Set Cast channel auth challenge to request SHA256 digests for signatu…

…res.

Add a new feature flag for the enforcement of the requested digest algorithm.

Support for SHA256 was added to the platform at the same time as the nonce
feature.

Record metric events for signature verification failures.

Bug: 747602
Test: Unittests
Change-Id: I61a2e8a95f111bff57ef5ea1ba073fba28a2b637
Reviewed-on: https://chromium-review.googlesource.com/582450
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Commit-Queue: Ryan Chung <ryanchung@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489458}

chrome/browser/extensions/api/networking_private/networking_private_crypto.cc

@@ -13,6 +13,7 @@
 #include "components/cast_certificate/cast_cert_validator.h"
 #include "crypto/openssl_util.h"
 #include "crypto/rsa_private_key.h"
+#include "net/cert/internal/signature_algorithm.h"
 #include "net/cert/pem_tokenizer.h"
 #include "third_party/boringssl/src/include/openssl/evp.h"
 #include "third_party/boringssl/src/include/openssl/rsa.h"
@@ -113,7 +114,8 @@ bool VerifyCredentialsAtTime(
 
   // Use the public key from verified certificate to verify |signature| over
   // |data|.
-  if (!verification_context->VerifySignatureOverData(signature, data)) {
+  if (!verification_context->VerifySignatureOverData(
+          signature, data, net::DigestAlgorithm::Sha1)) {
     LOG(ERROR) << kErrorPrefix
                << "Failed verifying signature using cast device cert";
     return false;

components/cast_certificate/cast_cert_validator.cc

@@ -114,13 +114,14 @@ class CertVerificationContextImpl : public CertVerificationContext {
                               const base::StringPiece& common_name)
       : spki_(spki.AsString()), common_name_(common_name.as_string()) {}
 
-  bool VerifySignatureOverData(const base::StringPiece& signature,
-                               const base::StringPiece& data) const override {
+  bool VerifySignatureOverData(
+      const base::StringPiece& signature,
+      const base::StringPiece& data,
+      net::DigestAlgorithm digest_algorithm) const override {
     // This code assumes the signature algorithm was RSASSA PKCS#1 v1.5 with
-    // SHA-1.
-    // TODO(eroman): Is it possible to use other hash algorithms?
+    // |digest_algorithm|.
     auto signature_algorithm =
-        net::SignatureAlgorithm::CreateRsaPkcs1(net::DigestAlgorithm::Sha1);
+        net::SignatureAlgorithm::CreateRsaPkcs1(digest_algorithm);
 
     // Use the same policy as was used for verifying signatures in
     // certificates. This will ensure for instance that the key used is at

components/cast_certificate/cast_cert_validator.h

@@ -16,6 +16,7 @@
 
 namespace net {
 class TrustStore;
+enum class DigestAlgorithm;
 }
 namespace cast_certificate {
 
@@ -68,11 +69,13 @@ class CertVerificationContext {
   virtual ~CertVerificationContext() {}
 
   // Use the public key from the verified certificate to verify a
-  // sha1WithRSAEncryption |signature| over arbitrary |data|. Both |signature|
-  // and |data| hold raw binary data. Returns true if the signature was
-  // correct.
-  virtual bool VerifySignatureOverData(const base::StringPiece& signature,
-                                       const base::StringPiece& data) const = 0;
+  // |digest_algorithm|WithRSAEncryption |signature| over arbitrary |data|.
+  // Both |signature| and |data| hold raw binary data. Returns true if the
+  // signature was correct.
+  virtual bool VerifySignatureOverData(
+      const base::StringPiece& signature,
+      const base::StringPiece& data,
+      net::DigestAlgorithm digest_algorithm) const = 0;
 
   // Retrieve the Common Name attribute of the subject's distinguished name from
   // the verified certificate, if present.  Returns an empty string if no Common

components/cast_certificate/cast_cert_validator_unittest.cc

@@ -7,6 +7,7 @@
 #include "components/cast_certificate/cast_cert_validator_test_helpers.h"
 #include "net/cert/internal/cert_errors.h"
 #include "net/cert/internal/parsed_certificate.h"
+#include "net/cert/internal/signature_algorithm.h"
 #include "net/cert/internal/trust_store_in_memory.h"
 #include "net/cert/x509_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -118,10 +119,12 @@ void RunTest(CastCertError expected_result,
   ASSERT_TRUE(context);
 
   // Test verification of some invalid signatures.
+  EXPECT_FALSE(context->VerifySignatureOverData("bogus signature", "bogus data",
+                                                net::DigestAlgorithm::Sha256));
+  EXPECT_FALSE(context->VerifySignatureOverData("", "bogus data",
+                                                net::DigestAlgorithm::Sha256));
   EXPECT_FALSE(
-      context->VerifySignatureOverData("bogus signature", "bogus data"));
-  EXPECT_FALSE(context->VerifySignatureOverData("", "bogus data"));
-  EXPECT_FALSE(context->VerifySignatureOverData("", ""));
+      context->VerifySignatureOverData("", "", net::DigestAlgorithm::Sha256));
 
   // If valid signatures are known for this device certificate, test them.
   if (!optional_signed_data_file_name.empty()) {
@@ -130,16 +133,13 @@ void RunTest(CastCertError expected_result,
 
     // Test verification of a valid SHA1 signature.
     EXPECT_TRUE(context->VerifySignatureOverData(signature_data.signature_sha1,
-                                                 signature_data.message));
-
-    // Test verification of a valid SHA256
-    //
-    // TODO(eroman): This fails because there isn't currently support
-    // for specifying a signature algorithm other than RSASSA PKCS#1 v1.5 with
-    // SHA1. Once support for different algorithms is added to the API this
-    // should be changed to expect success.
-    EXPECT_FALSE(context->VerifySignatureOverData(
-        signature_data.signature_sha256, signature_data.message));
+                                                 signature_data.message,
+                                                 net::DigestAlgorithm::Sha1));
+
+    // Test verification of a valid SHA256 signature.
+    EXPECT_TRUE(context->VerifySignatureOverData(
+        signature_data.signature_sha256, signature_data.message,
+        net::DigestAlgorithm::Sha256));
   }
 }
 
@@ -597,7 +597,8 @@ TEST(VerifyCastDeviceCertTest, VerifySignature1024BitRsa) {
       CertVerificationContextImplForTest(CreateString(kEx1PublicKeySpki));
 
   EXPECT_FALSE(context->VerifySignatureOverData(CreateString(kEx1Signature),
-                                                CreateString(kEx1Message)));
+                                                CreateString(kEx1Message),
+                                                net::DigestAlgorithm::Sha1));
 }
 
 // ------------------------------------------------------
@@ -671,7 +672,8 @@ TEST(VerifyCastDeviceCertTest, VerifySignature2048BitRsa) {
       CertVerificationContextImplForTest(CreateString(kEx2PublicKeySpki));
 
   EXPECT_TRUE(context->VerifySignatureOverData(CreateString(kEx2Signature),
-                                               CreateString(kEx2Message)));
+                                               CreateString(kEx2Message),
+                                               net::DigestAlgorithm::Sha1));
 }
 
 }  // namespace

