Add SignatureVerifier::VerifyInitRSAPSS for verifying RSA-PSS signatures.

crypto/signature_verifier_openssl.cc

Index: crypto/signature_verifier_openssl.cc
===================================================================
--- crypto/signature_verifier_openssl.cc	(revision 208575)
+++ crypto/signature_verifier_openssl.cc	(working copy)
@@ -16,8 +16,22 @@
 
 namespace crypto {
 
+namespace {
+
+const EVP_MD* ToOpenSSLDigest(SignatureVerifier::HashAlgorithm hash_alg) {
+  switch (hash_alg) {
+    case SignatureVerifier::SHA1:
+      return EVP_sha1();
+    case SignatureVerifier::SHA256:
+      return EVP_sha256();
+    default:

[agl, 2013/06/26 15:21:10]

Just a thought: if we omit the default case, would the compiler tell us that a
case was missing in the event that we add another value in the enum? A compile
time error would be nicer than failing/crashing.

[Ryan Sleevi, 2013/06/26 19:48:28]

Yes, it would. So +1 to omitting.

[wtc, 2013/06/27 02:23:51]

Done.

+      return NULL;
+  }
+}
+
+}  // namespace
+
 struct SignatureVerifier::VerifyContext {
-  ScopedOpenSSL<EVP_PKEY, EVP_PKEY_free> public_key;
   ScopedOpenSSL<EVP_MD_CTX, EVP_MD_CTX_destroy> ctx;
 };
 
@@ -29,24 +43,16 @@
   Reset();
 }
 
-bool SignatureVerifier::VerifyInit(const uint8* signature_algorithm,
-                                   int signature_algorithm_len,
+bool SignatureVerifier::CommonInit(const EVP_MD* digest,
                                    const uint8* signature,
                                    int signature_len,
                                    const uint8* public_key_info,
-                                   int public_key_info_len) {
+                                   int public_key_info_len,
+                                   EVP_PKEY_CTX** pkey_ctx) {
   DCHECK(!verify_context_);
   verify_context_ = new VerifyContext;
   OpenSSLErrStackTracer err_tracer(FROM_HERE);
 
-  ScopedOpenSSL<X509_ALGOR, X509_ALGOR_free> algorithm(
-      d2i_X509_ALGOR(NULL, &signature_algorithm, signature_algorithm_len));
-  if (!algorithm.get())
-    return false;
-
-  const EVP_MD* digest = EVP_get_digestbyobj(algorithm.get()->algorithm);
-  DCHECK(digest);
-
   signature_.assign(signature, signature + signature_len);
 
   // BIO_new_mem_buf is not const aware, but it does not modify the buffer.
@@ -56,30 +62,76 @@
   if (!bio.get())
     return false;
 
-  verify_context_->public_key.reset(d2i_PUBKEY_bio(bio.get(), NULL));
-  if (!verify_context_->public_key.get())
+  ScopedOpenSSL<EVP_PKEY, EVP_PKEY_free> public_key(
+      d2i_PUBKEY_bio(bio.get(), NULL));
+  if (!public_key.get())
     return false;
 
   verify_context_->ctx.reset(EVP_MD_CTX_create());
-  int rv = EVP_VerifyInit_ex(verify_context_->ctx.get(), digest, NULL);
+  int rv = EVP_DigestVerifyInit(verify_context_->ctx.get(), pkey_ctx,
+                                digest, NULL, public_key.get());
   return rv == 1;
 }
 
+bool SignatureVerifier::VerifyInit(const uint8* signature_algorithm,
+                                   int signature_algorithm_len,
+                                   const uint8* signature,
+                                   int signature_len,
+                                   const uint8* public_key_info,
+                                   int public_key_info_len) {
+  ScopedOpenSSL<X509_ALGOR, X509_ALGOR_free> algorithm(

[Ryan Sleevi, 2013/06/26 19:48:28]

BUG? No OpenSSLErrStackTracer to wipe errors here

[wtc, 2013/06/27 02:23:51]

Done.

+      d2i_X509_ALGOR(NULL, &signature_algorithm, signature_algorithm_len));
+  if (!algorithm.get())
+    return false;
+  const EVP_MD* digest = EVP_get_digestbyobj(algorithm.get()->algorithm);
+  DCHECK(digest);

[Ryan Sleevi, 2013/06/26 19:48:28]

Shouldn't this be a handled case?

It seems like if |digest| is NULL, then during the EVP_DigestVerifyInit() call,
OpenSSL will try to grab the default digest for the key type, which may result
in a hash confusion attack.

[wtc, 2013/06/27 02:23:51]

Done.

I assume |digest| can only be null if OpenSSL has a bug. But it is fine to
just handle this case.

+
+  return CommonInit(digest, signature, signature_len, public_key_info,
+                    public_key_info_len, NULL);
+}
+
+bool SignatureVerifier::VerifyInitRSAPSS(HashAlgorithm hash_alg,
+                                         HashAlgorithm mask_hash_alg,
+                                         int salt_len,
+                                         const uint8* signature,
+                                         int signature_len,
+                                         const uint8* public_key_info,
+                                         int public_key_info_len) {
+  const EVP_MD* digest = ToOpenSSLDigest(hash_alg);

[Ryan Sleevi, 2013/06/26 19:48:28]

BUG? No OpenSSLErrStackTracer to wipe errors here

[wtc, 2013/06/27 02:23:51]

Done.

+  DCHECK(digest);
+
+  EVP_PKEY_CTX* pkey_ctx;
+  if (!CommonInit(digest, signature, signature_len, public_key_info,
+                  public_key_info_len, &pkey_ctx)) {
+    return false;
+  }
+
+  int rv = EVP_PKEY_CTX_set_rsa_padding(pkey_ctx, RSA_PKCS1_PSS_PADDING);
+  if (rv != 1)
+    return false;
+  rv = EVP_PKEY_CTX_set_rsa_mgf1_md(pkey_ctx,
+                                    ToOpenSSLDigest(mask_hash_alg));
+  if (rv != 1)
+    return false;
+  rv = EVP_PKEY_CTX_set_rsa_pss_saltlen(pkey_ctx, salt_len);
+  return rv == 1;
+}
+
 void SignatureVerifier::VerifyUpdate(const uint8* data_part,
                                      int data_part_len) {
   DCHECK(verify_context_);
   OpenSSLErrStackTracer err_tracer(FROM_HERE);
-  int rv = EVP_VerifyUpdate(verify_context_->ctx.get(),
-                            data_part, data_part_len);
+  int rv = EVP_DigestVerifyUpdate(verify_context_->ctx.get(),
+                                  data_part, data_part_len);
   DCHECK_EQ(rv, 1);
 }
 
 bool SignatureVerifier::VerifyFinal() {
   DCHECK(verify_context_);
   OpenSSLErrStackTracer err_tracer(FROM_HERE);
-  int rv = EVP_VerifyFinal(verify_context_->ctx.get(),
-                           vector_as_array(&signature_), signature_.size(),
-                           verify_context_->public_key.get());
+  int rv = EVP_DigestVerifyFinal(verify_context_->ctx.get(),
+                                 vector_as_array(&signature_),
+                                 signature_.size());
   DCHECK_GE(rv, 0);
   Reset();
   return rv == 1;