# 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]