Bug 966856 - mozilla::pkix: support SHA-2 hashes in CertIDs in OCSP responses r=jschanck,djacksonNSS_3_72_1_RTM

Differential Revision: https://phabricator.services.mozilla.com/D133706

6 files changed, 194 insertions, 58 deletions

diff --git a/gtests/mozpkix_gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp b/gtests/mozpkix_gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp
index 58336dfae..c7e82368d 100644
--- a/gtests/mozpkix_gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp
+++ b/gtests/mozpkix_gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp
@@ -217,9 +217,13 @@ public:
                     const TestSignatureAlgorithm& signatureAlgorithm,
        /*optional*/ const ByteString* certs = nullptr,
        /*optional*/ OCSPResponseExtension* singleExtensions = nullptr,
-       /*optional*/ OCSPResponseExtension* responseExtensions = nullptr)
+       /*optional*/ OCSPResponseExtension* responseExtensions = nullptr,
+       /*optional*/ DigestAlgorithm certIDHashAlgorithm = DigestAlgorithm::sha1,
+       /*optional*/ ByteString certIDHashAlgorithmEncoded = ByteString())
   {
     OCSPResponseContext context(certID, producedAt);
+    context.certIDHashAlgorithm = certIDHashAlgorithm;
+    context.certIDHashAlgorithmEncoded = certIDHashAlgorithmEncoded;
     if (signerName) {
       context.signerNameDER = CNToDERName(signerName);
       EXPECT_FALSE(ENCODING_FAILED(context.signerNameDER));
@@ -466,6 +470,72 @@ TEST_F(pkixocsp_VerifyEncodedResponse_successful, ct_extension)
             trustDomain.signedCertificateTimestamps);
 }
 
+struct CertIDHashAlgorithm
+{
+  DigestAlgorithm hashAlgorithm;
+  ByteString encodedHashAlgorithm;
+  Result expectedResult;
+};
+
+// python DottedOIDToCode.py --alg id-sha1 1.3.14.3.2.26
+static const uint8_t alg_id_sha1[] = {
+  0x30, 0x07, 0x06, 0x05, 0x2b, 0x0e, 0x03, 0x02, 0x1a
+};
+// python DottedOIDToCode.py --alg id-sha256 2.16.840.1.101.3.4.2.1
+static const uint8_t alg_id_sha256[] = {
+  0x30, 0x0b, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01
+};
+static const uint8_t not_an_encoded_hash_oid[] = {
+  0x01, 0x02, 0x03, 0x04
+};
+
+static const CertIDHashAlgorithm CERTID_HASH_ALGORITHMS[] = {
+  { DigestAlgorithm::sha1, ByteString(), Success },
+  { DigestAlgorithm::sha256, ByteString(), Success },
+  { DigestAlgorithm::sha384, ByteString(), Success },
+  { DigestAlgorithm::sha512, ByteString(), Success },
+  { DigestAlgorithm::sha256, BytesToByteString(alg_id_sha1),
+    Result::ERROR_OCSP_MALFORMED_RESPONSE },
+  { DigestAlgorithm::sha1, BytesToByteString(alg_id_sha256),
+    Result::ERROR_OCSP_MALFORMED_RESPONSE },
+  { DigestAlgorithm::sha1, BytesToByteString(not_an_encoded_hash_oid),
+    Result::ERROR_OCSP_MALFORMED_RESPONSE },
+};
+
+class pkixocsp_VerifyEncodedResponse_CertIDHashAlgorithm
+  : public pkixocsp_VerifyEncodedResponse_successful
+  , public ::testing::WithParamInterface<CertIDHashAlgorithm>
+{
+};
+
+TEST_P(pkixocsp_VerifyEncodedResponse_CertIDHashAlgorithm, CertIDHashAlgorithm)
+{
+  ByteString responseString(
+               CreateEncodedOCSPSuccessfulResponse(
+                         OCSPResponseContext::good, *endEntityCertID, byKey,
+                         *rootKeyPair, oneDayBeforeNow,
+                         oneDayBeforeNow, &oneDayAfterNow,
+                         sha256WithRSAEncryption(),
+                         nullptr,
+                         nullptr,
+                         nullptr,
+                         GetParam().hashAlgorithm,
+                         GetParam().encodedHashAlgorithm));
+  Input response;
+  ASSERT_EQ(Success,
+            response.Init(responseString.data(), responseString.length()));
+  bool expired;
+  ASSERT_EQ(GetParam().expectedResult,
+            VerifyEncodedOCSPResponse(trustDomain, *endEntityCertID,
+                                      Now(), END_ENTITY_MAX_LIFETIME_IN_DAYS,
+                                      response, expired));
+  ASSERT_FALSE(expired);
+}
+
+INSTANTIATE_TEST_SUITE_P(pkixocsp_VerifyEncodedResponse_CertIDHashAlgorithm,
+                        pkixocsp_VerifyEncodedResponse_CertIDHashAlgorithm,
+                        testing::ValuesIn(CERTID_HASH_ALGORITHMS));
+
 ///////////////////////////////////////////////////////////////////////////////
 // indirect responses (signed by a delegated OCSP responder cert)
 
