Rewrite RSA_verify_PKCS1_PSS_mgf1 with size_t.

Splitting this out from most of the -Wshorten-64-to-32 fixes since it
non-trivially rewrites the function. While I'm here, move variable
declarations slightly closer to their use and document how the salt
check differs from the spec.

Bug: 516
Change-Id: I2e53afecb8ba720fd8c02da504b56c829c20c93b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54206
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>

crypto/fipsmodule/rsa/padding.c

diff --git a/crypto/fipsmodule/rsa/padding.c b/crypto/fipsmodule/rsa/padding.c
index 605647a..0dafed4 100644
--- a/crypto/fipsmodule/rsa/padding.c
+++ b/crypto/fipsmodule/rsa/padding.c
@@ -489,29 +489,23 @@
 int RSA_verify_PKCS1_PSS_mgf1(const RSA *rsa, const uint8_t *mHash,
                               const EVP_MD *Hash, const EVP_MD *mgf1Hash,
                               const uint8_t *EM, int sLen) {
-  int i;
-  int ret = 0;
-  int maskedDBLen, MSBits, emLen;
-  size_t hLen;
-  const uint8_t *H;
-  uint8_t *DB = NULL;
-  EVP_MD_CTX ctx;
-  uint8_t H_[EVP_MAX_MD_SIZE];
-  EVP_MD_CTX_init(&ctx);
-
   if (mgf1Hash == NULL) {
     mgf1Hash = Hash;
   }
 
-  hLen = EVP_MD_size(Hash);
+  int ret = 0;
+  uint8_t *DB = NULL;
+  EVP_MD_CTX ctx;
+  EVP_MD_CTX_init(&ctx);
   FIPS_service_indicator_lock_state();
 
   // Negative sLen has special meanings:
   //	-1	sLen == hLen
   //	-2	salt length is autorecovered from signature
   //	-N	reserved
+  size_t hLen = EVP_MD_size(Hash);
   if (sLen == -1) {
-    sLen = hLen;
+    sLen = (int)hLen;
   } else if (sLen == -2) {
     sLen = -2;
   } else if (sLen < -2) {
@@ -519,8 +513,8 @@
     goto err;
   }
 
-  MSBits = (BN_num_bits(rsa->n) - 1) & 0x7;
-  emLen = RSA_size(rsa);
+  unsigned MSBits = (BN_num_bits(rsa->n) - 1) & 0x7;
+  size_t emLen = RSA_size(rsa);
   if (EM[0] & (0xFF << MSBits)) {
     OPENSSL_PUT_ERROR(RSA, RSA_R_FIRST_OCTET_INVALID);
     goto err;
@@ -529,8 +523,9 @@
     EM++;
     emLen--;
   }
-  if (emLen < (int)hLen + 2 || emLen < ((int)hLen + sLen + 2)) {
-    // sLen can be small negative
+  // |sLen| may be -2 for the non-standard salt length recovery mode.
+  if (emLen < hLen + 2 ||
+      (sLen >= 0 && emLen < hLen + (size_t)sLen + 2)) {
     OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE);
     goto err;
   }
@@ -538,8 +533,8 @@
     OPENSSL_PUT_ERROR(RSA, RSA_R_LAST_OCTET_INVALID);
     goto err;
   }
-  maskedDBLen = emLen - hLen - 1;
-  H = EM + maskedDBLen;
+  size_t maskedDBLen = emLen - hLen - 1;
+  const uint8_t *H = EM + maskedDBLen;
   DB = OPENSSL_malloc(maskedDBLen);
   if (!DB) {
     OPENSSL_PUT_ERROR(RSA, ERR_R_MALLOC_FAILURE);
@@ -548,42 +543,49 @@
   if (!PKCS1_MGF1(DB, maskedDBLen, H, hLen, mgf1Hash)) {
     goto err;
   }
-  for (i = 0; i < maskedDBLen; i++) {
+  for (size_t i = 0; i < maskedDBLen; i++) {
     DB[i] ^= EM[i];
   }
   if (MSBits) {
     DB[0] &= 0xFF >> (8 - MSBits);
   }
-  for (i = 0; DB[i] == 0 && i < (maskedDBLen - 1); i++) {
+  // This step differs slightly from EMSA-PSS-VERIFY (RFC 8017) step 10 because
+  // it accepts a non-standard salt recovery flow. DB should be some number of
+  // zeros, a one, then the salt.
+  size_t salt_start;
+  for (salt_start = 0; DB[salt_start] == 0 && salt_start < maskedDBLen - 1;
+       salt_start++) {
     ;
   }
-  if (DB[i++] != 0x1) {
+  if (DB[salt_start] != 0x1) {
     OPENSSL_PUT_ERROR(RSA, RSA_R_SLEN_RECOVERY_FAILED);
     goto err;
   }
-  if (sLen >= 0 && (maskedDBLen - i) != sLen) {
+  salt_start++;
+  // If a salt length was specified, check it matches.
+  if (sLen >= 0 && maskedDBLen - salt_start != (size_t)sLen) {
     OPENSSL_PUT_ERROR(RSA, RSA_R_SLEN_CHECK_FAILED);
     goto err;
   }
+  uint8_t H_[EVP_MAX_MD_SIZE];
   if (!EVP_DigestInit_ex(&ctx, Hash, NULL) ||
       !EVP_DigestUpdate(&ctx, kPSSZeroes, sizeof(kPSSZeroes)) ||
       !EVP_DigestUpdate(&ctx, mHash, hLen) ||
-      !EVP_DigestUpdate(&ctx, DB + i, maskedDBLen - i) ||
+      !EVP_DigestUpdate(&ctx, DB + salt_start, maskedDBLen - salt_start) ||
       !EVP_DigestFinal_ex(&ctx, H_, NULL)) {
     goto err;
   }
-  if (OPENSSL_memcmp(H_, H, hLen)) {
+  if (OPENSSL_memcmp(H_, H, hLen) != 0) {
     OPENSSL_PUT_ERROR(RSA, RSA_R_BAD_SIGNATURE);
-    ret = 0;
-  } else {
-    ret = 1;
+    goto err;
   }
 
+  ret = 1;
+
 err:
   OPENSSL_free(DB);
   EVP_MD_CTX_cleanup(&ctx);
   FIPS_service_indicator_unlock_state();
-
   return ret;
 }