Extract Certificate Transparency SCTs from stapled OCSP responses

net/cert/asn1_util.cc

Index: net/cert/asn1_util.cc
diff --git a/net/cert/asn1_util.cc b/net/cert/asn1_util.cc
index 6dcff52ee65c90a4eb3ed033b226b0709a2e9b3d..323bd9eecf779bc3b327b74a0ffee776e8b8df21 100644
--- a/net/cert/asn1_util.cc
+++ b/net/cert/asn1_util.cc
@@ -326,6 +326,236 @@ bool ExtractCRLURLsFromDERCert(base::StringPiece cert,
   return true;
 }
 
+bool ExtractSCTExtensionFromOCSPResponse(
+    base::StringPiece ocsp_response,
+    const base::StringPiece& cert_serial_number,
+    base::StringPiece* sct_list_out) {
+  sct_list_out->clear();
+
+  // OCSPResponse ::= SEQUENCE {
+  //    responseStatus         OCSPResponseStatus,
+  //    responseBytes          [0] EXPLICIT ResponseBytes OPTIONAL }
+  base::StringPiece ocsp_response_sequence;
+  if (!GetElement(&ocsp_response, kSEQUENCE, &ocsp_response_sequence))
+    return false;
+  if (!ocsp_response.empty())
+    return false;
+
+  // responseStatus
+  if (!GetElement(&ocsp_response_sequence, kENUMERATED, NULL))
+    return false;
+
+  // responseBytes
+  base::StringPiece response_bytes_outer;
+  if (!GetElement(&ocsp_response_sequence,
+                  kOptional | kConstructed | kContextSpecific | 0,
+                  &response_bytes_outer))
+    return false;

[wtc, 2013/12/03 01:18:06]

Nit: add curly braces {} because the conditional expression spans multiple
lines.

Please fix other instances of this problem in this function.

[ekasper, 2013/12/03 13:50:51]

Done.

+  if (!ocsp_response_sequence.empty())
+    return false;
+
+  // ResponseBytes ::=       SEQUENCE {
+  //     responseType   OBJECT IDENTIFIER,
+  //     response       OCTET STRING } -- DER encoding of BasicOCSPResponse
+  base::StringPiece response_bytes;
+  if (!GetElement(&response_bytes_outer, kSEQUENCE, &response_bytes))
+    return false;
+  if (!response_bytes_outer.empty())
+    return false;
+
+  // responseType
+  base::StringPiece response_type;
+  if (!GetElement(&response_bytes, kOID, &response_type))
+    return false;
+
+  // id-ad-ocsp: 1.3.6.1.5.5.7.48.1.1
+  static const uint8 kBasicOCSPResponseOID[] = {0x2b, 0x06, 0x01, 0x05, 0x05,
+                                                0x07, 0x30, 0x01, 0x01};

[wtc, 2013/12/03 01:18:06]

Nit: add a space after '{' and before '}'.

[ekasper, 2013/12/03 13:50:51]

Done.

+
+  if (response_type.size() != sizeof(kBasicOCSPResponseOID) ||
+      memcmp(response_type.data(), kBasicOCSPResponseOID,
+             response_type.size()) != 0)
+    return false;
+
+  base::StringPiece wrapped_response;
+  if (!GetElement(&response_bytes, kOCTETSTRING, &wrapped_response))
+    return false;
+
+  if (!response_bytes.empty())
+    return false;
+
+  // BasicOCSPResponse       ::= SEQUENCE {
+  //       tbsResponseData      ResponseData,
+  //       signatureAlgorithm   AlgorithmIdentifier,
+  //       signature            BIT STRING,
+  //       certs                [0] EXPLICIT SEQUENCE OF Certificate OPTIONAL }
+  base::StringPiece basic_ocsp_response;
+  if (!GetElement(&wrapped_response, kSEQUENCE, &basic_ocsp_response))
+    return false;
+  if (!wrapped_response.empty())
+    return false;
+
+  // ResponseData ::= SEQUENCE {
+  //     version              [0] EXPLICIT Version DEFAULT v1,
+  //     responderID              ResponderID,
+  //     producedAt               GeneralizedTime,
+  //     responses                SEQUENCE OF SingleResponse,
+  //     responseExtensions   [1] EXPLICIT Extensions OPTIONAL }
+  base::StringPiece tbs_response_data;
+  if (!GetElement(&basic_ocsp_response, kSEQUENCE, &tbs_response_data))
+    return false;
+
+  // version
+  if (!GetElement(&tbs_response_data,
+                  kOptional | kConstructed | kContextSpecific | 0, NULL))
+    return false;
+
+  // ResponderID ::= CHOICE {
+  //    byName               [1] Name,
+  //    byKey                [2] KeyHash }
+  if (!GetElement(&tbs_response_data,
+                  kConstructed | kContextSpecific | 1, NULL) &&
+      !GetElement(&tbs_response_data,
+                  kConstructed | kContextSpecific | 2, NULL))
+    return false;
+
+  // producedAt
+  if (!GetElement(&tbs_response_data, kGENERALIZEDTIME, NULL))
+    return false;
+
+  // responses: SEQUENCE OF and SEQUENCE share the same tag.
+  base::StringPiece responses;
+  if (!GetElement(&tbs_response_data, kSEQUENCE, &responses))
+    return false;
+
+  while (responses.size() > 0) {

[wtc, 2013/12/03 01:18:06]

Nit: this function is very long. If we want to break it into smaller functions,
this while loop or line 426 may be a good point to start a new function that
parses the SEQUENCE OF SingleResponse.

[ekasper, 2013/12/03 13:50:51]

I've broken it up so that the seeking to the right extensions happens in
separate methods. However I didn't use "seek" a la "SeekToSPKI" because I found
the stringpiece naming to get too confusing.

+    // SingleResponse ::= SEQUENCE {
+    //   certID                       CertID,
+    //   certStatus                   CertStatus,
+    //   thisUpdate                   GeneralizedTime,
+    //   nextUpdate         [0]       EXPLICIT GeneralizedTime OPTIONAL,
+    //   singleExtensions   [1]       EXPLICIT Extensions OPTIONAL }
+    base::StringPiece single_response;
+    if (!GetElement(&responses, kSEQUENCE, &single_response))
+      return false;
+
+    // CertID ::= SEQUENCE {
+    //     hashAlgorithm            AlgorithmIdentifier,
+    //     issuerNameHash     OCTET STRING, -- Hash of Issuer's DN
+    //     issuerKeyHash      OCTET STRING, -- Hash of Issuers public key
+    //     serialNumber       CertificateSerialNumber }
+    base::StringPiece cert_id;
+    if (!GetElement(&single_response, kSEQUENCE, &cert_id))
+      return false;
+
+    // hashAlgorithm
+    if (!GetElement(&cert_id, kSEQUENCE, NULL))
+      return false;
+    // issuerNameHash
+    if (!GetElement(&cert_id, kOCTETSTRING, NULL))
+      return false;
+    // issuerKeyHash
+    if (!GetElement(&cert_id, kOCTETSTRING, NULL))
+      return false;
+
+    base::StringPiece serial_number;
+    if (!GetElement(&cert_id, kINTEGER, &serial_number))
+      return false;
+
+    if (serial_number != cert_serial_number) {
+      // Serial number mismatch - continue.
+      continue;
+    }
+
+    // Serial number match: continue to see if the CT extension is present.
+    // CertStatus ::= CHOICE {
+    //     good                [0]     IMPLICIT NULL,
+    //     revoked             [1]     IMPLICIT RevokedInfo,
+    //     unknown             [2]     IMPLICIT UnknownInfo }
+    if (!GetElement(&single_response, kContextSpecific | 0, NULL) &&
+        !GetElement(&single_response,
+                    kContextSpecific | kConstructed | 1, NULL) &&
+        !GetElement(&single_response, kContextSpecific | 2, NULL))
+      return false;
+
+    // thisUpdate
+    if (!GetElement(&single_response, kGENERALIZEDTIME, NULL))
+      return false;
+    // nextUpdate
+    if (!GetElement(&single_response,
+                    kConstructed | kOptional | kContextSpecific | 0, NULL))
+      return false;
+
+    base::StringPiece single_extensions_outer;
+    if (!GetElement(&single_response,
+                    kConstructed | kOptional | kContextSpecific | 1,
+                    &single_extensions_outer))
+      return false;
+    if (!single_response.empty())
+      return false;
+
+    if (single_extensions_outer.empty()) {
+      // No extensions; and since we should only have one singleResponse per
+      // certificate, we stop here.
+      return true;
+    }
+
+    // Extensions  ::=  SEQUENCE SIZE (1..MAX) OF Extension
+    base::StringPiece single_extensions;
+    if (!GetElement(&single_extensions_outer, kSEQUENCE, &single_extensions))
+      return false;
+
+    // Extension  ::=  SEQUENCE  {
+    //      extnID      OBJECT IDENTIFIER,
+    //      critical    BOOLEAN DEFAULT FALSE,
+    //      extnValue   OCTET STRING
+    //                  -- contains the DER encoding of an ASN.1 value
+    //                  -- corresponding to the extension type identified
+    //                  -- by extnID
+    //      }
+
+    while (!single_extensions.empty()) {
+      base::StringPiece extension;
+      if (!GetElement(&single_extensions, kSEQUENCE, &extension))
+        return false;
+
+      base::StringPiece extn_id;
+      if (!GetElement(&extension, kOID, &extn_id))
+        return false;
+
+      static const uint8 kSignedCertTimestampListOID[] = {
+        0x2b, 0x06, 0x01, 0x04, 0x01, 0xd6, 0x79, 0x02, 0x04, 0x05
+      };
+      if (extn_id.size() != sizeof(kSignedCertTimestampListOID) ||
+          memcmp(extn_id.data(), kSignedCertTimestampListOID,
+                 extn_id.size() != 0))
+        continue;
+
+      // critical
+      if (!GetElement(&extension, kBOOLEAN, NULL))
+        return false;
+
+      // extnValue
+      base::StringPiece extn_value;
+      if (!GetElement(&extension, kOCTETSTRING, &extn_value))
+        return false;
+      if (!extension.empty())
+        return false;
+
+      // The value of this extension is an OCTET STRING:
+      // SignedCertificateTimestampList ::= OCTET STRING
+      base::StringPiece sct_list;
+      if (!GetElement(&extn_value, kOCTETSTRING, &sct_list))
+        return false;
+      *sct_list_out = sct_list;
+      return true;
+    }

[wtc, 2013/12/03 01:18:06]

We should also return true at the end of this (inner) while loop, because we
have found the right singleResponse but it doesn't have the SCT extension.

One way to fix this is to change the "return true" statement on line 551 to a
"break" statement, and then add a "return true" between lines 552 and 553.

[ekasper, 2013/12/03 13:50:51]

Good catch, done as you suggested, break; followed by return true; after the
loop.

+  }
+
+  // No extension found.
+  return true;

[wtc, 2013/12/03 01:18:06]

With the change I proposed above, the only reason we get here is that the OCSP
response does not have a singleResponse for |cert_serial_number|. We probably
should return false in that case.

[ekasper, 2013/12/03 13:50:51]

Yes, a false is probably better in this case. This is now covered by
ExtractExtensionsWithMatchingCertID which returns false if no matching response
was found.

+}
+
 } // namespace asn1
 
 } // namespace net