391 lines
15 KiB
Plaintext
391 lines
15 KiB
Plaintext
# HG changeset patch
|
|
# Parent c8f9e7eadcb3068b796b575310a85411a0d90da1
|
|
|
|
diff --git a/security/manager/ssl/nsCertTree.cpp b/security/manager/ssl/nsCertTree.cpp
|
|
--- a/security/manager/ssl/nsCertTree.cpp
|
|
+++ b/security/manager/ssl/nsCertTree.cpp
|
|
@@ -16,6 +16,7 @@
|
|
#include "nsIX509CertValidity.h"
|
|
#include "nsNSSCertHelper.h"
|
|
#include "nsNSSCertificate.h"
|
|
+#include "nsNSSCertificateDB.h"
|
|
#include "nsNSSComponent.h" // for PIPNSS string bundle calls.
|
|
#include "nsNSSHelper.h"
|
|
#include "nsReadableUtils.h"
|
|
@@ -800,8 +801,8 @@ nsCertTree::DeleteEntryObject(uint32_t i
|
|
|
|
SECStatus srv = CERT_DecodeTrustString(&trust, ""); // no override
|
|
if (srv == SECSuccess) {
|
|
- CERT_ChangeCertTrust(CERT_GetDefaultCertDB(), nsscert.get(),
|
|
- &trust);
|
|
+ ChangeCertTrustWithPossibleAuthentication(nsscert, trust,
|
|
+ nullptr);
|
|
}
|
|
}
|
|
}
|
|
diff --git a/security/manager/ssl/nsNSSCertTrust.h b/security/manager/ssl/nsNSSCertTrust.h
|
|
--- a/security/manager/ssl/nsNSSCertTrust.h
|
|
+++ b/security/manager/ssl/nsNSSCertTrust.h
|
|
@@ -57,8 +57,7 @@ public:
|
|
/* set p <--> P */
|
|
void AddPeerTrust(bool ssl, bool email, bool objSign);
|
|
|
|
- /* get it (const?) (shallow?) */
|
|
- CERTCertTrust * GetTrust() { return &mTrust; }
|
|
+ CERTCertTrust& GetTrust() { return mTrust; }
|
|
|
|
private:
|
|
void addTrust(unsigned int *t, unsigned int v);
|
|
diff --git a/security/manager/ssl/nsNSSCertificateDB.cpp b/security/manager/ssl/nsNSSCertificateDB.cpp
|
|
--- a/security/manager/ssl/nsNSSCertificateDB.cpp
|
|
+++ b/security/manager/ssl/nsNSSCertificateDB.cpp
|
|
@@ -232,6 +232,38 @@ nsNSSCertificateDB::getCertsFromPackage(
|
|
return collectArgs;
|
|
}
|
|
|
|
+// When using the sql-backed softoken, trust settings are authenticated using a
|
|
+// key in the secret database. Thus, if the user has a password, we need to
|
|
+// authenticate to the token in order to be able to change trust settings.
|
|
+SECStatus
|
|
+ChangeCertTrustWithPossibleAuthentication(const UniqueCERTCertificate& cert,
|
|
+ CERTCertTrust& trust, void* ctx)
|
|
+{
|
|
+ MOZ_ASSERT(cert, "cert must be non-null");
|
|
+ if (!cert) {
|
|
+ PR_SetError(SEC_ERROR_LIBRARY_FAILURE, 0);
|
|
+ return SECFailure;
|
|
+ }
|
|
+ // NSS ignores the first argument to CERT_ChangeCertTrust
|
|
+ SECStatus srv = CERT_ChangeCertTrust(nullptr, cert.get(), &trust);
|
|
+ if (srv == SECSuccess || PR_GetError() != SEC_ERROR_TOKEN_NOT_LOGGED_IN) {
|
|
+ return srv;
|
|
+ }
|
|
+ if (cert->slot) {
|
|
+ // If this certificate is on an external PKCS#11 token, we have to
|
|
+ // authenticate to that token.
|
|
+ srv = PK11_Authenticate(cert->slot, PR_TRUE, ctx);
|
|
+ } else {
|
|
+ // Otherwise, the certificate is on the internal module.
|
|
+ UniquePK11SlotInfo internalSlot(PK11_GetInternalKeySlot());
|
|
+ srv = PK11_Authenticate(internalSlot.get(), PR_TRUE, ctx);
|
|
+ }
|
|
+ if (srv != SECSuccess) {
|
|
+ return srv;
|
|
+ }
|
|
+ return CERT_ChangeCertTrust(nullptr, cert.get(), &trust);
|
|
+}
|
|
+
|
|
nsresult
|
|
nsNSSCertificateDB::handleCACertDownload(NotNull<nsIArray*> x509Certs,
|
|
nsIInterfaceRequestor *ctx,
|
|
@@ -356,8 +388,8 @@ nsNSSCertificateDB::handleCACertDownload
|
|
if (srv != SECSuccess) {
|
|
return MapSECStatus(srv);
|
|
}
|
|
- // NSS ignores the first argument to CERT_ChangeCertTrust
|
|
- srv = CERT_ChangeCertTrust(nullptr, tmpCert.get(), trust.GetTrust());
|
|
+ srv = ChangeCertTrustWithPossibleAuthentication(tmpCert, trust.GetTrust(),
|
|
+ ctx);
|
|
if (srv != SECSuccess) {
|
|
return MapSECStatus(srv);
|
|
}
|
|
@@ -798,8 +830,8 @@ nsNSSCertificateDB::DeleteCertificate(ns
|
|
// the cert onto the card again at which point we *will* want to
|
|
// trust that cert if it chains up properly.
|
|
nsNSSCertTrust trust(0, 0, 0);
|
|
- srv = CERT_ChangeCertTrust(CERT_GetDefaultCertDB(),
|
|
- cert.get(), trust.GetTrust());
|
|
+ srv = ChangeCertTrustWithPossibleAuthentication(cert, trust.GetTrust(),
|
|
+ nullptr);
|
|
}
|
|
MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("cert deleted: %d", srv));
|
|
return (srv) ? NS_ERROR_FAILURE : NS_OK;
|
|
@@ -815,38 +847,33 @@ nsNSSCertificateDB::SetCertTrust(nsIX509
|
|
if (isAlreadyShutDown()) {
|
|
return NS_ERROR_NOT_AVAILABLE;
|
|
}
|
|
- nsNSSCertTrust trust;
|
|
- nsresult rv;
|
|
- UniqueCERTCertificate nsscert(cert->GetCert());
|
|
|
|
- SECStatus srv;
|
|
- if (type == nsIX509Cert::CA_CERT) {
|
|
- // always start with untrusted and move up
|
|
- trust.SetValidCA();
|
|
- trust.AddCATrust(!!(trusted & nsIX509CertDB::TRUSTED_SSL),
|
|
- !!(trusted & nsIX509CertDB::TRUSTED_EMAIL),
|
|
- !!(trusted & nsIX509CertDB::TRUSTED_OBJSIGN));
|
|
- srv = CERT_ChangeCertTrust(CERT_GetDefaultCertDB(),
|
|
- nsscert.get(),
|
|
- trust.GetTrust());
|
|
- } else if (type == nsIX509Cert::SERVER_CERT) {
|
|
- // always start with untrusted and move up
|
|
- trust.SetValidPeer();
|
|
- trust.AddPeerTrust(trusted & nsIX509CertDB::TRUSTED_SSL, 0, 0);
|
|
- srv = CERT_ChangeCertTrust(CERT_GetDefaultCertDB(),
|
|
- nsscert.get(),
|
|
- trust.GetTrust());
|
|
- } else if (type == nsIX509Cert::EMAIL_CERT) {
|
|
- // always start with untrusted and move up
|
|
- trust.SetValidPeer();
|
|
- trust.AddPeerTrust(0, !!(trusted & nsIX509CertDB::TRUSTED_EMAIL), 0);
|
|
- srv = CERT_ChangeCertTrust(CERT_GetDefaultCertDB(),
|
|
- nsscert.get(),
|
|
- trust.GetTrust());
|
|
- } else {
|
|
- // ignore user certs
|
|
- return NS_OK;
|
|
+ nsNSSCertTrust trust;
|
|
+ switch (type) {
|
|
+ case nsIX509Cert::CA_CERT:
|
|
+ trust.SetValidCA();
|
|
+ trust.AddCATrust(!!(trusted & nsIX509CertDB::TRUSTED_SSL),
|
|
+ !!(trusted & nsIX509CertDB::TRUSTED_EMAIL),
|
|
+ !!(trusted & nsIX509CertDB::TRUSTED_OBJSIGN));
|
|
+ break;
|
|
+ case nsIX509Cert::SERVER_CERT:
|
|
+ trust.SetValidPeer();
|
|
+ trust.AddPeerTrust(trusted & nsIX509CertDB::TRUSTED_SSL, false, false);
|
|
+ break;
|
|
+ case nsIX509Cert::EMAIL_CERT:
|
|
+ trust.SetValidPeer();
|
|
+ trust.AddPeerTrust(false, !!(trusted & nsIX509CertDB::TRUSTED_EMAIL),
|
|
+ false);
|
|
+ break;
|
|
+ default:
|
|
+ // Ignore any other type of certificate (including invalid types).
|
|
+ return NS_OK;
|
|
}
|
|
+
|
|
+ UniqueCERTCertificate nsscert(cert->GetCert());
|
|
+ SECStatus srv = ChangeCertTrustWithPossibleAuthentication(nsscert,
|
|
+ trust.GetTrust(),
|
|
+ nullptr);
|
|
return MapSECStatus(srv);
|
|
}
|
|
|
|
@@ -1311,7 +1338,7 @@ nsNSSCertificateDB::AddCertFromBase64(co
|
|
}
|
|
|
|
nsNSSCertTrust trust;
|
|
- if (CERT_DecodeTrustString(trust.GetTrust(), PromiseFlatCString(aTrust).get())
|
|
+ if (CERT_DecodeTrustString(&trust.GetTrust(), PromiseFlatCString(aTrust).get())
|
|
!= SECSuccess) {
|
|
return NS_ERROR_FAILURE;
|
|
}
|
|
@@ -1344,8 +1371,8 @@ nsNSSCertificateDB::AddCertFromBase64(co
|
|
if (srv != SECSuccess) {
|
|
return MapSECStatus(srv);
|
|
}
|
|
- // NSS ignores the first argument to CERT_ChangeCertTrust
|
|
- srv = CERT_ChangeCertTrust(nullptr, tmpCert.get(), trust.GetTrust());
|
|
+ srv = ChangeCertTrustWithPossibleAuthentication(tmpCert, trust.GetTrust(),
|
|
+ nullptr);
|
|
return MapSECStatus(srv);
|
|
}
|
|
|
|
@@ -1373,7 +1400,7 @@ nsNSSCertificateDB::SetCertTrustFromStri
|
|
}
|
|
UniqueCERTCertificate nssCert(cert->GetCert());
|
|
|
|
- srv = CERT_ChangeCertTrust(CERT_GetDefaultCertDB(), nssCert.get(), &trust);
|
|
+ srv = ChangeCertTrustWithPossibleAuthentication(nssCert, trust, nullptr);
|
|
return MapSECStatus(srv);
|
|
}
|
|
|
|
diff --git a/security/manager/ssl/nsNSSCertificateDB.h b/security/manager/ssl/nsNSSCertificateDB.h
|
|
--- a/security/manager/ssl/nsNSSCertificateDB.h
|
|
+++ b/security/manager/ssl/nsNSSCertificateDB.h
|
|
@@ -74,4 +74,8 @@ private:
|
|
{0xb3, 0x2c, 0x80, 0x12, 0x46, 0x93, 0xd8, 0x71} \
|
|
}
|
|
|
|
+SECStatus
|
|
+ChangeCertTrustWithPossibleAuthentication(
|
|
+ const mozilla::UniqueCERTCertificate& cert, CERTCertTrust& trust, void* ctx);
|
|
+
|
|
#endif // nsNSSCertificateDB_h
|
|
diff --git a/security/manager/ssl/nsNSSComponent.cpp b/security/manager/ssl/nsNSSComponent.cpp
|
|
--- a/security/manager/ssl/nsNSSComponent.cpp
|
|
+++ b/security/manager/ssl/nsNSSComponent.cpp
|
|
@@ -682,8 +682,8 @@ nsNSSComponent::MaybeImportFamilySafetyR
|
|
0,
|
|
0
|
|
};
|
|
- if (CERT_ChangeCertTrust(nullptr, nssCertificate.get(), &trust)
|
|
- != SECSuccess) {
|
|
+ if (ChangeCertTrustWithPossibleAuthentication(nssCertificate, trust,
|
|
+ nullptr) != SECSuccess) {
|
|
MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
|
|
("couldn't trust certificate for TLS server auth"));
|
|
return NS_ERROR_FAILURE;
|
|
@@ -769,8 +769,8 @@ nsNSSComponent::UnloadFamilySafetyRoot()
|
|
// they're the same. To work around this, we set a non-zero flag to ensure
|
|
// that the trust will get updated.
|
|
CERTCertTrust trust = { CERTDB_USER, 0, 0 };
|
|
- if (CERT_ChangeCertTrust(nullptr, mFamilySafetyRoot.get(), &trust)
|
|
- != SECSuccess) {
|
|
+ if (ChangeCertTrustWithPossibleAuthentication(mFamilySafetyRoot, trust,
|
|
+ nullptr) != SECSuccess) {
|
|
MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
|
|
("couldn't untrust certificate for TLS server auth"));
|
|
}
|
|
@@ -912,7 +912,9 @@ nsNSSComponent::UnloadEnterpriseRoots()
|
|
("library failure: CERTCertListNode null or lacks cert"));
|
|
continue;
|
|
}
|
|
- if (CERT_ChangeCertTrust(nullptr, n->cert, &trust) != SECSuccess) {
|
|
+ UniqueCERTCertificate cert(CERT_DupCertificate(n->cert));
|
|
+ if (ChangeCertTrustWithPossibleAuthentication(cert, trust, nullptr)
|
|
+ != SECSuccess) {
|
|
MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
|
|
("couldn't untrust certificate for TLS server auth"));
|
|
}
|
|
@@ -1068,8 +1070,8 @@ nsNSSComponent::ImportEnterpriseRootsFor
|
|
MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("couldn't add cert to list"));
|
|
continue;
|
|
}
|
|
- if (CERT_ChangeCertTrust(nullptr, nssCertificate.get(), &trust)
|
|
- != SECSuccess) {
|
|
+ if (ChangeCertTrustWithPossibleAuthentication(nssCertificate, trust,
|
|
+ nullptr) != SECSuccess) {
|
|
MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
|
|
("couldn't trust certificate for TLS server auth"));
|
|
}
|
|
diff --git a/security/manager/ssl/tests/unit/test_certDB_import_with_master_password.js b/security/manager/ssl/tests/unit/test_certDB_import_with_master_password.js
|
|
new file mode 100644
|
|
--- /dev/null
|
|
+++ b/security/manager/ssl/tests/unit/test_certDB_import_with_master_password.js
|
|
@@ -0,0 +1,120 @@
|
|
+// -*- indent-tabs-mode: nil; js-indent-level: 2 -*-
|
|
+// Any copyright is dedicated to the Public Domain.
|
|
+// http://creativecommons.org/publicdomain/zero/1.0/
|
|
+"use strict";
|
|
+
|
|
+// Tests that a CA certificate can still be imported if the user has a master
|
|
+// password set.
|
|
+
|
|
+do_get_profile();
|
|
+
|
|
+const gCertDB = Cc["@mozilla.org/security/x509certdb;1"]
|
|
+ .getService(Ci.nsIX509CertDB);
|
|
+
|
|
+const CA_CERT_COMMON_NAME = "importedCA";
|
|
+
|
|
+let gCACertImportDialogCount = 0;
|
|
+
|
|
+// Mock implementation of nsICertificateDialogs.
|
|
+const gCertificateDialogs = {
|
|
+ confirmDownloadCACert: (ctx, cert, trust) => {
|
|
+ gCACertImportDialogCount++;
|
|
+ equal(cert.commonName, CA_CERT_COMMON_NAME,
|
|
+ "CA cert to import should have the correct CN");
|
|
+ trust.value = Ci.nsIX509CertDB.TRUSTED_EMAIL;
|
|
+ return true;
|
|
+ },
|
|
+ setPKCS12FilePassword: (ctx, password) => {
|
|
+ // This is only relevant to exporting.
|
|
+ ok(false, "setPKCS12FilePassword() should not have been called");
|
|
+ },
|
|
+ getPKCS12FilePassword: (ctx, password) => {
|
|
+ // We don't test anything that calls this method yet.
|
|
+ ok(false, "getPKCS12FilePassword() should not have been called");
|
|
+ },
|
|
+ viewCert: (ctx, cert) => {
|
|
+ // This shouldn't be called for import methods.
|
|
+ ok(false, "viewCert() should not have been called");
|
|
+ },
|
|
+
|
|
+ QueryInterface: XPCOMUtils.generateQI([Ci.nsICertificateDialogs])
|
|
+};
|
|
+
|
|
+var gMockPrompter = {
|
|
+ passwordToTry: "password",
|
|
+ numPrompts: 0,
|
|
+
|
|
+ // This intentionally does not use arrow function syntax to avoid an issue
|
|
+ // where in the context of the arrow function, |this != gMockPrompter| due to
|
|
+ // how objects get wrapped when going across xpcom boundaries.
|
|
+ promptPassword(dialogTitle, text, password, checkMsg, checkValue) {
|
|
+ this.numPrompts++;
|
|
+ if (this.numPrompts > 1) { // don't keep retrying a bad password
|
|
+ return false;
|
|
+ }
|
|
+ equal(text,
|
|
+ "Please enter the master password for the Software Security Device.",
|
|
+ "password prompt text should be as expected");
|
|
+ equal(checkMsg, null, "checkMsg should be null");
|
|
+ ok(this.passwordToTry, "passwordToTry should be non-null");
|
|
+ password.value = this.passwordToTry;
|
|
+ return true;
|
|
+ },
|
|
+
|
|
+ QueryInterface: XPCOMUtils.generateQI([Ci.nsIPrompt]),
|
|
+
|
|
+ // Again with the arrow function issue.
|
|
+ getInterface(iid) {
|
|
+ if (iid.equals(Ci.nsIPrompt)) {
|
|
+ return this;
|
|
+ }
|
|
+
|
|
+ throw new Error(Cr.NS_ERROR_NO_INTERFACE);
|
|
+ }
|
|
+};
|
|
+
|
|
+function getCertAsByteArray(certPath) {
|
|
+ let certFile = do_get_file(certPath, false);
|
|
+ let certBytes = readFile(certFile);
|
|
+
|
|
+ let byteArray = [];
|
|
+ for (let i = 0; i < certBytes.length; i++) {
|
|
+ byteArray.push(certBytes.charCodeAt(i));
|
|
+ }
|
|
+
|
|
+ return byteArray;
|
|
+}
|
|
+
|
|
+function run_test() {
|
|
+ let certificateDialogsCID =
|
|
+ MockRegistrar.register("@mozilla.org/nsCertificateDialogs;1",
|
|
+ gCertificateDialogs);
|
|
+ do_register_cleanup(() => {
|
|
+ MockRegistrar.unregister(certificateDialogsCID);
|
|
+ });
|
|
+
|
|
+ // Set a master password.
|
|
+ let tokenDB = Cc["@mozilla.org/security/pk11tokendb;1"]
|
|
+ .getService(Ci.nsIPK11TokenDB);
|
|
+ let token = tokenDB.getInternalKeyToken();
|
|
+ token.initPassword("password");
|
|
+ token.logoutSimple();
|
|
+
|
|
+ // Sanity check the CA cert is missing.
|
|
+ throws(() => gCertDB.findCertByNickname(CA_CERT_COMMON_NAME),
|
|
+ /NS_ERROR_FAILURE/,
|
|
+ "CA cert should not be in the database before import");
|
|
+
|
|
+ // Import and check for success.
|
|
+ let caArray = getCertAsByteArray("test_certDB_import/importedCA.pem");
|
|
+ gCertDB.importCertificates(caArray, caArray.length, Ci.nsIX509Cert.CA_CERT,
|
|
+ gMockPrompter);
|
|
+ equal(gCACertImportDialogCount, 1,
|
|
+ "Confirmation dialog for the CA cert should only be shown once");
|
|
+
|
|
+ let caCert = gCertDB.findCertByNickname(CA_CERT_COMMON_NAME);
|
|
+ notEqual(caCert, null, "CA cert should now be found in the database");
|
|
+ ok(gCertDB.isCertTrusted(caCert, Ci.nsIX509Cert.CA_CERT,
|
|
+ Ci.nsIX509CertDB.TRUSTED_EMAIL),
|
|
+ "CA cert should be trusted for e-mail");
|
|
+}
|
|
diff --git a/security/manager/ssl/tests/unit/xpcshell.ini b/security/manager/ssl/tests/unit/xpcshell.ini
|
|
--- a/security/manager/ssl/tests/unit/xpcshell.ini
|
|
+++ b/security/manager/ssl/tests/unit/xpcshell.ini
|
|
@@ -55,6 +55,7 @@ run-sequentially = hardcoded ports
|
|
[test_cert_version.js]
|
|
[test_certDB_import.js]
|
|
[test_certDB_import_pkcs12.js]
|
|
+[test_certDB_import_with_master_password.js]
|
|
[test_certviewer_invalid_oids.js]
|
|
skip-if = toolkit == 'android'
|
|
[test_constructX509FromBase64.js]
|