Revert "Greatly expanded code coverage for cast_auth_util fuzz tests."

This reverts commit 3fd89cb3.

Reason for revert: Broke android-archive-rel. https://ci.chromium.org/p/chromium/builders/ci/android-archive-rel/12848

Original change's description:
> 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}

TBR=mfoltz@chromium.org,dougsteed@chromium.org,jrw@chromium.org

Change-Id: I99a5388d1b1a3f7d7b2b94a8d06d734e526dc74c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 796717, b/149843583
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2278141Reviewed-by: Fabrice de Gans-Riberi <fdegans@chromium.org>
Commit-Queue: Fabrice de Gans-Riberi <fdegans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784509}

components/cast_certificate/cast_crl.cc

@@ -38,14 +38,6 @@ 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
@@ -113,11 +105,6 @@ 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 =
@@ -141,9 +128,7 @@ 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.
@@ -166,10 +151,8 @@ 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.
@@ -187,9 +170,7 @@ 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.
@@ -197,15 +178,7 @@ 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;
-#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) {
+  for (const auto& cert : result.GetBestValidPath()->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;
@@ -266,19 +239,11 @@ 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();
@@ -315,11 +280,6 @@ 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,8 +3,6 @@
 # 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 = [
@@ -103,54 +101,6 @@ 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.
@@ -169,11 +119,23 @@ 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",
-    ":cast_channel_fuzzer_inputs",
+    "//components/cast_channel/proto:cast_channel_fuzzer_inputs_proto",
     "//third_party/libprotobuf-mutator",
     "//third_party/openscreen/src/cast/common/channel/proto:channel_proto",
   ]
@@ -184,7 +146,7 @@ fuzzer_test("cast_message_util_fuzzer") {
   dict = "fuzz.dict"
   deps = [
     ":cast_channel",
-    ":cast_channel_fuzzer_inputs",
+    "//components/cast_channel/proto:cast_channel_fuzzer_inputs_proto",
     "//third_party/libprotobuf-mutator",
     "//third_party/openscreen/src/cast/common/channel/proto:channel_proto",
   ]

components/cast_channel/README.md

@@ -13,27 +13,17 @@ Build the fuzz target:
 % ninja -C out/libfuzzer $TEST_NAME
 ```
 
-Create an empty corpus directory if you don't have one already.
+Create an empty corpus directory:
 
 ```shell
 % mkdir ${TEST_NAME}_corpus
 ```
 
-Turning off detection of ODR violations that occur in component builds:
+Run the fuzz target, 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,13 +453,9 @@ 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,152 +4,35 @@
 
 #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 {
 
+namespace {
 const char kCertData[] = {
-// Generated by //net/data/ssl/certificates:generate_fuzzer_cert_includes
 #include "net/data/ssl/certificates/wildcard.inc"
 };
+}  // namespace
 
-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);
+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
 
   switch (input_union.input_case()) {
     case CastAuthUtilInputs::kAuthenticateChallengeReplyInput: {
-      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.
+      const auto& input = input_union.authenticate_challenge_reply_input();
+      cast::channel::DeviceAuthMessage auth_message = input.auth_message();
+      AuthContext context = AuthContext::CreateForTest(input.nonce());
       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;
     }
@@ -158,6 +41,5 @@ DEFINE_PROTO_FUZZER(CastAuthUtilInputs& input_union) {
   }
 }
 
-}  // namespace
 }  // namespace fuzz
 }  // namespace cast_channel

components/cast_channel/fuzz.dict

@@ -63,27 +63,3 @@
 "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,49 +21,24 @@
 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 {
-    // 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;
+    required cast.channel.DeviceAuthMessage auth_message = 1;
+    required cast.channel.CastMessage cast_message = 2;
+    required string nonce = 3;
   }
 
   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
-  }
-}