Skip to content

Commit

Permalink
Switch crypto::SignatureVerifier to base::span.
Browse files Browse the repository at this point in the history
Bug: 830085, 559302
Change-Id: I5dc2bc41154846fd69e276c8d64727ecd6764052
Reviewed-on: https://chromium-review.googlesource.com/919318
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550795}
  • Loading branch information
davidben authored and Commit Bot committed Apr 13, 2018
1 parent 53e13c2 commit 8ed9231
Show file tree
Hide file tree
Showing 18 changed files with 106 additions and 179 deletions.
12 changes: 4 additions & 8 deletions chrome/browser/extensions/install_signer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -259,16 +259,12 @@ bool InstallSigner::VerifySignature(const InstallSignature& signature) {
return false;

crypto::SignatureVerifier verifier;
if (!verifier.VerifyInit(
crypto::SignatureVerifier::RSA_PKCS1_SHA1,
reinterpret_cast<const uint8_t*>(signature.signature.data()),
signature.signature.size(),
reinterpret_cast<const uint8_t*>(public_key.data()),
public_key.size()))
if (!verifier.VerifyInit(crypto::SignatureVerifier::RSA_PKCS1_SHA1,
base::as_bytes<const char>(signature.signature),
base::as_bytes<const char>(public_key)))
return false;

verifier.VerifyUpdate(reinterpret_cast<const uint8_t*>(signed_data.data()),
signed_data.size());
verifier.VerifyUpdate(base::as_bytes<const char>(signed_data));
return verifier.VerifyFinal();
}

