Bug 1615438 - Use CKA_NSS_SERVER_DISTRUST_AFTER from NSS for certificate validation. r=keeler
authorBenjamin Beurdouche <beurdouche@mozilla.com>
Thu, 28 May 2020 20:35:48 +0000
changeset 596604 adb3e43858403fab834a714418784e11c1c1b6e0
parent 596603 faf8e8097d2351b57063ad15b75d54c6ce32b24d
child 596605 12aba092c943a0820a74be0c97dcc7fc37b414c5
push id13186
push userffxbld-merge
push dateMon, 01 Jun 2020 09:52:46 +0000
treeherdermozilla-beta@3e7c70a1e4a1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1615438
milestone78.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1615438 - Use CKA_NSS_SERVER_DISTRUST_AFTER from NSS for certificate validation. r=keeler Differential Revision: https://phabricator.services.mozilla.com/D74662
security/certverifier/NSSCertDBTrustDomain.cpp
security/nss.symbols
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -37,16 +37,17 @@
 #include "nsNSSCertificateDB.h"
 #include "nsPrintfCString.h"
 #include "nsServiceManagerUtils.h"
 #include "nsThreadUtils.h"
 #include "nss.h"
 #include "pk11pub.h"
 #include "prerror.h"
 #include "secerr.h"
+#include "secder.h"
 
 #include "TrustOverrideUtils.h"
 #include "TrustOverride-AppleGoogleDigiCertData.inc"
 #include "TrustOverride-StartComAndWoSignData.inc"
 #include "TrustOverride-SymantecData.inc"
 
 using namespace mozilla;
 using namespace mozilla::pkix;
@@ -1150,16 +1151,53 @@ static Result CheckForStartComOrWoSign(c
     }
     if (CertDNIsInList(node->cert, StartComAndWoSignDNs)) {
       return Result::ERROR_REVOKED_CERTIFICATE;
     }
   }
   return Success;
 }
 
+nsresult isDistrustedCertificateChain(const UniqueCERTCertList& certList,
+                                      bool& isDistrusted) {
+  // Set the default result to be distrusted.
+  isDistrusted = true;
+
+  // Allocate objects and retreive the root and end-entity certificates.
+  const CERTCertificate* certRoot = CERT_LIST_TAIL(certList)->cert;
+  const CERTCertificate* certLeaf = CERT_LIST_HEAD(certList)->cert;
+
+  // Check if the distrust field of the root is filled.
+  if (!certRoot->distrust) {
+    isDistrusted = false;
+    return NS_OK;
+  }
+
+  // Get validity for the current end-entity certificate
+  // and get the distrust field for the root certificate.
+  PRTime certRootDistrustAfter;
+  PRTime certLeafNotBefore;
+
+  SECStatus rv1 = DER_DecodeTimeChoice(
+      &certRootDistrustAfter, &certRoot->distrust->serverDistrustAfter);
+  SECStatus rv2 =
+      DER_DecodeTimeChoice(&certLeafNotBefore, &certLeaf->validity.notBefore);
+
+  if ((rv1 != SECSuccess) || (rv2 != SECSuccess)) {
+    return NS_ERROR_FAILURE;
+  }
+
+  // Compare the validity of the end-entity certificate with
+  // the distrust value of the root.
+  if (certLeafNotBefore <= certRootDistrustAfter) {
+    isDistrusted = false;
+  }
+  return NS_OK;
+}
+
 Result NSSCertDBTrustDomain::IsChainValid(const DERArray& certArray, Time time,
                                           const CertPolicyId& requiredPolicy) {
   MOZ_LOG(gCertVerifierLog, LogLevel::Debug,
           ("NSSCertDBTrustDomain: IsChainValid"));
 
   UniqueCERTCertList certList;
   SECStatus srv =
       ConstructCERTCertListFromReversedDERArray(certArray, certList);
@@ -1213,16 +1251,29 @@ Result NSSCertDBTrustDomain::IsChainVali
     if (NS_FAILED(nsrv)) {
       return Result::FATAL_ERROR_LIBRARY_FAILURE;
     }
     if (!chainHasValidPins) {
       return Result::ERROR_KEY_PINNING_FAILURE;
     }
   }
 
+  // Check that the childs' certificate NotBefore date is anterior to
+  // the NotAfter value of the parent when the root is a builtin.
+  if (isBuiltInRoot) {
+    bool isDistrusted;
+    nsrv = isDistrustedCertificateChain(certList, isDistrusted);
+    if (NS_FAILED(nsrv)) {
+      return Result::FATAL_ERROR_LIBRARY_FAILURE;
+    }
+    if (isDistrusted) {
+      return Result::ERROR_UNTRUSTED_ISSUER;
+    }
+  }
+
   // See bug 1434300. If the root is a Symantec root, see if we distrust this
   // path. Since we already have the root available, we can check that cheaply
   // here before proceeding with the rest of the algorithm.
 
   // This algorithm only applies if we are verifying in the context of a TLS
   // handshake. To determine this, we check mHostname: If it isn't set, this is
   // not TLS, so don't run the algorithm.
   if (mHostname && CertDNIsInList(root.get(), RootSymantecDNs) &&
--- a/security/nss.symbols
+++ b/security/nss.symbols
@@ -152,16 +152,17 @@ CERT_SignedCrlTemplate @DATA@
 CERT_SignedDataTemplate @DATA@
 CERT_StartCertExtensions
 CERT_StartCertificateRequestAttributes
 CERT_SubjectPublicKeyInfoTemplate @DATA@
 CERT_TimeChoiceTemplate @DATA@
 CERT_VerifyCertificate
 CERT_VerifySignedDataWithPublicKeyInfo
 DER_AsciiToTime_Util
+DER_DecodeTimeChoice
 DER_DecodeTimeChoice_Util
 DER_Encode
 DER_EncodeTimeChoice_Util
 DER_Encode_Util
 DER_GeneralizedTimeToTime
 DER_GeneralizedTimeToTime_Util
 DER_GetInteger
 DER_GetInteger_Util