From 4f639ad73c008a6f9287665d45bceff9d4365105 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Thu, 28 Nov 2019 16:13:41 +0100 Subject: [PATCH] Fix intermittent SEC_ERROR_UNKNOWN_ISSUER (#1752303, #1648617) --- nss-3.47-certdb-temp-cert.patch | 230 ++++++++++++++++++++++++++++++++ nss.spec | 7 +- 2 files changed, 236 insertions(+), 1 deletion(-) create mode 100644 nss-3.47-certdb-temp-cert.patch diff --git a/nss-3.47-certdb-temp-cert.patch b/nss-3.47-certdb-temp-cert.patch new file mode 100644 index 0000000..b5623de --- /dev/null +++ b/nss-3.47-certdb-temp-cert.patch @@ -0,0 +1,230 @@ +# HG changeset patch +# User Daiki Ueno +# Date 1574953499 -3600 +# Thu Nov 28 16:04:59 2019 +0100 +# Node ID f1f705bd0528713216e16867233825c299d3e3b2 +# Parent 10722c590949819ed4d971ad5ae213bc8b11a1bf +Bug 1593167, certdb: prefer perm certs over temp certs when trust is not available + +Summary: +When a builtin root module is loaded after some temp certs being +loaded, our certificate lookup logic preferred those temp certs over +perm certs stored on the root module. This was a problem because such +temp certs are usually not accompanied with trust information. + +This makes the certificate lookup logic capable of handling such +situations by checking if the trust information is attached to temp +certs and otherwise falling back to perm certs. + +Reviewers: rrelyea, keeler + +Reviewed By: rrelyea + +Subscribers: heftig + +Bug #: 1593167 + +Differential Revision: https://phabricator.services.mozilla.com/D54726 + +diff --git a/lib/certdb/stanpcertdb.c b/lib/certdb/stanpcertdb.c +--- a/lib/certdb/stanpcertdb.c ++++ b/lib/certdb/stanpcertdb.c +@@ -340,6 +340,91 @@ CERT_AddTempCertToPerm(CERTCertificate * + return __CERT_AddTempCertToPerm(cert, nickname, trust); + } + ++static CERTCertificate * ++find_cert_by_der_cert(CERTCertDBHandle *handle, SECItem *derCert) ++{ ++ CERTCertificate *cc; ++ NSSCryptoContext *context; ++ NSSCertificate *cert = NULL; ++ NSSCertificate *tempCert = NULL; ++ NSSCertificate *permCert = NULL; ++ NSSDER encoding; ++ nssCertificateStoreTrace lockTrace = { NULL, NULL, PR_FALSE, PR_FALSE }; ++ nssCertificateStoreTrace unlockTrace = { NULL, NULL, PR_FALSE, PR_FALSE }; ++ ++ /* We retrieve a certificate instance for derCert in this order: ++ * 1. Look up a temp cert in the crypto context. If it is found ++ * and has a trust object associated, use it. ++ * 2. Look up a perm cert in the trust domain. If it is found, ++ * use it. Otherwise, use the temp cert. ++ */ ++ NSSITEM_FROM_SECITEM(&encoding, derCert); ++ context = STAN_GetDefaultCryptoContext(); ++ ++ /* First, see if it is already a temp cert */ ++ tempCert = NSSCryptoContext_FindCertificateByEncodedCertificate(context, ++ &encoding); ++ if (tempCert) { ++ NSSTrust *trust; ++ ++ trust = nssCryptoContext_FindTrustForCertificate(context, tempCert); ++ if (trust) { ++ nssTrust_Destroy(trust); ++ cert = tempCert; ++ tempCert = NULL; ++ } ++ } ++ ++ /* Then, see if it is already a perm cert */ ++ if (!cert && handle) { ++ permCert = NSSTrustDomain_FindCertificateByEncodedCertificate(handle, ++ &encoding); ++ if (permCert) { ++ /* Delete the temp instance */ ++ if (tempCert) { ++ nssCertificateStore_Lock(context->certStore, &lockTrace); ++ nssCertificateStore_RemoveCertLOCKED(context->certStore, ++ tempCert); ++ nssCertificateStore_Unlock(context->certStore, &lockTrace, ++ &unlockTrace); ++ } ++ cert = permCert; ++ permCert = NULL; ++ } else if (tempCert) { ++ cert = tempCert; ++ tempCert = NULL; ++ } ++ } ++ ++ if (tempCert) { ++ nssCertificate_Destroy(tempCert); ++ } ++ if (permCert) { ++ nssCertificate_Destroy(permCert); ++ } ++ ++ if (!cert) { ++ return NULL; ++ } ++ ++ /* Actually, that search ends up going by issuer/serial, ++ * so it is still possible to return a cert with the same ++ * issuer/serial but a different encoding, and we're ++ * going to reject that ++ */ ++ if (!nssItem_Equal(&cert->encoding, &encoding, NULL)) { ++ nssCertificate_Destroy(cert); ++ PORT_SetError(SEC_ERROR_REUSED_ISSUER_AND_SERIAL); ++ return NULL; ++ } ++ ++ cc = STAN_GetCERTCertificateOrRelease(cert); ++ if (!cc) { ++ CERT_MapStanError(); ++ } ++ return cc; ++} ++ + CERTCertificate * + CERT_NewTempCertificate(CERTCertDBHandle *handle, SECItem *derCert, + char *nickname, PRBool isperm, PRBool copyDER) +@@ -351,32 +436,8 @@ CERT_NewTempCertificate(CERTCertDBHandle + NSSCryptoContext *gCC = STAN_GetDefaultCryptoContext(); + NSSTrustDomain *gTD = STAN_GetDefaultTrustDomain(); + if (!isperm) { +- NSSDER encoding; +- NSSITEM_FROM_SECITEM(&encoding, derCert); +- /* First, see if it is already a temp cert */ +- c = NSSCryptoContext_FindCertificateByEncodedCertificate(gCC, +- &encoding); +- if (!c && handle) { +- /* Then, see if it is already a perm cert */ +- c = NSSTrustDomain_FindCertificateByEncodedCertificate(handle, +- &encoding); +- } +- if (c) { +- /* actually, that search ends up going by issuer/serial, +- * so it is still possible to return a cert with the same +- * issuer/serial but a different encoding, and we're +- * going to reject that +- */ +- if (!nssItem_Equal(&c->encoding, &encoding, NULL)) { +- nssCertificate_Destroy(c); +- PORT_SetError(SEC_ERROR_REUSED_ISSUER_AND_SERIAL); +- cc = NULL; +- } else { +- cc = STAN_GetCERTCertificateOrRelease(c); +- if (cc == NULL) { +- CERT_MapStanError(); +- } +- } ++ cc = find_cert_by_der_cert(handle, derCert); ++ if (cc) { + return cc; + } + } +@@ -598,19 +659,7 @@ CERT_FindCertByNickname(CERTCertDBHandle + CERTCertificate * + CERT_FindCertByDERCert(CERTCertDBHandle *handle, SECItem *derCert) + { +- NSSCryptoContext *cc; +- NSSCertificate *c; +- NSSDER encoding; +- NSSITEM_FROM_SECITEM(&encoding, derCert); +- cc = STAN_GetDefaultCryptoContext(); +- c = NSSCryptoContext_FindCertificateByEncodedCertificate(cc, &encoding); +- if (!c) { +- c = NSSTrustDomain_FindCertificateByEncodedCertificate(handle, +- &encoding); +- if (!c) +- return NULL; +- } +- return STAN_GetCERTCertificateOrRelease(c); ++ return find_cert_by_der_cert(handle, derCert); + } + + static CERTCertificate * +diff --git a/lib/pki/pkistore.c b/lib/pki/pkistore.c +--- a/lib/pki/pkistore.c ++++ b/lib/pki/pkistore.c +@@ -27,6 +27,8 @@ + + #include "prbit.h" + ++#include "secerr.h" ++ + /* + * Certificate Store + * +@@ -544,6 +546,13 @@ nssCertificateStore_FindCertificateByEnc + &serial); + PORT_Free(issuer.data); + PORT_Free(serial.data); ++ ++ if (rvCert && !nssItem_Equal(&rvCert->encoding, encoding, NULL)) { ++ nssCertificate_Destroy(rvCert); ++ PORT_SetError(SEC_ERROR_REUSED_ISSUER_AND_SERIAL); ++ return NULL; ++ } ++ + return rvCert; + } + +diff --git a/lib/pki/trustdomain.c b/lib/pki/trustdomain.c +--- a/lib/pki/trustdomain.c ++++ b/lib/pki/trustdomain.c +@@ -15,6 +15,7 @@ + #include "pk11pub.h" + #include "nssrwlk.h" + #include "pk11priv.h" ++#include "secerr.h" + + #define NSSTRUSTDOMAIN_DEFAULT_CACHE_SIZE 32 + +@@ -841,6 +842,13 @@ nssTrustDomain_FindCertificateByEncodedC + &serial); + PORT_Free(issuer.data); + PORT_Free(serial.data); ++ ++ if (rvCert && !nssItem_Equal(&rvCert->encoding, ber, NULL)) { ++ nssCertificate_Destroy(rvCert); ++ PORT_SetError(SEC_ERROR_REUSED_ISSUER_AND_SERIAL); ++ return NULL; ++ } ++ + return rvCert; + } + diff --git a/nss.spec b/nss.spec index 6d48d1a..68d4ed6 100644 --- a/nss.spec +++ b/nss.spec @@ -43,7 +43,7 @@ rpm.define(string.format("nss_release_tag NSS_%s_RTM", Summary: Network Security Services Name: nss Version: %{nss_version} -Release: 1%{?dist} +Release: 2%{?dist} License: MPLv2.0 URL: http://www.mozilla.org/projects/security/pki/nss/ Requires: nspr >= %{nspr_version} @@ -107,6 +107,8 @@ Patch2: nss-539183.patch Patch4: iquote.patch # add missing ike mechanism to softoken Patch10: nss-3.47-ike-fix.patch +# https://bugzilla.mozilla.org/show_bug.cgi?id=1593167 +Patch11: nss-3.47-certdb-temp-cert.patch %description Network Security Services (NSS) is a set of libraries designed to @@ -872,6 +874,9 @@ update-crypto-policies &> /dev/null || : %changelog +* Thu Nov 28 2019 Daiki Ueno - 3.47.1-2 +- Fix intermittent SEC_ERROR_UNKNOWN_ISSUER (#1752303, #1648617) + * Fri Nov 22 2019 Daiki Ueno - 3.47.1-1 - Update to NSS 3.47.1