Expand Down
8 changes: 3 additions & 5 deletions components/client_update_protocol/ecdsa.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,8 @@ bool Ecdsa::ValidateResponse(const base::StringPiece& response_body,

// Initialize the signature verifier.
crypto::SignatureVerifier verifier;
if (!verifier.VerifyInit(crypto::SignatureVerifier::ECDSA_SHA256,
&signature.front(), signature.size(),
&public_key_.front(), public_key_.size())) {
if (!verifier.VerifyInit(crypto::SignatureVerifier::ECDSA_SHA256, signature,
public_key_)) {
DVLOG(1) << "Couldn't init SignatureVerifier.";
return false;
}
Expand All @@ -182,8 +181,7 @@ bool Ecdsa::ValidateResponse(const base::StringPiece& response_body,
// * The buffer that the server signed does not match the buffer that the
// client assembled -- implying that either request body or response body
// was modified, or a different nonce value was used.
verifier.VerifyUpdate(&signed_message_hash.front(),
signed_message_hash.size());
verifier.VerifyUpdate(signed_message_hash);
return verifier.VerifyFinal();
}

Expand Down
21 changes: 8 additions & 13 deletions components/crx_file/crx_verifier.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ bool ReadHashAndVerifyArchive(base::File* file,
size_t len = 0;
while ((len = ReadAndHashBuffer(buffer, arraysize(buffer), file, hash)) > 0) {
for (auto& verifier : verifiers)
verifier->VerifyUpdate(buffer, len);
verifier->VerifyUpdate(base::make_span(buffer, len));
}
for (auto& verifier : verifiers) {
if (!verifier->VerifyFinal())
Expand Down Expand Up @@ -163,16 +163,12 @@ VerifierResult VerifyCrx3(
auto v = std::make_unique<crypto::SignatureVerifier>();
static_assert(sizeof(unsigned char) == sizeof(uint8_t),
"Unsupported char size.");
if (!v->VerifyInit(
proof_type.second, reinterpret_cast<const uint8_t*>(sig.data()),
sig.size(), reinterpret_cast<const uint8_t*>(key.data()),
key.size()))
if (!v->VerifyInit(proof_type.second, base::as_bytes<const char>(sig),
base::as_bytes<const char>(key)))
return VerifierResult::ERROR_SIGNATURE_INITIALIZATION_FAILED;
v->VerifyUpdate(kSignatureContext, arraysize(kSignatureContext));
v->VerifyUpdate(header_size_octets, arraysize(header_size_octets));
v->VerifyUpdate(
reinterpret_cast<const uint8_t*>(signed_header_data_str.data()),
signed_header_data_str.size());
v->VerifyUpdate(kSignatureContext);
v->VerifyUpdate(header_size_octets);
v->VerifyUpdate(base::as_bytes<const char>(signed_header_data_str));
verifiers.push_back(std::move(v));
}
}
Expand Down Expand Up @@ -221,9 +217,8 @@ VerifierResult VerifyCrx2(
return VerifierResult::ERROR_HEADER_INVALID;
std::vector<std::unique_ptr<crypto::SignatureVerifier>> verifiers;
verifiers.push_back(std::make_unique<crypto::SignatureVerifier>());
if (!verifiers[0]->VerifyInit(crypto::SignatureVerifier::RSA_PKCS1_SHA1,
sig.data(), sig.size(), key.data(),
key.size())) {
if (!verifiers[0]->VerifyInit(crypto::SignatureVerifier::RSA_PKCS1_SHA1, sig,
key)) {
return VerifierResult::ERROR_SIGNATURE_INITIALIZATION_FAILED;
}

Expand Down
9 changes: 3 additions & 6 deletions components/policy/core/common/cloud/cloud_policy_validator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -569,15 +569,12 @@ bool CloudPolicyValidatorBase::VerifySignature(const std::string& data,
return false;
}

if (!verifier.VerifyInit(
algorithm, reinterpret_cast<const uint8_t*>(signature.c_str()),
signature.size(), reinterpret_cast<const uint8_t*>(key.c_str()),
key.size())) {
if (!verifier.VerifyInit(algorithm, base::as_bytes<const char>(signature),
base::as_bytes<const char>(key))) {
DLOG(ERROR) << "Invalid verification signature/key format";
return false;
}
verifier.VerifyUpdate(reinterpret_cast<const uint8_t*>(data.c_str()),
data.size());
verifier.VerifyUpdate(base::as_bytes<const char>(data));
return verifier.VerifyFinal();
}

Expand Down
7 changes: 2 additions & 5 deletions components/variations/variations_seed_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,11 @@ VerifySignatureResult VerifySeedSignature(

crypto::SignatureVerifier verifier;
if (!verifier.VerifyInit(crypto::SignatureVerifier::ECDSA_SHA256,
reinterpret_cast<const uint8_t*>(signature.data()),
signature.size(), kPublicKey,
arraysize(kPublicKey))) {
base::as_bytes<const char>(signature), kPublicKey)) {
return VerifySignatureResult::INVALID_SIGNATURE;
}

verifier.VerifyUpdate(reinterpret_cast<const uint8_t*>(seed_bytes.data()),
seed_bytes.size());
verifier.VerifyUpdate(base::as_bytes<const char>(seed_bytes));
if (!verifier.VerifyFinal())
return VerifySignatureResult::INVALID_SEED;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,13 @@ bool VerifySignature(
// TODO(crbug.com/803774): This is missing the digitalSignature key usage bit
// check.
crypto::SignatureVerifier verifier;
if (!verifier.VerifyInit(
crypto::SignatureVerifier::RSA_PSS_SHA256, sig.data(), sig.size(),
reinterpret_cast<const uint8_t*>(spki.data()), spki.size())) {
if (!verifier.VerifyInit(crypto::SignatureVerifier::RSA_PSS_SHA256, sig,
base::as_bytes<const char>(spki))) {
signed_exchange_utils::RunErrorMessageCallbackAndEndTraceEvent(
"VerifySignature", error_message_callback, "VerifyInit failed.");
return false;
}
verifier.VerifyUpdate(msg.data(), msg.size());
verifier.VerifyUpdate(msg);
if (!verifier.VerifyFinal()) {
signed_exchange_utils::RunErrorMessageCallbackAndEndTraceEvent(
"VerifySignature", error_message_callback, "VerifyFinal failed.");
Expand Down
8 changes: 3 additions & 5 deletions crypto/ec_signature_creator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,9 @@ TEST(ECSignatureCreatorTest, BasicTest) {
ASSERT_TRUE(key_original->ExportPublicKey(&public_key_info));

crypto::SignatureVerifier verifier;
ASSERT_TRUE(verifier.VerifyInit(
crypto::SignatureVerifier::ECDSA_SHA256, &signature[0], signature.size(),
&public_key_info.front(), public_key_info.size()));
ASSERT_TRUE(verifier.VerifyInit(crypto::SignatureVerifier::ECDSA_SHA256,
signature, public_key_info));

verifier.VerifyUpdate(reinterpret_cast<const uint8_t*>(data.c_str()),
data.size());
verifier.VerifyUpdate(base::as_bytes<const char>(data));
ASSERT_TRUE(verifier.VerifyFinal());
}
24 changes: 9 additions & 15 deletions crypto/signature_creator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,10 @@ TEST(SignatureCreatorTest, BasicTest) {
ASSERT_TRUE(key_original->ExportPublicKey(&public_key_info));

crypto::SignatureVerifier verifier;
ASSERT_TRUE(verifier.VerifyInit(
crypto::SignatureVerifier::RSA_PKCS1_SHA1, &signature.front(),
signature.size(), &public_key_info.front(), public_key_info.size()));
ASSERT_TRUE(verifier.VerifyInit(crypto::SignatureVerifier::RSA_PKCS1_SHA1,
signature, public_key_info));

verifier.VerifyUpdate(reinterpret_cast<const uint8_t*>(data.c_str()),
data.size());
verifier.VerifyUpdate(base::as_bytes<const char>(data));
ASSERT_TRUE(verifier.VerifyFinal());
}

Expand Down Expand Up @@ -78,12 +76,10 @@ TEST(SignatureCreatorTest, SignDigestTest) {

// Verify the input data.
crypto::SignatureVerifier verifier;
ASSERT_TRUE(verifier.VerifyInit(
crypto::SignatureVerifier::RSA_PKCS1_SHA1, &signature.front(),
signature.size(), &public_key_info.front(), public_key_info.size()));
ASSERT_TRUE(verifier.VerifyInit(crypto::SignatureVerifier::RSA_PKCS1_SHA1,
signature, public_key_info));

verifier.VerifyUpdate(reinterpret_cast<const uint8_t*>(data.c_str()),
data.size());
verifier.VerifyUpdate(base::as_bytes<const char>(data));
ASSERT_TRUE(verifier.VerifyFinal());
}

Expand Down Expand Up @@ -113,11 +109,9 @@ TEST(SignatureCreatorTest, SignSHA256DigestTest) {

// Verify the input data.
crypto::SignatureVerifier verifier;
ASSERT_TRUE(verifier.VerifyInit(
crypto::SignatureVerifier::RSA_PKCS1_SHA256, &signature.front(),
signature.size(), &public_key_info.front(), public_key_info.size()));
ASSERT_TRUE(verifier.VerifyInit(crypto::SignatureVerifier::RSA_PKCS1_SHA256,
signature, public_key_info));

verifier.VerifyUpdate(reinterpret_cast<const uint8_t*>(data.c_str()),
data.size());
verifier.VerifyUpdate(base::as_bytes<const char>(data));
ASSERT_TRUE(verifier.VerifyFinal());
}
17 changes: 7 additions & 10 deletions crypto/signature_verifier.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,8 @@ SignatureVerifier::SignatureVerifier() = default;
SignatureVerifier::~SignatureVerifier() = default;

bool SignatureVerifier::VerifyInit(SignatureAlgorithm signature_algorithm,
const uint8_t* signature,
size_t signature_len,
const uint8_t* public_key_info,
size_t public_key_info_len) {
base::span<const uint8_t> signature,
base::span<const uint8_t> public_key_info) {
OpenSSLErrStackTracer err_tracer(FROM_HERE);

int pkey_type = EVP_PKEY_NONE;
Expand All @@ -57,10 +55,10 @@ bool SignatureVerifier::VerifyInit(SignatureAlgorithm signature_algorithm,
return false;

verify_context_.reset(new VerifyContext);
signature_.assign(signature, signature + signature_len);
signature_.assign(signature.data(), signature.data() + signature.size());

CBS cbs;
CBS_init(&cbs, public_key_info, public_key_info_len);
CBS_init(&cbs, public_key_info.data(), public_key_info.size());
bssl::UniquePtr<EVP_PKEY> public_key(EVP_parse_public_key(&cbs));
if (!public_key || CBS_len(&cbs) != 0 ||
EVP_PKEY_id(public_key.get()) != pkey_type) {
Expand All @@ -85,12 +83,11 @@ bool SignatureVerifier::VerifyInit(SignatureAlgorithm signature_algorithm,
return true;
}

void SignatureVerifier::VerifyUpdate(const uint8_t* data_part,
size_t data_part_len) {
void SignatureVerifier::VerifyUpdate(base::span<const uint8_t> data_part) {
DCHECK(verify_context_);
OpenSSLErrStackTracer err_tracer(FROM_HERE);
int rv = EVP_DigestVerifyUpdate(verify_context_->ctx.get(), data_part,
data_part_len);
int rv = EVP_DigestVerifyUpdate(verify_context_->ctx.get(), data_part.data(),
data_part.size());
DCHECK_EQ(rv, 1);
}

Expand Down
9 changes: 4 additions & 5 deletions crypto/signature_verifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <memory>
#include <vector>

#include "base/containers/span.h"
#include "build/build_config.h"
#include "crypto/crypto_export.h"

Expand Down Expand Up @@ -46,13 +47,11 @@ class CRYPTO_EXPORT SignatureVerifier {
// algorithm AlgorithmIdentifier,
// subjectPublicKey BIT STRING }
bool VerifyInit(SignatureAlgorithm signature_algorithm,
const uint8_t* signature,
size_t signature_len,
const uint8_t* public_key_info,
size_t public_key_info_len);
base::span<const uint8_t> signature,
base::span<const uint8_t> public_key_info);

// Feeds a piece of the data to the signature verifier.
void VerifyUpdate(const uint8_t* data_part, size_t data_part_len);
void VerifyUpdate(base::span<const uint8_t> data_part);

// Concludes a signature verification operation. Returns true if the
// signature is valid. Returns false if the signature is invalid or an
Expand Down
Loading

0 comments on commit 8ed9231

Please sign in to comment.