commit 05821b0ee37325131505ab98c0752367a7cecdc7
author David Benjamin <davidben@google.com> Tue May 02 14:15:21 2017 -0400
committer Adam Langley <agl@google.com> Tue May 02 20:29:47 2017 +0000
tree e350253525cf6144ef896775e0f386b775ebd701
parent 8a3a2a99b2ed875fd6bfb6a21ad30eb1ab66da7d

Consistently check length in RSA_add_pkcs1_prefix.

We check the length for MD5+SHA1 but not the normal cases. Instead,
EVP_PKEY_sign externally checks the length (largely because the silly
RSA-PSS padding function forces it). We especially should be checking
the length for these because otherwise the prefix built into the ASN.1
prefix is wrong.

The primary motivation is to avoid putting EVP_PKEY inside the FIPS
module. This means all logic for supported algorithms should live in
crypto/rsa.

This requires fixing up the verify_recover logic and some tests,
including bcm.c's KAT bits.

(evp_tests.txt is now this odd mixture of EVP-level and RSA-level error
codes. A follow-up change will add new APIs for RSA-PSS which will allow
p_rsa.c to be trimmed down and make things consistent.)

Change-Id: I29158e9695b28e8632b06b449234a5dded35c3e7
Reviewed-on: https://boringssl-review.googlesource.com/15824
Reviewed-by: Adam Langley <agl@google.com>

crypto/rsa/rsa.c

diff --git a/crypto/rsa/rsa.c b/crypto/rsa/rsa.c
index 64ca969..e6cdce9 100644
--- a/crypto/rsa/rsa.c
+++ b/crypto/rsa/rsa.c
@@ -63,8 +63,10 @@
 #include <openssl/engine.h>
 #include <openssl/err.h>
 #include <openssl/ex_data.h>
+#include <openssl/md5.h>
 #include <openssl/mem.h>
 #include <openssl/nid.h>
+#include <openssl/sha.h>
 #include <openssl/thread.h>
 
 #include "internal.h"
@@ -323,6 +325,8 @@
 struct pkcs1_sig_prefix {
   /* nid identifies the hash function. */
   int nid;
+  /* hash_len is the expected length of the hash function. */
+  uint8_t hash_len;
   /* len is the number of bytes of |bytes| which are valid. */
   uint8_t len;
   /* bytes contains the DER bytes. */
@@ -334,42 +338,48 @@
 static const struct pkcs1_sig_prefix kPKCS1SigPrefixes[] = {
     {
      NID_md5,
+     MD5_DIGEST_LENGTH,
      18,
      {0x30, 0x20, 0x30, 0x0c, 0x06, 0x08, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d,
       0x02, 0x05, 0x05, 0x00, 0x04, 0x10},
     },
     {
      NID_sha1,
+     SHA_DIGEST_LENGTH,
      15,
      {0x30, 0x21, 0x30, 0x09, 0x06, 0x05, 0x2b, 0x0e, 0x03, 0x02, 0x1a, 0x05,
       0x00, 0x04, 0x14},
     },
     {
      NID_sha224,
+     SHA224_DIGEST_LENGTH,
      19,
      {0x30, 0x2d, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03,
       0x04, 0x02, 0x04, 0x05, 0x00, 0x04, 0x1c},
     },
     {
      NID_sha256,
+     SHA256_DIGEST_LENGTH,
      19,
      {0x30, 0x31, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03,
       0x04, 0x02, 0x01, 0x05, 0x00, 0x04, 0x20},
     },
     {
      NID_sha384,
+     SHA384_DIGEST_LENGTH,
      19,
      {0x30, 0x41, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03,
       0x04, 0x02, 0x02, 0x05, 0x00, 0x04, 0x30},
     },
     {
      NID_sha512,
+     SHA512_DIGEST_LENGTH,
      19,
      {0x30, 0x51, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03,
       0x04, 0x02, 0x03, 0x05, 0x00, 0x04, 0x40},
     },
     {
-     NID_undef, 0, {0},
+     NID_undef, 0, 0, {0},
     },
 };
 
@@ -397,6 +407,11 @@
       continue;
     }
 
+    if (msg_len != sig_prefix->hash_len) {
+      OPENSSL_PUT_ERROR(RSA, RSA_R_INVALID_MESSAGE_LENGTH);
+      return 0;
+    }
+
     const uint8_t* prefix = sig_prefix->bytes;
     unsigned prefix_len = sig_prefix->len;
     unsigned signed_msg_len;