aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2017-12-11bn/asm/rsaz-avx2.pl: fix digit correction bug in rsaz_1024_mul_avx2.chromium-3282David Benjamin2-8/+13
Credit to OSS-Fuzz for finding this. CVE-2017-3738 (Imported from upstream's 5630661aecbea5fe3c4740f5fea744a1f07a6253 and 77d75993651b63e872244a3256e37967bb3c3e9e.) Confirmed with Intel SDE that the fix makes the test vector pass and that, without the fix, the test vector does not. (Well, we knew the latter already, since it was our test vector.) Change-Id: I167aa3407ddab3b434bacbd18e099c55aa40ac4c Reviewed-on: https://boringssl-review.googlesource.com/23884 Reviewed-by: Adam Langley <agl@google.com> (cherry picked from commit 296a61d6007688a1472798879b81517920e35dff) Reviewed-on: https://boringssl-review.googlesource.com/23964 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>
2017-12-07Revert "Support high tag numbers in CBS/CBB."David Benjamin6-225/+88
This reverts commit 66801feb175599a6d1eb3845eb7ce0ca84551fb5. This turned out to break a lot more than expected. Hopefully we can reland it soon, but we need to fix up some consumers first. Note due to work that went in later, this is not a trivial revert and should be re-reviewed. (cherry picked from commit 2fc4f362cdaab103241a6a3ca1bf16778944f36b) Change-Id: I5c26e2f8b65bcabdf4fe2b028486a42aad10ad71 Reviewed-on: https://boringssl-review.googlesource.com/23904 Reviewed-by: Steven Valdez <svaldez@google.com>
2017-11-28Bound the input to the bn_mod_exp fuzzer.David Benjamin1-0/+10
This is not a speedy operation, so the fuzzers need a bit of help to avoid timeouts. Bug: chromium:786049 Change-Id: Ib56281b63eb6c895057f21254f0cc7c5c2d85ee4 Reviewed-on: https://boringssl-review.googlesource.com/23484 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-11-28runner: Parse CertificateRequest with byteReader.David Benjamin1-146/+58
Bug: 212 Change-Id: I0ad4df330360789b16fc9db70565abdb3ae42a8f Reviewed-on: https://boringssl-review.googlesource.com/23448 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>
2017-11-28runner: Parse Certificate with byteReader.David Benjamin1-60/+21
Bug: 212 Change-Id: Ife51516ef0642730e601e146028b16ded99ab7ba Reviewed-on: https://boringssl-review.googlesource.com/23447 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-11-28runner: Parse SH/HRR/EE with byteReader.David Benjamin1-183/+94
Bug: 212 Change-Id: I454db0bfd59bac3729338c6f8d9e51efde0735eb Reviewed-on: https://boringssl-review.googlesource.com/23446 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-11-28runner: Send the right alert for handshake message parsing failures.David Benjamin1-1/+1
This throws me off every time. Change-Id: I19848927fe821f7656dea0343361d70dae4007c9 Reviewed-on: https://boringssl-review.googlesource.com/23445 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-11-27Reimplement OBJ_txt2obj and add a lower-level function.David Benjamin8-216/+191
OBJ_txt2obj is currently implemented using BIGNUMs which is absurd. It also depends on the giant OID table, which is undesirable. Write a new one and expose the low-level function so Chromium can use it without the OID table. Bug: chromium:706445 Change-Id: I61ff750a914194f8776cb8d81ba5d3eb5eaa3c3d Reviewed-on: https://boringssl-review.googlesource.com/23364 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>
2017-11-27runner: Add a byteReader type and convert ClientHello parsing.David Benjamin1-216/+228
Bug: 212 Change-Id: Iecbd8fddef1b55a438947ad60780e08cb4260c48 Reviewed-on: https://boringssl-review.googlesource.com/23444 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-11-27Add switch to enable draft 22.Steven Valdez1-2/+7
Change-Id: I60dc085fa02c152adb12a505b453fe8f84670d8b Reviewed-on: https://boringssl-review.googlesource.com/23464 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>
2017-11-27Pretty-print large INTEGERs and ENUMERATEDs in hex.David Benjamin2-2/+80
This avoids taking quadratic time to pretty-print certificates with excessively large integer fields. Very large integers aren't any more readable in decimal than hexadecimal anyway, and the i2s_* functions will parse either form. Found by libFuzzer. Change-Id: Id586cd1b0eef8936d38ff50433ae7c819f0054f3 Reviewed-on: https://boringssl-review.googlesource.com/23424 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>
2017-11-27Fix CBS tag class docs.David Benjamin1-3/+2
Change-Id: Ia7b3b5d9ce833a9cdfb94c8e0923f3cf17555fdd Reviewed-on: https://boringssl-review.googlesource.com/23449 Reviewed-by: Adam Langley <agl@google.com>
2017-11-24Remove spurious ;Daniel Wagner-Hall1-1/+1
DECLARE_STACK_OF adds a trailing ; so we don't need a second one added here. Compiling a project using boringssl which uses -Werror,-Wextra-semi I get errors: ``` third_party/boringssl/include/openssl/stack.h:374:1: error: extra ';' outside of a function [-Werror,-Wextra-semi] DEFINE_STACK_OF(void) ^ third_party/boringssl/include/openssl/stack.h:355:3: note: expanded from macro 'DEFINE_STACK_OF' BORINGSSL_DEFINE_STACK_OF_IMPL(type, type *, const type *) \ ^ third_party/boringssl/include/openssl/stack.h:248:25: note: expanded from macro 'BORINGSSL_DEFINE_STACK_OF_IMPL' DECLARE_STACK_OF(name); \ ^ third_party/boringssl/include/openssl/stack.h:375:1: error: extra ';' outside of a function [-Werror,-Wextra-semi] DEFINE_SPECIAL_STACK_OF(OPENSSL_STRING) ^ third_party/boringssl/include/openssl/stack.h:369:3: note: expanded from macro 'DEFINE_SPECIAL_STACK_OF' BORINGSSL_DEFINE_STACK_OF_IMPL(type, type, const type) ^ third_party/boringssl/include/openssl/stack.h:248:25: note: expanded from macro 'BORINGSSL_DEFINE_STACK_OF_IMPL' DECLARE_STACK_OF(name); \ ^ 2 errors generated. ``` Change-Id: Icc39e2341eb76544be72d2d7d0bd29e2f1ed0bf9 Reviewed-on: https://boringssl-review.googlesource.com/23404 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>
2017-11-22Const-correct X509_ALGOR_get0.David Benjamin3-5/+6
Matches the OpenSSL 1.1.0 spelling, which is what we advertise in OPENSSL_VERSION_NUMBER now. Otherwise third-party code which uses it will, in the long term, need ifdefs. Note this will require updates to any existing callers (there appear to only be a couple of them), but it should be straightforward. Change-Id: I9dd1013609abca547152728a293529055dacc239 Reviewed-on: https://boringssl-review.googlesource.com/23325 Reviewed-by: Adam Langley <agl@google.com>
2017-11-22Use some of the word-based functions for ECDSA verification.David Benjamin1-37/+39
This is only a hair faster than the signing change, but still something. I kept the call to BN_mod_inverse_odd as that appears to be faster (constant time is not a concern for verification). Before: Did 22855 ECDSA P-224 verify operations in 3015099us (7580.2 ops/sec) Did 21276 ECDSA P-256 verify operations in 3083284us (6900.4 ops/sec) Did 2635 ECDSA P-384 verify operations in 3032582us (868.9 ops/sec) Did 1240 ECDSA P-521 verify operations in 3068631us (404.1 ops/sec) After: Did 23310 ECDSA P-224 verify operations in 3056226us (7627.1 ops/sec) Did 21210 ECDSA P-256 verify operations in 3035765us (6986.7 ops/sec) Did 2666 ECDSA P-384 verify operations in 3023592us (881.7 ops/sec) Did 1209 ECDSA P-521 verify operations in 3054040us (395.9 ops/sec) Change-Id: Iec995b1a959dbc83049d0f05bdc525c14a95c28e Reviewed-on: https://boringssl-review.googlesource.com/23077 Reviewed-by: Adam Langley <agl@google.com>
2017-11-22Don't use BN_nnmod to convert from field element to scalar.David Benjamin2-17/+42
Hasse's theorem implies at most one subtraction is necessary. This is still using BIGNUM for now because field elements (EC_POINT_get_affine_coordinates_GFp) are BIGNUMs. This gives an additional 2% speedup for signing. Before: Did 16000 ECDSA P-224 signing operations in 1064799us (15026.3 ops/sec) Did 19000 ECDSA P-256 signing operations in 1007839us (18852.2 ops/sec) Did 1078 ECDSA P-384 signing operations in 1079413us (998.7 ops/sec) Did 484 ECDSA P-521 signing operations in 1083616us (446.7 ops/sec) After: Did 16000 ECDSA P-224 signing operations in 1054918us (15167.1 ops/sec) Did 20000 ECDSA P-256 signing operations in 1037338us (19280.1 ops/sec) Did 1045 ECDSA P-384 signing operations in 1049073us (996.1 ops/sec) Did 484 ECDSA P-521 signing operations in 1085492us (445.9 ops/sec) Change-Id: I2bfe214f968eca7a8e317928c0f3daf1a14bca90 Reviewed-on: https://boringssl-review.googlesource.com/23076 Reviewed-by: Adam Langley <agl@google.com>
2017-11-22Make ECDSA signing 10% faster and plug some timing leaks.David Benjamin12-394/+283
None of the asymmetric crypto we inherented from OpenSSL is constant-time because of BIGNUM. BIGNUM chops leading zeros off the front of everything, so we end up leaking information about the first word, in theory. BIGNUM functions additionally tend to take the full range of inputs and then call into BN_nnmod at various points. All our secret values should be acted on in constant-time, but k in ECDSA is a particularly sensitive value. So, ecdsa_sign_setup, in an attempt to mitigate the BIGNUM leaks, would add a couple copies of the order. This does not work at all. k is used to compute two values: k^-1 and kG. The first operation when computing k^-1 is to call BN_nnmod if k is out of range. The entry point to our tuned constant-time curve implementations is to call BN_nnmod if the scalar has too many bits, which this causes. The result is both corrections are immediately undone but cause us to do more variable-time work in the meantime. Replace all these computations around k with the word-based functions added in the various preceding CLs. In doing so, replace the BN_mod_mul calls (which internally call BN_nnmod) with Montgomery reduction. We can avoid taking k^-1 out of Montgomery form, which combines nicely with Brian Smith's trick in 3426d1011946b26ff1bb2fd98a081ba4753c9cc8. Along the way, we avoid some unnecessary mallocs. BIGNUM still affects the private key itself, as well as the EC_POINTs. But this should hopefully be much better now. Also it's 10% faster: Before: Did 15000 ECDSA P-224 signing operations in 1069117us (14030.3 ops/sec) Did 18000 ECDSA P-256 signing operations in 1053908us (17079.3 ops/sec) Did 1078 ECDSA P-384 signing operations in 1087853us (990.9 ops/sec) Did 473 ECDSA P-521 signing operations in 1069835us (442.1 ops/sec) After: Did 16000 ECDSA P-224 signing operations in 1064799us (15026.3 ops/sec) Did 19000 ECDSA P-256 signing operations in 1007839us (18852.2 ops/sec) Did 1078 ECDSA P-384 signing operations in 1079413us (998.7 ops/sec) Did 484 ECDSA P-521 signing operations in 1083616us (446.7 ops/sec) Change-Id: I2a25e90fc99dac13c0616d0ea45e125a4bd8cca1 Reviewed-on: https://boringssl-review.googlesource.com/23075 Reviewed-by: Adam Langley <agl@google.com>
2017-11-22Support high tag numbers in CBS/CBB.David Benjamin8-109/+263
Android's attestion format uses some ludicrously large tag numbers: https://developer.android.com/training/articles/security-key-attestation.html#certificate_schema Add support for these in CBS/CBB. The public API does not change for callers who were using the CBS_ASN1_* constants, but it is no longer the case that tag representations match their DER encodings for small tag numbers. Chromium needs https://chromium-review.googlesource.com/#/c/chromium/src/+/783254, but otherwise I don't expect this to break things. Bug: 214 Change-Id: I9b5dc27ae3ea020e9edaabec4d665fd73da7d31e Reviewed-on: https://boringssl-review.googlesource.com/23304 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-11-22Use dec/jnz instead of loop in bn_add_words and bn_sub_words.David Benjamin1-2/+4
Imported from upstream's a78324d95bd4568ce2c3b34bfa1d6f14cddf92ef. I think the "regression" part of that change is some tweak to BN_usub and I guess the bn_*_words was to compensate for it, but we may as well import it. Apparently the loop instruction is terrible. Before: Did 39871000 bn_add_words operations in 1000002us (39870920.3 ops/sec) Did 38621750 bn_sub_words operations in 1000001us (38621711.4 ops/sec) After: Did 64012000 bn_add_words operations in 1000007us (64011551.9 ops/sec) Did 81792250 bn_sub_words operations in 1000002us (81792086.4 ops/sec) loop sets no flags (even doing the comparison to zero without ZF) while dec sets all flags but CF, so Andres and I are assuming that because this prevents Intel from microcoding it to dec/jnz, they otherwise can't be bothered to add more circuitry since every compiler has internalized by now to never use loop. Change-Id: I3927cd1c7b707841bbe9963e3d4afd7ba9bd9b36 Reviewed-on: https://boringssl-review.googlesource.com/23344 Reviewed-by: Adam Langley <agl@google.com>
2017-11-22Remove DSA_sign_setup too.David Benjamin2-39/+11
Change-Id: Ib406e7d1653fa57a863dbd5d4eb04401caf5de0a Reviewed-on: https://boringssl-review.googlesource.com/23284 Reviewed-by: Adam Langley <agl@google.com>
2017-11-22Remove ECDSA_sign_setup and friends.David Benjamin4-103/+17
These allow precomputation of k, but bypass our nonce hardening and also make it harder to excise BIGNUM. As a bonus, ECDSATest.SignTestVectors is now actually covering the k^-1 and r computations. Change-Id: I4c71dae162874a88a182387ac43999be9559ddd7 Reviewed-on: https://boringssl-review.googlesource.com/23074 Reviewed-by: Adam Langley <agl@google.com>
2017-11-22Add some missing OpenSSL 1.1.0 accessors.David Benjamin5-1/+37
wpa_supplicant appear to be using these. Change-Id: I1f220cae69162901bcd9452e8daf67379c5e276c Reviewed-on: https://boringssl-review.googlesource.com/23324 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-11-21Unwind legacy SSL_PRIVATE_KEY_METHOD hooks.David Benjamin7-184/+12
After much procrastinating, we finally moved Chromium to the new stuff. We can now delete this. This is a breaking change for SSL_PRIVATE_KEY_METHOD consumers, but it should be trivial (remove some unused fields in the struct). I've bumped BORINGSSL_API_VERSION to ease any multi-sided changes that may be needed. Change-Id: I9fe562590ad938bcb4fcf9af0fadeff1d48745fb Reviewed-on: https://boringssl-review.googlesource.com/23224 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>
2017-11-21Fix memory leak on sk_X509_EXTENSION_push failure.David Benjamin1-7/+15
(Imported from upstream's c29f83c05f3a3c5641c5ddf054789a29d2163bf3.) ext was being leaked. Upstream also did some stuff around *x which wasn't strictly necessary (usually OpenSSL only provides basic exception safety, not strong exception safety), but ah well. Change-Id: I52d230990b05501b4cee6deee8dcacba4a926c18 Reviewed-on: https://boringssl-review.googlesource.com/23204 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-11-21Add a CFI build flag.David Benjamin1-0/+18
This uses Clang's CFI feature. Bug: 201 Change-Id: I7a42ec73dc8bfb3893ec69f2d2f4d7e3a2fd2cc4 Reviewed-on: https://boringssl-review.googlesource.com/23225 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>
2017-11-20Include a couple of missing header files.Adam Langley2-0/+2
mem.h for |OPENSSL_cleanse| and bn/internal.h for things like |bn_less_than_words| and |bn_correct_top|. Change-Id: I3c447a565dd9e4f18fb2ff5d59f80564b4df8cea Reviewed-on: https://boringssl-review.googlesource.com/23164 Reviewed-by: Adam Langley <agl@google.com>
2017-11-20Fix fuzzer mode suppressions.David Benjamin1-9/+9
Change-Id: I82f92019dccfaf927f7180a5af53c9ffae111861 Reviewed-on: https://boringssl-review.googlesource.com/23145 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>
2017-11-20Remove unused function.David Benjamin2-47/+0
Change-Id: Id12ab478b6ba441fb1b6f4c2f9479384fc3fbdb6 Reviewed-on: https://boringssl-review.googlesource.com/23144 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2017-11-20Test that EC_POINT_mul works with the order.David Benjamin1-0/+32
|EC_POINT_mul| is almost exclusively used with reduced scalars, with this exception. This comes from consumers following NIST SP 800-56A section 5.6.2.3.2. (Though all our curves have cofactor one, so this check isn't useful.) Add a test for this so we don't accidentally break it. Change-Id: I42492db38a1ea03acec4febdd7945c8a3933530a Reviewed-on: https://boringssl-review.googlesource.com/23084 Reviewed-by: Adam Langley <agl@google.com>
2017-11-20Run TLS 1.3 tests at all variants and fix bugs.David Benjamin8-1239/+1399
We were only running a random subset of TLS 1.3 tests with variants and let a lot of bugs through as a result. - HelloRetryRequest-EmptyCookie wasn't actually testing what we were trying to test. - The second HelloRetryRequest detection needs tweaks in draft-22. - The empty HelloRetryRequest logic can't be based on non-empty extensions in draft-22. - We weren't sending ChangeCipherSpec correctly in HRR or testing it right. - Rework how runner reads ChangeCipherSpec by setting a flag which affects the next readRecord. This cuts down a lot of cases and works correctly if the client didn't send early data. (In that case, we don't flush CCS until EndOfEarlyData and runner deadlocks waiting for the ChangeCipherSpec to arrive.) Change-Id: I559c96ea3a8b350067e391941231713c6edb2f78 Reviewed-on: https://boringssl-review.googlesource.com/23125 Reviewed-by: Steven Valdez <svaldez@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>
2017-11-20Add EndOfEarlyData to per-message tests.David Benjamin1-2/+31
Change-Id: I9da9734625d1d9d2c783830d8b4aecd34f51acc6 Reviewed-on: https://boringssl-review.googlesource.com/23124 Reviewed-by: Steven Valdez <svaldez@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>
2017-11-20Add missing error path.David Benjamin1-0/+1
Error paths must always have OPENSSL_PUT_ERROR. Change-Id: I0ed8c8288484a4ea69ec58317064ad3cd90ddd64 Reviewed-on: https://boringssl-review.googlesource.com/23104 Reviewed-by: Steven Valdez <svaldez@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>
2017-11-20Deduplicate built-in curves and give custom curves an order_mont.David Benjamin3-86/+64
I still need to revive the original CL, but right now I'm interested in giving every EC_GROUP an order_mont and having different ownership of that field between built-in and custom groups is kind of a nuisance. If I'm going to do that anyway, better to avoid computing the entire EC_GROUP in one go. I'm using some manual locking rather than CRYPTO_once here so that it behaves well in the face of malloc errors. Not that we especially care, but it was easy to do. This speeds up our ECDH benchmark a bit which otherwise must construct the EC_GROUP each time (matching real world usage). Before: Did 7619 ECDH P-224 operations in 1003190us (7594.8 ops/sec) Did 7518 ECDH P-256 operations in 1060844us (7086.8 ops/sec) Did 572 ECDH P-384 operations in 1055878us (541.7 ops/sec) Did 264 ECDH P-521 operations in 1062375us (248.5 ops/sec) After: Did 8415 ECDH P-224 operations in 1066695us (7888.9 ops/sec) Did 7952 ECDH P-256 operations in 1022819us (7774.6 ops/sec) Did 572 ECDH P-384 operations in 1055817us (541.8 ops/sec) Did 264 ECDH P-521 operations in 1060008us (249.1 ops/sec) Bug: 20 Change-Id: I7446cd0a69a840551dcc2dfabadde8ee1e3ff3e2 Reviewed-on: https://boringssl-review.googlesource.com/23073 Reviewed-by: Adam Langley <agl@google.com>
2017-11-20Enforce some bounds and invariants on custom curves.David Benjamin2-0/+48
Later code will take advantage of these invariants. Enforcing them on custom curves avoids making them go through a custom codepath. Change-Id: I23cee72a90c2e4846b41e03e6be26bc3abeb4a45 Reviewed-on: https://boringssl-review.googlesource.com/23072 Reviewed-by: Adam Langley <agl@google.com>
2017-11-20Add bn_mod_exp_mont_small and bn_mod_inverse_prime_mont_small.David Benjamin4-7/+214
These can be used to invert values in ECDSA. Unlike their BIGNUM counterparts, the caller is responsible for taking values in and out of Montgomery domain. This will save some work later on in the ECDSA computation. Change-Id: Ib7292900a0fdeedce6cb3e9a9123c94863659043 Reviewed-on: https://boringssl-review.googlesource.com/23071 Reviewed-by: Adam Langley <agl@google.com>
2017-11-20Add "small" variants of Montgomery logic.David Benjamin3-6/+159
These use the square and multiply functions added earlier. Change-Id: I723834f9a227a9983b752504a2d7ce0223c43d24 Reviewed-on: https://boringssl-review.googlesource.com/23070 Reviewed-by: Adam Langley <agl@google.com>
2017-11-20Split BN_from_montgomery_word into a non-BIGNUM core.David Benjamin1-47/+50
bn_from_montgomery_in_place is actually constant-time. It is, of course, only used by non-constant-time BIGNUM callers, but that will soon be fixed. Change-Id: I2b2c9943dc3b8d6a4b5b19a5bc4fa9ebad532bac Reviewed-on: https://boringssl-review.googlesource.com/23069 Reviewed-by: Adam Langley <agl@google.com>
2017-11-20Add bn_mul_small and bn_sqr_small.David Benjamin4-9/+154
As part of excising BIGNUM from EC scalars, we will need a "words" version of BN_mod_mul_montgomery. That, in turn, requires BN_sqr and BN_mul for cases where we don't have bn_mul_mont. BN_sqr and BN_mul have a lot of logic in there, with the most complex cases being not even remotely constant time. Fortunately, those only apply to RSA-sized numbers, not EC-sized numbers. (With the exception, I believe, of 32-bit P-521 which just barely exceeds the cutoff.) Imposing a limit also makes it easier to stack-allocate temporaries (BN_CTX serves a similar purpose in BIGNUM). Extract bn_mul_small and bn_sqr_small and test them as part of bn_tests.txt. Later changes will build on these. If we end up reusing these functions for RSA in the future (though that would require tending to the egregiously non-constant-time code in the no-asm build), we probably want to extract a version where there is an explicit tmp parameter as in bn_sqr_normal rather than the stack bits. Change-Id: If414981eefe12d6664ab2f5e991a359534aa7532 Reviewed-on: https://boringssl-review.googlesource.com/23068 Reviewed-by: Adam Langley <agl@google.com>
2017-11-20Const-correct some of the low-level BIGNUM functions.David Benjamin4-13/+14
Change-Id: I8c6257e336f54a3a1786df9c4103fcf29177030a Reviewed-on: https://boringssl-review.googlesource.com/23067 Reviewed-by: Adam Langley <agl@google.com>
2017-11-20size_t a bunch of bn words bits.David Benjamin4-62/+56
Also replace a pointless call to bn_mul_words with a memset. Change-Id: Ief30ddab0e84864561b73fe2776bd0477931cf7f Reviewed-on: https://boringssl-review.googlesource.com/23066 Reviewed-by: Adam Langley <agl@google.com>
2017-11-20Make BN_generate_dsa_nonce internally constant-time.David Benjamin7-97/+212
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-20Fix timing leak in BN_from_montgomery_word.David Benjamin1-44/+30
BN_from_montgomery_word doesn't have a constant memory access pattern. Replace the pointer trick with constant_time_select_w. There is, of course, still the bn_correct_top leak pervasive in BIGNUM itself. I wasn't able to measure a performance on RSA operations before or after this change, but the benchmarks would vary wildly run to run. But one would assume the logic here is nothing compared to the actual reduction. Change-Id: Ide761fde3a091a93679f0a803a287aa5d0d4600d Reviewed-on: https://boringssl-review.googlesource.com/22904 Reviewed-by: Adam Langley <agl@google.com>
2017-11-17Add ECDSA tests for custom curves.David Benjamin1-69/+109
We don't currently have test coverage for the order_mont bits (or lack thereof) for custom curves. Change-Id: I865d547c783226a5a3d3d203e10b0e59bad36984 Reviewed-on: https://boringssl-review.googlesource.com/23064 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>
2017-11-15Clarify the documentation for |BN_is_bit_set|.Daniel Hirche1-2/+2
Change-Id: Ic859f19edff281334bd6975dd3c3b2931c901021 Reviewed-on: https://boringssl-review.googlesource.com/23044 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>
2017-11-14Add tests for post-handshake CCS in draft "22".David Benjamin3-0/+24
The current PR says the sender only skips it during the handshake. Add a test that we got this right. Change-Id: Ib27eb942f11d955b8a24e32321efe474037f5254 Reviewed-on: https://boringssl-review.googlesource.com/23024 Reviewed-by: David Benjamin <davidben@google.com> Reviewed-by: Steven Valdez <svaldez@chromium.org> Commit-Queue: David Benjamin <davidben@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-11-14Fix TLSInnerPlaintext limit.David Benjamin2-44/+108
See https://github.com/tlswg/tls13-spec/pull/1083. We misread the original text spec, but it turns out the original spec text required senders have version-specific maximum send fragments. The PR fixes this off-by-one issue. Align with the new spec text uniformly. This is a wire format change for our existing drafts *only if* records have padding. We don't currently send padding, so this is fine. Unpadded records continue to be capped at 2^14 bytes of plaintext (or 2^14+1 bytes of TLSInnerPlaintext structure). Change-Id: I01017cfd13162504bb163dd59afd74aff0896cc4 Reviewed-on: https://boringssl-review.googlesource.com/23004 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>
2017-11-12Disable 'draft 22' by default.Steven Valdez2-6/+8
Change-Id: I1a0f264cbfa0eb5d4adac96d0fc24fa342f2b6a3 Reviewed-on: https://boringssl-review.googlesource.com/22946 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>
2017-11-11Fix early data printout in bssl client.David Benjamin1-2/+3
Because the handshake returns early, it should query SSL_in_early_data. Change-Id: I64d4c0e8de753832207d5c198c50d660f87afac6 Reviewed-on: https://boringssl-review.googlesource.com/22945 Reviewed-by: Steven Valdez <svaldez@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>
2017-11-11Refresh TLS fuzzer corpora.David Benjamin486-0/+0
Change-Id: Ie5055d6d1d33690f27cdd978a0aa696307880579 Reviewed-on: https://boringssl-review.googlesource.com/22964 Reviewed-by: Steven Valdez <svaldez@chromium.org> Reviewed-by: David Benjamin <davidben@google.com>
2017-11-11Implement PR 1091 (TLS 1.3 draft '22').Steven Valdez16-110/+392
This introduces a wire change to Experiment2/Experiment3 over 0RTT, however as there is never going to be a 0RTT deployment with Experiment2/Experiment3, this is valid. Change-Id: Id541d195cbc4bbb3df7680ae2a02b53bb8ae3eab Reviewed-on: https://boringssl-review.googlesource.com/22744 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: David Benjamin <davidben@google.com> Reviewed-by: Steven Valdez <svaldez@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>