Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1313)

Unified Diff: net/cert/internal/verify_signed_data.cc

Issue 1259313002: Add some policy controls for VerifySignedData(). (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@add_python
Patch Set: Created 5 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: net/cert/internal/verify_signed_data.cc
diff --git a/net/cert/internal/verify_signed_data.cc b/net/cert/internal/verify_signed_data.cc
index fd68328694be015b60e39de5d041bfdd3a875678..c95e6d781cc28e3091164a4e2be519f8d4c58c36 100644
--- a/net/cert/internal/verify_signed_data.cc
+++ b/net/cert/internal/verify_signed_data.cc
@@ -18,7 +18,8 @@ namespace net {
bool VerifySignedData(const SignatureAlgorithm& signature_algorithm,
const der::Input& signed_data,
const der::Input& signature_value_bit_string,
- const der::Input& public_key) {
+ const der::Input& public_key,
+ const VerificationPolicy* policy) {
NOTIMPLEMENTED();
return false;
}
@@ -38,6 +39,7 @@ bool VerifySignedData(const SignatureAlgorithm& signature_algorithm,
#include "crypto/openssl_util.h"
#include "crypto/scoped_openssl_types.h"
#include "net/cert/internal/signature_algorithm.h"
+#include "net/cert/internal/verification_policy.h"
#include "net/der/input.h"
#include "net/der/parser.h"
@@ -46,9 +48,14 @@ namespace net {
namespace {
// Converts a DigestAlgorithm to an equivalent EVP_MD*.
-WARN_UNUSED_RESULT bool GetDigest(DigestAlgorithm digest, const EVP_MD** out) {
+WARN_UNUSED_RESULT bool GetDigest(DigestAlgorithm digest,
+ const VerificationPolicy* policy,
+ const EVP_MD** out) {
*out = nullptr;
+ if (!policy->IsAcceptableDigestAlgorithm(digest))
+ return false;
+
switch (digest) {
case DigestAlgorithm::Sha1:
*out = EVP_sha1();
@@ -69,7 +76,8 @@ WARN_UNUSED_RESULT bool GetDigest(DigestAlgorithm digest, const EVP_MD** out) {
// Sets the RSASSA-PSS parameters on |pctx|. Returns true on success.
WARN_UNUSED_RESULT bool ApplyRsaPssOptions(const RsaPssParameters* params,
- EVP_PKEY_CTX* pctx) {
+ EVP_PKEY_CTX* pctx,
+ const VerificationPolicy* policy) {
// BoringSSL takes a signed int for the salt length, and interprets
// negative values in a special manner. Make sure not to silently underflow.
base::CheckedNumeric<int> salt_length_bytes_int(params->salt_length());
@@ -77,7 +85,7 @@ WARN_UNUSED_RESULT bool ApplyRsaPssOptions(const RsaPssParameters* params,
return false;
const EVP_MD* mgf1_hash;
- if (!GetDigest(params->mgf1_hash(), &mgf1_hash))
+ if (!GetDigest(params->mgf1_hash(), policy, &mgf1_hash))
return false;
return EVP_PKEY_CTX_set_rsa_padding(pctx, RSA_PKCS1_PSS_PADDING) &&
@@ -160,8 +168,18 @@ WARN_UNUSED_RESULT bool ImportPkeyFromSpki(const der::Input& spki,
//
// Following RFC 3279 in this case.
WARN_UNUSED_RESULT bool ParseRsaKeyFromSpki(const der::Input& public_key_spki,
- crypto::ScopedEVP_PKEY* pkey) {
- return ImportPkeyFromSpki(public_key_spki, EVP_PKEY_RSA, pkey);
+ crypto::ScopedEVP_PKEY* pkey,
+ const VerificationPolicy* policy) {
+ if (!ImportPkeyFromSpki(public_key_spki, EVP_PKEY_RSA, pkey))
+ return false;
+
+ // Extract the modulus length from the key.
+ crypto::ScopedRSA rsa(EVP_PKEY_get1_RSA(pkey->get()));
+ if (!rsa.get())
+ return false;
+ unsigned int modulus_length_bits = BN_num_bits(rsa.get()->n);
+
+ return policy->IsAcceptableModulusLengthForRsa(modulus_length_bits);
}
// Does signature verification using either RSA or ECDSA.
@@ -171,7 +189,8 @@ WARN_UNUSED_RESULT bool ParseRsaKeyFromSpki(const der::Input& public_key_spki,
WARN_UNUSED_RESULT bool DoVerify(const SignatureAlgorithm& algorithm,
const der::Input& signed_data,
const der::Input& signature_value,
- EVP_PKEY* public_key) {
+ EVP_PKEY* public_key,
+ const VerificationPolicy* policy) {
DCHECK(algorithm.algorithm() == SignatureAlgorithmId::RsaPkcs1 ||
algorithm.algorithm() == SignatureAlgorithmId::RsaPss ||
algorithm.algorithm() == SignatureAlgorithmId::Ecdsa);
@@ -182,7 +201,7 @@ WARN_UNUSED_RESULT bool DoVerify(const SignatureAlgorithm& algorithm,
EVP_PKEY_CTX* pctx = nullptr; // Owned by |ctx|.
const EVP_MD* digest;
- if (!GetDigest(algorithm.digest(), &digest))
+ if (!GetDigest(algorithm.digest(), policy, &digest))
return false;
if (!EVP_DigestVerifyInit(ctx.get(), &pctx, digest, nullptr, public_key))
@@ -190,7 +209,7 @@ WARN_UNUSED_RESULT bool DoVerify(const SignatureAlgorithm& algorithm,
// Set the RSASSA-PSS specific options.
if (algorithm.algorithm() == SignatureAlgorithmId::RsaPss &&
- !ApplyRsaPssOptions(algorithm.ParamsForRsaPss(), pctx)) {
+ !ApplyRsaPssOptions(algorithm.ParamsForRsaPss(), pctx, policy)) {
return false;
}
@@ -203,21 +222,6 @@ WARN_UNUSED_RESULT bool DoVerify(const SignatureAlgorithm& algorithm,
signature_value.Length());
}
-// Returns true if the given curve is allowed for ECDSA. The input is a
-// BoringSSL NID.
-//
-// TODO(eroman): Extract policy decisions such as allowed curves, hashes, RSA
-// modulus size, to somewhere more central.
-WARN_UNUSED_RESULT bool IsAllowedCurveName(int curve_nid) {
- switch (curve_nid) {
- case NID_X9_62_prime256v1:
- case NID_secp384r1:
- case NID_secp521r1:
- return true;
- }
- return false;
-}
-
// Parses an EC public key from SPKI to an EVP_PKEY.
//
// Returns true on success.
@@ -263,18 +267,18 @@ WARN_UNUSED_RESULT bool IsAllowedCurveName(int curve_nid) {
// ... -- Extensible
// }
WARN_UNUSED_RESULT bool ParseEcKeyFromSpki(const der::Input& public_key_spki,
- crypto::ScopedEVP_PKEY* pkey) {
+ crypto::ScopedEVP_PKEY* pkey,
+ const VerificationPolicy* policy) {
if (!ImportPkeyFromSpki(public_key_spki, EVP_PKEY_EC, pkey))
return false;
- // Enforce policy on allowed curves in case ImportPkeyFromSpki() were to
- // recognize and allow use of a weak curve.
+ // Extract the curve name.
crypto::ScopedEC_KEY ec(EVP_PKEY_get1_EC_KEY(pkey->get()));
if (!ec.get())
return false; // Unexpected.
-
int curve_nid = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec.get()));
- return IsAllowedCurveName(curve_nid);
+
+ return policy->IsAcceptableCurveForEcdsa(curve_nid);
}
} // namespace
@@ -282,18 +286,24 @@ WARN_UNUSED_RESULT bool ParseEcKeyFromSpki(const der::Input& public_key_spki,
bool VerifySignedData(const SignatureAlgorithm& signature_algorithm,
const der::Input& signed_data,
const der::Input& signature_value_bit_string,
- const der::Input& public_key_spki) {
+ const der::Input& public_key_spki,
+ const VerificationPolicy* policy) {
+ // Optimization: Check the digest before parsing the SPKI. This isn't
+ // strictly necessary as it will also get checked by GetDigest().
+ if (!policy->IsAcceptableDigestAlgorithm(signature_algorithm.digest()))
+ return false;
+
crypto::ScopedEVP_PKEY public_key;
// Parse the SPKI to an EVP_PKEY appropriate for the signature algorithm.
switch (signature_algorithm.algorithm()) {
case SignatureAlgorithmId::RsaPkcs1:
case SignatureAlgorithmId::RsaPss:
- if (!ParseRsaKeyFromSpki(public_key_spki, &public_key))
+ if (!ParseRsaKeyFromSpki(public_key_spki, &public_key, policy))
return false;
break;
case SignatureAlgorithmId::Ecdsa:
- if (!ParseEcKeyFromSpki(public_key_spki, &public_key))
+ if (!ParseEcKeyFromSpki(public_key_spki, &public_key, policy))
return false;
break;
}
@@ -310,7 +320,7 @@ bool VerifySignedData(const SignatureAlgorithm& signature_algorithm,
return false;
return DoVerify(signature_algorithm, signed_data, signature_value,
- public_key.get());
+ public_key.get(), policy);
}
} // namespace net

Powered by Google App Engine
This is Rietveld 408576698