Adding OCSP Parser

net/cert/internal/parse_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 <algorithm>
+
+#include "base/sha1.h"
+#include "crypto/sha2.h"
+#include "net/cert/internal/extended_key_usage.h"
+#include "net/cert/internal/parse_ocsp.h"
+#include "net/cert/internal/signature_policy.h"
+#include "net/cert/internal/verify_name_match.h"
+#include "net/cert/internal/verify_signed_data.h"
+
+namespace net {
+
+namespace {
+
+// Continues reading from |parser| to extract an OCSP CertStatus (RFC 6960)
+// and stores the result in the OCSPSingleResponse |out|. Returns whether
+// the parsing was successful.
+bool ParseOCSPStatus(der::Parser* parser, OCSPCertStatus* out) {

[eroman, 2016/02/13 00:56:50]

I would find it useful when reading this to duplicate the ASN.1 documentations
here, or inline. I realize it is duplicated with the header, but der/asn.1
parsing is incredibly subtle when written manually.

[svaldez, 2016/02/16 17:25:11]

Acknowledged.

+  der::Tag status_tag;
+  der::Input status;
+
+  if (!parser->ReadTagAndValue(&status_tag, &status))
+    return false;
+
+  if (status_tag == der::ContextSpecificPrimitive(0)) {
+    out->status = OCSPCertStatus::Status::GOOD;
+  } else if (status_tag == der::ContextSpecificConstructed(1)) {
+    out->status = OCSPCertStatus::Status::REVOKED;
+    der::Parser revoked_info_parser(status);
+    der::Input revocation_time;
+    der::Input revocation_reason_input;
+    if (!revoked_info_parser.ReadGeneralizedTime(&(out->revocation_time)))
+      return false;
+    if (!revoked_info_parser.ReadOptionalTag(der::ContextSpecificConstructed(0),
+                                             &revocation_reason_input,
+                                             &(out->has_reason))) {

[eroman, 2016/02/13 00:56:49]

How about initializing has_reason to false at the start of this function, to
catch potential abuses of the API.

The header documentation should also call out which fields are valid to access,
since they depend on the Status and are otherwise un-initialized.

[svaldez, 2016/02/16 17:25:11]

Done.

+      return false;
+    }
+    if (revoked_info_parser.HasMore())
+      return false;
+    if (!out->has_reason)
+      return true;

[eroman, 2016/02/13 00:56:49]

From a future-proofing perspective, earlier returning success scares me.

This looks correct, but I would prefer if the parsing of RevokedInfo was in its
own function, so it is more clear what this is skipping past.

[svaldez, 2016/02/16 17:25:11]

Done.

+    der::Parser reason_parser(revocation_reason_input);
+    der::Input revocation_reason;
+    if (!reason_parser.ReadTag(der::kEnumerated, &revocation_reason))

[eroman, 2016/02/13 00:56:49]

Why is this der::kEnumerated?

I assumed the definition for CRLReason comes from RFC 5912:

   CRLReason ::= INTEGER

In which case shouldn't the tag be 0x02.

Please provide the definition and reference for what you are using.

[svaldez, 2016/02/16 17:25:10]

CRLReason is defined as an ENUMERATED in RFC5912 (In the appendix) and
PKIX1Implicit-2009. There is errata for the section 4 definition.

+      return false;
+    uint8_t revocation_reason_value;
+    if (!der::ParseUint8(revocation_reason, &revocation_reason_value))
+      return false;
+    if (revocation_reason_value >=
+        (uint8_t)OCSPCertStatus::RevocationReason::REVOCATION_REASON_MAX) {

[eroman, 2016/02/13 00:56:49]

static_cast<>

[svaldez, 2016/02/16 17:25:10]

Done.

+      return false;
+    }
+    out->revocation_reason =
+        static_cast<OCSPCertStatus::RevocationReason>(revocation_reason_value);

[eroman, 2016/02/13 00:56:49]

I don't see that reason_parser.HasMore() is checked anywhere.
I believe unconsumed data in here should not be permitted.

[svaldez, 2016/02/16 17:25:11]

Done.

+  } else if (status_tag == der::ContextSpecificPrimitive(2)) {
+    out->status = OCSPCertStatus::Status::UNKNOWN;

[eroman, 2016/02/13 00:56:49]

For sanity it would be a good idea to reset all the unused fields for these
other cases.

[svaldez, 2016/02/16 17:25:10]

Done.

+  } else {
+    return false;
+  }
+
+  return true;
+}
+
+// Continues reading from |parser| to extract the ResponderID (RFC 6960) and
+// stores the result in |out|. Returns whether the parsing was successful.
+bool ParseOCSPResponder(der::Parser* parser,
+                        OCSPResponseData::ResponderID* out) {
+  der::Tag id_tag;
+  der::Input responder_id_input;
+  if (!parser->ReadTagAndValue(&id_tag, &responder_id_input))
+    return false;
+
+  OCSPResponseData::ResponderID responder_id;

[eroman, 2016/02/13 00:56:50]

Why this temporary rather than writing directly to |out| ?

[svaldez, 2016/02/16 17:25:11]

Done.

+  if (id_tag == der::ContextSpecificConstructed(1)) {
+    responder_id.type = OCSPResponseData::ResponderType::NAME;
+    responder_id.name = responder_id_input;
+  } else if (id_tag == der::ContextSpecificConstructed(2)) {
+    der::Parser key_parser(responder_id_input);
+    der::Input responder_key;
+    if (!key_parser.ReadTag(der::kOctetString, &responder_key))
+      return false;
+    if (key_parser.HasMore())
+      return false;
+
+    SHA1HashValue key_hash;
+    if (responder_key.Length() != sizeof(key_hash.data))
+      return false;
+    memcpy(key_hash.data, responder_key.UnsafeData(), base::kSHA1Length);

[eroman, 2016/02/13 00:56:49]

can you use sizeof(key_hash.data) instead of base::kSHA1Length ?

[svaldez, 2016/02/16 17:25:11]

Done.

+    responder_id.type = OCSPResponseData::ResponderType::KEY_HASH;
+    responder_id.key_hash = HashValue(key_hash);
+  } else {
+    return false;
+  }
+  *out = responder_id;
+  return true;
+}
+
+// Continues reading from |parser| to extract a BasicOCSPResponse (RFC 6960)
+// and stores the result in OCSPResponse |out|. Returns whether the parsing
+// was successful.
+bool ParseBasicOCSPResponse(der::Parser* parser, OCSPResponse* out) {
+  der::Input sigalg_tlv;
+  if (!parser->ReadRawTLV(&(out->data)))
+    return false;
+  if (!parser->ReadRawTLV(&sigalg_tlv))
+    return false;
+  out->signature_algorithm = SignatureAlgorithm::CreateFromDer(sigalg_tlv);
+  if (!out->signature_algorithm)
+    return false;
+
+  if (!parser->ReadBitString(&(out->signature)))
+    return false;
+  der::Input certs_input;
+  bool certs_present;
+  if (!parser->ReadOptionalTag(der::ContextSpecificConstructed(0), &certs_input,
+                               &certs_present)) {
+    return false;
+  }
+
+  if (parser->HasMore())

[eroman, 2016/02/13 00:56:50]

I think this would be more appropriate in the caller.

Based on the streaming API for this (consumes a BasicOCSPResponse from
|parser|), checking for remaining data seems like the responsibility of the
caller (ParseOCSPResponse) instead.

(If this function took a der::Input instead then it would make sense to verify
everything had been consumed).

[svaldez, 2016/02/16 17:25:10]

Done.

+    return false;
+
+  if (!certs_present)
+    return true;

[eroman, 2016/02/13 00:56:49]

Same comment as earlier - "return true" short cuicuits scare me, as they can
easily introduce bugs when code is added to a function. I would rather structure
this as an if statement, or smaller helper functions.

In fact the code below should be re-setting out->certs.clear() before looping.
In which case a return true here would effectively skip over that and be bad

[svaldez, 2016/02/16 17:25:10]

Done.

+  der::Parser certs_input_parser(certs_input);
+  der::Parser certs_parser;
+  if (!certs_input_parser.ReadSequence(&certs_parser))
+    return false;
+  if (certs_input_parser.HasMore())
+    return false;
+  while (certs_parser.HasMore()) {
+    der::Input cert_tlv;
+    if (!certs_parser.ReadRawTLV(&cert_tlv))
+      return false;
+    out->certs.push_back(cert_tlv);

[eroman, 2016/02/13 00:56:50]

Where is the array first initialized?

In other words, should there be a call to out->certs.clear() before entering
this loop?

That would be consistent with what you did for out->responses.clear() later in
this file, which I agree is a good approach.

[svaldez, 2016/02/16 17:25:10]

Done.

+  }
+
+  return true;
+}
+
+}  // namespace
+
+OCSPCertID::OCSPCertID() {}
+OCSPCertID::~OCSPCertID() {}
+
+OCSPSingleResponse::OCSPSingleResponse() {}
+OCSPSingleResponse::~OCSPSingleResponse() {}
+
+OCSPResponseData::OCSPResponseData() {}
+OCSPResponseData::~OCSPResponseData() {}
+
+OCSPResponse::OCSPResponse() {}
+OCSPResponse::~OCSPResponse() {}
+
+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);
+}
+
+bool ParseOCSPCertID(der::Input raw_tlv, OCSPCertID* out) {
+  der::Parser outer_parser(raw_tlv);
+  der::Parser parser;
+  if (!outer_parser.ReadSequence(&parser))
+    return false;
+  der::Input sigalg_tlv;
+  if (!parser.ReadRawTLV(&sigalg_tlv))
+    return false;
+  if (!ParseHashAlgorithm(sigalg_tlv, &(out->hash_algorithm)))
+    return false;
+  if (!parser.ReadTag(der::kOctetString, &(out->issuer_name_hash)))
+    return false;
+  if (!parser.ReadTag(der::kOctetString, &(out->issuer_key_hash)))
+    return false;
+  if (!parser.ReadTag(der::kInteger, &(out->serial_number)))

[eroman, 2016/02/13 00:56:50]

This is more lenient then the certificate serial number parsing we do, is that
intentional?

https://code.google.com/p/chromium/codesearch#chromium/src/net/cert/internal/...

[svaldez, 2016/02/16 17:25:11]

Done.

+    return false;
+  return !parser.HasMore();
+}
+
+bool ParseOCSPSingleResponse(der::Input raw_tlv, OCSPSingleResponse* out) {
+  der::Parser response_parser(raw_tlv);
+  der::Parser parser;
+  if (!response_parser.ReadSequence(&parser))
+    return false;
+  if (response_parser.HasMore())
+    return false;
+  if (!parser.ReadRawTLV(&(out->cert_id_tlv)))
+    return false;
+  if (!ParseOCSPStatus(&parser, &(out->cert_status)))
+    return false;
+  der::Input this_update;
+  if (!parser.ReadGeneralizedTime(&(out->this_update)))
+    return false;
+  der::Input next_update_input;
+  if (!parser.ReadOptionalTag(der::ContextSpecificConstructed(0),
+                              &next_update_input, &(out->has_next_update))) {
+    return false;
+  }
+
+  if (out->has_next_update) {
+    der::Parser next_update_parser(next_update_input);
+    der::Input next_update;
+    if (!next_update_parser.ReadGeneralizedTime(&(out->next_update)))
+      return false;
+    if (next_update_parser.HasMore())
+      return false;
+  }
+
+  if (!parser.ReadOptionalTag(der::ContextSpecificConstructed(1),
+                              &(out->extensions), &(out->has_extensions))) {
+    return false;
+  }
+
+  return !parser.HasMore();
+}
+
+bool ParseOCSPResponseData(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),
+                              &version_input, &version_present)) {
+    return false;
+  }
+
+  // We ignore the restriction that the DEFAULT Version v1 shouldn't be

