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