diff options
author | btolsch <btolsch@chromium.org> | 2020-01-06 11:40:53 -0800 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2020-01-06 19:50:37 +0000 |
commit | d4b6a806880aaeb29c41c8a8804afd7de6a198ee (patch) | |
tree | dcd8022f880dcd1265c84d4da9ee8f85d27088de /cast/sender | |
parent | ecf313250e37db6ec3a761b7927ac997c6c7c3ad (diff) | |
download | openscreen-d4b6a806880aaeb29c41c8a8804afd7de6a198ee.tar.gz |
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>
Diffstat (limited to 'cast/sender')
-rw-r--r-- | cast/sender/channel/cast_auth_util.cc | 34 | ||||
-rw-r--r-- | cast/sender/channel/cast_auth_util.h | 5 | ||||
-rw-r--r-- | cast/sender/channel/cast_auth_util_unittest.cc | 36 |
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; |