Clear false positives in RSA-OAEP constant time validation

We check OAEP padding in constant time, but once the padding is
determined to be valid (or not), this fact and, if valid, the output
length are public.

Change-Id: I2aa6a707ca9a91761776746264416736c820977c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56845
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>

crypto/fipsmodule/rsa/padding.c

@@ -457,10 +457,16 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(uint8_t *out, size_t *out_len,
 
   bad |= looking_for_one_byte;
 
-  if (bad) {
+  // Whether the overall padding was valid or not in OAEP is public.
+  if (constant_time_declassify_w(bad)) {
     goto decoding_err;
   }
 
+  // Once the padding is known to be valid, the output length is also public.
+  static_assert(sizeof(size_t) <= sizeof(crypto_word_t),
+                "size_t does not fit in crypto_word_t");
+  one_index = constant_time_declassify_w(one_index);
+
   one_index++;
   size_t mlen = dblen - one_index;
   if (max_out < mlen) {
@@ -475,8 +481,8 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(uint8_t *out, size_t *out_len,
   return 1;
 
 decoding_err:
-  // to avoid chosen ciphertext attacks, the error message should not reveal
-  // which kind of decoding error happened
+  // To avoid chosen ciphertext attacks, the error message should not reveal
+  // which kind of decoding error happened.
   OPENSSL_PUT_ERROR(RSA, RSA_R_OAEP_DECODING_ERROR);
  err:
   OPENSSL_free(db);

crypto/internal.h

@@ -479,7 +479,7 @@ static inline int constant_time_select_int(crypto_word_t mask, int a, int b) {
 
 #endif  // BORINGSSL_CONSTANT_TIME_VALIDATION
 
-static inline int constant_time_declassify_int(int v) {
+static inline crypto_word_t constant_time_declassify_w(crypto_word_t v) {
   // Return |v| through a value barrier to be safe. Valgrind-based constant-time
   // validation is partly to check the compiler has not undone any constant-time
   // work. Any place |BORINGSSL_CONSTANT_TIME_VALIDATION| influences
@@ -491,8 +491,14 @@ static inline int constant_time_declassify_int(int v) {
   //
   // Thus, to be safe, stick a value barrier, in hopes of comparably inhibiting
   // compiler analysis.
+  CONSTTIME_DECLASSIFY(&v, sizeof(v));
+  return value_barrier_w(v);
+}
+
+static inline int constant_time_declassify_int(int v) {
   static_assert(sizeof(uint32_t) == sizeof(int),
                 "int is not the same size as uint32_t");
+  // See comment above.
   CONSTTIME_DECLASSIFY(&v, sizeof(v));
   return value_barrier_u32(v);
 }