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>
https://boringssl-review.googlesource.com/10520 and then later
https://boringssl-review.googlesource.com/25285 made BN_MONT_CTX_set
constant-time, which is necessary for RSA's mont_p and mont_q. However,
due to a typo in the benchmark, they did not correctly measure.
Split BN_MONT_CTX creation into a constant-time and variable-time one.
The constant-time one uses our current algorithm and the latter restores
the original BN_mod codepath.
Should we wish to avoid BN_mod, I have an alternate version lying
around:
First, BN_set_bit + bn_mod_lshift1_consttime as now to count up to 2*R.
Next, observe that 2*R = BN_to_montgomery(2) and R*R =
BN_to_montgomery(R) = BN_to_montgomery(2^r_bits) Also observe that
BN_mod_mul_montgomery only needs n0, not RR. Split the core of
BN_mod_exp_mont into its own function so the caller handles conversion.
Raise 2*R to the r_bits power to get 2^r_bits*R = R*R.
The advantage of that algorithm is that it is still constant-time, so we
only need one BN_MONT_CTX_new. Additionally, it avoids BN_mod which is
otherwise (almost, but the remaining links should be easy to cut) out of
the critical path for correctness. One less operation to worry about.
The disadvantage is that it is gives a 25% (RSA-2048) or 32% (RSA-4096)
slower RSA verification speed. I went with the BN_mod one for the time
being.
Before:
Did 9204 RSA 2048 signing operations in 10052053us (915.6 ops/sec)
Did 326000 RSA 2048 verify (same key) operations in 10028823us (32506.3 ops/sec)
Did 50830 RSA 2048 verify (fresh key) operations in 10033794us (5065.9 ops/sec)
Did 1269 RSA 4096 signing operations in 10019204us (126.7 ops/sec)
Did 88435 RSA 4096 verify (same key) operations in 10031129us (8816.1 ops/sec)
Did 14552 RSA 4096 verify (fresh key) operations in 10053411us (1447.5 ops/sec)
After:
Did 9150 RSA 2048 signing operations in 10022831us (912.9 ops/sec)
Did 322000 RSA 2048 verify (same key) operations in 10028604us (32108.2 ops/sec)
Did 289000 RSA 2048 verify (fresh key) operations in 10017205us (28850.4 ops/sec)
Did 1270 RSA 4096 signing operations in 10072950us (126.1 ops/sec)
Did 87480 RSA 4096 verify (same key) operations in 10036328us (8716.3 ops/sec)
Did 80730 RSA 4096 verify (fresh key) operations in 10073614us (8014.0 ops/sec)
Change-Id: Ie8916d1634ccf8513ceda458fa302f09f3e93c07
Reviewed-on: https://boringssl-review.googlesource.com/27287
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>
The first non-zero window (which we can condition on for public
exponents) always multiplies by one. This means we can cut out one
Montgomery multiplication. It also means we never actually need to
initialize r to one, saving another Montgomery multiplication for P-521.
This, in turn, means we don't need the bn_one_to_montgomery optimization
for the public-exponent exponentations, so we can delete
bn_one_to_montgomery_small. (The function does currently promise to
handle p = 0, but this is not actually reachable, so it can just do a
reduction on RR.)
For RSA, where we're not doing many multiplications to begin with,
saving one is noticeable.
Before:
Did 92000 RSA 2048 verify (same key) operations in 3002557us (30640.6 ops/sec)
Did 25165 RSA 4096 verify (same key) operations in 3045046us (8264.2 ops/sec)
After:
Did 100000 RSA 2048 verify (same key) operations in 3002483us (33305.8 ops/sec)
Did 26603 RSA 4096 verify (same key) operations in 3010942us (8835.4 ops/sec)
(Not looking at the fresh key number yet as that still needs to be
fixed.)
Change-Id: I81a025a68d9b0f8eb0f9c6c04ec4eedf0995a345
Reviewed-on: https://boringssl-review.googlesource.com/27286
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>
Change-Id: If3d93648cf6561c02c208895526ae1f1cbfa2b51
Reviewed-on: https://boringssl-review.googlesource.com/27524
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Otherwise Clang has to assign a file entry to the label which conflicts with
later, explicit, file entries.
Change-Id: Ifc782821517aa7b48ba3ef304d4468f2bc850ac2
Reviewed-on: https://boringssl-review.googlesource.com/27544
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This schism came up in passing again, and I realized we never added a
TLS-level test for this. Fix that.
Change-Id: I10f910bb5a975d6b3b73d99e7412ade35654fddb
Reviewed-on: https://boringssl-review.googlesource.com/27224
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>
It's defined to return one in Montgomery form, not a normal one.
(Not that this matters. This function is only used to Fermat's Little
Theorem. Probably it should have been less general, though we'd need to
make new test vectors first.)
Change-Id: Ia8d7588e6a413b25f01280af9aacef0192283771
Reviewed-on: https://boringssl-review.googlesource.com/27285
Reviewed-by: Adam Langley <agl@google.com>
BN_mod_exp_mont is intended to protect the base, but not the exponent.
Accordingly, it shouldn't treat a base of zero as special.
Change-Id: Ib053e8ce65ab1741973a9f9bfeff8c353567439c
Reviewed-on: https://boringssl-review.googlesource.com/27284
Reviewed-by: Adam Langley <agl@google.com>
Our technique to perform the reduction only works for balanced key
sizes. For unbalanced keys, we fall back to variable-time logic.
Instead, fall back earlier to the non-CRT codepath, which is still
secure, just slower. This also aligns with the advice here:
https://github.com/HACS-workshop/spectre-mitigations/blob/master/crypto_guidelines.md#1-do-not-conditionally-choose-between-constant-and-non-constant-time
Update-Note: This is a performance hit (some keys will run 3x slower),
but only for keys with different-sized primes. I believe the Windows
crypto APIs will not accept such keys at all. There are two scenarios to
be concerned with for RSA performance:
1. Performance of reasonably-generated keys. Keys that BoringSSL or
anyone else reasonable generates will all be balanced, so this change
does not affect them.
2. Worst-case performance for DoS purposes. This CL does not change the
worst-case performance for RSA at a given bit size. In fact, it improves
it slightly. A sufficiently unbalanced RSA key is as slow as not doing
CRT at all.
In both cases, this change does not affect performance. The affected
keys are pathologically-generated ones that were not quite pathological
enough.
Bug: 235
Change-Id: Ie298dabb549ab9108fa9374aa86ebffe8b6c6c88
Reviewed-on: https://boringssl-review.googlesource.com/27504
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>
data_plus_mac_size is secret. Values derived from it cannot quite be
safely divided by md_block_size because SHA-384 ciphers prevent that
field from being constant. We know the value is a power of two, so do
the strength reduction by hand.
Change-Id: Id62ab9e646f4e21d507a7059cfe84d49bbb986e6
Reviewed-on: https://boringssl-review.googlesource.com/27505
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>
This too is connection-level state to be reset on SSL_clear.
Change-Id: I071c9431c28a7d0ff3eb20c679784d4aa4c236a5
Reviewed-on: https://boringssl-review.googlesource.com/27490
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>
Chrome uses the platform certificate verifier and thus cannot reliably
expect PSS signatures to work in all configurations. Add an API for the
consumer to inform BoringSSL of this ability. We will then adjust our
advertisements accordingly.
Note that, because TLS 1.2 does not have the signature_algorithms_cert
extension, turning off TLS 1.3 and using this API will stop advertising
RSA-PSS. I believe this is the correct behavior given the semantics of
that code point.
The tests check the various combinations here, as well as checking that
the peer never sends signature_algorithms_cert identical to
signature_algorithms.
Bug: 229
Change-Id: I8c33a93efdc9252097e3899425b49548fc42a93a
Reviewed-on: https://boringssl-review.googlesource.com/27488
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>
It's hard to diagnose "20".
Change-Id: I57e8d0fb6e4937ddeca45b3645463ca0dc872ea6
Reviewed-on: https://boringssl-review.googlesource.com/27487
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>
This reflects the change to add the key type into the constant. The old
constants are left around for now as legacy aliases and will be removed
later.
Change-Id: I67f1b50c01fbe0ebf4a2e9e89d3e7d5ed5f5a9d7
Reviewed-on: https://boringssl-review.googlesource.com/27486
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>
Update-Note: I believe everything relying on this overload has since
been updated.
Change-Id: I7facf59cde56098e5e3c79470293b67abb715f4c
Reviewed-on: https://boringssl-review.googlesource.com/27485
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>
These are connection state, so they should be reset on SSL_clear.
Change-Id: I861fe52578836615d2719c9e1ff0911c798f336e
Reviewed-on: https://boringssl-review.googlesource.com/27384
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>
Conscrypt need this function right now. They ought to be fixed up to not
need this but, in the meantime, this API is also provided by OpenSSL and
will clear one most consumer reaching into SSL_SESSION.
Bumping the API since Conscrypt often involves multi-sided stuff.
Change-Id: I665ca6b6a17ef479133c29c23fc639f278128c69
Reviewed-on: https://boringssl-review.googlesource.com/27405
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
In addition, make use of bssl::ScopedEVP_MD_CTX in |SpeedHashChunk|,
otherwise the ctx doesn't get destroyed on failure.
Change-Id: I5828080cb9f4eb7c77cc2ff185d9aa8135311385
Reviewed-on: https://boringssl-review.googlesource.com/27464
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
OpenSSL 1.1.0 renamed that. Also clang-format wanted to smush it all
onto one line.
Change-Id: Icdaa0eefc503c4aab1b309ccb34625f5e811c537
Reviewed-on: https://boringssl-review.googlesource.com/27404
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>
Change-Id: Iceed87c194201d28c4a51b1c19a59fe2f20b6a5e
Reviewed-on: https://boringssl-review.googlesource.com/27444
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
In the handoff+handback case, bssl_shim.cc creates 3 |SSL| objects:
one to receive the ClientHello, one to receive the handoff, and a
third one to receive the handback.
Before 56986f9, only the first of these received any configuration.
Since that commit, all 3 of them receive the same configuration. That
means that the handback message no longer needs to serialize as many
things.
N.B. even before 56986f9, not all of the fields were necessary. For
example, there was no reason to serialize |conf_max_version| and
|conf_min_version| in the handback, so far as I can tell.
This commit is mechanical: it simply removes everything that doesn't
cause any tests to fail. In the long run, I'll need to carefully
check for two possibilities:
- Knobs that affect the handshake after the server's first message it
sent. These are troublesome because that portion of the handshake
may run on a different |SSL|, depending on whether the handback is
early or late.
- Getters that may be called post-handshake, and that callers may
reasonably expect to reflect the value that was used during
handshake.
(I'm not sure that either case exists!)
Change-Id: Ibf6e0be6609ad6e83ab50e69199e9b2d51e59a87
Reviewed-on: https://boringssl-review.googlesource.com/27364
Commit-Queue: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
(Found by fuzzing: a zero value causes an infinite loop.)
Change-Id: I984fd88d85fb87616b5e806795c10334f4379744
Reviewed-on: https://boringssl-review.googlesource.com/27345
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This separates the source lists for the crypto and ssl targets from
their headers, so the header files can be listed in the 'public'
section of the targets. This allows tighter GN checking and expresses
the build structure more cleanly.
Change-Id: Ifb20c90977d7e858734654d9a03949be19a9c43a
Reviewed-on: https://boringssl-review.googlesource.com/27344
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The last-minute TLS 1.3 change was done partly for consistency with DTLS
1.3, where authenticating the record header is less obviously pointless
than in TLS. There, reconstructing it would be messy. Instead, pass in
the record header and let SSLAEADContext decide whether or not to
assemble its own.
(While I'm here, reorder all the flags so the AD and nonce ones are
grouped together.)
Change-Id: I06e65d526b21a08019e5ca6f1b7c7e0e579e7760
Reviewed-on: https://boringssl-review.googlesource.com/27024
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>
https://boringssl-review.googlesource.com/10522 didn't actually do what
it was supposed to do. In fact, it appears, not paying attention to it,
we've managed to make RSA verify slower than ECDSA verify. Oops.
Did 32000 RSA 2048 verify (same key) operations in 1016746us (31473.0 ops/sec)
Did 5525 RSA 2048 verify (fresh key) operations in 1067209us (5177.1 ops/sec)
Did 8957 ECDSA P-256 verify operations in 1078570us (8304.5 ops/sec)
The difference is in setting up the BN_MONT_CTX, either computing R^2 or n0.
I'm guessing R^2. The current algorithm needs to be constant-time, but we can
split out a variable-time one if necessary.
Change-Id: Ie064a0e464aaa803815b56a6734bc9e2becef1a7
Reviewed-on: https://boringssl-review.googlesource.com/27244
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>
If we don't have OID data for an object then we should fail if we
are asked to encode the ASN.1 for that OID.
(Imported from upstream's f3f8e72f494b36d05e0d04fe418f92b692fbb261.)
Change-Id: I3c3d3a3b236bca374fde3c0d02504140f2992602
Reviewed-on: https://boringssl-review.googlesource.com/27065
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>
This is helpful at smaller sizes because the benefits of an unlikely hit
by trival-division are smaller.
The full set of kPrimes eliminates about 94.3% of random numbers. The
first quarter eliminates about 93.2% of them. But the little extra power
of the full set seems to be borderline for RSA 3072 and clearly positive
for RSA 4096.
Did 316 RSA 2048 key-gen operations in 30035598us (10.5 ops/sec)
min: 19423us, median: 80448us, max: 394265us
Change-Id: Iee53f721329674ae7a08fabd85b4f645c24e119d
Reviewed-on: https://boringssl-review.googlesource.com/26944
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I7298c878bd2c8187dbd25903e397e8f0c2575aa4
Reviewed-on: https://boringssl-review.googlesource.com/26846
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
The generic code special-cases affine points, but this leaks
information. (Of course, the generic code also doesn't have a
constant-time multiply and other problems, but one thing at a time.)
The optimization in point doubling is not useful. Point multiplication
more-or-less never doubles an affine point. The optimization in point
addition *is* useful because the wNAF code converts the tables to
affine. Accordingly, align with the P-256 code which adds a 'mixed'
parameter.
(I haven't aligned the formally-verified point formulas themselves yet;
initial testing suggests that the large number of temporaries take a
perf hit with BIGNUM. I'll check the results in EC_FELEM, which will be
stack-allocated, to see if we still need to help the compiler out.)
Strangly, it actually got a bit faster with this change. I'm guessing
because now it doesn't need to bother with unnecessary comparisons and
maybe was kinder to the branch predictor?
Before:
Did 2201 ECDH P-384 operations in 3068341us (717.3 ops/sec)
Did 4092 ECDSA P-384 signing operations in 3076981us (1329.9 ops/sec)
Did 3503 ECDSA P-384 verify operations in 3024753us (1158.1 ops/sec)
Did 992 ECDH P-521 operations in 3017884us (328.7 ops/sec)
Did 1798 ECDSA P-521 signing operations in 3059000us (587.8 ops/sec)
Did 1581 ECDSA P-521 verify operations in 3033142us (521.2 ops/sec)
After:
Did 2310 ECDH P-384 operations in 3092648us (746.9 ops/sec)
Did 4080 ECDSA P-384 signing operations in 3044588us (1340.1 ops/sec)
Did 3520 ECDSA P-384 verify operations in 3056070us (1151.8 ops/sec)
Did 992 ECDH P-521 operations in 3012779us (329.3 ops/sec)
Did 1792 ECDSA P-521 signing operations in 3019459us (593.5 ops/sec)
Did 1600 ECDSA P-521 verify operations in 3047749us (525.0 ops/sec)
Bug: 239
Change-Id: If5d13825fc98e4c58bdd1580cf0245bf7ce93a82
Reviewed-on: https://boringssl-review.googlesource.com/27004
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>
This changes the contract for split handshakes such that on the
receiving side, the connection is to be driven until it returns
|SSL_ERROR_HANDBACK|, rather than until SSL_do_handshake() returns
success.
Change-Id: Idd1ebfbd943d88474d7c934f4c0ae757ff3c0f37
Reviewed-on: https://boringssl-review.googlesource.com/26864
Commit-Queue: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This used to work, but I broke it on accident in the recent rewrite.
Change-Id: I06ab5e06eb0c0a6b67ecc97919654e386f3c2198
Reviewed-on: https://boringssl-review.googlesource.com/26984
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The relevant result (Damgård, Landrock, and Pomerance, Average Case
Error Estimates for the Strong Probably Prime Test) is only applicable
for randomly selected candidates. It relies on there being very few odd
composites with many false witnesses.
(If testing an adversarially-selected composite, false witnesses are
bounded by ϕ(n)/4 for n != 9, so one needs about 40 iterations for a
2^-80 false positive rate.)
Change-Id: I2a063dac5f9042dcb9e6affee8d2ae575f2238a9
Reviewed-on: https://boringssl-review.googlesource.com/26972
Reviewed-by: Adam Langley <agl@google.com>
This is in preparation for representing field elements with
stack-allocated types in the generic code. While there is likely little
benefit in threading all the turned field arithmetic through all the
generic code, and the P-224 logic, in particular, does not have a tight
enough abstraction for this, the current implementations depend on
BN_div, which is not compatible with stack-allocating things and avoiding
malloc.
This also speeds things up slightly, now that benchmarks cover point
validation.
Before:
Did 82786 ECDH P-224 operations in 10024326us (8258.5 ops/sec)
After:
Did 89991 ECDH P-224 operations in 10012429us (8987.9 ops/sec)
Change-Id: I468483b49f5dc69187aebd62834365ce5caab795
Reviewed-on: https://boringssl-review.googlesource.com/26971
Reviewed-by: Adam Langley <agl@google.com>
Alas, it is reachable by way of the legacy custom curves API. Add a
basic test to ensure those codepaths work.
Change-Id: If631110045a664001133a0d07fdac4c67971a15f
Reviewed-on: https://boringssl-review.googlesource.com/26970
Reviewed-by: Adam Langley <agl@google.com>
This includes a point validation, which figures into the overall cost of
an ECDH operation. If, say, point validation is slow because it uses
generic code, we'd like it to show up in benchmarks.
(Later I'd like to replace this mess with a simple byte-oriented ECDH
API. When that happens, I'll update the benchmark accordingly.)
Change-Id: If8c33542d4b40572aac0a71ea2f658e7bc501f4b
Reviewed-on: https://boringssl-review.googlesource.com/26969
Reviewed-by: Adam Langley <agl@google.com>
ECDSA converts digests to scalars by taking the leftmost n bits, where n
is the number of bits in the group order. This does not necessarily
produce a fully-reduced scalar.
Montgomery multiplication actually tolerates this slightly looser bound,
so we did not bother with the conditional subtraction. However, this
subtraction is free compared to the multiplication, inversion, and base
point multiplication. Simplify things by keeping it fully-reduced.
Change-Id: If49dffefccc21510f40418dc52ea4da7e3ff198f
Reviewed-on: https://boringssl-review.googlesource.com/26968
Reviewed-by: Adam Langley <agl@google.com>
ECDSA's logic for converting digests to scalars sometimes produces
slightly unreduced values. Test these cases.
Change-Id: I67a5078db684ee82c286f41e71b13b57c3ee707b
Reviewed-on: https://boringssl-review.googlesource.com/26967
Reviewed-by: Adam Langley <agl@google.com>
May as well use it. Also avoid an overflow with digest_len if someone
asks to sign a truly enormous digest.
Change-Id: Ia0a53007a496f9c7cadd44b1020ec2774b310936
Reviewed-on: https://boringssl-review.googlesource.com/26966
Reviewed-by: Adam Langley <agl@google.com>
For non-custom curves, this only comes up with P-521 and, even then,
only with excessively large hashes. Still, we should have test coverage
for this.
Change-Id: Id17a6f47d59d6dd4a43a93857fd3df490f9fa965
Reviewed-on: https://boringssl-review.googlesource.com/26965
Reviewed-by: Adam Langley <agl@google.com>
We do this in four different places, with the same long comment, and I'm
about to add yet another one.
Change-Id: If28e3f87ea71020d9b07b92e8947f3848473d99d
Reviewed-on: https://boringssl-review.googlesource.com/26964
Reviewed-by: Adam Langley <agl@google.com>
RSA keygen uses this to pick primes. May as well avoid bouncing on
malloc. (The BIGNUM internally allocates, of course, but that allocation
will be absorbed by BN_CTX in RSA keygen.)
Change-Id: Ie2243a6e48b9c55f777153cbf67ba5c06688c2f1
Reviewed-on: https://boringssl-review.googlesource.com/26887
Reviewed-by: Adam Langley <agl@google.com>
AUTHORITY_INFO_ACCESS is a STACK_OF(ACCESS_DESCRIPTION), so we want to
add a deleter for ACCESS_DESCRIPTION, at which point
AUTHORITY_INFO_ACCESS's deleter will show up for free.
Change-Id: Id9efb74093868c39a893de67dd26f1fc15379252
Reviewed-on: https://boringssl-review.googlesource.com/26973
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
With this, in 0.02% of 1024-bit primes (which is what's used with an RSA
2048 generation), we'll leak that we struggled to generate values less
than the prime. I.e. that there's a greater likelihood of zero bits
after the leading 1 bit in the prime.
But this recovers all the speed loss from making key generation
constant-time, and then some.
Did 273 RSA 2048 key-gen operations in 30023223us (9.1 ops/sec)
min: 23867us, median: 93688us, max: 421466us
Did 66 RSA 3072 key-gen operations in 30041763us (2.2 ops/sec)
min: 117044us, median: 402095us, max: 1096538us
Did 31 RSA 4096 key-gen operations in 31673405us (1.0 ops/sec)
min: 245109us, median: 769480us, max: 2659386us
Change-Id: Id82dedde35f5fbb36b278189c0685a13c7824590
Reviewed-on: https://boringssl-review.googlesource.com/26924
Reviewed-by: Adam Langley <alangley@gmail.com>
