commit 31302a473afcde1bc60acdeab0b0cb0498b5aa66
author David Benjamin <davidben@google.com> Sun Oct 13 11:18:30 2019 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Tue Oct 15 20:50:47 2019 +0000
tree fb53d4517589dfe0623733bd301eb96dd0162557
parent a7a75f208caea8a303615724d4cc5f4e8dfb9695

Fix up BN_GENCB_call calls.

Use the constants when defined. Also OpenSSL uses 0-indexed iteration
counts rather than 1-indexed. This likely changed when we tried to align
with the 1-indexed FIPS 186-4 algorithm.

Also fix the safe prime call. BN_GENCB_call(cb, i, c1 - 1) doesn't make
sense since the first parameter should be an event constant. OpenSSL
does BN_GENCB_call(cb, 2, c1 - 1).

This also doesn't make sense. OpenSSL documents 2 as meaning the prime
has been found. That function is interleaving the p and (p-1)/2 checks
to save the full iteration count on p if (p-1)/2 is composite anyway.

That also doesn't work because the blinding mechanism runs even if the
iteration count is 1, so we're actually paying for the blinding four
times. Add a TODO to address this.

(I can only assume we just never try to generate safe primes. Moreover,
we don't even use BN_generate_prime_ex in RSA keygen. Still, that
function needs work.)

Change-Id: I6f0b7cd10da28484362c92db0c806c1c3045d415
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/38169
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>

crypto/dsa/dsa.c

diff --git a/crypto/dsa/dsa.c b/crypto/dsa/dsa.c
index cb419c8..cc98225 100644
--- a/crypto/dsa/dsa.c
+++ b/crypto/dsa/dsa.c
@@ -256,7 +256,7 @@
     // Find q.
     for (;;) {
       // step 1
-      if (!BN_GENCB_call(cb, 0, m++)) {
+      if (!BN_GENCB_call(cb, BN_GENCB_GENERATED, m++)) {
         goto err;
       }
 
@@ -319,7 +319,7 @@
     n = (bits - 1) / 160;
 
     for (;;) {
-      if ((counter != 0) && !BN_GENCB_call(cb, 0, counter)) {
+      if ((counter != 0) && !BN_GENCB_call(cb, BN_GENCB_GENERATED, counter)) {
         goto err;
       }
 

crypto/fipsmodule/bn/prime.c

diff --git a/crypto/fipsmodule/bn/prime.c b/crypto/fipsmodule/bn/prime.c
index 9df4f95..efaec65 100644
--- a/crypto/fipsmodule/bn/prime.c
+++ b/crypto/fipsmodule/bn/prime.c
@@ -443,6 +443,11 @@
       goto err;
     }
 
+    // Interleave |ret| and |t|'s primality tests to avoid paying the full
+    // iteration count on |ret| only to quickly discover |t| is composite.
+    //
+    // TODO(davidben): This doesn't quite work because an iteration count of 1
+    // still runs the blinding mechanism.
     for (i = 0; i < checks; i++) {
       j = BN_is_prime_fasttest_ex(ret, 1, ctx, 0, NULL);
       if (j == -1) {
@@ -458,7 +463,7 @@
         goto loop;
       }
 
-      if (!BN_GENCB_call(cb, i, c1 - 1)) {
+      if (!BN_GENCB_call(cb, BN_GENCB_PRIME_TEST, i)) {
         goto err;
       }
       // We have a safe prime test pass
@@ -669,7 +674,7 @@
       *out_is_probably_prime = BN_is_word(w, prime);
       return 1;
     }
-    if (!BN_GENCB_call(cb, 1, -1)) {
+    if (!BN_GENCB_call(cb, BN_GENCB_PRIME_TEST, -1)) {
       return 0;
     }
   }
@@ -755,7 +760,7 @@
     }
 
     // Step 4.7
-    if (!BN_GENCB_call(cb, 1, i)) {
+    if (!BN_GENCB_call(cb, BN_GENCB_PRIME_TEST, i - 1)) {
       goto err;
     }
   }
@@ -910,7 +915,7 @@
 
  loop:
     // Step 4.15
-    if (!BN_GENCB_call(cb, 1, i)) {
+    if (!BN_GENCB_call(cb, BN_GENCB_PRIME_TEST, i - 1)) {
       goto err;
     }
   }