From 32d6c776ad5c489efea9f380fdac296a63209ea5 Mon Sep 17 00:00:00 2001
From: Alexander Scheel <ascheel@redhat.com>
Date: Wed, 4 Sep 2019 08:33:14 -0400
Subject: [PATCH] Fix root certificate validation
When the Leaf and Chain OCSP checking policy is enabled in
CryptoManager, JSS will switch to alternative certificate verification
logic in JSSL_DefaultCertAuthCallback. In this method, the root
certificate was incorrectly trusted without being verified to exist in
the trust store.
This patch cleans up the logic in JSSL_verifyCertPKIX and makes it
more explicit in addition to fixing the error.
Fixes CVE-2019-14823
Signed-off-by: Alexander Scheel <ascheel@redhat.com>
---
org/mozilla/jss/ssl/common.c | 239 ++++++++++++++++++++---------------
1 file changed, 138 insertions(+), 101 deletions(-)
diff --git a/org/mozilla/jss/ssl/common.c b/org/mozilla/jss/ssl/common.c
index 8c2a2240..030588c8 100644
--- a/org/mozilla/jss/ssl/common.c
+++ b/org/mozilla/jss/ssl/common.c
@@ -936,170 +936,207 @@ finish:
return root;
}
-/* Verify a cert using explicit PKIX call.
- * For now only used in OCSP AIA context.
- * The result of this call will be a full chain
- * and leaf network AIA ocsp validation.
- * The policy param will be used in the future to
- * handle more scenarios.
- */
-
-SECStatus JSSL_verifyCertPKIX(CERTCertificate *cert,
- SECCertificateUsage certificateUsage,secuPWData *pwdata, int ocspPolicy,
- CERTVerifyLog *log, SECCertificateUsage *usage)
+/* Internal helper for the below call. */
+static SECStatus
+JSSL_verifyCertPKIXInternal(CERTCertificate *cert,
+ SECCertificateUsage certificateUsage, secuPWData *pwdata, int ocspPolicy,
+ CERTVerifyLog *log, SECCertificateUsage *usage,
+ CERTCertList *trustedCertList)
{
-
- /* put the first set of possible flags internally here first */
- /* later there could be a more complete list to choose from */
- /* support our hard core fetch aia ocsp policy for now */
-
- static PRUint64 ocsp_Enabled_Hard_Policy_LeafFlags[2] = {
+ /* Put the first set of possible flags internally here first. Later
+ * there could be a more complete list to choose from; for now we only
+ * support our hard core fetch AIA OCSP policy. Note that we disable
+ * CRL fetching as Dogtag doesn't support it. Additionally, enable OCSP
+ * checking on the chained CA certificates. Since NSS/PKIX's
+ * CERT_GetClassicOCSPEnabledHardFailurePolicy doesn't do what we want,
+ * we construct the policy ourselves. */
+ PRUint64 ocsp_Enabled_Hard_Policy_LeafFlags[2] = {
/* crl */
- 0,
+ CERT_REV_M_DO_NOT_TEST_USING_THIS_METHOD,
/* ocsp */
CERT_REV_M_TEST_USING_THIS_METHOD |
- CERT_REV_M_FAIL_ON_MISSING_FRESH_INFO
+ CERT_REV_M_FAIL_ON_MISSING_FRESH_INFO
};
- static PRUint64 ocsp_Enabled_Hard_Policy_ChainFlags[2] = {
+ PRUint64 ocsp_Enabled_Hard_Policy_ChainFlags[2] = {
/* crl */
- 0,
+ CERT_REV_M_DO_NOT_TEST_USING_THIS_METHOD,
/* ocsp */
CERT_REV_M_TEST_USING_THIS_METHOD |
- CERT_REV_M_FAIL_ON_MISSING_FRESH_INFO
+ CERT_REV_M_FAIL_ON_MISSING_FRESH_INFO
};
- static CERTRevocationMethodIndex
- ocsp_Enabled_Hard_Policy_Method_Preference = {
- cert_revocation_method_ocsp
- };
-
- static CERTRevocationFlags ocsp_Enabled_Hard_Policy = {
- { /* leafTests */
- 2,
- ocsp_Enabled_Hard_Policy_LeafFlags,
- 1,
- &ocsp_Enabled_Hard_Policy_Method_Preference,
- 0 },
- { /* chainTests */
- 2,
- ocsp_Enabled_Hard_Policy_ChainFlags,
- 1,
- &ocsp_Enabled_Hard_Policy_Method_Preference,
- 0 }
+ CERTRevocationMethodIndex ocsp_Enabled_Hard_Policy_Method_Preference[1] = {
+ cert_revocation_method_ocsp
};
- /* for future expansion */
+ CERTRevocationFlags ocsp_Enabled_Hard_Policy = {
+ /* CERTRevocationTests - leafTests */
+ {
+ /* number_of_defined_methods */
+ 2,
+ /* cert_rev_flags_per_method */
+ ocsp_Enabled_Hard_Policy_LeafFlags,
+ /* number_of_preferred_methods */
+ 1,
+ /* preferred_methods */
+ ocsp_Enabled_Hard_Policy_Method_Preference,
+ /* cert_rev_method_independent_flags */
+ 0
+ },
+ /* CERTRevocationTests - chainTests */
+ {
+ /* number_of_defined_methods */
+ 2,
+ /* cert_rev_flags_per_method */
+ ocsp_Enabled_Hard_Policy_ChainFlags,
+ /* number_of_preferred_methods */
+ 1,
+ /* preferred_methods */
+ ocsp_Enabled_Hard_Policy_Method_Preference,
+ /* cert_rev_method_independent_flags */
+ 0
+ }
+ };
- CERTValOutParam cvout[20] = {0};
- CERTValInParam cvin[20] = {0};
+ /* The size of these objects are defined here based upon maximum possible
+ * inputs. A dynamic allocation could reallocate based upon actual usage,
+ * however this would affect the size by at most one or two. Note that,
+ * due to the required usage of cert_pi_end/cert_po_end, these sizes are
+ * inflated by one. */
+ CERTValOutParam cvout[3] = {{0}};
+ CERTValInParam cvin[6] = {{0}};
+ int usageIndex = -1;
int inParamIndex = 0;
int outParamIndex = 0;
- CERTRevocationFlags *rev = NULL;
- CERTCertList *trustedCertList = NULL;
-
- PRBool fetchCerts = PR_FALSE;
-
- SECCertUsage certUsage = certUsageSSLClient /* 0 */;
-
SECStatus res = SECFailure;
- if(cert == NULL) {
+
+ if (cert == NULL) {
goto finish;
}
- if(ocspPolicy != OCSP_LEAF_AND_CHAIN_POLICY) {
+ if (ocspPolicy != OCSP_LEAF_AND_CHAIN_POLICY) {
goto finish;
}
- /* Force the strict ocsp network check on chain
- and leaf.
- */
-
- fetchCerts = PR_TRUE;
- rev = &ocsp_Enabled_Hard_Policy;
-
- /* fetch aia over net */
-
+ /* Enable live AIA fetching over the network. */
cvin[inParamIndex].type = cert_pi_useAIACertFetch;
- cvin[inParamIndex].value.scalar.b = fetchCerts;
- inParamIndex++;
-
- /* time */
+ cvin[inParamIndex].value.scalar.b = PR_TRUE;
+ inParamIndex++;
+ /* By setting the time to zero, we choose the current time when the
+ * check is performed. */
cvin[inParamIndex].type = cert_pi_date;
- cvin[inParamIndex].value.scalar.time = PR_Now();
+ cvin[inParamIndex].value.scalar.time = 0;
inParamIndex++;
- /* flags */
-
+ /* Force the strict OCSP check on both the leaf and its chain. */
cvin[inParamIndex].type = cert_pi_revocationFlags;
- cvin[inParamIndex].value.pointer.revocation = rev;
+ cvin[inParamIndex].value.pointer.revocation = &ocsp_Enabled_Hard_Policy;
inParamIndex++;
- /* establish trust anchor */
-
- /* We need to convert the SECCertificateUsage to a SECCertUsage to obtain
- * the root.
- */
-
- SECCertificateUsage testUsage = certificateUsage;
- while (0 != (testUsage = testUsage >> 1)) { certUsage++; }
-
- CERTCertificate *root = getRoot(cert,certUsage);
-
- /* Try to add the root as the trust anchor so all the
- other memebers of the ca chain will get validated.
- */
-
- if( root != NULL ) {
- trustedCertList = CERT_NewCertList();
- CERT_AddCertToListTail(trustedCertList, root);
-
+ /* Establish a trust anchor if it is passed to us. NOTE: this trust anchor
+ * must previously be validated before it is passed to us here. */
+ if (trustedCertList != NULL) {
cvin[inParamIndex].type = cert_pi_trustAnchors;
cvin[inParamIndex].value.pointer.chain = trustedCertList;
-
inParamIndex++;
}
+ /* Done establishing input parameters. */
cvin[inParamIndex].type = cert_pi_end;
- if(log != NULL) {
+ /* When we need to log rationale for failure, pass it as an output
+ * parameter. */
+ if (log != NULL) {
cvout[outParamIndex].type = cert_po_errorLog;
cvout[outParamIndex].value.pointer.log = log;
outParamIndex ++;
}
- int usageIndex = 0;
- if(usage != NULL) {
+ /* When we need to inquire about the resulting certificate usage, pass it
+ * here. */
+ if (usage != NULL) {
usageIndex = outParamIndex;
cvout[outParamIndex].type = cert_po_usages;
cvout[outParamIndex].value.scalar.usages = 0;
outParamIndex ++;
}
+ /* Done establishing output parameters. */
cvout[outParamIndex].type = cert_po_end;
+ /* Call into NSS's PKIX library to validate our certificate. */
res = CERT_PKIXVerifyCert(cert, certificateUsage, cvin, cvout, &pwdata);
finish:
- /* clean up any trusted cert list */
-
+ /* Clean up any certificates in the trusted certificate list. This was
+ * a passed input parameter, but by taking ownership of it and clearing it,
+ * we enable tail calls to this function. */
if (trustedCertList) {
+ /* CERT_DestroyCertList destroys interior certs for us. */
CERT_DestroyCertList(trustedCertList);
trustedCertList = NULL;
}
- /* CERT_DestroyCertList destroys interior certs for us. */
-
- if(root) {
- root = NULL;
- }
-
- if(res == SECSuccess && usage) {
+ if (res == SECSuccess && usage && usageIndex != -1) {
*usage = cvout[usageIndex].value.scalar.usages;
}
return res;
}
+
+/* Verify a cert using an explicit PKIX call. For now only perform this call
+ * when the OCSP policy is set to leaf and chain. Performs a blocking, online
+ * OCSP status refresh. The result of this call will be a full-chain OCSP
+ * validation.
+ *
+ * In the future, we'll use ocspPolicy to condition around additional policies
+ * and handle them all with this method (and a call to PKIX).
+ *
+ * Note that this currently requires the certificate to be added directly
+ * to the NSS DB. We can't otherwise validate against root certificates in
+ * the default NSS DB.
+ */
+SECStatus JSSL_verifyCertPKIX(CERTCertificate *cert,
+ SECCertificateUsage certificateUsage, secuPWData *pwdata, int ocspPolicy,
+ CERTVerifyLog *log, SECCertificateUsage *usage)
+{
+ SECCertUsage certUsage = certUsageSSLClient /* 0 */;
+
+ /* We need to convert the SECCertificateUsage to a SECCertUsage to obtain
+ * the root.
+ */
+
+ SECCertificateUsage testUsage = certificateUsage;
+ while (0 != (testUsage = testUsage >> 1)) { certUsage++; }
+
+ CERTCertificate *root = getRoot(cert, certUsage);
+
+ // Two cases: either the root is present, or it isn't.
+ if (root == NULL) {
+ /* In this case, we've had a hard time finding the root. In all
+ * likelihood, the following call will fail to validate the end cert
+ * as well and thus fail to validate. I don't believe there's a risk
+ * in trying it however. */
+ return JSSL_verifyCertPKIXInternal(cert, certificateUsage, pwdata,
+ ocspPolicy, log, usage, NULL);
+ } else {
+ /* In this case, we've found the root certificate. Before passing it
+ * to the leaf, explicitly validate it with strict OCSP checking. Then
+ * validate the leaf certificate with a known and trusted root
+ * certificate. */
+ SECStatus ret = JSSL_verifyCertPKIXInternal(root, certificateUsageSSLCA,
+ pwdata, ocspPolicy, log, usage, NULL);
+ if (ret != SECSuccess) {
+ return ret;
+ }
+
+ CERTCertList *rootList = CERT_NewCertList();
+ CERT_AddCertToListTail(rootList, root);
+ return JSSL_verifyCertPKIXInternal(cert, certificateUsage, pwdata,
+ ocspPolicy, log, usage, rootList);
+ }
+}
--
2.21.0