Greatly expanded code coverage for cast_auth_util fuzz tests.

Bug: 796717, b/149843583
Change-Id: Iee9b406604c923d6109cc9f2391effedb33043d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2252691
Commit-Queue: John Williams <jrw@chromium.org>
Reviewed-by: Doug Steedman <dougsteed@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784502}

components/cast_certificate/cast_crl.cc

@@ -38,6 +38,14 @@ using cast::certificate::TbsCrl;
 
 namespace {
 
+#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+// During fuzz testing, we won't have valid hashes for certificate revocation,
+// so we use the empty string as a placeholder where a hash code is needed in
+// production.  This allows us to test the revocation logic without needing the
+// fuzzing engine to produce a valid hash code.
+constexpr char kFakeHashForFuzzing[] = "fake_hash_code";
+#endif
+
 enum CrlVersion {
   // version 0: Spki Hash Algorithm = SHA-256
   //            Signature Algorithm = RSA-PKCS1 V1.5 with SHA-256
@@ -105,6 +113,11 @@ bool VerifyCRL(const Crl& crl,
                const base::Time& time,
                net::TrustStore* trust_store,
                net::der::GeneralizedTime* overall_not_after) {
+  if (!crl.has_signature() || !crl.has_signer_cert()) {
+    VLOG(2) << "CRL - Missing fields";
+    return false;
+  }
+
   // Verify the trust of the CRL authority.
   net::CertErrors parse_errors;
   scoped_refptr<net::ParsedCertificate> parsed_cert =
@@ -128,7 +141,9 @@ bool VerifyCRL(const Crl& crl,
           *signature_algorithm_type, net::der::Input(&crl.tbs_crl()),
           signature_value_bit_string, parsed_cert->tbs().spki_tlv)) {
     VLOG(2) << "CRL - Signature verification failed";
+#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
     return false;
+#endif
   }
 
   // Verify the issuer certificate.
@@ -151,8 +166,10 @@ bool VerifyCRL(const Crl& crl,
   net::CertPathBuilder::Result result = path_builder.Run();
   if (!result.HasValidPath()) {
     VLOG(2) << "CRL - Issuer certificate verification failed.";
+#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
     // TODO(crbug.com/634443): Log the error information.
     return false;
+#endif
   }
   // There are no requirements placed on the leaf certificate having any
   // particular KeyUsages. Leaf certificate checks are bypassed.
@@ -170,7 +187,9 @@ bool VerifyCRL(const Crl& crl,
   }
   if ((verification_time < not_before) || (verification_time > not_after)) {
     VLOG(2) << "CRL - Not time-valid.";
+#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
     return false;
+#endif
   }
 
   // Set CRL expiry to the earliest of the cert chain expiry and CRL expiry.
@@ -178,7 +197,15 @@ bool VerifyCRL(const Crl& crl,
   // "expiration" of the trust anchor is handled instead by its
   // presence in the trust store.
   *overall_not_after = not_after;
-  for (const auto& cert : result.GetBestValidPath()->certs) {
+#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+  // We don't expect to have a valid path during fuzz testing, so just use a
+  // single cert.
+  const net::ParsedCertificateList path_certs = {parsed_cert};
+#else
+  const net::ParsedCertificateList& path_certs =
+      result.GetBestValidPath()->certs;
+#endif
+  for (const auto& cert : path_certs) {
     net::der::GeneralizedTime cert_not_after = cert->tbs().validity_not_after;
     if (cert_not_after < *overall_not_after)
       *overall_not_after = cert_not_after;
@@ -239,11 +266,19 @@ CastCRLImpl::CastCRLImpl(const TbsCrl& tbs_crl,
   // Parse the revoked hashes.
   for (const auto& hash : tbs_crl.revoked_public_key_hashes()) {
     revoked_hashes_.insert(hash);
+#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+    // Save fake hash code for later lookups.
+    revoked_hashes_.insert(kFakeHashForFuzzing);
+#endif
   }
 
   // Parse the revoked serial ranges.
   for (const auto& range : tbs_crl.revoked_serial_number_ranges()) {
     std::string issuer_hash = range.issuer_public_key_hash();
+#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+    // Save range under fake hash code for later lookups.
+    issuer_hash = kFakeHashForFuzzing;
+#endif
 
     uint64_t first_serial_number = range.first_serial_number();
     uint64_t last_serial_number = range.last_serial_number();
@@ -280,6 +315,11 @@ bool CastCRLImpl::CheckRevocation(
 
     // Calculate the public key's hash to check for revocation.
     std::string spki_hash = crypto::SHA256HashString(spki_tlv.AsString());
+#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+    // Revocation data (if any) was saved in the constructor using this fake
+    // hash code.
+    spki_hash = kFakeHashForFuzzing;
+#endif
     if (revoked_hashes_.find(spki_hash) != revoked_hashes_.end()) {
       VLOG(2) << "Public key is revoked.";
       return false;

components/cast_channel/BUILD.gn

@@ -3,6 +3,8 @@
 # found in the LICENSE file.
 
 import("//testing/libfuzzer/fuzzer_test.gni")
+import("//third_party/libprotobuf-mutator/fuzzable_proto_library.gni")
+import("//third_party/protobuf/proto_library.gni")
 
 static_library("cast_channel") {
   sources = [
@@ -101,6 +103,54 @@ source_set("unit_tests") {
   ]
 }
 
+fuzzable_proto_library("cast_channel_fuzzer_inputs") {
+  sources = [ "proto/fuzzer_inputs.proto" ]
+  import_dirs = [
+    "//third_party/openscreen/src/cast/common/certificate/proto",
+    "//third_party/openscreen/src/cast/common/channel/proto",
+  ]
+  proto_out_dir = "components/cast_channel/fuzz_proto"
+}
+
+protoc_convert("cast_auth_util_fuzzer_convert_corpus") {
+  sources = [
+    "test/data/error.textproto",
+    "test/data/good.textproto",
+  ]
+
+  inputs = [ "proto/fuzzer_inputs.proto" ]
+
+  output_pattern = "$target_gen_dir/cast_auth_util_fuzzer_corpus/{{source_name_part}}.binarypb"
+
+  args = [
+    "--encode=cast_channel.fuzz.CastAuthUtilInputs",
+    "-I",
+    rebase_path("//third_party/openscreen/src/cast/common/channel/proto"),
+    "-I",
+    rebase_path("//third_party/openscreen/src/cast/common/certificate/proto"),
+    "-I",
+    rebase_path("proto"),
+    "fuzzer_inputs.proto",
+  ]
+}
+
+fuzzer_test("cast_auth_util_fuzzer") {
+  sources = [ "cast_auth_util_fuzzer.cc" ]
+  seed_corpus = "$target_gen_dir/cast_auth_util_fuzzer_corpus"
+  dict = "fuzz.dict"
+  deps = [
+    ":cast_auth_util_fuzzer_convert_corpus",
+    ":cast_channel",
+    ":cast_channel_fuzzer_inputs",
+    "//components/cast_certificate",
+    "//components/cast_certificate:test_support",
+    "//net:test_support",
+    "//net/data/ssl/certificates:generate_fuzzer_cert_includes",
+    "//third_party/libprotobuf-mutator",
+    "//third_party/openscreen/src/cast/common/certificate/proto:certificate_proto",
+  ]
+}
+
 # TODO(jrw): Rename target to cast_framer_ingest_fuzzer.  The name
 # is left unchanged for now to avoid the need to get reviews for
 # various files that include it.
@@ -119,23 +169,11 @@ fuzzer_test("cast_message_fuzzer") {
   libfuzzer_options = [ "max_len=65535" ]
 }
 
-fuzzer_test("cast_auth_util_fuzzer") {
-  sources = [ "cast_auth_util_fuzzer.cc" ]
-  dict = "fuzz.dict"
-  deps = [
-    ":cast_channel",
-    "//components/cast_channel/proto:cast_channel_fuzzer_inputs_proto",
-    "//net/data/ssl/certificates:generate_fuzzer_cert_includes",
-    "//third_party/libprotobuf-mutator",
-    "//third_party/openscreen/src/cast/common/channel/proto:channel_proto",
-  ]
-}
-
 fuzzer_test("cast_framer_serialize_fuzzer") {
   sources = [ "cast_framer_serialize_fuzzer.cc" ]
   deps = [
     ":cast_channel",
-    "//components/cast_channel/proto:cast_channel_fuzzer_inputs_proto",
+    ":cast_channel_fuzzer_inputs",
     "//third_party/libprotobuf-mutator",
     "//third_party/openscreen/src/cast/common/channel/proto:channel_proto",
   ]
@@ -146,7 +184,7 @@ fuzzer_test("cast_message_util_fuzzer") {
   dict = "fuzz.dict"
   deps = [
     ":cast_channel",
-    "//components/cast_channel/proto:cast_channel_fuzzer_inputs_proto",
+    ":cast_channel_fuzzer_inputs",
     "//third_party/libprotobuf-mutator",
     "//third_party/openscreen/src/cast/common/channel/proto:channel_proto",
   ]

components/cast_channel/README.md

@@ -13,17 +13,27 @@ Build the fuzz target:
 % ninja -C out/libfuzzer $TEST_NAME
 ```
 
-Create an empty corpus directory:
+Create an empty corpus directory if you don't have one already.
 
 ```shell
 % mkdir ${TEST_NAME}_corpus
 ```
 
-Run the fuzz target, turning off detection of ODR violations that occur in
-component builds:
+Turning off detection of ODR violations that occur in component builds:
 
 ```shell
 % export ASAN_OPTIONS=detect_odr_violation=0
+```
+
+If the test has a seed corpus:
+
+```shell
+% ./out/libfuzzer/$TEST_NAME ${TEST_NAME}_corpus out/libfuzzer/gen/components/cast_channel/${TEST_NAME}_corpus
+```
+
+If the test has no seed corpus, omit the last parameter:
+
+```shell
 % ./out/libfuzzer/$TEST_NAME ${TEST_NAME}_corpus
 ```
 

components/cast_channel/cast_auth_util.cc

@@ -453,9 +453,13 @@ AuthResult VerifyCredentialsImpl(const AuthResponse& response,
 
   if (!verification_context->VerifySignatureOverData(
           response.signature(), signature_input, digest_algorithm)) {
+    // For fuzz testing we just pretend the signature was OK.  The signature is
+    // normally verified using boringssl, which has its own fuzz tests.
+#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
     RecordSignatureEvent(SIGNATURE_VERIFY_FAILED);
     return AuthResult("Failed verifying signature over data.",
                       AuthResult::ERROR_SIGNED_BLOBS_MISMATCH);
+#endif
   }
   RecordSignatureEvent(SIGNATURE_OK);
 

components/cast_channel/cast_auth_util_fuzzer.cc

@@ -4,35 +4,152 @@
 
 #include <cstdlib>
 #include <iostream>
+#include <string>
+#include <vector>
 
+#include "base/no_destructor.h"
 #include "base/notreached.h"
+#include "base/time/time_override.h"
+#include "components/cast_certificate/cast_cert_validator_test_helpers.h"
 #include "components/cast_channel/cast_auth_util.h"
 #include "components/cast_channel/fuzz_proto/fuzzer_inputs.pb.h"
 #include "net/cert/x509_certificate.h"
+#include "net/cert/x509_util.h"
+#include "net/test/test_certificate_data.h"
 #include "testing/libfuzzer/proto/lpm_interface.h"
 
 namespace cast_channel {
 namespace fuzz {
-
 namespace {
+
 const char kCertData[] = {
+// Generated by //net/data/ssl/certificates:generate_fuzzer_cert_includes
 #include "net/data/ssl/certificates/wildcard.inc"
 };
-}  // namespace
 
-DEFINE_PROTO_FUZZER(const CastAuthUtilInputs& input_union) {
-  // TODO(crbug.com/796717): Add tests for AuthenticateChallengeReply and
-  // VerifyTLSCertificate if necessary.  Refer to updates on the bug, and check
-  // to see if there is already coverage through BoringSSL
+base::NoDestructor<std::vector<std::string>> certs;
+
+static bool InitializeOnce() {
+  *certs = cast_certificate::testing::ReadCertificateChainFromFile(
+      "certificates/chromecast_gen1.pem");
+  DCHECK(certs->size() >= 1);
+  return true;
+}
+
+void UpdateTime(TimeBoundCase c, const base::Time* time, int direction) {
+  auto& mtime = const_cast<base::Time&>(*time);
+  switch (c) {
+    case TimeBoundCase::VALID:
+      // Create bound that include the current date.
+      mtime = base::Time::Now() + base::TimeDelta::FromDays(direction);
+      break;
+    case TimeBoundCase::INVALID:
+      // Create a bound that excludes the current date.
+      mtime = base::Time::Now() + base::TimeDelta::FromDays(-direction);
+      break;
+    case TimeBoundCase::OOB:
+      // Create a bound so far in the past/future it's not valid.
+      mtime = base::Time::Now() + base::TimeDelta::FromDays(direction * 10000);
+      break;
+    case TimeBoundCase::MISSING:
+      // Remove any existing bound.
+      mtime = base::Time();
+      break;
+    default:
+      NOTREACHED();
+  }
+}
+
+DEFINE_PROTO_FUZZER(CastAuthUtilInputs& input_union) {
+  static bool init = InitializeOnce();
+  CHECK(init);
 
   switch (input_union.input_case()) {
     case CastAuthUtilInputs::kAuthenticateChallengeReplyInput: {
-      const auto& input = input_union.authenticate_challenge_reply_input();
-      cast::channel::DeviceAuthMessage auth_message = input.auth_message();
-      AuthContext context = AuthContext::CreateForTest(input.nonce());
+      auto& input = *input_union.mutable_authenticate_challenge_reply_input();
+
+      // If we have a DeviceAuthMessage, use it to override the cast_message()
+      // payload with a more interesting value.
+      if (input.has_auth_message()) {
+        // Optimization: if the payload_binary() field is going to be
+        // overwritten, insist that it has to be empty initially.  This cuts
+        // down on how much time is spent generating identical arguments for
+        // AuthenticateChallengeReply() from different values of |input|.
+        if (input.cast_message().has_payload_binary())
+          return;
+
+        if (!input.auth_message().has_response()) {
+          // Optimization.
+          if (input.nonce_ok() || input.response_certs_ok() ||
+              input.tbs_crls_size() || input.crl_certs_ok() ||
+              input.crl_signatures_ok()) {
+            return;
+          }
+        } else {
+          auto& response = *input.mutable_auth_message()->mutable_response();
+
+          // Maybe force the nonce to be the correct value.
+          if (input.nonce_ok()) {
+            // Optimization.
+            if (response.has_sender_nonce())
+              return;
+
+            response.set_sender_nonce(input.nonce());
+          }
+
+          // Maybe force the response certs to be valid.
+          if (input.response_certs_ok()) {
+            // Optimization.
+            if (!response.client_auth_certificate().empty() ||
+                response.intermediate_certificate_size() > 0)
+              return;
+
+            response.set_client_auth_certificate(certs->front());
+            response.clear_intermediate_certificate();
+            for (std::size_t i = 1; i < certs->size(); i++) {
+              response.add_intermediate_certificate(certs->at(i));
+            }
+          }
+
+          // Maybe replace the crl() field in the response with valid data.
+          if (input.tbs_crls_size() == 0) {
+            // Optimization.
+            if (input.crl_certs_ok() || input.crl_signatures_ok())
+              return;
+          } else {
+            // Optimization.
+            if (response.has_crl())
+              return;
+
+            cast::certificate::CrlBundle crl_bundle;
+            for (const auto& tbs_crl : input.tbs_crls()) {
+              cast::certificate::Crl& crl = *crl_bundle.add_crls();
+              if (input.crl_certs_ok())
+                crl.set_signer_cert(certs->at(0));
+              if (input.crl_signatures_ok())
+                crl.set_signature("");
+              tbs_crl.SerializeToString(crl.mutable_tbs_crl());
+            }
+            crl_bundle.SerializeToString(response.mutable_crl());
+          }
+        }
+
+        input.mutable_cast_message()->set_payload_type(CastMessage::BINARY);
+        input.auth_message().SerializeToString(
+            input.mutable_cast_message()->mutable_payload_binary());
+      }
+
+      // Build a well-formed cert with start and expiry times relative to the
+      // current time.  The actual cert doesn't matter for testing purposes
+      // because validation failures are ignored.
       scoped_refptr<net::X509Certificate> peer_cert =
           net::X509Certificate::CreateFromBytes(kCertData,
                                                 base::size(kCertData));
+      UpdateTime(input.start_case(), &peer_cert->valid_start(), -1);
+      UpdateTime(input.expiry_case(), &peer_cert->valid_expiry(), +1);
+
+      AuthContext context = AuthContext::CreateForTest(input.nonce());
+
       AuthenticateChallengeReply(input.cast_message(), *peer_cert, context);
       break;
     }
@@ -41,5 +158,6 @@ DEFINE_PROTO_FUZZER(const CastAuthUtilInputs& input_union) {
   }
 }
 
+}  // namespace
 }  // namespace fuzz
 }  // namespace cast_channel

components/cast_channel/fuzz.dict

@@ -63,3 +63,27 @@
 "type"
 "userAgent"
 "version"
+
+# Names from cast_channel.proto
+"UNSPECIFIED"
+"RSASSA_PKCS1v15"
+"RSASSA_PSS"
+"INTERNAL_ERROR"
+"NO_TLS"
+"SIGNATURE_ALGORITHM_UNAVAILABLE"
+"SHA1"
+"SHA256"
+"signature_algorithm"
+"sender_nonce"
+"hash_algorithm"
+"signature"
+"client_auth_certificate"
+"intermediate_certificate"
+"signature_algorithm"
+"sender_nonce"
+"hash_algorithm"
+"crl"
+"error_type"
+"challenge"
+"response"
+"error"

components/cast_channel/proto/BUILD.gn

-# Copyright 2014 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.
-
-import("//third_party/libprotobuf-mutator/fuzzable_proto_library.gni")
-import("//third_party/protobuf/proto_library.gni")
-
-fuzzable_proto_library("cast_channel_fuzzer_inputs_proto") {
-  sources = [ "fuzzer_inputs.proto" ]
-  import_dirs = [ "//third_party/openscreen/src/cast/common/channel/proto" ]
-  proto_out_dir = "components/cast_channel/fuzz_proto"
-}

components/cast_channel/proto/fuzzer_inputs.proto

@@ -21,24 +21,49 @@
 syntax = "proto2";
 
 import "cast_channel.proto";
+import "revocation.proto";
 
 option optimize_for = LITE_RUNTIME;
 
 package cast_channel.fuzz;
 
+enum TimeBoundCase {
+  VALID = 0;
+  INVALID = 1;
+  OOB = 2;
+  MISSING = 3;
+}
+
 // Inputs for functions in cast_auth_utils.cc
 message CastAuthUtilInputs {
   message AuthenticateChallengeReplyInput {
-    required cast.channel.DeviceAuthMessage auth_message = 1;
-    required cast.channel.CastMessage cast_message = 2;
-    required string nonce = 3;
+    // The actual input to the function, parts of which may be
+    // overridden based on the additional fields below.
+    required cast.channel.CastMessage cast_message = 1;
+    required string nonce = 2;
+
+    // Values used to set the start and end times.
+    required TimeBoundCase start_case = 3;
+    required TimeBoundCase expiry_case = 4;
+
+    // Value used to replace the payload of |cast_message| with data
+    // that is more likely to be valid (and thus exercise more code
+    // paths).
+    optional cast.channel.DeviceAuthMessage auth_message = 5;
+
+    // Values used to replace the |crl| field of
+    // |auth_message.response|.
+    repeated cast.certificate.TbsCrl tbs_crls = 7;
+
+    // Flags that force certain fields to have correct values.
+    required bool nonce_ok = 8;
+    required bool response_certs_ok = 9;
+    required bool crl_certs_ok = 10;
+    required bool crl_signatures_ok = 11;
   }
 
   oneof input {
     AuthenticateChallengeReplyInput authenticate_challenge_reply_input = 1;
-    // TODO(crbug.com/796717): Add inputs for other functions to test:
-    // - VerifyTLSCertificate
-    // - VerifyCredentials
   }
 }
 

components/cast_channel/test/data/error.textproto

+authenticate_challenge_reply_input {
+  cast_message {
+    protocol_version: CASTV2_1_0
+    source_id: ""
+    destination_id: ""
+    namespace: ""
+    payload_type: BINARY
+  }
+  nonce: ""
+
+  start_case: VALID
+  expiry_case: VALID
+  nonce_ok: false
+  response_certs_ok: false
+  crl_certs_ok: false
+  crl_signatures_ok: false
+
+  auth_message {
+    error {
+      error_type: INTERNAL_ERROR
+    }
+  }
+}

components/cast_channel/test/data/good.textproto

+authenticate_challenge_reply_input {
+  cast_message {
+    protocol_version: CASTV2_1_0
+    source_id: ""
+    destination_id: ""
+    namespace: ""
+    payload_type: BINARY
+  }
+  nonce: ""
+  start_case: VALID
+  expiry_case: VALID
+  nonce_ok: true
+  response_certs_ok: true
+  crl_certs_ok: true
+  crl_signatures_ok: true
+
+  auth_message {
+    response {
+      signature: ""
+      signature_algorithm: RSASSA_PKCS1v15
+      hash_algorithm: SHA1
+      client_auth_certificate: ""
+    }
+  }
+
+  tbs_crls {
+    revoked_public_key_hashes: ""
+    revoked_serial_number_ranges {
+      issuer_public_key_hash: ""
+      first_serial_number: 0
+      last_serial_number: 0
+    }
+    version: 0
+    not_before_seconds: 0
+    not_after_seconds: 0
+  }
+}