diff --git a/lib/mozpkix/include/pkix-test/pkixtestutil.h b/lib/mozpkix/include/pkix-test/pkixtestutil.h
index 55c435419..70c0fee94 100644
--- a/lib/mozpkix/include/pkix-test/pkixtestutil.h
+++ b/lib/mozpkix/include/pkix-test/pkixtestutil.h
@@ -349,6 +349,14 @@ class OCSPResponseContext final {
   OCSPResponseContext(const CertID& certID, std::time_t time);
 
   const CertID& certID;
+  // What digest algorithm to use to produce issuerNameHash and issuerKeyHash.
+  // Defaults to sha1.
+  DigestAlgorithm certIDHashAlgorithm;
+  // If non-empty, the sequence of bytes to use for hashAlgorithm when encoding
+  // this response. If empty, the sequence of bytes corresponding to
+  // certIDHashAlgorithm will be used. Defaults to empty.
+  ByteString certIDHashAlgorithmEncoded;
+
   // TODO(bug 980538): add a way to specify what certificates are included.
 
   // The fields below are in the order that they appear in an OCSP response.
diff --git a/lib/mozpkix/include/pkix/pkixutil.h b/lib/mozpkix/include/pkix/pkixutil.h
index ca5b5a2d7..b224aa1f7 100644
--- a/lib/mozpkix/include/pkix/pkixutil.h
+++ b/lib/mozpkix/include/pkix/pkixutil.h
@@ -259,6 +259,20 @@ Result CheckSubjectPublicKeyInfo(Input subjectPublicKeyInfo,
 #else
 #error Unsupported compiler for MOZILLA_PKIX_UNREACHABLE_DEFAULT.
 #endif
+
+inline size_t DigestAlgorithmToSizeInBytes(DigestAlgorithm digestAlgorithm) {
+  switch (digestAlgorithm) {
+    case DigestAlgorithm::sha1:
+      return 160 / 8;
+    case DigestAlgorithm::sha256:
+      return 256 / 8;
+    case DigestAlgorithm::sha384:
+      return 384 / 8;
+    case DigestAlgorithm::sha512:
+      return 512 / 8;
+      MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
+  }
+}
 }
 }  // namespace mozilla::pkix
 
diff --git a/lib/mozpkix/lib/pkixocsp.cpp b/lib/mozpkix/lib/pkixocsp.cpp
index a81154417..d725e55ac 100644
--- a/lib/mozpkix/lib/pkixocsp.cpp
+++ b/lib/mozpkix/lib/pkixocsp.cpp
@@ -28,12 +28,6 @@
 #include "mozpkix/pkixcheck.h"
 #include "mozpkix/pkixutil.h"
 
