Update nss-3.47-certdb-temp-cert.patch to the final version

This commit is contained in:
Daiki Ueno 2019-12-03 09:31:24 +01:00
parent 4f639ad73c
commit 704f2e22d6
2 changed files with 24 additions and 230 deletions

View File

@ -1,230 +1,21 @@
# HG changeset patch
# User Daiki Ueno <dueno@redhat.com>
# 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/lib/pki/pki3hack.c b/lib/pki/pki3hack.c
--- a/lib/pki/pki3hack.c
+++ b/lib/pki/pki3hack.c
@@ -921,11 +921,11 @@
}
if (!cc->nssCertificate || forceUpdate) {
fill_CERTCertificateFields(c, cc, forceUpdate);
- } else if (CERT_GetCertTrust(cc, &certTrust) != SECSuccess &&
- !c->object.cryptoContext) {
- /* if it's a perm cert, it might have been stored before the
- * trust, so look for the trust again. But a temp cert can be
- * ignored.
+ } else if (CERT_GetCertTrust(cc, &certTrust) != SECSuccess) {
+ /* If it's a perm cert, it might have been stored before the
+ * trust, so look for the trust again. If it's a temp cert, it
+ * might have been stored before the builtin module is loaded,
+ * so still need to look for the trust again.
*/
CERTCertTrust *trust = NULL;
trust = nssTrust_GetCERTCertTrustForCert(c, cc);

View File

@ -43,7 +43,7 @@ rpm.define(string.format("nss_release_tag NSS_%s_RTM",
Summary: Network Security Services
Name: nss
Version: %{nss_version}
Release: 2%{?dist}
Release: 3%{?dist}
License: MPLv2.0
URL: http://www.mozilla.org/projects/security/pki/nss/
Requires: nspr >= %{nspr_version}
@ -874,6 +874,9 @@ update-crypto-policies &> /dev/null || :
%changelog
* Tue Dec 3 2019 Daiki Ueno <dueno@redhat.com> - 3.47.1-3
- Update nss-3.47-certdb-temp-cert.patch to the final version
* Thu Nov 28 2019 Daiki Ueno <dueno@redhat.com> - 3.47.1-2
- Fix intermittent SEC_ERROR_UNKNOWN_ISSUER (#1752303, #1648617)