[eroman, 2016/02/13 00:56:49]

Per previous comment, try to avoid "We" when possible (see style thread)

[svaldez, 2016/02/16 17:25:10]

Done.

+  // specified due to many implementations including it regardless of the DER
+  // spec.
+  if (version_present) {
+    der::Parser version_parser(version_input);
+    der::Input version;
+    if (!version_parser.ReadTag(der::kInteger, &version))
+      return false;
+    if (version_parser.HasMore())
+      return false;
+    if (!der::ParseUint8(version, &(out->version)))
+      return false;
+  } else {
+    out->version = 0;
+  }
+
+  if (!ParseOCSPResponder(&parser, &(out->responder_id)))
+    return false;
+
+  der::Input produced_at;
+  if (!parser.ReadGeneralizedTime(&(out->produced_at)))
+    return false;
+
+  der::Parser responses_parser;
+  if (!parser.ReadSequence(&responses_parser))
+    return false;
+  out->responses.clear();
+  while (responses_parser.HasMore()) {
+    der::Input single_response;
+    if (!responses_parser.ReadRawTLV(&single_response))
+      return false;
+    out->responses.push_back(single_response);
+  }
+
+  if (!parser.ReadOptionalTag(der::ContextSpecificConstructed(1),
+                              &(out->extensions), &(out->has_extensions))) {
+    return false;
+  }
+
+  return !parser.HasMore();
+}
+
+bool ParseOCSPResponse(der::Input ocsp_response, OCSPResponse* out) {

[eroman, 2016/02/13 00:56:49]

Would be useful to see the ASN.1 structure duplicated in comments here

[svaldez, 2016/02/16 17:25:11]

Done.

+  der::Parser parser(ocsp_response);
+  der::Input response_status;
+  der::Parser ocsp_response_parser;
+  if (!parser.ReadSequence(&ocsp_response_parser))
+    return false;
+  if (parser.HasMore())
+    return false;
+  if (!ocsp_response_parser.ReadTag(der::kEnumerated, &response_status))
+    return false;
+  uint8_t response_status_value;
+  if (!der::ParseUint8(response_status, &response_status_value))
+    return false;
+  if (response_status_value >=
+      (uint8_t)OCSPResponse::ResponseStatus::RESPONSE_STATUS_MAX) {

[eroman, 2016/02/13 00:56:49]

static_cast

[svaldez, 2016/02/16 17:25:10]

Done.

+    return false;
+  }
+  out->status =
+      static_cast<OCSPResponse::ResponseStatus>(response_status_value);
+  if (out->status != OCSPResponse::ResponseStatus::SUCCESSFUL)
+    return true;

[eroman, 2016/02/13 00:56:50]

Same comment as earlier regarding "return true;" short-circuits.

Also this doesn't seem right -- need to still validate the rest of the DER.

The RFC says:

   If the value of responseStatus is one of the error conditions, the
responseBytes
   field is not set.

So should be checking !ocsp_response_parser.HasMore() before succeeding.
Or if this is a "should" rather than a "must" condition, need to validate that
it is at least the right stuff following.

[svaldez, 2016/02/16 17:25:11]

Done.

+
+  der::Input response_type_oid;
+  der::Input response_string;
+  der::Parser response_bytes_input_parser;
+  der::Parser response_bytes_parser;
+  if (!ocsp_response_parser.ReadConstructed(der::ContextSpecificConstructed(0),

[eroman, 2016/02/13 00:56:50]

Is this allowed to be omitted in the case of success? (not sure what that would
mean, but based on the ASN.1 at least it is optional)

[svaldez, 2016/02/16 17:25:10]

responseBytes must be set if the status is SUCCESSFUL.

+                                            &response_bytes_input_parser)) {
+    return false;
+  }
+  if (ocsp_response_parser.HasMore())
+    return false;
+  if (!response_bytes_input_parser.ReadSequence(&response_bytes_parser))

[eroman, 2016/02/13 00:56:49]

I would find it easier to read if each sequence is broken out into its own
function (i.e. have a separate function for parsing ResponseBytes here). Yes
some will be small, but easier to identify and verify each piece. I get a bit
lost when reading a larger stream of instructions, especially:
  * the variable names for the "parser" become longer
  * harder to verify the HasMore() calls

[svaldez, 2016/02/16 17:25:10]

Moved the sequence parsing into BasicResponse parsing which should make it more
straightforward.

+    return false;
+  if (response_bytes_input_parser.HasMore())
+    return false;
+  if (!response_bytes_parser.ReadTag(der::kOid, &response_type_oid))
+    return false;
+  if (response_type_oid != BasicOCSPResponseOid())

[eroman, 2016/02/13 00:56:50]

(Note: Not familiar enough if this is the only OID we should support)

[svaldez, 2016/02/16 17:25:10]

Its the only defined OID currently.

+    return false;
+  if (!response_bytes_parser.ReadTag(der::kOctetString, &response_string))

[eroman, 2016/02/13 00:56:50]

Please provide documentation here that the octet string is being treated as a
BasicOCSPResponse (i.e. reference the RFC, otherwise this looks a bit magical
when reading). RFC 6960 section 4.2.1 ?

[svaldez, 2016/02/16 17:25:11]

Done.

+    return false;
+  if (response_bytes_parser.HasMore())
+    return false;
+  der::Parser response_parser(response_string);
+  der::Parser basic_response_parser;
+  if (!response_parser.ReadSequence(&basic_response_parser))
+    return false;
+  if (response_parser.HasMore())
+    return false;
+  return ParseBasicOCSPResponse(&basic_response_parser, out);
+}
+
+namespace {
+
+bool CheckResponder(OCSPResponseData::ResponderID id,

[eroman, 2016/02/13 00:56:50]

The non-parsing functions feel like they belong in a different CL and file.

I have not reviewed the non-parsing bits in this review pass

[svaldez, 2016/02/16 17:25:11]

I'll split out the non-parsing bits after finishing running the parser/verifier
against all the Alexa sites.

+                    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) ||
+        !der::Parser(cert.subject_tlv).ReadTag(der::kSequence, &cert_rdn))
+      return false;
+    return VerifyNameMatch(name_rdn, cert_rdn);
+  } else {
+    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);
+  }
+}
+
+bool CheckCertID(der::Input id_tlv,
+                 ParsedTbsCertificate issuer,
+                 der::Input serial_number) {
+  OCSPCertID id;
+  if (!ParseOCSPCertID(id_tlv, &id))
+    return false;
+
+  HashValueTag type;
+  switch (id.hash_algorithm) {
+    case DigestAlgorithm::Sha1:
+      type = HASH_VALUE_SHA1;
+      break;
+    case DigestAlgorithm::Sha256:
+      type = HASH_VALUE_SHA256;
+      break;
+    default:
+      NOTIMPLEMENTED();
+      return false;
+  }
+
+  HashValue id_name_hash(type);
+  if (id.issuer_name_hash.Length() != id_name_hash.size())
+    return false;
+  memcpy(id_name_hash.data(), id.issuer_name_hash.UnsafeData(),
+         id_name_hash.size());
+  HashValue issuer_name_hash(type);
+  der::Input name = issuer.subject_tlv;
+  if (type == HASH_VALUE_SHA1)
+    base::SHA1HashBytes(name.UnsafeData(), name.Length(),
+                        issuer_name_hash.data());
+  else {
+    std::string hash = crypto::SHA256HashString(name.AsString());
+    memcpy(issuer_name_hash.data(), hash.data(), issuer_name_hash.size());
+  }
+  if (!id_name_hash.Equals(issuer_name_hash))
+    return false;
+
+  HashValue id_key_hash(type);
+  if (id.issuer_key_hash.Length() != id_key_hash.size())
+    return false;
+  memcpy(id_key_hash.data(), id.issuer_key_hash.UnsafeData(),
+         id_key_hash.size());
+  HashValue issuer_key_hash(type);
+  der::Parser parser(issuer.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();
+  if (type == HASH_VALUE_SHA1)
+    base::SHA1HashBytes(key.UnsafeData(), key.Length(), issuer_key_hash.data());
+  else {
+    std::string hash = crypto::SHA256HashString(key.AsString());
+    memcpy(issuer_key_hash.data(), hash.data(), issuer_key_hash.size());
+  }
+
+  if (!id_key_hash.Equals(issuer_key_hash))
+    return false;
+
+  return id.serial_number == serial_number;
+}
+
+}  // namespace
+
+bool VerifyOCSPResponse(const OCSPResponse* response,
+                        const ParsedCertificate* issuer_cert) {
+  SimpleSignaturePolicy signature_policy(1024);
+
+  OCSPResponseData response_data;
+  if (!ParseOCSPResponseData(response->data, &response_data))
+    return false;
+
+  ParsedTbsCertificate issuer;
+  ParsedTbsCertificate responder;
+  if (!issuer_cert ||
+      !ParseTbsCertificate(issuer_cert->tbs_certificate_tlv, &issuer))
+    return false;
+
+  if (CheckResponder(response_data.responder_id, issuer)) {
+    responder = issuer;
+  } else {
+    bool found = false;
+    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;
+
+      if (CheckResponder(response_data.responder_id, tbs_cert)) {
+        found = true;
+        responder = tbs_cert;
+
+        scoped_ptr<SignatureAlgorithm> signature_algorithm =
+            SignatureAlgorithm::CreateFromDer(
+                responder_cert.signature_algorithm_tlv);
+        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;
+        }
+
+        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;
+        break;
+      }
+    }
+    if (!found)
+      return false;
+  }
+  return VerifySignedData(*(response->signature_algorithm), response->data,
+                          response->signature, responder.spki_tlv,
+                          &signature_policy);
+}
+
+bool GetOCSPCertStatus(const OCSPResponseData* response_data,
+                       const ParsedCertificate* issuer,
+                       const ParsedCertificate* cert,
+                       OCSPCertStatus* out) {
+  out->status = OCSPCertStatus::Status::UNKNOWN;
+
+  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;
+
+  for (const auto& response : response_data->responses) {
+    OCSPSingleResponse single_response;
+    if (!ParseOCSPSingleResponse(response, &single_response))
+      return false;
+    if (CheckCertID(single_response.cert_id_tlv, issuer_tbs_cert,
+                    tbs_cert.serial_number)) {
+      *out = single_response.cert_status;
+      if (single_response.cert_status.status != OCSPCertStatus::Status::GOOD)
+        return true;
+    }
+  }
+
+  return true;
+}
+
+}  // namespace net