-namespace {
-
-const size_t SHA1_DIGEST_LENGTH = 160 / 8;
-
-} // namespace
-
 namespace mozilla { namespace pkix {
 
 // These values correspond to the tag values in the ASN.1 CertStatus
@@ -181,10 +175,12 @@ static inline Result MatchCertID(Reader& input,
                                  const Context& context,
                                  /*out*/ bool& match);
 static Result MatchKeyHash(TrustDomain& trustDomain,
+                           DigestAlgorithm hashAlgorithm,
                            Input issuerKeyHash,
                            Input issuerSubjectPublicKeyInfo,
                            /*out*/ bool& match);
 static Result KeyHash(TrustDomain& trustDomain,
+                      DigestAlgorithm hashAlgorithm,
                       Input subjectPublicKeyInfo,
                       /*out*/ uint8_t* hashBuf, size_t hashBufSize);
 
@@ -213,7 +209,7 @@ MatchResponderID(TrustDomain& trustDomain,
       if (rv != Success) {
         return rv;
       }
-      return MatchKeyHash(trustDomain, keyHash,
+      return MatchKeyHash(trustDomain, DigestAlgorithm::sha1, keyHash,
                           potentialSignerSubjectPublicKeyInfo, match);
     }
 
@@ -741,36 +737,36 @@ MatchCertID(Reader& input, const Context& context, /*out*/ bool& match)
     return Success;
   }
 
-  // TODO: support SHA-2 hashes.
-
-  if (hashAlgorithm != DigestAlgorithm::sha1) {
-    // Again, not interested in this response. Consume input, return success.
-    input.SkipToEnd();
-    return Success;
-  }
-
-  if (issuerNameHash.GetLength() != SHA1_DIGEST_LENGTH) {
+  size_t hashAlgorithmLength = DigestAlgorithmToSizeInBytes(hashAlgorithm);
+  if (issuerNameHash.GetLength() != hashAlgorithmLength) {
     return Result::ERROR_OCSP_MALFORMED_RESPONSE;
   }
 
   // From http://tools.ietf.org/html/rfc6960#section-4.1.1:
   // "The hash shall be calculated over the DER encoding of the
   // issuer's name field in the certificate being checked."
-  uint8_t hashBuf[SHA1_DIGEST_LENGTH];
+  uint8_t hashBuf[MAX_DIGEST_SIZE_IN_BYTES];
+  if (hashAlgorithmLength > sizeof(hashBuf)) {
+    return Result::FATAL_ERROR_LIBRARY_FAILURE;
+  }
   rv = context.trustDomain.DigestBuf(context.certID.issuer,
-                                     DigestAlgorithm::sha1, hashBuf,
-                                     sizeof(hashBuf));
+                                     hashAlgorithm, hashBuf,
+                                     hashAlgorithmLength);
+  if (rv != Success) {
+    return rv;
+  }
+  Input computed;
+  rv = computed.Init(hashBuf, hashAlgorithmLength);
   if (rv != Success) {
     return rv;
   }
-  Input computed(hashBuf);
   if (!InputsAreEqual(computed, issuerNameHash)) {
     // Again, not interested in this response. Consume input, return success.
     input.SkipToEnd();
     return Success;
   }
 
-  return MatchKeyHash(context.trustDomain, issuerKeyHash,
+  return MatchKeyHash(context.trustDomain, hashAlgorithm, issuerKeyHash,
                       context.certID.issuerSubjectPublicKeyInfo, match);
 }
 
@@ -784,30 +780,53 @@ MatchCertID(Reader& input, const Context& context, /*out*/ bool& match)
 //                          -- BIT STRING subjectPublicKey [excluding
 //                          -- the tag, length, and number of unused
 //                          -- bits] in the responder's certificate)
+//
+// From https://datatracker.ietf.org/doc/html/rfc6960#section-4.1.1:
+//    CertID          ::=     SEQUENCE {
+//        hashAlgorithm       AlgorithmIdentifier,
+//        issuerNameHash      OCTET STRING, -- Hash of issuer's DN
+//        issuerKeyHash       OCTET STRING, -- Hash of issuer's public key
+//        serialNumber        CertificateSerialNumber }
+// ...
+//    o  hashAlgorithm is the hash algorithm used to generate the
+//       issuerNameHash and issuerKeyHash values.
+// ...
+//    o  issuerKeyHash is the hash of the issuer's public key.  The hash
+//       shall be calculated over the value (excluding tag and length) of
+//       the subject public key field in the issuer's certificate.
 static Result
-MatchKeyHash(TrustDomain& trustDomain, Input keyHash,
-             const Input subjectPublicKeyInfo, /*out*/ bool& match)
+MatchKeyHash(TrustDomain& trustDomain, DigestAlgorithm hashAlgorithm,
+             Input keyHash, const Input subjectPublicKeyInfo,
+             /*out*/ bool& match)
 {
-  if (keyHash.GetLength() != SHA1_DIGEST_LENGTH)  {
+  size_t hashLength = DigestAlgorithmToSizeInBytes(hashAlgorithm);
+  if (keyHash.GetLength() != hashLength)  {
     return Result::ERROR_OCSP_MALFORMED_RESPONSE;
   }
-  uint8_t hashBuf[SHA1_DIGEST_LENGTH];
-  Result rv = KeyHash(trustDomain, subjectPublicKeyInfo, hashBuf,
-                      sizeof hashBuf);
+  uint8_t hashBuf[MAX_DIGEST_SIZE_IN_BYTES];
+  if (hashLength > MAX_DIGEST_SIZE_IN_BYTES) {
+    return Result::FATAL_ERROR_LIBRARY_FAILURE;
+  }
+  Result rv = KeyHash(trustDomain, hashAlgorithm, subjectPublicKeyInfo,
+                      hashBuf, hashLength);
+  if (rv != Success) {
+    return rv;
+  }
+  Input computed;
+  rv = computed.Init(hashBuf, hashLength);
   if (rv != Success) {
     return rv;
   }
-  Input computed(hashBuf);
   match = InputsAreEqual(computed, keyHash);
   return Success;
 }
 
-// TODO(bug 966856): support SHA-2 hashes
 Result
-KeyHash(TrustDomain& trustDomain, const Input subjectPublicKeyInfo,
-        /*out*/ uint8_t* hashBuf, size_t hashBufSize)
+KeyHash(TrustDomain& trustDomain, DigestAlgorithm hashAlgorithm,
+        const Input subjectPublicKeyInfo, /*out*/ uint8_t* hashBuf,
+        size_t hashBufSize)
 {
-  if (!hashBuf || hashBufSize != SHA1_DIGEST_LENGTH) {
+  if (!hashBuf || hashBufSize != DigestAlgorithmToSizeInBytes(hashAlgorithm)) {
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
 
@@ -840,8 +859,8 @@ KeyHash(TrustDomain& trustDomain, const Input subjectPublicKeyInfo,
     return rv;
   }
 
-  return trustDomain.DigestBuf(subjectPublicKey, DigestAlgorithm::sha1,
-                               hashBuf, hashBufSize);
+  return trustDomain.DigestBuf(subjectPublicKey, hashAlgorithm, hashBuf,
+                               hashBufSize);
 }
 
 Result
@@ -921,8 +940,6 @@ CreateEncodedOCSPRequest(TrustDomain& trustDomain, const struct CertID& certID,
   // and thus more likely to fit within the 255 byte limit for OCSP GET that
   // is specified in RFC 5019 Section 5.
 
-  // Bug 966856: Add the id-pkix-ocsp-pref-sig-algs extension.
-
   // Since we don't know whether the OCSP responder supports anything other
   // than SHA-1, we have no choice but to use SHA-1 for issuerNameHash and
   // issuerKeyHash.
@@ -986,7 +1003,8 @@ CreateEncodedOCSPRequest(TrustDomain& trustDomain, const struct CertID& certID,
   // reqCert.issuerKeyHash (OCTET STRING)
   *d++ = 0x04;
   *d++ = hashLen;
-  rv = KeyHash(trustDomain, certID.issuerSubjectPublicKeyInfo, d, hashLen);
+  rv = KeyHash(trustDomain, DigestAlgorithm::sha1,
+               certID.issuerSubjectPublicKeyInfo, d, hashLen);
   if (rv != Success) {
     return rv;
   }
diff --git a/lib/mozpkix/lib/pkixverify.cpp b/lib/mozpkix/lib/pkixverify.cpp
index 8ceb2c184..bec570de6 100644
--- a/lib/mozpkix/lib/pkixverify.cpp
+++ b/lib/mozpkix/lib/pkixverify.cpp
@@ -43,14 +43,7 @@ DigestSignedData(TrustDomain& trustDomain,
     return Result::ERROR_BAD_DER;
   }
 
-  size_t digestLen;
-  switch (signedDigest.digestAlgorithm) {
-    case DigestAlgorithm::sha512: digestLen = 512 / 8; break;
-    case DigestAlgorithm::sha384: digestLen = 384 / 8; break;
-    case DigestAlgorithm::sha256: digestLen = 256 / 8; break;
-    case DigestAlgorithm::sha1: digestLen = 160 / 8; break;
-    MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
-  }
+  size_t digestLen = DigestAlgorithmToSizeInBytes(signedDigest.digestAlgorithm);
   assert(digestLen <= sizeof(digestBuf));
 
   rv = trustDomain.DigestBuf(signedData.data, signedDigest.digestAlgorithm,
diff --git a/lib/mozpkix/test-lib/pkixtestutil.cpp b/lib/mozpkix/test-lib/pkixtestutil.cpp
index b1b89c07e..306f7a0e4 100644
--- a/lib/mozpkix/test-lib/pkixtestutil.cpp
+++ b/lib/mozpkix/test-lib/pkixtestutil.cpp
@@ -156,6 +156,8 @@ OCSPResponseExtension::OCSPResponseExtension()
 
 OCSPResponseContext::OCSPResponseContext(const CertID& aCertID, time_t time)
   : certID(aCertID)
+  , certIDHashAlgorithm(DigestAlgorithm::sha1)
+  , certIDHashAlgorithmEncoded(ByteString())
   , responseStatus(successful)
   , skipResponseBytes(false)
   , producedAt(time)
@@ -184,25 +186,26 @@ static ByteString CertID(OCSPResponseContext& context);
 static ByteString CertStatus(OCSPResponseContext& context);
 
 static ByteString
-SHA1(const ByteString& toHash)
+HASH(const ByteString& toHash, DigestAlgorithm digestAlgorithm)
 {
-  uint8_t digestBuf[20];
+  uint8_t digestBuf[MAX_DIGEST_SIZE_IN_BYTES];
   Input input;
   if (input.Init(toHash.data(), toHash.length()) != Success) {
     abort();
   }
-  Result rv = TestDigestBuf(input, DigestAlgorithm::sha1, digestBuf,
-                            sizeof(digestBuf));
+  size_t digestLen = DigestAlgorithmToSizeInBytes(digestAlgorithm);
+  assert(digestLen <= sizeof(digestBuf));
+  Result rv = TestDigestBuf(input, digestAlgorithm, digestBuf, digestLen);
   if (rv != Success) {
     abort();
   }
-  return ByteString(digestBuf, sizeof(digestBuf));
+  return ByteString(digestBuf, digestLen);
 }
 
 static ByteString
-HashedOctetString(const ByteString& bytes)
+HashedOctetString(const ByteString& bytes, DigestAlgorithm digestAlgorithm)
 {
-  ByteString digest(SHA1(bytes));
+  ByteString digest(HASH(bytes, digestAlgorithm));
   if (ENCODING_FAILED(digest)) {
     return ByteString();
   }
@@ -993,7 +996,7 @@ ResponderID(OCSPResponseContext& context)
 ByteString
 KeyHash(const ByteString& subjectPublicKey)
 {
-  return HashedOctetString(subjectPublicKey);
+  return HashedOctetString(subjectPublicKey, DigestAlgorithm::sha1);
 }
 
 // SingleResponse ::= SEQUENCE {
@@ -1050,7 +1053,7 @@ CertID(OCSPResponseContext& context)
 {
   ByteString issuerName(context.certID.issuer.UnsafeGetData(),
                         context.certID.issuer.GetLength());
-  ByteString issuerNameHash(HashedOctetString(issuerName));
+  ByteString issuerNameHash(HashedOctetString(issuerName, context.certIDHashAlgorithm));
   if (ENCODING_FAILED(issuerNameHash)) {
     return ByteString();
   }
@@ -1074,8 +1077,8 @@ CertID(OCSPResponseContext& context)
           != Success) {
       return ByteString();
     }
-    issuerKeyHash = KeyHash(ByteString(subjectPublicKey.UnsafeGetData(),
-                                       subjectPublicKey.GetLength()));
+    issuerKeyHash = HashedOctetString(ByteString(subjectPublicKey.UnsafeGetData(),
+        subjectPublicKey.GetLength()), context.certIDHashAlgorithm);
     if (ENCODING_FAILED(issuerKeyHash)) {
       return ByteString();
     }
@@ -1089,9 +1092,39 @@ CertID(OCSPResponseContext& context)
   static const uint8_t alg_id_sha1[] = {
     0x30, 0x07, 0x06, 0x05, 0x2b, 0x0e, 0x03, 0x02, 0x1a
   };
+  // python DottedOIDToCode.py --alg id-sha256 2.16.840.1.101.3.4.2.1
+  static const uint8_t alg_id_sha256[] = {
+    0x30, 0x0b, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01
+  };
+  // python DottedOIDToCode.py --alg id-sha384 2.16.840.1.101.3.4.2.2
+  static const uint8_t alg_id_sha384[] = {
+    0x30, 0x0b, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02
+  };
+  // python DottedOIDToCode.py --alg id-sha512 2.16.840.1.101.3.4.2.3
+  static const uint8_t alg_id_sha512[] = {
+    0x30, 0x0b, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03
+  };
 
   ByteString value;
-  value.append(alg_id_sha1, sizeof(alg_id_sha1));
+  if (!context.certIDHashAlgorithmEncoded.empty()) {
+    value.append(context.certIDHashAlgorithmEncoded);
+  } else {
+    switch (context.certIDHashAlgorithm) {
+      case DigestAlgorithm::sha1:
+        value.append(alg_id_sha1, sizeof(alg_id_sha1));
+        break;
+      case DigestAlgorithm::sha256:
+        value.append(alg_id_sha256, sizeof(alg_id_sha256));
+        break;
+      case DigestAlgorithm::sha384:
+        value.append(alg_id_sha384, sizeof(alg_id_sha384));
+        break;
+      case DigestAlgorithm::sha512:
+        value.append(alg_id_sha512, sizeof(alg_id_sha512));
+        break;
+      MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
+    }
+  }
   value.append(issuerNameHash);
   value.append(issuerKeyHash);
   value.append(serialNumber);