components/cast_channel/cast_auth_util.cc

@@ -20,6 +20,7 @@
 #include "components/cast_channel/cast_channel_enum.h"
 #include "components/cast_channel/cast_message_util.h"
 #include "crypto/random.h"
+#include "net/cert/internal/signature_algorithm.h"
 #include "net/cert/x509_certificate.h"
 #include "net/der/parse_values.h"
 
@@ -51,10 +52,16 @@ const base::Feature kEnforceRevocationChecking{
 // the one sent to the device. As a result, the nonce can be empty and omitted
 // from the signature. This allows backwards compatibility with legacy Cast
 // receivers.
-
 const base::Feature kEnforceNonceChecking{"CastNonceEnforced",
                                           base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Enforce the use of SHA256 digest for signatures.
+// If disabled, the device may respond with a signature with SHA1 digest even
+// though a signature with SHA256 digest was requested in the challenge. This
+// allows for backwards compatibility with legacy Cast receivers.
+const base::Feature kEnforceSHA256Checking{"CastSHA256Enforced",
+                                           base::FEATURE_DISABLED_BY_DEFAULT};
+
 namespace cast_crypto = ::cast_certificate;
 
 // Extracts an embedded DeviceAuthMessage payload from an auth challenge reply
@@ -154,6 +161,16 @@ enum NonceVerificationStatus {
   NONCE_COUNT,
 };
 
+// Must match with the histogram enum CastSignature.
+// This should never be reordered.
+enum SignatureStatus {
+  SIGNATURE_OK,
+  SIGNATURE_EMPTY,
+  SIGNATURE_VERIFY_FAILED,
+  SIGNATURE_ALGORITHM_UNSUPPORTED,
+  SIGNATURE_COUNT,
+};
+
 // Record certificate verification histogram events.
 void RecordCertificateEvent(CertVerificationStatus event) {
   UMA_HISTOGRAM_ENUMERATION("Cast.Channel.Certificate", event,
@@ -165,6 +182,11 @@ void RecordNonceEvent(NonceVerificationStatus event) {
   UMA_HISTOGRAM_ENUMERATION("Cast.Channel.Nonce", event, NONCE_COUNT);
 }
 
+// Record signature verification histogram events.
+void RecordSignatureEvent(SignatureStatus event) {
+  UMA_HISTOGRAM_ENUMERATION("Cast.Channel.Signature", event, SIGNATURE_COUNT);
+}
+
 // Maps CastCertError to AuthResult.
 // If crl_required is set to false, all revocation related errors are ignored.
 AuthResult MapToAuthResult(cast_certificate::CastCertError error,
@@ -258,6 +280,24 @@ AuthResult AuthContext::VerifySenderNonce(
   return AuthResult();
 }
 
+AuthResult VerifyAndMapDigestAlgorithm(HashAlgorithm response_digest_algorithm,
+                                       net::DigestAlgorithm* digest_algorithm) {
+  switch (response_digest_algorithm) {
+    case SHA1:
+      RecordSignatureEvent(SIGNATURE_ALGORITHM_UNSUPPORTED);
+      *digest_algorithm = net::DigestAlgorithm::Sha1;
+      if (base::FeatureList::IsEnabled(kEnforceSHA256Checking)) {
+        return AuthResult("Unsupported digest algorithm.",
+                          AuthResult::ERROR_DIGEST_UNSUPPORTED);
+      }
+      break;
+    case SHA256:
+      *digest_algorithm = net::DigestAlgorithm::Sha256;
+      break;
+  }
+  return AuthResult();
+}
+
 // Verifies the peer certificate and populates |peer_cert_der| with the DER
 // encoded certificate.
 AuthResult VerifyTLSCertificate(const net::X509Certificate& peer_cert,
@@ -386,11 +426,24 @@ AuthResult VerifyCredentialsImpl(const AuthResponse& response,
 
   // The certificate is verified at this point.
   RecordCertificateEvent(CERT_STATUS_OK);
-  if (!verification_context->VerifySignatureOverData(response.signature(),
-                                                     signature_input)) {
-    return AuthResult("Failed verifying signature over data",
+
+  if (response.signature().empty() && !signature_input.empty()) {
+    RecordSignatureEvent(SIGNATURE_EMPTY);
+    return AuthResult("Signature is empty.", AuthResult::ERROR_SIGNATURE_EMPTY);
+  }
+  net::DigestAlgorithm digest_algorithm;
+  AuthResult digest_result =
+      VerifyAndMapDigestAlgorithm(response.hash_algorithm(), &digest_algorithm);
+  if (!digest_result.success())
+    return digest_result;
+
+  if (!verification_context->VerifySignatureOverData(
+          response.signature(), signature_input, digest_algorithm)) {
+    RecordSignatureEvent(SIGNATURE_VERIFY_FAILED);
+    return AuthResult("Failed verifying signature over data.",
                       AuthResult::ERROR_SIGNED_BLOBS_MISMATCH);
   }
+  RecordSignatureEvent(SIGNATURE_OK);
 
   AuthResult success;
 

components/cast_channel/cast_auth_util.h

@@ -46,6 +46,8 @@ struct AuthResult {
     ERROR_CRL_INVALID,
     ERROR_CERT_REVOKED,
     ERROR_SENDER_NONCE_MISMATCH,
+    ERROR_DIGEST_UNSUPPORTED,
+    ERROR_SIGNATURE_EMPTY,
   };
 
   enum PolicyType { POLICY_NONE = 0, POLICY_AUDIO_ONLY = 1 << 0 };

components/cast_channel/cast_auth_util_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <string>
 
+#include "base/logging.h"
 #include "base/macros.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/time/time.h"
@@ -29,7 +30,8 @@ class CastAuthUtilTest : public testing::Test {
   void SetUp() override {}
 
  protected:
-  static AuthResponse CreateAuthResponse(std::string* signed_data) {
+  static AuthResponse CreateAuthResponse(std::string* signed_data,
+                                         HashAlgorithm digest_algorithm) {
     auto chain = cast_certificate::testing::ReadCertificateChainFromFile(
         "certificates/chromecast_gen1.pem");
     CHECK(!chain.empty());
@@ -43,7 +45,15 @@ class CastAuthUtilTest : public testing::Test {
     for (size_t i = 1; i < chain.size(); ++i)
       response.add_intermediate_certificate(chain[i]);
 
-    response.set_signature(signature_data.signature_sha1);
+    response.set_hash_algorithm(digest_algorithm);
+    switch (digest_algorithm) {
+      case SHA1:
+        response.set_signature(signature_data.signature_sha1);
+        break;
+      case SHA256:
+        response.set_signature(signature_data.signature_sha256);
+        break;
+    }
     *signed_data = signature_data.message;
 
     return response;
@@ -58,7 +68,7 @@ class CastAuthUtilTest : public testing::Test {
 // being verified doesn't expire until 2032!
 TEST_F(CastAuthUtilTest, VerifySuccess) {
   std::string signed_data;
-  AuthResponse auth_response = CreateAuthResponse(&signed_data);
+  AuthResponse auth_response = CreateAuthResponse(&signed_data, SHA256);
   base::Time now = base::Time::Now();
   AuthResult result = VerifyCredentialsForTest(
       auth_response, signed_data, cast_certificate::CRLPolicy::CRL_OPTIONAL,
@@ -69,7 +79,7 @@ TEST_F(CastAuthUtilTest, VerifySuccess) {
 
 TEST_F(CastAuthUtilTest, VerifyBadCA) {
   std::string signed_data;
-  AuthResponse auth_response = CreateAuthResponse(&signed_data);
+  AuthResponse auth_response = CreateAuthResponse(&signed_data, SHA256);
   MangleString(auth_response.mutable_intermediate_certificate(0));
   AuthResult result = VerifyCredentials(auth_response, signed_data);
   EXPECT_FALSE(result.success());
@@ -78,7 +88,7 @@ TEST_F(CastAuthUtilTest, VerifyBadCA) {
 
 TEST_F(CastAuthUtilTest, VerifyBadClientAuthCert) {
   std::string signed_data;
-  AuthResponse auth_response = CreateAuthResponse(&signed_data);
+  AuthResponse auth_response = CreateAuthResponse(&signed_data, SHA256);
   MangleString(auth_response.mutable_client_auth_certificate());
   AuthResult result = VerifyCredentials(auth_response, signed_data);
   EXPECT_FALSE(result.success());
@@ -88,16 +98,49 @@ TEST_F(CastAuthUtilTest, VerifyBadClientAuthCert) {
 
 TEST_F(CastAuthUtilTest, VerifyBadSignature) {
   std::string signed_data;
-  AuthResponse auth_response = CreateAuthResponse(&signed_data);
+  AuthResponse auth_response = CreateAuthResponse(&signed_data, SHA256);
   MangleString(auth_response.mutable_signature());
   AuthResult result = VerifyCredentials(auth_response, signed_data);
   EXPECT_FALSE(result.success());
   EXPECT_EQ(AuthResult::ERROR_SIGNED_BLOBS_MISMATCH, result.error_type);
 }
 
+TEST_F(CastAuthUtilTest, VerifyEmptySignature) {
+  std::string signed_data;
+  AuthResponse auth_response = CreateAuthResponse(&signed_data, SHA256);
+  auth_response.mutable_signature()->clear();
+  AuthResult result = VerifyCredentials(auth_response, signed_data);
+  EXPECT_FALSE(result.success());
+  EXPECT_EQ(AuthResult::ERROR_SIGNATURE_EMPTY, result.error_type);
+}
+
+TEST_F(CastAuthUtilTest, VerifyUnsupportedDigest) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      base::Feature{"CastSHA256Enforced", base::FEATURE_DISABLED_BY_DEFAULT});
+  std::string signed_data;
+  AuthResponse auth_response = CreateAuthResponse(&signed_data, SHA1);
+  base::Time now = base::Time::Now();
+  AuthResult result = VerifyCredentialsForTest(
+      auth_response, signed_data, cast_certificate::CRLPolicy::CRL_OPTIONAL,
+      nullptr, nullptr, now);
+  EXPECT_FALSE(result.success());
+  EXPECT_EQ(AuthResult::ERROR_DIGEST_UNSUPPORTED, result.error_type);
+}
+
+TEST_F(CastAuthUtilTest, VerifyBackwardsCompatibleDigest) {
+  std::string signed_data;
+  AuthResponse auth_response = CreateAuthResponse(&signed_data, SHA1);
+  base::Time now = base::Time::Now();
+  AuthResult result = VerifyCredentialsForTest(
+      auth_response, signed_data, cast_certificate::CRLPolicy::CRL_OPTIONAL,
+      nullptr, nullptr, now);
+  EXPECT_TRUE(result.success());
+}
+
 TEST_F(CastAuthUtilTest, VerifyBadPeerCert) {
   std::string signed_data;
-  AuthResponse auth_response = CreateAuthResponse(&signed_data);
+  AuthResponse auth_response = CreateAuthResponse(&signed_data, SHA256);
   MangleString(&signed_data);
   AuthResult result = VerifyCredentials(auth_response, signed_data);
   EXPECT_FALSE(result.success());

components/cast_channel/cast_channel_enum.h

@@ -97,6 +97,8 @@ enum class ChallengeReplyError {
   CRL_INVALID,
   CERT_REVOKED,
   SENDER_NONCE_MISMATCH,
+  SIGNATURE_EMPTY,
+  DIGEST_UNSUPPORTED,
 };
 
 // Used by CastSocket/CastTransport to track connection state.

components/cast_channel/cast_message_util.cc

@@ -68,7 +68,12 @@ void CreateAuthChallengeMessage(CastMessage* message_proto,
                                 const AuthContext& auth_context) {
   CHECK(message_proto);
   DeviceAuthMessage auth_message;
-  auth_message.mutable_challenge()->set_sender_nonce(auth_context.nonce());
+
+  AuthChallenge* challenge = auth_message.mutable_challenge();
+  DCHECK(challenge);
+  challenge->set_sender_nonce(auth_context.nonce());
+  challenge->set_hash_algorithm(SHA256);
+
   std::string auth_message_string;
   auth_message.SerializeToString(&auth_message_string);
 

components/cast_channel/logger.cc

@@ -60,6 +60,10 @@ ChallengeReplyError AuthErrorToChallengeReplyError(
       return ChallengeReplyError::CERT_REVOKED;
     case AuthResult::ERROR_SENDER_NONCE_MISMATCH:
       return ChallengeReplyError::SENDER_NONCE_MISMATCH;
+    case AuthResult::ERROR_SIGNATURE_EMPTY:
+      return ChallengeReplyError::SIGNATURE_EMPTY;
+    case AuthResult::ERROR_DIGEST_UNSUPPORTED:
+      return ChallengeReplyError::DIGEST_UNSUPPORTED;
     default:
       NOTREACHED();
       return ChallengeReplyError::NONE;