Commit Graph

11 Commits

Author SHA1 Message Date
David Benjamin
a2938719a4 Improve the RSA key generation failure probability.
The FIPS 186-4 algorithm we use includes a limit which hits a 2^-20
failure probability, assuming my math is right. We've observed roughly
2^-23. This is a little large at scale. (See b/77854769.)

To avoid modifying the FIPS algorithm, retry the whole thing four times
to bring the failure rate down to 2^-80. Along the way, now that I have
the derivation on hand, adjust
https://boringssl-review.googlesource.com/22584 to target the same
failure probability.

Along the way, fix an issue with RSA_generate_key where, if callers
don't check for failure, there may be half a key in there.

Change-Id: I0e1da98413ebd4ffa65fb74c67a58a0e0cd570ff
Reviewed-on: https://boringssl-review.googlesource.com/27288
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-04-20 21:34:05 +00:00
David Benjamin
c1c6eeb5e2 Check d is mostly-reduced in RSA_check_key.
We don't check it is fully reduced because different implementations use
Carmichael vs Euler totients, but if d exceeds n, something is wrong.
Note the fixed-width BIGNUM changes already fail operations with
oversized d.

Update-Note: Some blatantly invalid RSA private keys will be rejected at
    RSA_check_key time. Note that most of those keys already are not
    usable with BoringSSL anyway. This CL moves the failure from
    sign/decrypt to RSA_check_key.

Change-Id: I468dbba74a148aa58c5994cc27f549e7ae1486a2
Reviewed-on: https://boringssl-review.googlesource.com/26374
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-03-30 19:54:10 +00:00
David Benjamin
8d9ee7d1fe Replace rsa_greater_than_pow2 with BN_cmp.
It costs us a malloc, but it's one less function to test and implement
in constant time, now that BN_cmp and BIGNUM are okay.

Median of 29 RSA keygens: 0m0.207s -> 0m0.210s
(Accuracy beyond 0.1s is questionable.)

Bug: 238
Change-Id: Ic56f92f0dcf04da1f542290a7e8cdab8036699ed
Reviewed-on: https://boringssl-review.googlesource.com/26367
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-03-30 19:53:18 +00:00
David Benjamin
380fc326c3 Add RSA_check_key tests.
Change-Id: I5ac52de4217b32631b1d455f5d693d7b2aec665f
Reviewed-on: https://boringssl-review.googlesource.com/26372
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-03-19 22:29:40 +00:00
David Benjamin
73df153be8 Make BN_generate_dsa_nonce internally constant-time.
This rewrites the internals with a "words" variant that can avoid
bn_correct_top. It still ultimately calls bn_correct_top as the calling
convention is sadly still BIGNUM, but we can lift that calling
convention out incrementally.

Performance seems to be comparable, if not faster.

Before:
Did 85000 ECDSA P-256 signing operations in 5030401us (16897.3 ops/sec)
Did 34278 ECDSA P-256 verify operations in 5048029us (6790.4 ops/sec)

After:
Did 85000 ECDSA P-256 signing operations in 5021057us (16928.7 ops/sec)
Did 34086 ECDSA P-256 verify operations in 5010416us (6803.0 ops/sec)

Change-Id: I1159746dfcc00726dc3f28396076a354556e6e7d
Reviewed-on: https://boringssl-review.googlesource.com/23065
Reviewed-by: Adam Langley <agl@google.com>
2017-11-20 16:18:30 +00:00
David Benjamin
a37f286f4e Remove the buggy RSA parser.
I've left EVP_set_buggy_rsa_parser as a no-op stub for now, but it
shouldn't need to last very long. (Just waiting for a CL to land in a
consumer.)

Bug: chromium:735616
Change-Id: I6426588f84dd0803661a79c6636a0414f4e98855
Reviewed-on: https://boringssl-review.googlesource.com/22124
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-10-24 17:39:46 +00:00
David Benjamin
808f832917 Run the comment converter on libcrypto.
crypto/{asn1,x509,x509v3,pem} were skipped as they are still OpenSSL
style.

Change-Id: I3cd9a60e1cb483a981aca325041f3fbce294247c
Reviewed-on: https://boringssl-review.googlesource.com/19504
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-08-18 21:49:04 +00:00
David Benjamin
e55b32ddff Don't crash when decrypting with public keys.
Public and private RSA keys have the same type in OpenSSL, so it's
probably prudent for us to catch this case with an error rather than
crash. (As we do if you, say, configure RSA-PSS parameters on an Ed25519
EVP_PKEY.) Bindings libraries, in particular, tend to hit this sort of
then when their callers do silly things.

Change-Id: I2555e9bfe716a9f15273abd887a8459c682432dd
Reviewed-on: https://boringssl-review.googlesource.com/17325
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-06-22 15:20:15 +00:00
Adam Langley
8379978bc8 Allow |RSA_FLAG_NO_BLINDING| to be set with |e| set.
This change allows blinding to be disabled without also having to remove
|e|, which would disable the CRT and the glitch checks. This is to
support disabling blinding in the FIPS power-on tests.

(Note: the case where |e| isn't set is tested by RSATest.OnlyDGiven.)

Change-Id: I28f18beda33b1687bf145f4cbdfd37ce262dd70f
Reviewed-on: https://boringssl-review.googlesource.com/17146
Commit-Queue: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-06-13 20:27:25 +00:00
Steven Valdez
467d3220f8 Add FIPS-compliant key generation that calls check_fips for RSA and EC.
Change-Id: Ie466b7b55bdd679c5baf2127bd8de4a5058fc3b7
Reviewed-on: https://boringssl-review.googlesource.com/16346
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-05-17 16:30:48 +00:00
Adam Langley
96dec443d9 Move rsa/ to fipsmodule/rsa/
Change-Id: Id20d371ae7a88a91aaba7a9e23574eccb9caeb3c
Reviewed-on: https://boringssl-review.googlesource.com/15849
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-05-04 21:22:39 +00:00