Allocate one string for nonce + certificate data

This change removes an extra string allocation + copy in
AuthenticateChallengeReply.  It also removes an overly redundant size
check.

Bug: None
Change-Id: Ief845b3b347cb6455219efe3e857b545214a1db1
Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/1976983
Commit-Queue: Brandon Tolsch <btolsch@chromium.org>
Reviewed-by: Ryan Keane <rwkeane@google.com>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>

3 files changed, 41 insertions, 34 deletions

diff --git a/cast/sender/channel/cast_auth_util.cc b/cast/sender/channel/cast_auth_util.cc
index 6954980a..fc103e9b 100644
--- a/cast/sender/channel/cast_auth_util.cc
+++ b/cast/sender/channel/cast_auth_util.cc
@@ -6,7 +6,7 @@
 
 #include <openssl/rand.h>
 
-#include <vector>
+#include <algorithm>
 
 #include "cast/common/certificate/cast_cert_validator.h"
 #include "cast/common/certificate/cast_cert_validator_internal.h"
@@ -221,7 +221,7 @@ Error VerifyTLSCertificateValidity(X509* peer_cert,
 
 ErrorOr<CastDeviceCertPolicy> VerifyCredentialsImpl(
     const AuthResponse& response,
-    const std::string& signature_input,
+    const std::vector<uint8_t>& signature_input,
     const CRLPolicy& crl_policy,
     TrustStore* cast_trust_store,
     TrustStore* crl_trust_store,
@@ -256,21 +256,22 @@ ErrorOr<CastDeviceCertPolicy> AuthenticateChallengeReplyImpl(
     return result;
   }
 
-  int len = i2d_X509(peer_cert, nullptr);
-  if (len <= 0) {
+  int cert_len = i2d_X509(peer_cert, nullptr);
+  if (cert_len <= 0) {
     return Error(Error::Code::kErrCertsParse, "Serializing cert failed.");
   }
-  std::string peer_cert_der(len, 0);
-  uint8_t* data = reinterpret_cast<uint8_t*>(&peer_cert_der[0]);
+  size_t nonce_response_size = nonce_response.size();
+  std::vector<uint8_t> nonce_plus_peer_cert_der(nonce_response_size + cert_len,
+                                                0);
+  std::copy(nonce_response.begin(), nonce_response.end(),
+            &nonce_plus_peer_cert_der[0]);
+  uint8_t* data = &nonce_plus_peer_cert_der[nonce_response_size];
   if (!i2d_X509(peer_cert, &data)) {
     return Error(Error::Code::kErrCertsParse, "Serializing cert failed.");
   }
-  size_t actual_size = data - reinterpret_cast<uint8_t*>(&peer_cert_der[0]);
-  OSP_DCHECK_EQ(actual_size, peer_cert_der.size());
-  peer_cert_der.resize(actual_size);
 
-  return VerifyCredentialsImpl(response, nonce_response + peer_cert_der,
-                               crl_policy, cast_trust_store, crl_trust_store,
+  return VerifyCredentialsImpl(response, nonce_plus_peer_cert_der, crl_policy,
+                               cast_trust_store, crl_trust_store,
                                verification_time, false);
 }
 
@@ -318,7 +319,7 @@ ErrorOr<CastDeviceCertPolicy> AuthenticateChallengeReplyForTest(
 //   |signature_input| by |response.client_auth_certificate|'s public key.
 ErrorOr<CastDeviceCertPolicy> VerifyCredentialsImpl(
     const AuthResponse& response,
-    const std::string& signature_input,
+    const std::vector<uint8_t>& signature_input,
     const CRLPolicy& crl_policy,
     TrustStore* cast_trust_store,
     TrustStore* crl_trust_store,
@@ -368,9 +369,8 @@ ErrorOr<CastDeviceCertPolicy> VerifyCredentialsImpl(
   ConstDataSpan signature = {
       reinterpret_cast<const uint8_t*>(response.signature().data()),
       static_cast<uint32_t>(response.signature().size())};
-  ConstDataSpan siginput = {
-      reinterpret_cast<const uint8_t*>(signature_input.data()),
-      static_cast<uint32_t>(signature_input.size())};
+  ConstDataSpan siginput = {signature_input.data(),
+                            static_cast<uint32_t>(signature_input.size())};
   if (!verification_context->VerifySignatureOverData(signature, siginput,
                                                      digest_algorithm)) {
     return Error(Error::Code::kCastV2SignedBlobsMismatch,
@@ -382,7 +382,7 @@ ErrorOr<CastDeviceCertPolicy> VerifyCredentialsImpl(
 
 ErrorOr<CastDeviceCertPolicy> VerifyCredentials(
     const AuthResponse& response,
-    const std::string& signature_input,
+    const std::vector<uint8_t>& signature_input,
     bool enforce_revocation_checking,
     bool enforce_sha256_checking) {
   DateTime now = {};
@@ -395,7 +395,7 @@ ErrorOr<CastDeviceCertPolicy> VerifyCredentials(
 
 ErrorOr<CastDeviceCertPolicy> VerifyCredentialsForTest(
     const AuthResponse& response,
-    const std::string& signature_input,
+    const std::vector<uint8_t>& signature_input,
     CRLPolicy crl_policy,
     TrustStore* cast_trust_store,
     TrustStore* crl_trust_store,
diff --git a/cast/sender/channel/cast_auth_util.h b/cast/sender/channel/cast_auth_util.h
index 467dfb82..4ca6df5e 100644
--- a/cast/sender/channel/cast_auth_util.h
+++ b/cast/sender/channel/cast_auth_util.h
@@ -9,6 +9,7 @@
 
 #include <chrono>  // NOLINT
 #include <string>
+#include <vector>
 
 #include "cast/common/certificate/cast_cert_validator.h"
 #include "platform/base/error.h"
@@ -79,7 +80,7 @@ Error VerifyTLSCertificateValidity(X509* peer_cert,
 // |signature_input|.
 ErrorOr<CastDeviceCertPolicy> VerifyCredentials(
     const ::cast::channel::AuthResponse& response,
-    const std::string& signature_input,
+    const std::vector<uint8_t>& signature_input,
     bool enforce_revocation_checking = false,
     bool enforce_sha256_checking = false);
 
@@ -89,7 +90,7 @@ ErrorOr<CastDeviceCertPolicy> VerifyCredentials(
 // trust stores, and verification times.
 ErrorOr<CastDeviceCertPolicy> VerifyCredentialsForTest(
     const ::cast::channel::AuthResponse& response,
-    const std::string& signature_input,
+    const std::vector<uint8_t>& signature_input,
     CRLPolicy crl_policy,
     TrustStore* cast_trust_store,
     TrustStore* crl_trust_store,
diff --git a/cast/sender/channel/cast_auth_util_unittest.cc b/cast/sender/channel/cast_auth_util_unittest.cc
index 3fd80855..5c6afd22 100644
--- a/cast/sender/channel/cast_auth_util_unittest.cc
+++ b/cast/sender/channel/cast_auth_util_unittest.cc
@@ -118,7 +118,7 @@ class CastAuthUtilTest : public ::testing::Test {
 
  protected:
   static AuthResponse CreateAuthResponse(
-      std::string* signed_data,
+      std::vector<uint8_t>* signed_data,
       ::cast::channel::HashAlgorithm digest_algorithm) {
     std::vector<std::string> chain = testing::ReadCertificatesFromPemFile(
         TEST_DATA_PREFIX "certificates/chromecast_gen1.pem");
@@ -147,21 +147,27 @@ class CastAuthUtilTest : public ::testing::Test {
                         signatures.sha256.length));
         break;
     }
-    signed_data->assign(reinterpret_cast<const char*>(signatures.message.data),
-                        signatures.message.length);
+    *signed_data = std::vector<uint8_t>(
+        signatures.message.data,
+        signatures.message.data + signatures.message.length);
 
     return response;
   }
 
   // Mangles a string by inverting the first byte.
   static void MangleString(std::string* str) { (*str)[0] = ~(*str)[0]; }
+
+  // Mangles a vector by inverting the first byte.
+  static void MangleData(std::vector<uint8_t>* data) {
+    (*data)[0] = ~(*data)[0];
+  }
 };
 
 // Note on expiration: VerifyCredentials() depends on the system clock. In
 // practice this shouldn't be a problem though since the certificate chain
 // being verified doesn't expire until 2032.
 TEST_F(CastAuthUtilTest, VerifySuccess) {
-  std::string signed_data;
+  std::vector<uint8_t> signed_data;
   AuthResponse auth_response =
       CreateAuthResponse(&signed_data, ::cast::channel::SHA256);
   DateTime now = {};
@@ -174,7 +180,7 @@ TEST_F(CastAuthUtilTest, VerifySuccess) {
 }
 
 TEST_F(CastAuthUtilTest, VerifyBadCA) {
-  std::string signed_data;
+  std::vector<uint8_t> signed_data;
   AuthResponse auth_response =
       CreateAuthResponse(&signed_data, ::cast::channel::SHA256);
   MangleString(auth_response.mutable_intermediate_certificate(0));
@@ -185,7 +191,7 @@ TEST_F(CastAuthUtilTest, VerifyBadCA) {
 }
 
 TEST_F(CastAuthUtilTest, VerifyBadClientAuthCert) {
-  std::string signed_data;
+  std::vector<uint8_t> signed_data;
   AuthResponse auth_response =
       CreateAuthResponse(&signed_data, ::cast::channel::SHA256);
   MangleString(auth_response.mutable_client_auth_certificate());
@@ -196,7 +202,7 @@ TEST_F(CastAuthUtilTest, VerifyBadClientAuthCert) {
 }
 
 TEST_F(CastAuthUtilTest, VerifyBadSignature) {
-  std::string signed_data;
+  std::vector<uint8_t> signed_data;
   AuthResponse auth_response =
       CreateAuthResponse(&signed_data, ::cast::channel::SHA256);
   MangleString(auth_response.mutable_signature());
@@ -207,7 +213,7 @@ TEST_F(CastAuthUtilTest, VerifyBadSignature) {
 }
 
 TEST_F(CastAuthUtilTest, VerifyEmptySignature) {
-  std::string signed_data;
+  std::vector<uint8_t> signed_data;
   AuthResponse auth_response =
       CreateAuthResponse(&signed_data, ::cast::channel::SHA256);
   auth_response.mutable_signature()->clear();
@@ -218,7 +224,7 @@ TEST_F(CastAuthUtilTest, VerifyEmptySignature) {
 }
 
 TEST_F(CastAuthUtilTest, VerifyUnsupportedDigest) {
-  std::string signed_data;
+  std::vector<uint8_t> signed_data;
   AuthResponse auth_response =
       CreateAuthResponse(&signed_data, ::cast::channel::SHA1);
   DateTime now = {};
@@ -231,7 +237,7 @@ TEST_F(CastAuthUtilTest, VerifyUnsupportedDigest) {
 }
 
 TEST_F(CastAuthUtilTest, VerifyBackwardsCompatibleDigest) {
-  std::string signed_data;
+  std::vector<uint8_t> signed_data;
   AuthResponse auth_response =
       CreateAuthResponse(&signed_data, ::cast::channel::SHA1);
   DateTime now = {};
@@ -243,10 +249,10 @@ TEST_F(CastAuthUtilTest, VerifyBackwardsCompatibleDigest) {
 }
 
 TEST_F(CastAuthUtilTest, VerifyBadPeerCert) {
-  std::string signed_data;
+  std::vector<uint8_t> signed_data;
   AuthResponse auth_response =
       CreateAuthResponse(&signed_data, ::cast::channel::SHA256);
-  MangleString(&signed_data);
+  MangleData(&signed_data);
   ErrorOr<CastDeviceCertPolicy> result =
       VerifyCredentials(auth_response, signed_data);
   EXPECT_FALSE(result);
@@ -370,9 +376,9 @@ ErrorOr<CastDeviceCertPolicy> TestVerifyRevocation(
   CRLPolicy crl_policy = CRLPolicy::kCrlRequired;
   if (!crl_required && crl_bundle.empty())
     crl_policy = CRLPolicy::kCrlOptional;
-  ErrorOr<CastDeviceCertPolicy> result =
-      VerifyCredentialsForTest(response, "", crl_policy, cast_trust_store,
-                               crl_trust_store, verification_time);
+  ErrorOr<CastDeviceCertPolicy> result = VerifyCredentialsForTest(
+      response, std::vector<uint8_t>(), crl_policy, cast_trust_store,
+      crl_trust_store, verification_time);
   // This test doesn't set the signature so it will just fail there.
   EXPECT_FALSE(result);
   return result;