nss/nss-3.47-certdb-temp-cert.patch

# 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;
 }