security/pkix: align pkix with nss' mozpkix bug966856 fix

security/pkix/include/pkix/pkixutil.h

@@ -241,6 +241,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
 

security/pkix/lib/pkixocsp.cpp

@@ -10,12 +10,6 @@
 #include "pkix/pkixcheck.h"
 #include "pkix/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
@@ -163,10 +157,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);
 
@@ -195,7 +191,7 @@ MatchResponderID(TrustDomain& trustDomain,
       if (rv != Success) {
         return rv;
       }
-      return MatchKeyHash(trustDomain, keyHash,
+      return MatchKeyHash(trustDomain, DigestAlgorithm::sha1, keyHash,
                           potentialSignerSubjectPublicKeyInfo, match);
     }
 
@@ -723,36 +719,36 @@ MatchCertID(Reader& input, const Context& context, /*out*/ bool& match)
     return Success;
   }
 
-  // TODO: support SHA-2 hashes.
+  size_t hashAlgorithmLength = DigestAlgorithmToSizeInBytes(hashAlgorithm);
-
+  if (issuerNameHash.GetLength() != hashAlgorithmLength) {
-  if (hashAlgorithm != DigestAlgorithm::sha1) {
-    // Again, not interested in this response. Consume input, return success.
-    input.SkipToEnd();
-    return Success;
-  }
-
-  if (issuerNameHash.GetLength() != SHA1_DIGEST_LENGTH) {
     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,
+                                     hashAlgorithm, hashBuf,
-                                     sizeof(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);
 }
 
@@ -766,30 +762,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,
+MatchKeyHash(TrustDomain& trustDomain, DigestAlgorithm hashAlgorithm,
-             const Input subjectPublicKeyInfo, /*out*/ bool& match)
+             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];
+  uint8_t hashBuf[MAX_DIGEST_SIZE_IN_BYTES];
-  Result rv = KeyHash(trustDomain, subjectPublicKeyInfo, hashBuf,
+  if (hashLength > MAX_DIGEST_SIZE_IN_BYTES) {
-                      sizeof hashBuf);
+    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,
+KeyHash(TrustDomain& trustDomain, DigestAlgorithm hashAlgorithm,
-        /*out*/ uint8_t* hashBuf, size_t hashBufSize)
+        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;
   }
 
@@ -822,8 +841,8 @@ KeyHash(TrustDomain& trustDomain, const Input subjectPublicKeyInfo,
     return rv;
   }
 
-  return trustDomain.DigestBuf(subjectPublicKey, DigestAlgorithm::sha1,
+  return trustDomain.DigestBuf(subjectPublicKey, hashAlgorithm, hashBuf,
-                               hashBuf, hashBufSize);
+                               hashBufSize);
 }
 
 Result
@@ -903,8 +922,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.
@@ -968,7 +985,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;
   }

security/pkix/lib/pkixverify.cpp

@@ -25,14 +25,7 @@ DigestSignedData(TrustDomain& trustDomain,
     return Result::ERROR_BAD_DER;
   }
 
-  size_t digestLen;
+  size_t digestLen = DigestAlgorithmToSizeInBytes(signedDigest.digestAlgorithm);
-  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
-  }
   assert(digestLen <= sizeof(digestBuf));
 
   rv = trustDomain.DigestBuf(signedData.data, signedDigest.digestAlgorithm,