Adding OCSP Verification

net/cert/internal/ocsp.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "net/cert/internal/ocsp.h"
+
 #include <algorithm>
 
 #include "base/sha1.h"
 #include "crypto/sha2.h"
-#include "net/cert/internal/parse_ocsp.h"
+#include "net/cert/internal/extended_key_usage.h"
+#include "net/cert/internal/verify_name_match.h"
+#include "net/cert/internal/verify_signed_data.h"
 
 namespace net {
 
 OCSPCertID::OCSPCertID() {}
 OCSPCertID::~OCSPCertID() {}
 
 OCSPSingleResponse::OCSPSingleResponse() {}
 OCSPSingleResponse::~OCSPSingleResponse() {}
 
 OCSPResponseData::OCSPResponseData() {}
 OCSPResponseData::~OCSPResponseData() {}
 
 OCSPResponse::OCSPResponse() {}
 OCSPResponse::~OCSPResponse() {}
 
-der::Input BasicOCSPResponseOid() {
+WARN_UNUSED_RESULT der::Input BasicOCSPResponseOid() {
   // From RFC 6960:
   //
   // id-pkix-ocsp           OBJECT IDENTIFIER ::= { id-ad-ocsp }
   // id-pkix-ocsp-basic     OBJECT IDENTIFIER ::= { id-pkix-ocsp 1 }
   //
   // In dotted notation: 1.3.6.1.5.5.7.48.1.1
   static const uint8_t oid[] = {0x2b, 0x06, 0x01, 0x05, 0x05,
                                 0x07, 0x30, 0x01, 0x01};
   return der::Input(oid);
 }
 
 // CertID ::= SEQUENCE {
 //    hashAlgorithm           AlgorithmIdentifier,
 //    issuerNameHash          OCTET STRING, -- Hash of issuer's DN
 //    issuerKeyHash           OCTET STRING, -- Hash of issuer's public key
 //    serialNumber            CertificateSerialNumber
 // }
-bool ParseOCSPCertID(const der::Input& raw_tlv, OCSPCertID* out) {
+WARN_UNUSED_RESULT bool ParseOCSPCertID(const der::Input& raw_tlv,
+                                        OCSPCertID* out) {
   der::Parser outer_parser(raw_tlv);
   der::Parser parser;
   if (!outer_parser.ReadSequence(&parser))
     return false;
   if (outer_parser.HasMore())
     return false;
 
   der::Input sigalg_tlv;
   if (!parser.ReadRawTLV(&sigalg_tlv))
     return false;
@@ skipping 14 matching lines @@
 namespace {
 
 // Parses |raw_tlv| to extract an OCSP RevokedInfo (RFC 6960) and stores the
 // result in the OCSPCertStatus |out|. Returns whether the parsing was
 // successful.
 //
 // RevokedInfo ::= SEQUENCE {
 //      revocationTime              GeneralizedTime,
 //      revocationReason    [0]     EXPLICIT CRLReason OPTIONAL
 // }
-bool ParseRevokedInfo(const der::Input& raw_tlv, OCSPCertStatus* out) {
+WARN_UNUSED_RESULT bool ParseRevokedInfo(const der::Input& raw_tlv,
+                                         OCSPCertStatus* out) {
   der::Parser parser(raw_tlv);
   if (!parser.ReadGeneralizedTime(&(out->revocation_time)))
     return false;
 
   der::Input reason_input;
   if (!parser.ReadOptionalTag(der::ContextSpecificConstructed(0), &reason_input,
                               &(out->has_reason))) {
     return false;
   }
   if (out->has_reason) {
@@ skipping 22 matching lines @@
 // result in the OCSPCertStatus |out|. Returns whether the parsing was
 // successful.
 //
 // CertStatus ::= CHOICE {
 //      good        [0]     IMPLICIT NULL,
 //      revoked     [1]     IMPLICIT RevokedInfo,
 //      unknown     [2]     IMPLICIT UnknownInfo
 // }
 //
 // UnknownInfo ::= NULL
-bool ParseCertStatus(const der::Input& raw_tlv, OCSPCertStatus* out) {
+WARN_UNUSED_RESULT bool ParseCertStatus(const der::Input& raw_tlv,
+                                        OCSPCertStatus* out) {
   der::Parser parser(raw_tlv);
   der::Tag status_tag;
   der::Input status;
   if (!parser.ReadTagAndValue(&status_tag, &status))
     return false;
 
   out->has_reason = false;
   if (status_tag == der::ContextSpecificPrimitive(0)) {
     out->status = OCSPCertStatus::Status::GOOD;
   } else if (status_tag == der::ContextSpecificConstructed(1)) {
@@ skipping 11 matching lines @@
 
 }  // namespace
 
 // SingleResponse ::= SEQUENCE {
 //      certID                       CertID,
 //      certStatus                   CertStatus,
 //      thisUpdate                   GeneralizedTime,
 //      nextUpdate         [0]       EXPLICIT GeneralizedTime OPTIONAL,
 //      singleExtensions   [1]       EXPLICIT Extensions OPTIONAL
 // }
-bool ParseOCSPSingleResponse(const der::Input& raw_tlv,
-                             OCSPSingleResponse* out) {
+WARN_UNUSED_RESULT bool ParseOCSPSingleResponse(const der::Input& raw_tlv,
+                                                OCSPSingleResponse* out) {
   der::Parser outer_parser(raw_tlv);
   der::Parser parser;
   if (!outer_parser.ReadSequence(&parser))
     return false;
   if (outer_parser.HasMore())
     return false;
 
   if (!parser.ReadRawTLV(&(out->cert_id_tlv)))
     return false;
   der::Input status_tlv;
@@ skipping 27 matching lines @@
 
 namespace {
 
 // Parses |raw_tlv| to extract a ResponderID (RFC 6960) and stores the
 // result in the ResponderID |out|. Returns whether the parsing was successful.
 //
 // ResponderID ::= CHOICE {
 //      byName               [1] Name,
 //      byKey                [2] KeyHash
 // }
-bool ParseResponderID(const der::Input& raw_tlv,
-                      OCSPResponseData::ResponderID* out) {
+WARN_UNUSED_RESULT bool ParseResponderID(const der::Input& raw_tlv,
+                                         OCSPResponseData::ResponderID* out) {
   der::Parser parser(raw_tlv);
   der::Tag id_tag;
   der::Input id_input;
   if (!parser.ReadTagAndValue(&id_tag, &id_input))
     return false;
 
   if (id_tag == der::ContextSpecificConstructed(1)) {
     out->type = OCSPResponseData::ResponderType::NAME;
     out->name = id_input;
   } else if (id_tag == der::ContextSpecificConstructed(2)) {
@@ skipping 18 matching lines @@
 
 }  // namespace
 
 // ResponseData ::= SEQUENCE {
 //      version              [0] EXPLICIT Version DEFAULT v1,
 //      responderID              ResponderID,
 //      producedAt               GeneralizedTime,
 //      responses                SEQUENCE OF SingleResponse,
 //      responseExtensions   [1] EXPLICIT Extensions OPTIONAL
 // }
-bool ParseOCSPResponseData(const der::Input& raw_tlv, OCSPResponseData* out) {
+WARN_UNUSED_RESULT bool ParseOCSPResponseData(const der::Input& raw_tlv,
+                                              OCSPResponseData* out) {
   der::Parser outer_parser(raw_tlv);
   der::Parser parser;
   if (!outer_parser.ReadSequence(&parser))
     return false;
   if (outer_parser.HasMore())
     return false;
 
   der::Input version_input;
   bool version_present;
   if (!parser.ReadOptionalTag(der::ContextSpecificConstructed(0),
@@ skipping 49 matching lines @@
 // Parses |raw_tlv| to extract a BasicOCSPResponse (RFC 6960) and stores the
 // result in the OCSPResponse |out|. Returns whether the parsing was
 // successful.
 //
 // BasicOCSPResponse       ::= SEQUENCE {
 //      tbsResponseData      ResponseData,
 //      signatureAlgorithm   AlgorithmIdentifier,
 //      signature            BIT STRING,
 //      certs            [0] EXPLICIT SEQUENCE OF Certificate OPTIONAL
 // }
-bool ParseBasicOCSPResponse(const der::Input& raw_tlv, OCSPResponse* out) {
+WARN_UNUSED_RESULT bool ParseBasicOCSPResponse(const der::Input& raw_tlv,
+                                               OCSPResponse* out) {
   der::Parser outer_parser(raw_tlv);
   der::Parser parser;
   if (!outer_parser.ReadSequence(&parser))
     return false;
   if (outer_parser.HasMore())
     return false;
 
   if (!parser.ReadRawTLV(&(out->data)))
     return false;
   der::Input sigalg_tlv;
@@ skipping 33 matching lines @@
 
 // OCSPResponse ::= SEQUENCE {
 //      responseStatus         OCSPResponseStatus,
 //      responseBytes          [0] EXPLICIT ResponseBytes OPTIONAL
 // }
 //
 // ResponseBytes ::=       SEQUENCE {
 //      responseType   OBJECT IDENTIFIER,
 //      response       OCTET STRING
 // }
-bool ParseOCSPResponse(const der::Input& raw_tlv, OCSPResponse* out) {
+WARN_UNUSED_RESULT bool ParseOCSPResponse(const der::Input& raw_tlv,
+                                          OCSPResponse* out) {
   der::Parser outer_parser(raw_tlv);
   der::Parser parser;
   if (!outer_parser.ReadSequence(&parser))
     return false;
   if (outer_parser.HasMore())
     return false;
 
   der::Input response_status_input;
   uint8_t response_status;
   if (!parser.ReadTag(der::kEnumerated, &response_status_input))
@@ skipping 36 matching lines @@
     if (bytes_parser.HasMore())
       return false;
   }
 
   return !parser.HasMore();
 }
 
 namespace {
 
 // Checks that the |type| hash of |value| is equal to |hash|
-bool VerifyHash(HashValueTag type,
-                const der::Input& hash,
-                const der::Input& value) {
+WARN_UNUSED_RESULT bool VerifyHash(HashValueTag type,
+                                   const der::Input& hash,
+                                   const der::Input& value) {
   HashValue target(type);
   if (target.size() != hash.Length())
     return false;
   memcpy(target.data(), hash.UnsafeData(), target.size());
 
   HashValue value_hash(type);
   if (type == HASH_VALUE_SHA1) {
     base::SHA1HashBytes(value.UnsafeData(), value.Length(), value_hash.data());
   } else if (type == HASH_VALUE_SHA256) {
     std::string hash_string = crypto::SHA256HashString(value.AsString());
     memcpy(value_hash.data(), hash_string.data(), value_hash.size());
   } else {
     return false;
   }
 
   return target.Equals(value_hash);
 }
 
 // Checks that the input |id_tlv| parses to a valid CertID and matches the
 // issuer |issuer| name and key, as well as the serial number |serial_number|.
-bool CheckCertID(const der::Input& id_tlv,
-                 const ParsedTbsCertificate& certificate,
-                 const ParsedTbsCertificate& issuer,
-                 const der::Input& serial_number) {
+WARN_UNUSED_RESULT bool CheckCertID(const der::Input& id_tlv,
+                                    const ParsedTbsCertificate& certificate,
+                                    const ParsedTbsCertificate& issuer,
+                                    const der::Input& serial_number) {
   OCSPCertID id;
   if (!ParseOCSPCertID(id_tlv, &id))
     return false;
 
   HashValueTag type = HASH_VALUE_SHA1;
   switch (id.hash_algorithm) {
     case DigestAlgorithm::Sha1:
       type = HASH_VALUE_SHA1;
       break;
     case DigestAlgorithm::Sha256:
@@ skipping 25 matching lines @@
     return false;
   der::Input key_tlv = key_bits.bytes();
   if (!VerifyHash(type, id.issuer_key_hash, key_tlv))
     return false;
 
   return id.serial_number == serial_number;
 }
 
 }  // namespace
 
-bool GetOCSPCertStatus(const OCSPResponseData& response_data,
-                       const ParsedCertificate& issuer,
-                       const ParsedCertificate& cert,
-                       OCSPCertStatus* out) {
+WARN_UNUSED_RESULT bool GetOCSPCertStatus(const OCSPResponseData& response_data,
+                                          const ParsedCertificate& issuer,
+                                          const ParsedCertificate& cert,
+                                          OCSPCertStatus* out) {
   out->status = OCSPCertStatus::Status::GOOD;
 
   ParsedTbsCertificate tbs_cert;
   if (!ParseTbsCertificate(cert.tbs_certificate_tlv, &tbs_cert))
     return false;
   ParsedTbsCertificate issuer_tbs_cert;
   if (!ParseTbsCertificate(issuer.tbs_certificate_tlv, &issuer_tbs_cert))
     return false;
 
   bool found = false;
@@ skipping 13 matching lines @@
       }
     }
   }
 
   if (!found)
     out->status = OCSPCertStatus::Status::UNKNOWN;
 
   return found;
 }
 
+namespace {
+
+// Checks that the ResponderID |id| matches the certificate |cert| either
+// by verifying the name matches that of the certificate or that the hash
+// matches the certificate's public key hash (RFC 6960, 4.2.2.3).
+WARN_UNUSED_RESULT bool CheckResponder(const OCSPResponseData::ResponderID& id,

[eroman, 2016/05/31 19:12:46]

Can you use a more specific name -- how about ResponderIdMatches(ID, Cert), to
reflect what check on the responder it is doing? (In particular it was confusing
when seeing the caller, as I thought the actual responder check that the "else"
branch was doing would go here).

+                                       const ParsedTbsCertificate& cert) {
+  if (id.type == OCSPResponseData::ResponderType::NAME) {
+    der::Input name_rdn;
+    der::Input cert_rdn;
+    if (!der::Parser(id.name).ReadTag(der::kSequence, &name_rdn) ||

[eroman, 2016/05/31 19:12:46]

Can you add documentation for ResponderID indicating this assumption? (i.e. that
ResponderID after parsing is a single SEQUENCE?)

+        !der::Parser(cert.subject_tlv).ReadTag(der::kSequence, &cert_rdn))
+      return false;
+    return VerifyNameMatch(name_rdn, cert_rdn);
+  }

[eroman, 2016/05/31 19:12:46]

For future proofing my preference is a switch(). But could also use this
structure and add an else or a DCHECK() for the remaining possibility for
id.type.

+
+  // Otherwise we extract the SPKI and compare the public key hash to key

[eroman, 2016/05/31 19:12:46]

avoid "we" in comments.

+  // in the ResponderID.
+  der::Parser parser(cert.spki_tlv);
+  der::Parser spki_parser;
+  der::BitString key_bits;
+  if (!parser.ReadSequence(&spki_parser))
+    return false;
+  if (!spki_parser.SkipTag(der::kSequence))
+    return false;
+  if (!spki_parser.ReadBitString(&key_bits))
+    return false;
+
+  der::Input key = key_bits.bytes();
+  HashValue key_hash(HASH_VALUE_SHA1);
+  base::SHA1HashBytes(key.UnsafeData(), key.Length(), key_hash.data());
+  return key_hash.Equals(id.key_hash);
+}
+
+}  // namespace
+
+WARN_UNUSED_RESULT bool VerifyOCSPResponse(
+    const OCSPResponse& response,
+    const ParsedCertificate& issuer_cert,
+    const SignaturePolicy& signature_policy) {
+  OCSPResponseData response_data;
+  if (!ParseOCSPResponseData(response.data, &response_data))
+    return false;
+
+  ParsedTbsCertificate issuer;
+  ParsedTbsCertificate responder;
+  if (!ParseTbsCertificate(issuer_cert.tbs_certificate_tlv, &issuer))
+    return false;
+
+  // In order to verify the OCSP signature, a valid responder matching the OCSP
+  // Responder ID must be located (RFC 6960, 4.2.2.2). The responder is allowed
+  // to be either the certificate issuer or a delegated authority directly
+  // signed by the issuer.
+  if (CheckResponder(response_data.responder_id, issuer)) {
+    responder = issuer;
+  } else {
+    bool found = false;
+    // If the authority to sign OCSP responses has been delegated to another
+    // authority, the OCSP Response will contain the responder certificate.

[eroman, 2016/05/31 19:12:46]

Is there any normative reference for this logic?

I couldn't find it in 6960 at least, which just said:

   The responder MAY include certificates in the certs field of
   BasicOCSPResponse that help the OCSP client verify the responder's
   signature.

Which says nothing about their ordering, validity, or interpretation.

This code checks only signatures of certificates (nothing else, including
presence of critical unsupported extensions).

Also, should it permit a chain of certs from the issuer to the responder? (here
the responder is assumed to be directly signed by issuer).

+    for (const auto& responder_cert_tlv : response.certs) {
+      ParsedCertificate responder_cert;
+      ParsedTbsCertificate tbs_cert;
+      if (!ParseCertificate(responder_cert_tlv, &responder_cert))
+        return false;
+      if (!ParseTbsCertificate(responder_cert.tbs_certificate_tlv, &tbs_cert))
+        return false;
+
+      // The OCSP Responder ID is checked against each provided certificate to
+      // determine whether it is the correct Authorized Responder.
+      if (CheckResponder(response_data.responder_id, tbs_cert)) {
+        found = true;
+        responder = tbs_cert;
+
+        // The Authorized Responder must be directly signed by the issuer of the
+        // certificate being checked.
+        std::unique_ptr<SignatureAlgorithm> signature_algorithm =
+            SignatureAlgorithm::CreateFromDer(
+                responder_cert.signature_algorithm_tlv);
+        if (!signature_algorithm)
+          return false;
+        der::Input issuer_spki = issuer.spki_tlv;
+        if (!VerifySignedData(*signature_algorithm,
+                              responder_cert.tbs_certificate_tlv,
+                              responder_cert.signature_value, issuer_spki,
+                              &signature_policy)) {
+          return false;
+        }
+
+        // The Authorized Responder must include the value id-kp-OCSPSigning as
+        // part of the extended key usage extension.
+        std::map<der::Input, ParsedExtension> extensions;
+        std::vector<der::Input> eku;
+        if (!ParseExtensions(responder.extensions_tlv, &extensions))
+          return false;
+        if (!ParseEKUExtension(extensions[ExtKeyUsageOid()].value, &eku))
+          return false;
+        if (std::find(eku.begin(), eku.end(), OCSPSigning()) == eku.end())
+          return false;
+
+        // TODO(svaldez): Add Revocation Checking of Authorized Responder.
+        break;

[eroman, 2016/05/31 19:12:46]

Can you pull this out to a separate function instead of the "break" and "found"
logic?

+      }
+    }
+    if (!found)
+      return false;
+  }
+  // Once a valid responder has been found, the signature of the OCSP
+  // response is verified against the public key of the responder.
+  return VerifySignedData(*(response.signature_algorithm), response.data,
+                          response.signature, responder.spki_tlv,
+                          &signature_policy);
+}
+
 }  // namespace net