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>
(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>
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>
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>
|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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
This was primarily for my own understanding, but this should hopefully
also be clearer and more amenable to using unsigned indices later.
Change-Id: I09cc3d55de0f7d9284d3b3168d8b0446274b2ab7
Reviewed-on: https://boringssl-review.googlesource.com/22889
Reviewed-by: Adam Langley <agl@google.com>
Normal shifts do the trick just fine and are less likely to tempt the
compiler into inserting a jump.
Change-Id: Iaa1da1b6f986fd447694fcde8f3525efb9eeaf11
Reviewed-on: https://boringssl-review.googlesource.com/22888
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I482093000ee2e4ba371c78b4f7f8e8b121e71640
Reviewed-on: https://boringssl-review.googlesource.com/22886
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We capitalize things Go-style.
Change-Id: Id002efb8a85e4e1886164421bba059d9ca425964
Reviewed-on: https://boringssl-review.googlesource.com/22885
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
BN_generate_dsa_nonce will never generate a zero value of k.
Change-Id: I06964b815bc82aa678ffbc80664f9d788cf3851d
Reviewed-on: https://boringssl-review.googlesource.com/22884
Commit-Queue: David Benjamin <davidben@google.com>
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>
Even without strict-aliasing, C does not allow casting pointers to types
that don't match their alignment. After this change, UBSan is happy with
our code at default settings but for the negative left shift language
bug.
Note: architectures without unaligned loads do not generate the same
code for memcpy and pointer casts. But even ARMv6 can perform unaligned
loads and stores (ARMv5 couldn't), so we should be okay here.
Before:
Did 11086000 AES-128-GCM (16 bytes) seal operations in 5000391us (2217026.6 ops/sec): 35.5 MB/s
Did 370000 AES-128-GCM (1350 bytes) seal operations in 5005208us (73923.0 ops/sec): 99.8 MB/s
Did 63000 AES-128-GCM (8192 bytes) seal operations in 5029958us (12525.0 ops/sec): 102.6 MB/s
Did 9894000 AES-256-GCM (16 bytes) seal operations in 5000017us (1978793.3 ops/sec): 31.7 MB/s
Did 316000 AES-256-GCM (1350 bytes) seal operations in 5005564us (63129.7 ops/sec): 85.2 MB/s
Did 54000 AES-256-GCM (8192 bytes) seal operations in 5054156us (10684.3 ops/sec): 87.5 MB/s
After:
Did 11026000 AES-128-GCM (16 bytes) seal operations in 5000197us (2205113.1 ops/sec): 35.3 MB/s
Did 370000 AES-128-GCM (1350 bytes) seal operations in 5005781us (73914.5 ops/sec): 99.8 MB/s
Did 63000 AES-128-GCM (8192 bytes) seal operations in 5032695us (12518.1 ops/sec): 102.5 MB/s
Did 9831750 AES-256-GCM (16 bytes) seal operations in 5000010us (1966346.1 ops/sec): 31.5 MB/s
Did 316000 AES-256-GCM (1350 bytes) seal operations in 5005702us (63128.0 ops/sec): 85.2 MB/s
Did 54000 AES-256-GCM (8192 bytes) seal operations in 5053642us (10685.4 ops/sec): 87.5 MB/s
(Tested with the no-asm builds; most of this code isn't reachable
otherwise.)
Change-Id: I025c365d26491abed0116b0de3b7612159e52297
Reviewed-on: https://boringssl-review.googlesource.com/22804
Reviewed-by: Adam Langley <agl@google.com>
This avoids upsetting the C compiler. UBSan is offended by the alignment
violations in those functions. The business with offset is also
undefined behavior (pointer arithmetic is supposed to stay within a
single object).
There is a small performance cost, however:
Before:
Did 6636000 ChaCha20-Poly1305 (16 bytes) seal operations in 5000475us (1327073.9 ops/sec): 21.2 MB/s
Did 832000 ChaCha20-Poly1305 (1350 bytes) seal operations in 5003481us (166284.2 ops/sec): 224.5 MB/s
Did 155000 ChaCha20-Poly1305 (8192 bytes) seal operations in 5026933us (30833.9 ops/sec): 252.6 MB/s
After:
Did 6508000 ChaCha20-Poly1305 (16 bytes) seal operations in 5000160us (1301558.4 ops/sec): 20.8 MB/s
Did 831000 ChaCha20-Poly1305 (1350 bytes) seal operations in 5002865us (166104.8 ops/sec): 224.2 MB/s
Did 155000 ChaCha20-Poly1305 (8192 bytes) seal operations in 5013204us (30918.4 ops/sec): 253.3 MB/s
(Tested with the no-asm build which disables the custom stitched mode
assembly and ends up using this one.)
Change-Id: I76d74183f1e04ad3726463a8871ee64be04ce674
Reviewed-on: https://boringssl-review.googlesource.com/22784
Reviewed-by: Adam Langley <agl@google.com>
These functions don't appear to do any stack manipulation thus all they
need are start/end directives in order for the correct CFI tables to be
emitted.
Change-Id: I4c94a9446030d363fa4bcb7c8975c689df3d21dc
Reviewed-on: https://boringssl-review.googlesource.com/22765
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: Id70cfc78c8d103117d4c2195206b023a5d51edc3
Reviewed-on: https://boringssl-review.googlesource.com/22764
Commit-Queue: Adam Langley <agl@google.com>
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>
It's not clear if it's a feature or bug, but binutils-2.29[.1]
interprets 'adr' instruction with Thumb2 code reference differently,
in a way that affects calculation of addresses of constants' tables.
(Imported from upstream's b82acc3c1a7f304c9df31841753a0fa76b5b3cda.)
Change-Id: Ia0f5233a9fcfaf18b9d1164bf1c88217c0cbb60d
Reviewed-on: https://boringssl-review.googlesource.com/22724
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>
This change doesn't actually introduce any Fiat code yet. It sets up the
directory structure to make the diffs in the next change clearer.
Change-Id: I38a21fb36b18a08b0907f9d37b7ef5d7d3137ede
Reviewed-on: https://boringssl-review.googlesource.com/22624
Reviewed-by: David Benjamin <davidben@google.com>
Generating a 2048-bit RSA key with e = 3 (don't do this), the failure
rate at 5*bits iterations appears to be around 7 failures in 1000 tries.
Bump the limit up to 32*bits. This should give a failure rate of around
2 failures in 10^14 tries.
(The FIPS 186-4 algorithm is meant for saner values of e, like 65537. e
= 3 implies a restrictive GCD requirement: the primes must both be 2 mod
3.)
Change-Id: Icd373f61e2eb90df5afaff9a0fc2b2fbb6ec3f0a
Reviewed-on: https://boringssl-review.googlesource.com/22584
Commit-Queue: David Benjamin <davidben@google.com>
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>
Previously, the ed25519 and SPAKE implementations called field element
operations in ways that did not satisfy the preconditions about ranges
of limbs. Furthermore, replacing signed field arithmetic with unsigned field
arithmetic with similar specifications caused tests to fail. This commit
addresses this in three steps:
(1) Split fe into fe and fe_loose, tracking the bounds
(2) Insert carry operations before uses of fe_add/fe_sub/fe_neg whose
input is already within only the loose bounds
(3) Assert that each field element is within the appropriate bounds at
the beginning and end of every field operation.
Throughput diff:
Ed25519 key generation: -2%
Ed25519 signing: -2%
Ed25519 verify: -2%
X25519: roughly unchanged
Detailed benchmarks on Google Cloud's unidentified Intel Xeon with AVX2:
git checkout $VARIANT && ( cd build && rm -rf * && CC=clang CXX=clang++ cmake -GNinja -DCMAKE_TOOLCHAIN_FILE=../util/32-bit-toolchain.cmake -DCMAKE_BUILD_TYPE=Release .. && ninja && ./tool/bssl speed -filter 25519 )
this branch:
Did 11206 Ed25519 key generation operations in 1029462us (10885.3 ops/sec)
Did 11104 Ed25519 signing operations in 1035735us (10720.9 ops/sec)
Did 3278 Ed25519 verify operations in 1087969us (3013.0 ops/sec)
Did 12000 Curve25519 base-point multiplication operations in 1078962us (11121.8 ops/sec)
Did 3610 Curve25519 arbitrary point multiplication operations in 1002767us (3600.0 ops/sec)
Did 11662 Ed25519 key generation operations in 1077690us (10821.3 ops/sec)
Did 10780 Ed25519 signing operations in 1011474us (10657.7 ops/sec)
Did 3289 Ed25519 verify operations in 1083638us (3035.1 ops/sec)
Did 12000 Curve25519 base-point multiplication operations in 1087477us (11034.7 ops/sec)
Did 3610 Curve25519 arbitrary point multiplication operations in 1017023us (3549.6 ops/sec)
Did 11018 Ed25519 key generation operations in 1011606us (10891.6 ops/sec)
Did 11000 Ed25519 signing operations in 1029961us (10680.0 ops/sec)
Did 3124 Ed25519 verify operations in 1045163us (2989.0 ops/sec)
Did 12000 Curve25519 base-point multiplication operations in 1081770us (11092.9 ops/sec)
Did 3610 Curve25519 arbitrary point multiplication operations in 1014503us (3558.4 ops/sec)
master:
Did 11662 Ed25519 key generation operations in 1059449us (11007.6 ops/sec)
Did 10908 Ed25519 signing operations in 1000081us (10907.1 ops/sec)
Did 3333 Ed25519 verify operations in 1078798us (3089.5 ops/sec)
Did 12000 Curve25519 base-point multiplication operations in 1072831us (11185.4 ops/sec)
Did 3850 Curve25519 arbitrary point multiplication operations in 1075821us (3578.7 ops/sec)
Did 11102 Ed25519 key generation operations in 1017540us (10910.6 ops/sec)
Did 11000 Ed25519 signing operations in 1013279us (10855.8 ops/sec)
Did 3311 Ed25519 verify operations in 1066866us (3103.5 ops/sec)
Did 12000 Curve25519 base-point multiplication operations in 1069668us (11218.4 ops/sec)
Did 3905 Curve25519 arbitrary point multiplication operations in 1095501us (3564.6 ops/sec)
Did 11206 Ed25519 key generation operations in 1014127us (11049.9 ops/sec)
Did 10908 Ed25519 signing operations in 1015821us (10738.1 ops/sec)
Did 3344 Ed25519 verify operations in 1100592us (3038.4 ops/sec)
Did 12000 Curve25519 base-point multiplication operations in 1072847us (11185.2 ops/sec)
Did 3570 Curve25519 arbitrary point multiplication operations in 1009373us (3536.8 ops/sec)
Change-Id: Ia014386daf36c913f3ea44c5f9a420b98670e465
Reviewed-on: https://boringssl-review.googlesource.com/22104
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>
It always returns one, so just void it.
Change-Id: I8733cc3d6b20185e782cf0291e9c0dc57712bb63
Reviewed-on: https://boringssl-review.googlesource.com/22564
Reviewed-by: Adam Langley <agl@google.com>
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>
Credit to OSS-Fuzz for finding this.
CVE-2017-3736
(Imported from upstream's 668a709a8d7ea374ee72ad2d43ac72ec60a80eee and
420b88cec8c6f7c67fad07bf508dcccab094f134.)
This bug does not affect BoringSSL as we do not enable the ADX code.
Note the test vector had to be tweaked to take things in and out of
Montgomery form. (There may be something to be said for test vectors for
just BN_mod_mul_montgomery, though we'd need separate 64-bit and 32-bit
ones because R can be different.)
Change-Id: I832070731ac1c5f893f9c1746892fc4a32f023f5
Reviewed-on: https://boringssl-review.googlesource.com/22484
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>
Due to a copy-paste error, the call to |left_shift_3| is missing after
reducing the password scalar in SPAKE2. This means that three bits of
the password leak in Alice's message. (Two in Bob's message as the point
N happens to have order 4l, not 8l.)
The “correct” fix is to put in the missing call to |left_shift_3|, but
that would be a breaking change. In order to fix this in a unilateral
way, we add points of small order to the masking point to bring it into
prime-order subgroup.
BUG=chromium:778101
Change-Id: I440931a3df7f009b324d2a3e3af2d893a101804f
Reviewed-on: https://boringssl-review.googlesource.com/22445
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This partially reverts commit 38636aba74.
Some build on Android seems to break now. I'm not really sure what the
situation is, but if the weird common symbols are still there (can we
remove them?), they probably ought to have the right flags.
Change-Id: Ief589d763d16b995ac6be536505acf7596a87b30
Reviewed-on: https://boringssl-review.googlesource.com/22404
Commit-Queue: David Benjamin <davidben@google.com>
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>
Those EXPECTs should be ASSERTs to ensure bn is not null.
Change-Id: Icb54c242ffbde5f8eaa67f19f214c9eef13705ea
Reviewed-on: https://boringssl-review.googlesource.com/22366
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>
An embedded item wasn't allocated separately on the heap, so don't
free it as if it was.
Issue discovered by Pavel Kopyl
(Imported from upstream's cdc3307d4257f4fcebbab3b2b44207e1a399da05 and
65d414434aeecd5aa86a46adbfbcb59b4344503a.)
I do not believe this is actually reachable in BoringSSL, even in the
face of malloc errors. The only field which sets ASN1_TFLG_COMBINE is in
X509_ATTRIBUTE. That field's value is X509_ATTRIBUTE_SET which cannot
fail to initialize. (It is a CHOICE whose initialization consists of
setting the selector to -1 and calling the type's callback which is
unset for this type.)
Change-Id: I29c080f8a4ddc2f3ef9c119d0d90a899d3cb78c5
Reviewed-on: https://boringssl-review.googlesource.com/22365
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>
1 << 31 is technically an undefined shift. It should be 1u << 31 to shut
UBSan up. I've also converted the others for consistency.
Change-Id: I1c6fe282f55c7032cea39f5ff1035a7711155f02
Reviewed-on: https://boringssl-review.googlesource.com/22344
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>
Currently we only check that the underlying EC_METHODs match, which
avoids the points being in different forms, but not that the points are
on the same curves. (We fixed the APIs early on so off-curve EC_POINTs
cannot be created.)
In particular, this comes up with folks implementating Java's crypto
APIs with ECDH_compute_key. These APIs are both unfortunate and should
not be mimicked, as they allow folks to mismatch the groups on the two
multiple EC_POINTs. Instead, ECDH APIs should take the public value as a
byte string.
Thanks also to Java's poor crypto APIs, we must support custom curves,
which makes this particularly gnarly. This CL makes EC_GROUP_cmp work
with custom curves and adds an additional subtle requirement to
EC_GROUP_set_generator.
Annoyingly, this change is additionally subtle because we now have a
reference cycle to hack around.
Change-Id: I2efbc4bd5cb65fee5f66527bd6ccad6b9d5120b9
Reviewed-on: https://boringssl-review.googlesource.com/22245
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>
(Credit to libFuzzer for finding this.)
Change-Id: I0353d686d883703d39145c5bdd1e56368a587a35
Reviewed-on: https://boringssl-review.googlesource.com/22324
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Also switch them to accepting a u16 length prefix. We appear not to have
any such tests right now, but RSA-2048 would involve modulus well larger
and primes just a hair larger than a u8 length prefix alows.
Change-Id: Icce8f1d976e159b945302fbba732e72913c7b724
Reviewed-on: https://boringssl-review.googlesource.com/22284
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>
I really need to resurrect the CL to make them entirely static
(https://crbug.com/boringssl/20), but, in the meantime, to make
replacing the EC_METHOD pointer in EC_POINT with EC_GROUP not
*completely* insane, make them refcounted.
OpenSSL did not do this because their EC_GROUPs are mutable
(EC_GROUP_set_asn1_flag and EC_GROUP_set_point_conversion_form). Ours
are immutable but for the two-function dance around custom curves (more
of OpenSSL's habit of making their objects too complex), which is good
enough to refcount.
Change-Id: I3650993737a97da0ddcf0e5fb7a15876e724cadc
Reviewed-on: https://boringssl-review.googlesource.com/22244
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 an OpenSSL thing to support platforms where BN_ULONG is not
actually the size it claims to be. We define BN_ULONG to uint32_t and
uint64_t which are guaranteed by C to implement arithemetic modulo 2^32
and 2^64, respectively. Thus there is no need for any of this.
Change-Id: I098cd4cc050a136b9f2c091dfbc28dd83e01f531
Reviewed-on: https://boringssl-review.googlesource.com/21784
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>
This reverts commit f6942f0d22.
Reason for revert: This doesn't actually work in clang-cl. I
forgot we didn't have the clang-cl try bots enabled! :-( I
believe __asm__ is still okay, but I'll try it by hand
tomorrow.
Original change's description:
> Use uint128_t and __asm__ in clang-cl.
>
> clang-cl does not define __GNUC__ but is still a functioning clang. We
> should be able to use our uint128_t and __asm__ code in it on Windows.
>
> Change-Id: I67310ee68baa0c0c947b2441c265b019ef12af7e
> Reviewed-on: https://boringssl-review.googlesource.com/22184
> 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>
TBR=agl@google.com,davidben@google.com
Change-Id: I5c7e0391cd9c2e8cc0dfde37e174edaf5d17db22
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://boringssl-review.googlesource.com/22224
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>
clang-cl does not define __GNUC__ but is still a functioning clang. We
should be able to use our uint128_t and __asm__ code in it on Windows.
Change-Id: I67310ee68baa0c0c947b2441c265b019ef12af7e
Reviewed-on: https://boringssl-review.googlesource.com/22184
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>
There is also no need to make the struct public. Also tidy up includes a
bit.
Change-Id: I188848dfd8f9ed42925b2c55da8dc4751c29f146
Reviewed-on: https://boringssl-review.googlesource.com/22126
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>
Some of the complaints seem a bit questionable or their replacements
problematic, but not using strcat, strcpy, and strncpy is easy and
safer.
Change-Id: I64faf24b4f39d1ea410e883f026350094975a9b5
Reviewed-on: https://boringssl-review.googlesource.com/22125
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>
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>
Our assembly does not use the GOT to reference symbols, which means
references to visible symbols will often require a TEXTREL. This is
undesirable, so all assembly-referenced symbols should be hidden. CPU
capabilities are the only such symbols defined in C.
These symbols may be hidden by doing at least one of:
1. Build with -fvisibility=hidden
2. __attribute__((visibility("hidden"))) in C.
3. .extern + .hidden in some assembly file referencing the symbol.
We have lots of consumers and can't always rely on (1) happening. We
were doing (3) by way of d216b71f90 and
16e38b2b8f, but missed 32-bit x86 because
it doesn't cause a linker error.
Those two patches are not in upstream. Upstream instead does (3) by way
of x86cpuid.pl and friends, but we have none of these files.
Standardize on doing (2). This avoids accidentally getting TEXTRELs on
some 32-bit x86 build configurations. This also undoes
d216b71f90 and
16e38b2b8f. They are no now longer needed
and reduce the upstream diff.
Change-Id: Ib51c43fce6a7d8292533635e5d85d3c197a93644
Reviewed-on: https://boringssl-review.googlesource.com/22064
Commit-Queue: Matt Braithwaite <mab@google.com>
Reviewed-by: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This removes the last place where non-app-data hooks leave anything
uncomsumed in rrec. (There is still a place where non-app-data hooks see
a non-empty rrec an entrance. read_app_data calls into read_handshake.
That'll be fixed in a later patch in this series.)
This should not change behavior, though some error codes may change due
to some processing happening in a slightly different order.
Since we do this in a few places, this adds a BUF_MEM_append with tests.
Change-Id: I9fe1fc0103e47f90e3c9f4acfe638927aecdeff6
Reviewed-on: https://boringssl-review.googlesource.com/21345
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This will be useful for the SSL stack to properly resurface handshake
failures. Leave this in a private header and, along the way, hide the
various types.
(ERR_NUM_ERRORS didn't change in meaning. The old documentation was
wrong.)
Bug: 206
Change-Id: I4c6ca98d162d11ad5e17e4baf439a18fbe371018
Reviewed-on: https://boringssl-review.googlesource.com/21284
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>
Our build logic needed to revised and and clang implements more warnings
than MSVC, so GTest needed more fixes.
Bug: 200
Change-Id: I84c5dd0c51079dd9c990e08dbea7f9022a7d6842
Reviewed-on: https://boringssl-review.googlesource.com/21204
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>
Right now, compiling with the stock gcc on debian, cmake is compiling
with -Wall which gives an error because -Wunused-value.
The gcc version is gcc (Debian 4.7.2-5) 4.7.2.
Change-Id: Iafd4cc14a22fe788d4c7bdb05202fd856f0c6395
Reviewed-on: https://boringssl-review.googlesource.com/21144
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>
ERR_FLAGS_STRING is meaningless and we can use a bitfield for the mark
bit.
Change-Id: I6f677b55b11316147512171629196c651cb33ca9
Reviewed-on: https://boringssl-review.googlesource.com/21084
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>
sha1-altivec.c is not sensitive to OPENSSL_NO_ASM, so sha1.c needs to
disable the generic implementation accordingly.
Bug: 204
Change-Id: Ic655f8b76907f07da33afa863d1b24d62d42e23a
Reviewed-on: https://boringssl-review.googlesource.com/21064
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>
Cut down on the number of cases we need to worry about here. In
particular, it would be useful for the handshake to be able to replay an
error.
Change-Id: I2345faaff5503ede1324a5599e680de83f4b106e
Reviewed-on: https://boringssl-review.googlesource.com/21004
Commit-Queue: David Benjamin <davidben@google.com>
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>
crypto/bio/bio_test.cc - I'm not sure where this was added for, but none
of the functions used there appear to have feature macros documented.
crypto/bio/printf.c - -std=c99 provides (v)snprintf.
crypto/lhash/lhash_test.cc - we no longer call rand_r.
crypto/mem.c - we no longer call strdup and -std=c99 provides (v)snprintf.
Apple messed up their headers and, if _POSIX_C_SOURCE is defined but
_DARWIN_C_SOURCE isn't, pthread.h no longer defines mach_port_t. They
then shipped a version of libc++ headers that is missing this fix, so
the build breaks:
bcc92d75df
If one uses XCode, they've hacked their pthread.h to provide mach_port_t
if defined(__cplusplus), but the standalone tools appear to be old and
missing this.
We can work around this by also defining _DARWIN_C_SOURCE in C++ files
that need _POSIX_C_SOURCE, but it appears none of these files actually
need it.
Change-Id: I5df9453730696100eb22b809febeb65053701322
Reviewed-on: https://boringssl-review.googlesource.com/20964
Reviewed-by: Adam Langley <agl@google.com>
In case the XCode install is at, say "/Applications/Xcode 9.app". This
won't work if the path contains quotes, but it doesn't appear CMake
itself makes any effort to handle that right.
Change-Id: Ifecf6147d44ffdae8c2692b2d6c94bfafd8d7714
Reviewed-on: https://boringssl-review.googlesource.com/20944
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>
The exponent is secret, so we should be using the consttime variant. See
also upstream's f9cbf470180841966338db1f4c28d99ec4debec4.
Change-Id: I233d4223ded5b80711d7c8f906e3579c36b24cd0
Reviewed-on: https://boringssl-review.googlesource.com/20924
Reviewed-by: Adam Langley <agl@google.com>
Although we are derived from 1.0.2, we mimic 1.1.0 in some ways around
our FOO_up_ref functions and opaque libssl types. This causes some
difficulties when porting third-party code as any OPENSSL_VERSION_NUMBER
checks for 1.1.0 APIs we have will be wrong.
Moreover, adding accessors without changing OPENSSL_VERSION_NUMBER can
break external projects. It is common to implement a compatibility
version of an accessor under #ifdef as a static function. This then
conflicts with our headers if we, unlike OpenSSL 1.0.2, have this
function.
This change switches OPENSSL_VERSION_NUMBER to 1.1.0 and atomically adds
enough accessors for software with 1.1.0 support already. The hope is
this will unblock hiding SSL_CTX and SSL_SESSION, which will be
especially useful with C++-ficiation. The cost is we will hit some
growing pains as more 1.1.0 consumers enter the ecosystem and we
converge on the right set of APIs to import from upstream.
It does not remove any 1.0.2 APIs, so we will not require that all
projects support 1.1.0. The exception is APIs which changed in 1.1.0 but
did not change the function signature. Those are breaking changes.
Specifically:
- SSL_CTX_sess_set_get_cb is now const-correct.
- X509_get0_signature is now const-correct.
For C++ consumers only, this change temporarily includes an overload
hack for SSL_CTX_sess_set_get_cb that keeps the old callback working.
This is a workaround for Node not yet supporting OpenSSL 1.1.0.
The version number is set at (the as yet unreleased) 1.1.0g to denote
that this change includes https://github.com/openssl/openssl/pull/4384.
Bug: 91
Change-Id: I5eeb27448a6db4c25c244afac37f9604d9608a76
Reviewed-on: https://boringssl-review.googlesource.com/10340
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>
Chromium's OCSP code needs the OIDs and we already have them on hand.
Change-Id: Icab012ba4ae15ce029cbfe3ed93f89470137e7f6
Reviewed-on: https://boringssl-review.googlesource.com/20724
Commit-Queue: David Benjamin <davidben@google.com>
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>
We haven't supported MSVC 2013 for a while (we may even be able to drop
2015 in not too long). There is also no need to pull in stdalign.h in
C++. alignof and alignas are keywords.
Change-Id: Ib31d8166282592bcb9e1c543e57758ff55746404
Reviewed-on: https://boringssl-review.googlesource.com/20704
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>
First, I spelled the wildcard name constraint in many_constraints.pem
wrong. It's .test, not *.test for name constraints. (This doesn't matter
for some_names*.pem, but it does to avoid a false negative in
many_names3.pem.)
Second, the CN of certs should be a host, not "Leaf". OpenSSL 1.1.0
checks "host-like" CNs against name constraints too and "Leaf" is
host-like.
I've also made the generator deterministic and checked it in, as PEM
blobs are not reviewable.
Change-Id: I195d9846315168a792cca829aff25c986339b8f5
Reviewed-on: https://boringssl-review.googlesource.com/20584
Reviewed-by: David Benjamin <davidben@google.com>
Fixes failed compile with [-Werror=implicit-fallthrough=], which is
default on gcc-7.x on distributions like fedora.
Enabling no implicit fallthrough for more than just clang as well to
catch this going forward.
Change-Id: I6cd880dac70ec126bd7812e2d9e5ff804d32cadd
Signed-off-by: Vincent Batts <vbatts@redhat.com>
Reviewed-on: https://boringssl-review.googlesource.com/20564
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Thanks to Lennart Beringer for pointing that that malloc failures could
lead to invalid EVP_MD_CTX states. This change cleans up the code in
general so that fallible operations are all performed before mutating
objects. Thus failures should leave objects in a valid state.
Also, |ctx_size| is never zero and a hash with no context is not
sensible, so stop handling that case and simply assert that it doesn't
occur.
Change-Id: Ia60c3796dcf2f772f55e12e49431af6475f64d52
Reviewed-on: https://boringssl-review.googlesource.com/20544
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
I'll fully remove this once Chrome 62 hits stable, in case any bug
reports come in for Chrome 61. Meanwhile switch the default to off so
that other consumers pick up the behavior. (Should have done this sooner
and forgot.)
Bug: chromium:735616
Change-Id: Ib27c4072f228cd3b5cce283accd22732eeef46b2
Reviewed-on: https://boringssl-review.googlesource.com/20484
Commit-Queue: David Benjamin <davidben@google.com>
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>
We don't get up to 16-byte alignment without additional work like
https://boringssl-review.googlesource.com/20204. This just makes UBSan
unhappy at us.
Change-Id: I55d9cb5b40e5177c3c7aac7828c1d22f2bfda9a6
Reviewed-on: https://boringssl-review.googlesource.com/20464
Commit-Queue: David Benjamin <davidben@google.com>
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>
crypto/asn1 routinely switches between int and long without overflow
checks. Fortunately, it funnels everything into a common entrypoint, so
we can uniformly bound all inputs to something which comfortably fits in
an int.
Change-Id: I340674c6b07820309dc5891024498878c82e225b
Reviewed-on: https://boringssl-review.googlesource.com/20366
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>
Thes are remnants of some old setup.
Change-Id: I09151fda9419fbe7514f2f609f70284965694bfa
Reviewed-on: https://boringssl-review.googlesource.com/20365
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 to keep Chromium building.
Bug: chromium:765754
Change-Id: I312f747e27e53590a948305f80abc240bfd2063c
Reviewed-on: https://boringssl-review.googlesource.com/20344
Reviewed-by: Aaron Green <aarongreen@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Fuchsia needed to rename Magenta to Zircon. Several syscalls and status
codes changed as a result.
Change-Id: I64b5ae4537ccfb0a318452fed34040a2e8f5012e
Reviewed-on: https://boringssl-review.googlesource.com/20324
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>
Windows provides _aligned_malloc, so we could provide an
|OPENSSL_aligned_malloc| in the future. However, since we're still
trying to get the zeroisation change landed everywhere, a self-contained
change seems easier until that has settled down.
Change-Id: I47bbd811a7fa1758f3c0a8a766a1058523949b7f
Reviewed-on: https://boringssl-review.googlesource.com/20204
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>
This guards against the name constraints check consuming large amounts
of CPU time when certificates in the presented chain contain an
excessive number of names (specifically subject email names or subject
alternative DNS names) and/or name constraints.
Name constraints checking compares the names presented in a certificate
against the name constraints included in a certificate higher up in the
chain using two nested for loops.
Move the name constraints check so that it happens after signature
verification so peers cannot exploit this using a chain with invalid
signatures. Also impose a hard limit on the number of name constraints
check loop iterations to further mitigate the issue.
Thanks to NCC for finding this issue.
Change-Id: I112ba76fe75d1579c45291042e448850b830cbb7
Reviewed-on: https://boringssl-review.googlesource.com/19164
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
c2i_ASN1_BIT_STRING takes length as a long but uses it as an int. Check bounds
before doing so. Previously, excessively large inputs to the function could
write a single byte outside the target buffer. (This is unreachable as
asn1_ex_c2i already uses int for the length.)
Thanks to NCC for finding this issue.
Change-Id: I7ae42214ca620d4159fa01c942153717a7647c65
Reviewed-on: https://boringssl-review.googlesource.com/19204
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Allocations by |OPENSSL_malloc| are prefixed with their length.
|OPENSSL_free| zeros the allocation before calling free(), eliminating
the need for a separate call to |OPENSSL_cleanse| for sensitive data.
This change will be followed up by the cleanup in
https://boringssl-review.googlesource.com/c/boringssl/+/19824.
Change-Id: Ie272f07e9248d7d78af9aea81dacec0fdb7484c4
Reviewed-on: https://boringssl-review.googlesource.com/19544
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Rather than clear them, even on failure, detect if an individual test
failed and dump the error queue there. We already do this at the GTest
level in ErrorTestEventListener, but that is too coarse-grained for the
file tests.
Change-Id: I3437626dcf3ec43f6fddd98153b0af73dbdcce84
Reviewed-on: https://boringssl-review.googlesource.com/19966
Reviewed-by: Steven Valdez <svaldez@google.com>
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>
We have no tests for encryption right now, and evp_tests.txt needs to
force RSA-PSS to have salt length 0, even though other salt values are
more common. This also lets us test the salt length -2 silliness.
Change-Id: I30f52d36c38732c9b63a02c66ada1d08488417d4
Reviewed-on: https://boringssl-review.googlesource.com/19965
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>
We do not expose EVP_PKEY_CTX_ctrl, so we can freely change the
semantics of EVP_PKEY_CTRL_RSA_OAEP_LABEL. That means we can pass in an
actual size_t rather than an int.
Not that anyone is actually going to exceed an INT_MAX-length RSA-OAEP
label.
Change-Id: Ifc4eb296ff9088c8815f4f8cd88100a407e4d969
Reviewed-on: https://boringssl-review.googlesource.com/19984
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>
It was pointed out that we have no test coverage of this. Fix this. Test
vector generated using Go's implementation.
Change-Id: Iddbc50d3b422e853f8afd50117492f4666a47373
Reviewed-on: https://boringssl-review.googlesource.com/19964
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>
linux/random.h is not really needed if FIPS mode is not enabled. Note
that use of the getrandom syscall is unaffected by this header.
Fixes commit bc7daec4d8
Change-Id: Ia367aeffb3f2802ba97fd1507de0b718d9ac2c55
Reviewed-on: https://boringssl-review.googlesource.com/19644
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>
No need to have two of these.
Change-Id: I5ff1ba24757828d8113321cd3262fed3d4defcdb
Reviewed-on: https://boringssl-review.googlesource.com/19525
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>
One less macro to worry about in bcm.c.
Change-Id: I321084c0d4ed1bec38c541b04f5b3468350c6eaa
Reviewed-on: https://boringssl-review.googlesource.com/19565
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>
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>
These groups are terrible, we got the function wrong (unused ENGINE
parameter does not match upstream), and the functions are unused. Unwind
them. This change doesn't unwind the X9.42 Diffie-Hellman machinery, so
the checks are still present and tested.
(We can probably get rid of the X9.42 machinery too, but it is reachable
from DSA_dup_DH. That's only used by wpa_supplicant and, if that code
ever ran, it'd be ignored because we don't support DHE in TLS. I've left
it alone for the time being.)
Bug: 2
Change-Id: I8d9396983c8d40ed46a03ba6947720da7e9b689a
Reviewed-on: https://boringssl-review.googlesource.com/19384
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>
It's confusing to have both mont and mont_data on EC_GROUP. The
documentation was also wrong.
Change-Id: I4e2e3169ed79307018212fba51d015bbbe5c4227
Reviewed-on: https://boringssl-review.googlesource.com/10348
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>
Someone tried to build us with Ubuntu's MinGW. This is too old to be
supported (the tests rather badly fail to build), but some of the fixes
will likely be useful for eventually building Clang for Windows
standalone too.
Change-Id: I6d279a0da1346b4e0813de51df3373b7412de33a
Reviewed-on: https://boringssl-review.googlesource.com/19364
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>
This is never used.
Change-Id: I20498cab5b59ec141944d4a5e907a1164d0ae559
Reviewed-on: https://boringssl-review.googlesource.com/19184
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>
The ticket encryption key is rotated automatically once every 24 hours,
unless a key has been configured manually (i.e. using
|SSL_CTX_set_tlsext_ticket_keys|) or one of the custom ticket encryption
methods is used.
Change-Id: I0dfff28b33e58e96b3bbf7f94dcd6d2642f37aec
Reviewed-on: https://boringssl-review.googlesource.com/18924
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>
Fuchsia isn't POSIX and doesn't have /etc. This CL adds the
location for the system certificate store on Fuchsia.
Change-Id: I2b48e0e13525a32fa5e2c5c48b8db41d76c26872
Reviewed-on: https://boringssl-review.googlesource.com/19224
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Using ADX instructions requires relatively new assemblers. Conscrypt are
currently using Yasm 1.2.0. Revert these for the time being to unbreak
their build.
Change-Id: Iaba5761ccedcafaffb5ca79a8eaf7fa565583c32
Reviewed-on: https://boringssl-review.googlesource.com/19244
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Refactor bio_io() to use a switch/case statement to call the correct BIO
method. This is cleaner and eliminates calling a function pointer cast
to an incompatible type signature, which conflicts with LLVMs
implementation of control flow integrity for indirect calls.
Change-Id: I5456635e1c9857cdce810758ba0000577cc94b01
Reviewed-on: https://boringssl-review.googlesource.com/19084
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This loosens the earlier restriction to match Channel ID. Both may be
configured and offered, but the server is obligated to select only one
of them. This aligns with the current tokbind + 0-RTT draft where the
combination is signaled by a separate extension.
Bug: 183
Change-Id: I786102a679999705d399f0091f76da236be091c2
Reviewed-on: https://boringssl-review.googlesource.com/19124
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
We can test these with Intel SDE now. The AVX2 code just affects the two
select functions while the ADX code is a separate implementation.
Haswell numbers:
Before:
Did 84630 ECDH P-256 operations in 10031494us (8436.4 ops/sec)
Did 206000 ECDSA P-256 signing operations in 10015055us (20569.0 ops/sec)
Did 77256 ECDSA P-256 verify operations in 10064556us (7676.0 ops/sec)
After:
Did 86112 ECDH P-256 operations in 10015008us (8598.3 ops/sec)
Did 211000 ECDSA P-256 signing operations in 10025104us (21047.2 ops/sec)
Did 79344 ECDSA P-256 verify operations in 10017076us (7920.9 ops/sec)
Skylake numbers:
Before:
Did 75684 ECDH P-256 operations in 10016019us (7556.3 ops/sec)
Did 185000 ECDSA P-256 signing operations in 10012090us (18477.7 ops/sec)
Did 72885 ECDSA P-256 verify operations in 10027154us (7268.8 ops/sec)
After:
Did 89598 ECDH P-256 operations in 10032162us (8931.1 ops/sec)
Did 203000 ECDSA P-256 signing operations in 10019739us (20260.0 ops/sec)
Did 87040 ECDSA P-256 verify operations in 10000441us (8703.6 ops/sec)
The code was slightly patched for delocate.go compatibility.
Change-Id: Ic44ced4eca65c656bbe07d5a7fee91ec6925eb59
Reviewed-on: https://boringssl-review.googlesource.com/18967
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This is a reland of https://boringssl-review.googlesource.com/18965
which was reverted due to Windows toolchain problems that have since
been fixed.
We have an SDE bot now and can more easily test things. We also enabled
ADX in rsaz-avx2.pl which does not work without x86_64-mont*.pl enabled.
rsa-avx2.pl's ADX code only turns itself off so that the faster ADX code
can be used... but we disable it.
Verified, after reverting the fix, the test vectors we imported combined
with Intel SDE catches CVE-2016-7055, so we do indeed have test
coverage. Also verified on the Windows version of Intel SDE.
Thanks to Alexey Ivanov for pointing out the discrepancy.
Skylake numbers:
Before:
Did 7296 RSA 2048 signing operations in 10038191us (726.8 ops/sec)
Did 209000 RSA 2048 verify operations in 10030629us (20836.2 ops/sec)
Did 1080 RSA 4096 signing operations in 10072221us (107.2 ops/sec)
Did 60836 RSA 4096 verify operations in 10053929us (6051.0 ops/sec)
ADX consistently off:
Did 9360 RSA 2048 signing operations in 10025823us (933.6 ops/sec)
Did 220000 RSA 2048 verify operations in 10024339us (21946.6 ops/sec)
Did 1048 RSA 4096 signing operations in 10006782us (104.7 ops/sec)
Did 61936 RSA 4096 verify operations in 10088011us (6139.6 ops/sec)
After (ADX consistently on):
Did 10444 RSA 2048 signing operations in 10006781us (1043.7 ops/sec)
Did 323000 RSA 2048 verify operations in 10012192us (32260.7 ops/sec)
Did 1610 RSA 4096 signing operations in 10044930us (160.3 ops/sec)
Did 96000 RSA 4096 verify operations in 10075606us (9528.0 ops/sec)
Change-Id: I2502ce80e9cfcdea40907512682e3a6663000faa
Reviewed-on: https://boringssl-review.googlesource.com/19105
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Other projects are starting to use them. Having two APIs for the same
thing is silly, so deprecate all our old ones.
Change-Id: Iaf6b6995bc9e4b624140d5c645000fbf2cb08162
Reviewed-on: https://boringssl-review.googlesource.com/19064
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>
The AVX2 code has alignment requirements.
Change-Id: Ieb0774f7595a76eef0f3a15aabd63d056bbaa463
Reviewed-on: https://boringssl-review.googlesource.com/18966
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 reverts commit 83d1a3d3c8.
Reason for revert: Our Windows setup can't handle these instructions.
Will investigate tomorrow, possibly by turning ADX off on Windows.
Change-Id: I378fc0906c59b9bac9da17a33ba8280c70fdc995
Reviewed-on: https://boringssl-review.googlesource.com/19004
Reviewed-by: David Benjamin <davidben@google.com>
We have an SDE bot now and can more easily test things. We also enabled
ADX in rsaz-avx2.pl which does not work without x86_64-mont*.pl enabled.
rsa-avx2.pl's ADX code only turns itself off so that the faster ADX code
can be used... but we disable it.
Verified, after reverting the fix, the test vectors we imported combined
with Intel SDE catches CVE-2016-7055, so we do indeed have test
coverage.
Thanks to Alexey Ivanov for pointing out the discrepancy.
Skylake numbers:
Before:
Did 7296 RSA 2048 signing operations in 10038191us (726.8 ops/sec)
Did 209000 RSA 2048 verify operations in 10030629us (20836.2 ops/sec)
Did 1080 RSA 4096 signing operations in 10072221us (107.2 ops/sec)
Did 60836 RSA 4096 verify operations in 10053929us (6051.0 ops/sec)
ADX consistently off:
Did 9360 RSA 2048 signing operations in 10025823us (933.6 ops/sec)
Did 220000 RSA 2048 verify operations in 10024339us (21946.6 ops/sec)
Did 1048 RSA 4096 signing operations in 10006782us (104.7 ops/sec)
Did 61936 RSA 4096 verify operations in 10088011us (6139.6 ops/sec)
After (ADX consistently on):
Did 10444 RSA 2048 signing operations in 10006781us (1043.7 ops/sec)
Did 323000 RSA 2048 verify operations in 10012192us (32260.7 ops/sec)
Did 1610 RSA 4096 signing operations in 10044930us (160.3 ops/sec)
Did 96000 RSA 4096 verify operations in 10075606us (9528.0 ops/sec)
Change-Id: Icbbd4f06dde60d1a42a691c511b34c47b9a2da5f
Reviewed-on: https://boringssl-review.googlesource.com/18965
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
See upstream's 5292833132cc863b66574fe2bbf55e4b2eff7949. Syncing just to
reduce the diff for the time being.
Change-Id: I0992d538b283d7348ef1d993973291f5416edce6
Reviewed-on: https://boringssl-review.googlesource.com/18804
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 memcpy of a pointer looks like a typo, though it isn't. Instead,
transcribe what the functions expect into a union and let C fill it in.
Change-Id: Iba4c824295e8908c5bda68ac35673040a8cff116
Reviewed-on: https://boringssl-review.googlesource.com/18744
Commit-Queue: David Benjamin <davidben@google.com>
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>
There are still a ton of them, almost exclusively complaints that
function declaration and definitions have different parameter names. I
just fixed a few randomly.
Change-Id: I1072f3dba8f63372cda92425aa94f4aa9e3911fa
Reviewed-on: https://boringssl-review.googlesource.com/18706
Reviewed-by: Steven Valdez <svaldez@google.com>
Change-Id: I84b9a7606aaf28e582c79ada47df95b46ff2c2c2
Reviewed-on: https://boringssl-review.googlesource.com/18624
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>
Similarly, add EVP_AEAD_CTX_tag_len which computes the exact tag length
for required by EVP_AEAD_CTX_seal_scatter.
Change-Id: I069b0ad16fab314fd42f6048a3c1dc45e8376f7f
Reviewed-on: https://boringssl-review.googlesource.com/18324
Reviewed-by: Adam Langley <agl@google.com>
Apparently C does not promise this, only that casting zero to a pointer
gives NULL. No compiler will be insane enough to violate this, but it's
an easy assumption to document.
Change-Id: Ie255d42af655a4be07bcaf48ca90584a85c6aefd
Reviewed-on: https://boringssl-review.googlesource.com/18584
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The changes to the assembly files are synced from upstream's
64d92d74985ebb3d0be58a9718f9e080a14a8e7f. cpu-intel.c is translated to C
from that commit and d84df594404ebbd71d21fec5526178d935e4d88d.
Change-Id: I02c8f83aa4780df301c21f011ef2d8d8300e2f2a
Reviewed-on: https://boringssl-review.googlesource.com/18411
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Also clear AVX512 bits if %xmm and %ymm registers are not preserved. See
also upstream's 66bee01c822c5dd26679cad076c52b3d81199668.
Change-Id: I1bcaf4cf355e3ca0adb5d207ae6185f9b49c0245
Reviewed-on: https://boringssl-review.googlesource.com/18410
Reviewed-by: Steven Valdez <svaldez@google.com>
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>
X.509 functions and the like should not vary their behaviour based on
the configured locale, but tolower(3), strcasecmp(3) and strncasecmp(3)
change behaviour based on that.
For example, with tr_TR.utf8, 'I' is not the upper-case version of 'i'.
Change-Id: I896a285767ae0c22e6ce06b9908331c625e90af2
Reviewed-on: https://boringssl-review.googlesource.com/18412
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>
At this point, the security policy document will be maintained in the
BoringSSL repo for change control.
Change-Id: I9ece51a0e9a506267e2f3b5215fb0d516d0d834b
Reviewed-on: https://boringssl-review.googlesource.com/18184
Reviewed-by: David Benjamin <davidben@google.com>
The former is defined by the kernel and is a straightforward number. The
latter is defined by glibc as:
#define SYS_getrandom __NR_getrandom
which does not work when kernel headers are older than glibc headers.
Instead, use the kernel values.
Bug: chromium:742260
Change-Id: Id162f125db660643269e0b1329633437048575c4
Reviewed-on: https://boringssl-review.googlesource.com/17864
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>
This change updates the ChaCha20-Poly1305 AEAD to be able to process
|extra_in| data. It does this by encrypting the extra data byte-by-byte
(because extra data should be very small). Both the generic and assembly
code is updated to be able to include this extra ciphertext in the
Poly1305 calculation.
Change-Id: I751ed31fb7e1f4db6974e9ed31721a43177cf8cb
Reviewed-on: https://boringssl-review.googlesource.com/17465
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
This isn't all of our pointer games by far, but for any code which
doesn't run on armv6, memcpy and pointer cast compile to the same code.
For code with does care about armv6 (do we care?), it'll need a bit more
work. armv6 makes memcpy into a function call.
Ironically, the one platform where C needs its alignment rules is the
one platform that makes it hard to honor C's alignment rules.
Change-Id: Ib9775aa4d9df9381995df8698bd11eb260aac58c
Reviewed-on: https://boringssl-review.googlesource.com/17707
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 tag doesn't actually do anything, except cause UBSan to point out
that malloc doesn't align that tightly. malloc does, however, appear to
align up to 16-bytes, which is the actual alignment requirement of that
code. So just replace 64 with 16.
When we're juggling less things, it'd be nice to see what toolchain
support for the various aligned allocators looks like. Or maybe someday
we can use C++ new which one hopes is smart enough to deal with all
this.
Change-Id: Idbdde66852d5dad25a044d4c68ffa3b3f213025a
Reviewed-on: https://boringssl-review.googlesource.com/17706
Commit-Queue: David Benjamin <davidben@google.com>
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>
This is the last of the non-GTest tests. We never did end up writing
example files or doc.go tooling for them. And probably examples should
be in C++ at this point.
Bug: 129
Change-Id: Icbc43c9639cfed7423df20df1cdcb8c35f23fc1a
Reviewed-on: https://boringssl-review.googlesource.com/17669
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>
We've got three versions of DATA_TOO_LARGE and two versions of
DATA_TOO_SMALL with no apparent distinction between them.
Change-Id: I18ca2cb71ffc31b04c8fd0be316c362da4d7daf9
Reviewed-on: https://boringssl-review.googlesource.com/17529
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>
When tree_calculate_user_set() fails, a jump to error failed to
deallocate a possibly allocated |auth_nodes|.
(Imported from upstream's 58314197b54cc1417cfa62d1987462f72a2559e0.)
Also sync up a couple of comments from that revision. Upstream's
reformat script mangled them and we never did the manual fixup.
Change-Id: I1ed896d13ec94d122d71df72af5a3be4eb0eb9d1
Reviewed-on: https://boringssl-review.googlesource.com/17644
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>
This adds sections on running CAVP tests, breaking FIPS tests and the
RNG design.
Change-Id: I859290e8e2e6ab087aa2b6570a30176b42b01073
Reviewed-on: https://boringssl-review.googlesource.com/17585
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I683481b12e66966729297466748f1869de0b913b
Reviewed-on: https://boringssl-review.googlesource.com/17584
Commit-Queue: Adam Langley <agl@google.com>
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 imports bf5b8ff17dd7039b15cbc6468cd865cbc219581d and
a696708ae6bbe42f409748b3e31bb2f3034edbf3 from upstream. I missed them at
some point.
Change-Id: I882d995868e4c0461b7ca51a854691cf4faa7260
Reviewed-on: https://boringssl-review.googlesource.com/17384
Reviewed-by: Adam Langley <agl@google.com>
The __clang__-guarded #defines cause gas to complain if clang is passed
-fno-integrated-as. Emitting .syntax unified when those are used fixes
this. This matches the change made to ghash-armv4.pl in upstream's
6cf412c473d8145562b76219ce3da73b201b3255.
See also https://github.com/openssl/openssl/pull/3694. This fixes the
build with the latest Android NDK (use the NDK-supplied toolchain file)
with the armeabi ABI.
Bug: chromium:732066
Change-Id: Ic6ca633a58edbe8ae8c7d501bd9515c2476fd7c2
Reviewed-on: https://boringssl-review.googlesource.com/17404
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>
There's a |tag_len| in the generic AEAD context now so keeping a second
copy only invites confusion.
Change-Id: I029d8a8ee366e3af7f61408177c950d5b1a740a9
Reviewed-on: https://boringssl-review.googlesource.com/17424
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
WatchGuard's bug is very distinctive. Report a dedicated error code out
of BoringSSL so we can better track this.
Bug: chromium:733223
Change-Id: Ia42abd8654e7987b1d43c63a4f454f35f6aa873b
Reviewed-on: https://boringssl-review.googlesource.com/17328
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>
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>
Bug: chromium:731280
Change-Id: I87161a3400ac5119401ec157df5843249971327a
Reviewed-on: https://boringssl-review.googlesource.com/17246
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Aaron Green <aarongreen@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I8512c6bfb62f1a83afc8f763d681bf5db3b4ceae
Reviewed-on: https://boringssl-review.googlesource.com/17144
Commit-Queue: Adam Langley <alangley@gmail.com>
Reviewed-by: David Benjamin <davidben@google.com>
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>
Three modules were left behind in
I59df0b567e8e80befe5c399f817d6410ddafc577.
(Imported from upstream's c93f06c12f10c07cea935abd78a07a037e27f155.)
This actually meant functions defined in those two files were
non-functional. I'm guessing no one noticed upstream because, if you go
strictly by iOS compile-time capabilities, all this code is unreachable
on ios32, only ios64.
Change-Id: I55035edf2aebf96d14bdf66161afa2374643d4ec
Reviewed-on: https://boringssl-review.googlesource.com/17113
Reviewed-by: David Benjamin <davidben@google.com>
(Imported from upstream's 413b6a82594ab45192dda233a77efe5637d656d6.)
This doesn't affect us but is imported to make future imports easier.
Change-Id: I8cc97d658df6cc25da69bff840b96a47e2946ddb
Reviewed-on: https://boringssl-review.googlesource.com/17112
Reviewed-by: Adam Langley <agl@google.com>
This change was made by copying over the files as of that commit and
then discarding the parts of the diff which corresponding to our own
changes.
Change-Id: I28c5d711f7a8cec30749b8174687434129af5209
Reviewed-on: https://boringssl-review.googlesource.com/17111
Reviewed-by: Adam Langley <agl@google.com>
Close difference gap on Cortex-A9, which resulted in further improvement
even on other processors.
(Imported from upstream's 8eed3289b21d25583ed44742db43a2d727b79643.)
Performance numbers on a Nexus 5X in AArch32 mode:
$ ./bssl.old speed -filter RSA -timeout 5
Did 355 RSA 2048 signing operations in 5009578us (70.9 ops/sec)
Did 20577 RSA 2048 verify operations in 5079000us (4051.4 ops/sec)
Did 66 RSA 4096 signing operations in 5057941us (13.0 ops/sec)
Did 5564 RSA 4096 verify operations in 5086902us (1093.8 ops/sec)
$ ./bssl speed -filter RSA -timeout 5
Did 411 RSA 2048 signing operations in 5010206us (82.0 ops/sec)
Did 27720 RSA 2048 verify operations in 5048114us (5491.2 ops/sec)
Did 86 RSA 4096 signing operations in 5056160us (17.0 ops/sec)
Did 8216 RSA 4096 verify operations in 5048719us (1627.3 ops/sec)
Change-Id: I8c5be9ff9405ec1796dcf4cfe7df8a89e5a50ce5
Reviewed-on: https://boringssl-review.googlesource.com/17109
Reviewed-by: Adam Langley <agl@google.com>
As some of ARM processors, more specifically Cortex-Mx series, are
Thumb2-only, we need to support Thumb2-only builds even in assembly.
(Imported from upstream's 11208dcfb9105e8afa37233185decefd45e89e17.)
Change-Id: I7cb48ce6a842cf3cfdf553f6e6e6227d52d525c0
Reviewed-on: https://boringssl-review.googlesource.com/17108
Reviewed-by: Adam Langley <agl@google.com>
This reverts commit 2cd63877b5. We've
since imported a change to upstream which adds some #defines that should
do the same thing on clang. (Though if gas accepts unified assembly too,
that does seem a better approach. Ah well. Diverging on these files is
expensive.)
This is to reduce the diff and make applying some subsequent changes
easier.
Change-Id: I3f5eae2a71919b291a8de9415b894d8f0c67e3cf
Reviewed-on: https://boringssl-review.googlesource.com/17107
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 5e5ece561d1f7e557c8e0ea202a8c1f3008361ce.)
This doesn't matter but reduces the diff for changes past it.
Change-Id: Ib2e979eedad2a0b89c9d172207f6b7e610bf211f
Reviewed-on: https://boringssl-review.googlesource.com/17106
Commit-Queue: David Benjamin <davidben@google.com>
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>
We patch arm-xlate.pl to add the ifdefs, so this isn't needed and
reduces our upstream diff.
(We do still have a diff from upstream here. Will go through them
shortly.)
Change-Id: I5b1e301b9111969815f58d69a98591c973465f42
Reviewed-on: https://boringssl-review.googlesource.com/17105
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 imports upstream's scrypt implementation, though it's been heavily
revised. I lost track of words vs. blocks vs. bigger blocks too many
times in the original code and introduced a typedef for the fixed-width
Salsa20 blocks. The downside is going from bytes to blocks is a bit
trickier, so I took advantage of our little-endian assumption.
This also adds an missing check for N < 2^32. Upstream's code is making
this assumption in Integerify. I'll send that change back upstream. I've
also removed the weird edge case where a NULL out_key parameter means to
validate N/r/p against max_mem and nothing else. That's just in there to
get a different error code out of their PKCS#12 code.
Performance-wise, the cleanup appears to be the same (up to what little
precision I was able to get here), but an optimization to use bitwise
AND rather than modulus makes us measurably faster. Though scrypt isn't
a fast operation to begin with, so hopefully it isn't anyone's
bottleneck.
This CL does not route scrypt up to the PKCS#12 code, though we could
write our own version of that if we need to later.
BUG=chromium:731993
Change-Id: Ib2f43344017ed37b6bafd85a2c2b103d695020b8
Reviewed-on: https://boringssl-review.googlesource.com/17084
Reviewed-by: Adam Langley <agl@google.com>
Rather than adding a new mode to EVP_PKEY_CTX, upstream chose to tie
single-shot signing to EVP_MD_CTX, adding functions which combine
EVP_Digest*Update and EVP_Digest*Final. This adds a weird vestigial
EVP_MD_CTX and makes the signing digest parameter non-uniform, slightly
complicating things. But it means APIs like X509_sign_ctx can work
without modification.
Align with upstream's APIs. This required a bit of fiddling around
evp_test.cc. For consistency and to avoid baking details of parameter
input order, I made it eagerly read all inputs before calling
SetupContext. Otherwise which attributes are present depend a lot on the
shape of the API we use---notably the NO_DEFAULT_DIGEST tests for RSA
switch to failing before consuming an input, which is odd.
(This only matters because we have some tests which expect the operation
to abort the operation early with parameter errors and match against
Error. Those probably should not use FileTest to begin with, but I'll
tease that apart a later time.)
Upstream also named NID_Ed25519 as NID_ED25519, even though the
algorithm is normally stylized as "Ed25519". Switch it to match.
Change-Id: Id6c8f5715930038e754de50338924d044e908045
Reviewed-on: https://boringssl-review.googlesource.com/17044
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>
These behave like EVP_AEAD_CTX_{seal,open} respectively, but receive
ciphertext and authentication tag as separate arguments, rather than one
contiguous out or in buffer.
Change-Id: Ia4f1b83424bc7067c55dd9e5a68f18061dab4d07
Reviewed-on: https://boringssl-review.googlesource.com/16924
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
These are never referenced within the library or externally. Some of the
constants have been unused since SSLeay.
Change-Id: I597511208dab1ab3816e5f730fcadaea9a733dff
Reviewed-on: https://boringssl-review.googlesource.com/17025
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Originally we had some confusion around whether the features could be
toggled individually or not. Per the ARM C Language Extensions doc[1],
__ARM_FEATURE_CRYPTO implies the "crypto extension" which encompasses
all of them. The runtime CPUID equivalent can report the features
individually, but it seems no one separates them in practice, for now.
(If they ever do, probably there'll be a new set of #defines.)
[1] http://infocenter.arm.com/help/topic/com.arm.doc.ihi0053c/IHI0053C_acle_2_0.pdf
Change-Id: I12915dfc308f58fb005286db75e50d8328eeb3ea
Reviewed-on: https://boringssl-review.googlesource.com/16991
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>
These are, in turn, just taken from RFC 8032 and are all in
ed25519_tests.txt. But it's probably good to test non-empty inputs at
the EVP_PKEY layer too.
Change-Id: I21871a6efaad5c88b828d2e90d757c325a550b2a
Reviewed-on: https://boringssl-review.googlesource.com/16989
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This was specific to some old software on the test machine. Shrinking
the critical section to not cover getrandom is probably worthwhile
anyway though, so keep it around but make the comment less scary.
Change-Id: I8c17b6688ae93f6aef5d89c252900985d9e7bb52
Reviewed-on: https://boringssl-review.googlesource.com/16992
Reviewed-by: Adam Langley <agl@google.com>
This matches the example code in IG 9.10.
Change-Id: Ie010d135d6c30acb9248b689302b0a27d65bc4f7
Reviewed-on: https://boringssl-review.googlesource.com/17006
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This is a fairly shallow conversion because of the somewhat screwy Error
lines in the test which may target random functions like
EVP_PKEY_CTX_set_signature_md. We probably should revise this, perhaps
moving those to normal tests and leaving error codes to the core
operation itself.
BUG=129
Change-Id: I27dcc945058911b2de40cd48466d4e0366813a12
Reviewed-on: https://boringssl-review.googlesource.com/16988
Commit-Queue: David Benjamin <davidben@google.com>
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>
This is less likely to make the compiler grumpy and generates the same
code. (Although this file has worse casts here which I'm still trying to
get the compiler to cooperate on.)
Change-Id: If7ac04c899d2cba2df34eac51d932a82d0c502d9
Reviewed-on: https://boringssl-review.googlesource.com/16986
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>
POWER8 has hardware transactional memory, which glibc uses to implement
locks. In some cases, taking a lock begins a transaction, wrapping
arbitrary user code (!) until the lock is released. If the transaction
is aborted, everything rewinds and glibc tries again with some other
implementation.
The kernel will abort the transaction in a variety of cases. Notably, on
a syscall, the transaction aborts and the syscall *does not happen*.
https://www.kernel.org/doc/Documentation/powerpc/transactional_memory.txt
Yet, for some reason, although the relevant change does appear to be in
the kernel, the transaction is being rewound with getrandom happening
anyway. This does not work very well.
Instead, only guard the DRBG access with the lock, not CRYPTO_sysrand.
This lock is only used to protect the DRBG from the destructor that
zeros everything.
Change-Id: Ied8350f1e808a09300651de4200c7b0d07b3a158
Reviewed-on: https://boringssl-review.googlesource.com/16985
Commit-Queue: David Benjamin <davidben@google.com>
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>
BUG=129
Change-Id: Ia8b0639489fea817be4bb24f0457629f0fd6a815
Reviewed-on: https://boringssl-review.googlesource.com/16947
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>
Change-Id: I4e0da85857e820f8151e2fb50d699f14fedee97b
Reviewed-on: https://boringssl-review.googlesource.com/16966
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>
Change-Id: I2e7b9e80419758a5ee4f53915f13334bbf8e0447
Reviewed-on: https://boringssl-review.googlesource.com/16965
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>
Change-Id: Ic22ea72b0134aa7884f1e75433dd5c18247f57ab
Reviewed-on: https://boringssl-review.googlesource.com/16964
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>
When building with OPENSSL_NO_ASM do not try to enable_language(ASM).
Even though the assembly source isn't being built this still causes
CMake to look for the assembler which will fail on platforms where one
is not available.
Change-Id: Ie4893f606143e8f8ca0807114068e577dc1e23e9
Reviewed-on: https://boringssl-review.googlesource.com/16904
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>
Drop some redundant instructions in reduction in ecp_nistz256_sqr_montx.
(Imported from upstream's 8fc063dcc9668589fd95533d25932396d60987f9.)
I believe this is a no-op for us as we do not currently enable the
ADX-based optimizations.
Change-Id: I34a5f5ffb965d59c67f6b9f0ca7937e49ba6e820
Reviewed-on: https://boringssl-review.googlesource.com/16884
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>
BUG=129
Change-Id: I1fef45d662743e7210f93e4dc1bae0c55f75d3fe
Reviewed-on: https://boringssl-review.googlesource.com/16864
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>
Change-Id: Ie88363c4f02016ee743b37a79e76432823b948a0
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/16844
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 order to use AES-GCM-SIV in the open-source QUIC boxer, it needs to
be moved out from OPENSSL_SMALL. (Hopefully the linker can still discard
it in the vast majority of cases.)
Additionally, the input to the key schedule function comes from outside
and may not be aligned, thus we need to use unaligned instructions to
read it.
Change-Id: I02c261fe0663d13a96c428174943c7e5ac8415a7
Reviewed-on: https://boringssl-review.googlesource.com/16824
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>
Without this, trying to trigger the CRNGT on a system with RDRAND won't
work.
Change-Id: I0658a1f045620a2800df36277f67305bc0efff8b
Reviewed-on: https://boringssl-review.googlesource.com/16766
Reviewed-by: Adam Langley <agl@google.com>
We want to clarify that this isn't the PWCT that FIPS generally means,
but rather the power-on self-test. Since ECDSA is non-deterministic, we
have to implement that power-on self-test as a PWCT, but we have a
different flag to break that actual PWCT.
Change-Id: I3e27c6a6b0483a6c04e764d6af8a4a863e0b8b77
Reviewed-on: https://boringssl-review.googlesource.com/16765
Reviewed-by: Adam Langley <agl@google.com>
FIPS requires that the CTR-DRBG state be zeroed on process exit, however
destructors for thread-local data aren't called when the process exits.
This change maintains a linked-list of thread-local state which is
walked on exit to zero each thread's PRNG state. Any concurrently
running threads block until the process finishes exiting.
Change-Id: Ie5dc18e1bb2941a569d8b309411cf20c9bdf52ef
Reviewed-on: https://boringssl-review.googlesource.com/16764
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>
Comments in CAVP are semantically important and we need to copy them
from the input to the output.
Change-Id: Ib798c4ad79de924487d0c4a0f8fc16b757e766d8
Reviewed-on: https://boringssl-review.googlesource.com/16725
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>
Most importantly, this version of delocate works for ppc64le. It should
also work for x86-64, but will need significant testing to make sure
that it covers all the cases that the previous delocate.go covered.
It's less stringtastic than the old code, however the parser isn't as
nice as I would have liked. I thought that the reason we put up with
AT&T syntax with Intel is so that assembly syntax could be somewhat
consistent across platforms. At least for ppc64le, that does not appear
to be the case.
Change-Id: Ic7e3c6acc3803d19f2c3ff5620c5e39703d74212
Reviewed-on: https://boringssl-review.googlesource.com/16464
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>
Change-Id: I1a17860245b7726a24576f5e1bddb0645171f28e
Reviewed-on: https://boringssl-review.googlesource.com/16486
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 symbol “rcon” should be local in order to avoid collisions and it's
much easier on delocate if some of the expressions are evalulated in
Perl rather than left in the resulting .S file.
Also fix the perlasm style so the symbols are actually local.
Change-Id: Iddfc661fc3a6504bcc5732abaa1174da89ad805e
Reviewed-on: https://boringssl-review.googlesource.com/16524
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 change replace the cmovq scheme with slightly faster SSE2 code.
The SSE2 code was first introduced in Go's curve25519 implementation.
See: https://go-review.googlesource.com/c/39693/
The implementation is basicly copied from the Go assembly.
Change-Id: I25931a421ba141ce33809875699f048b0941c061
Reviewed-on: https://boringssl-review.googlesource.com/16564
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>
The only place it is used is EC_KEY_{dup,copy} and no one calls that
function on an EC_KEY with ex_data. This aligns with functions like
RSAPublicKey_dup which do not copy ex_data. The logic is also somewhat
subtle in the face of malloc errors (upstream's PR 3323).
In fact, we'd even changed the function pointer signature from upstream,
so BoringSSL-only code is needed to pass this pointer in anyway. (I
haven't switched it to CRYPTO_EX_unused because there are some callers
which pass in an implementation anyway.)
Note, in upstream, the dup hook is also used for SSL_SESSIONs when those
are duplicated (for TLS 1.2 ticket renewal or TLS 1.3 resumption). Our
interpretation is that callers should treat those SSL_SESSIONs
equivalently to newly-established ones. This avoids every consumer
providing a dup hook and simplifies the interface.
(I've gone ahead and removed the TODO(fork). I don't think we'll be able
to change this API. Maybe introduce a new one, but it may not be worth
it? Then again, this API is atrocious... I've never seen anyone use argl
and argp even.)
BUG=21
Change-Id: I6c9e9d5a02347cb229d4c084c1e85125bd741d2b
Reviewed-on: https://boringssl-review.googlesource.com/16344
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>
It shouldn't have been defined for variable-length nonces at all, but so
it goes. EVP_CIPHER rejected this by way of EVP_CTRL_GCM_SET_IVLEN
comparing <= 0, but the EVP_AEAD API did not.
I've done the test in a separate file on the assumption that aead_test
will become GTest shortly, at which point it will be easy to stick extra
tests into the same file as the FileTest ones.
Thanks to Daniel Bleichenbacher and Thanh Bui of Project Wycheproof for
the report.
Change-Id: Ic4616b39a1d7fe74a1f14fb58cccec2ce7c4f2f3
Reviewed-on: https://boringssl-review.googlesource.com/16544
Reviewed-by: Adam Langley <agl@google.com>
This introduces machinery to start embedding the test data files into
the crypto_test binary. Figuring out every CI's test data story is more
trouble than is worth it. The GTest FileTest runner is considerably
different from the old one:
- It returns void and expects failures to use the GTest EXPECT_* and
ASSERT_* macros, rather than ExpectBytesEqual. This is more monkey
work to convert, but ultimately less work to add new tests. I think
it's also valuable for our FileTest and normal test patterns to align
as much as possible. The line number is emitted via SCOPED_TRACE.
- I've intentionally omitted the Error attribute handling, since that
doesn't work very well with the new callback. This means evp_test.cc
will take a little more work to convert, but this is again to keep our
two test patterns aligned.
- The callback takes a std::function rather than a C-style void pointer.
This means we can go nuts with lambdas. It also places the path first
so clang-format doesn't go nuts.
BUG=129
Change-Id: I0d1920a342b00e64043e3ea05f5f5af57bfe77b3
Reviewed-on: https://boringssl-review.googlesource.com/16507
Reviewed-by: Adam Langley <agl@google.com>
In GTest, we'll just burn the files into the binary and not worry about
this. Apparently test files is a one of computer science's great
unsolved problems and everyone has their own special-snowflake way of
doing it. Burning them into the executable is easier.
BUG=129
Change-Id: Ib39759ed4dba6eb9ba97f0282f000739ddf931fe
Reviewed-on: https://boringssl-review.googlesource.com/16506
Reviewed-by: Adam Langley <agl@google.com>
Instead of a script which generates macros, emit static inlines in
individual header (or C files). This solves a few issues with the
original setup:
- The documentation was off. We match the documentation now.
- The stack macros did not check constness; see some of the fixes in
crypto/x509.
- Type errors did not look like usual type errors.
- Any type which participated in STACK_OF had to be made partially
public. This allows stack types to be defined an internal header or
even an individual file.
- One could not pass sk_FOO_free into something which expects a function
pointer.
Thanks to upstream's 411abf2dd37974a5baa54859c1abcd287b3c1181 for the
idea.
Change-Id: Ie5431390ccad761c17596b0e93941b0d7a68f904
Reviewed-on: https://boringssl-review.googlesource.com/16087
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I7bf485a9bfe0d7b7a3dc3081f86278fee87b8c74
Reviewed-on: https://boringssl-review.googlesource.com/16485
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>
Change-Id: Iab7a738a8981de7c56d1585050e78699cb876dab
Reviewed-on: https://boringssl-review.googlesource.com/16467
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 change causes FIPS mode to use RDRAND in preference to the kernel's
entropy pool. This prevents issues where the ioctl that we have to do
when getrandom isn't supported transiently reports that the pool is
“empty” and causes us to block.
Change-Id: Iad50e443d88b168bf0b85fe1e91e153d79ab3703
Reviewed-on: https://boringssl-review.googlesource.com/16466
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>
Rather than comparing against both endpoints, subtract the minimum and
rely on unsigned wraparound to do both comparisons at once. This seems
to be slightly faster.
In addition, constant_time_lt_8 becomes much simpler if it can assume
that |a| and |b| have the same MSB. But we can arrange that by casting
up to |crypto_word_t| (which is otherwise happening anyway).
Change-Id: I82bd676e487eb7bb079ba7286df724c1c380bbb4
Reviewed-on: https://boringssl-review.googlesource.com/16445
Reviewed-by: Adam Langley <agl@google.com>
With the constant-time base64 decode, base64_ascii_to_bin is a bit more
expensive. This check is redundant with the one in base64_decode_quad,
though it does mean syntax error reporting will be slightly deferred by
four bytes.
Change-Id: I71f23ea23feba2ee5b41df79ce09026fb56996d3
Reviewed-on: https://boringssl-review.googlesource.com/16444
Reviewed-by: Adam Langley <agl@google.com>
Saves having it in several places.
Change-Id: I329e1bf4dd4a7f51396e36e2604280fcca32b58c
Reviewed-on: https://boringssl-review.googlesource.com/16026
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>
AES-GCM-SIV specifies that the counter is a 32-bit, unsigned number.
These test vectors are crafted to trigger a wrap-around and ensure that
corner of the spec is implemented correctly.
Change-Id: I911482ca0b6465a7623ee1b74a6cb1d5e54ddbea
Reviewed-on: https://boringssl-review.googlesource.com/16324
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 makes things a little easier for some of our tooling.
Change-Id: Ia7e73daf0a5150b106cf9b03b10cae194cb8fc5a
Reviewed-on: https://boringssl-review.googlesource.com/15104
Reviewed-by: Matt Braithwaite <mab@google.com>
Reviewed-by: Adam Langley <agl@google.com>
43e5a26b53 removed the .file directive
from x86asm.pl. This removes the parameter from asm_init altogether. See
also upstream's e195c8a2562baef0fdcae330556ed60b1e922b0e.
Change-Id: I65761bc962d09f9210661a38ecf6df23eae8743d
Reviewed-on: https://boringssl-review.googlesource.com/16247
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>
We're not using the MASM output, so don't bother maintaining a diff on
it.
Change-Id: I7321e58c8b267be91d58849927139b74cc96eddc
Reviewed-on: https://boringssl-review.googlesource.com/16246
Commit-Queue: David Benjamin <davidben@google.com>
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>
Due to issues with CMake enable_language, we have to delay setting
CMAKE_ASM_FLAGS until after enable_language(ASM) has been called.
We also need to remove the '.file' macro from x86gas.pl to prevent the
filenames from being overridden from those provided by the build
system.
Change-Id: I436f57ec45e4751714af49e1211a0d7810e4e56a
Reviewed-on: https://boringssl-review.googlesource.com/16127
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>
This allows breaking Known Answer Tests for AES-GCM, DES, SHA-1,
SHA-256, SHA-512, RSA signing and DRBG as required by FIPS.
Change-Id: I8e59698a5048656021f296195229a09ca5cd767c
Reviewed-on: https://boringssl-review.googlesource.com/16088
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>
It's about time we got rid of this. As a first step, introduce a flag,
so that some consumers may stage this change in appropriately.
BUG=chromium:534766,chromium:532048
Change-Id: Id53f0bacf5bdbf85dd71d1262d9f3a9ce3c4111f
Reviewed-on: https://boringssl-review.googlesource.com/16104
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 has since been done.
Change-Id: I498f845fa4ba3d1c04a5892831be4b07f31536d4
Reviewed-on: https://boringssl-review.googlesource.com/16124
Commit-Queue: David Benjamin <davidben@google.com>
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>
This is needed when unrandom.c is compiled on its own.
Change-Id: Ia46e06d267c097e5fa0296092a7270a4cd0b2044
Reviewed-on: https://boringssl-review.googlesource.com/16085
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>
This is required by FIPS testing.
Change-Id: Ia399a0bf3d03182499c0565278a3713cebe771e3
Reviewed-on: https://boringssl-review.googlesource.com/16044
Commit-Queue: Adam Langley <agl@google.com>
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>
SHA-512 is faster to calculate on 64-bit systems and that's what we were
using before. (Though, realistically, this doesn't show up at all.)
Change-Id: Id4f386ca0b5645a863b36405eef03bc62d0f29b3
Reviewed-on: https://boringssl-review.googlesource.com/16006
Commit-Queue: David Benjamin <davidben@google.com>
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>
SHA-512 is faster to calculate on 64-bit systems and we're only
targetting 64-bit systems with FIPS.
Change-Id: I5e9b8419ad4ddc72ec682c4193ffb17975d228e5
Reviewed-on: https://boringssl-review.googlesource.com/16025
Commit-Queue: Adam Langley <agl@google.com>
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>
ASAN prevents the integrity test from running, so don't indicate FIPS
mode in that case.
Change-Id: I14c79e733e53ef16f164132bc1fded871ce3f133
Reviewed-on: https://boringssl-review.googlesource.com/16024
Commit-Queue: Adam Langley <agl@google.com>
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>
Perl, multiple versions, for some reason occasionally takes issue with
letter b[?] in ox([0-9a-f]+) regex. As result some constants, such as
0xb1 came out wrong when generating code for MASM. Fixes upstream
GH#3241.
(Imported from upstream's c47aea8af1e28e46e1ad5e2e7468b49fec3f4f29.)
This does not affect of the configurations we generate and is imported
to avoid a diff against upstream.
Change-Id: Iacde0ca5220c3607681fad081fbe72d8d613518f
Reviewed-on: https://boringssl-review.googlesource.com/15985
Reviewed-by: Adam Langley <agl@google.com>
This avoids depending the FIPS module on crypto/bytestring and moves
ECDSA_SIG_{new,free} into the module.
Change-Id: I7b45ef07f1140873a0da300501141b6ae272a5d9
Reviewed-on: https://boringssl-review.googlesource.com/15984
Reviewed-by: Adam Langley <agl@google.com>
This dates to ded93581f1, but we have
since switched to building with nasm, to match upstream's supported
assemblers. Since this doesn't affect anything we generate, remove the
workaround to reduce the diff against upstream.
Change-Id: I549ae97ad6d6f28836f6c9d54dcf51c518de7521
Reviewed-on: https://boringssl-review.googlesource.com/15986
Reviewed-by: Adam Langley <agl@google.com>
EVP_AEAD_CTX is otherwise a pain to use from C++ when you need to keep
it around.
Change-Id: I1dff926b33a3246680be21b89b69dfb336d25cd5
Reviewed-on: https://boringssl-review.googlesource.com/15965
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>
Change-Id: Icf1d6ec9d3fb33a124a9f61c75d29248a2582680
Reviewed-on: https://boringssl-review.googlesource.com/15964
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>
Nearly all of the assembly code was written by Shay and is submitted
under the CLA.
Change-Id: Ia70952d4ba2713ccc5e96a0952c22e5400c90f3a
Reviewed-on: https://boringssl-review.googlesource.com/15649
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>
The names in the P-224 code collided with the P-256 code and thus many
of the functions and constants in the P-224 code have been prefixed.
Change-Id: I6bcd304640c539d0483d129d5eaf1702894929a8
Reviewed-on: https://boringssl-review.googlesource.com/15847
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>
FIPS 186-4 wants d = e^-1 (mod lcm(p-1, q-1)), not (p-1)*(q-1).
Note this means the size of d might reveal information about p-1 and
q-1. However, we do operations with Chinese Remainder Theorem, so we
only use d (mod p-1) and d (mod q-1) as exponents. Using a minimal
totient does not affect those two values.
This removes RSA_recover_crt_params. Using a minimal d breaks (or rather
reveals an existing bug in) the function.
While I'm here, rename those ridiculous variable names.
Change-Id: Iaf623271d49cd664ba0eca24aa25a393f5666fac
Reviewed-on: https://boringssl-review.googlesource.com/15944
Commit-Queue: David Benjamin <davidben@google.com>
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>
Nothing is using them. For encrypt, there's generally no need to swap
out public key operations. keygen seems especially pointless as one
could just as easily call the other function directly.
The one behavior change is RSA_encrypt now gracefully detects if called
on an empty RSA, to match the other un-RSA_METHOD-ed functions which had
similar treatments. (Conscrypt was filling in the encrypt function
purely to provide a non-crashing no-op function. They leave the public
bits blank and pass their custom keys through sufficiently many layers
of Java crypto goo that it's not obvious whether this is reachable.)
We still can't take the function pointers out, but once
96bbe03dfd
trickles back into everything, we can finally prune RSA_METHOD.
Bump BORINGSSL_API_VERSION as a convenience so I can land the
corresponding removal in Conscrypt immediately.
Change-Id: Ia2ef4780a5dfcb869b224e1ff632daab8d378b2e
Reviewed-on: https://boringssl-review.googlesource.com/15864
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>
(Imported from upstream's 54538204d870b97c751d13efeefa876bd792a44b.)
Change-Id: If9967b67a74ab7dea175e97ea8bda195c3cd0478
Reviewed-on: https://boringssl-review.googlesource.com/15835
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>
ASN1_GENERALIZEDTIME and ASN1_UTCTIME may be specified using offsets,
even though that's not supported within certificates. [davidben: This
commit message seems off as crypto/x509 does not reject them. It merely
has a comment telling you that it's doing it wrong.]
To convert the offset time back to GMT, the offsets are supposed to be
subtracted, not added. e.g. 1759-0500 == 2359+0100 == 2259Z.
(Imported from upstream's d2335f30970ed3edc1c7c11700ab7f34396cf086.)
Change-Id: Id0d4c5b650e77db3b04b15e66b069807f6f31266
Reviewed-on: https://boringssl-review.googlesource.com/15834
Commit-Queue: David Benjamin <davidben@google.com>
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>
I forgot to scrub these files when they moved and their macros are
currently leaking into other files. This isn't a problem, but does
prevent ec/ code from being moved into the module at the moment.
Change-Id: I5433fb043e90a03ae3dc5c38cb3a69563aada007
Reviewed-on: https://boringssl-review.googlesource.com/15845
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>
Another synthesized function which may be referenced directly.
Change-Id: Ic75fe66ce7244246a2d4a707b6a5fee24cac6941
Reviewed-on: https://boringssl-review.googlesource.com/15831
Commit-Queue: David Benjamin <davidben@google.com>
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>
This will let us keep CBS/CBB out of the module. It also makes the PWCT
actually use a hard-coded public key since kEC was using the
private-key-only serialization.
Change-Id: I3769fa26fc789c4797a56534df73f810cf5441c4
Reviewed-on: https://boringssl-review.googlesource.com/15830
Reviewed-by: Adam Langley <agl@google.com>
This will let us keep CBS/CBB out of the module.
Change-Id: I780de0fa2c102cf27eee2cc242ee23740fbc16ce
Reviewed-on: https://boringssl-review.googlesource.com/15829
Commit-Queue: David Benjamin <davidben@google.com>
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>
RSA_verify_raw is the same as RSA_public_decrypt and fits the calling
convention better. This also avoids the extra copy.
Change-Id: Ib7e3152af26872440290a289f178c9a1d9bc673f
Reviewed-on: https://boringssl-review.googlesource.com/15826
Reviewed-by: Adam Langley <agl@google.com>
This allows us to implement RSA-PSS in the FIPS module without pulling
in EVP_PKEY. It also allows people to use RSA-PSS on an RSA*.
Empirically folks seem to use the low-level padding functions a lot,
which is unfortunate.
This allows us to remove a now redundant length check in p_rsa.c.
Change-Id: I5270e01c6999d462d378865db2b858103c335485
Reviewed-on: https://boringssl-review.googlesource.com/15825
Reviewed-by: Adam Langley <agl@google.com>
We check the length for MD5+SHA1 but not the normal cases. Instead,
EVP_PKEY_sign externally checks the length (largely because the silly
RSA-PSS padding function forces it). We especially should be checking
the length for these because otherwise the prefix built into the ASN.1
prefix is wrong.
The primary motivation is to avoid putting EVP_PKEY inside the FIPS
module. This means all logic for supported algorithms should live in
crypto/rsa.
This requires fixing up the verify_recover logic and some tests,
including bcm.c's KAT bits.
(evp_tests.txt is now this odd mixture of EVP-level and RSA-level error
codes. A follow-up change will add new APIs for RSA-PSS which will allow
p_rsa.c to be trimmed down and make things consistent.)
Change-Id: I29158e9695b28e8632b06b449234a5dded35c3e7
Reviewed-on: https://boringssl-review.googlesource.com/15824
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I167b7045c537d95294d387936f3d7bad530e1c6f
Reviewed-on: https://boringssl-review.googlesource.com/15844
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>
Change-Id: I64533d2b4a6b075fa3ccea1abfd0ec5106673453
Reviewed-on: https://boringssl-review.googlesource.com/15704
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 works better with util/generate_build_files.py.
Change-Id: Icb55dc74e0a004aca3e09978640455b66f0473ff
Reviewed-on: https://boringssl-review.googlesource.com/15648
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>
This is a remnant of the ECDSA code returning a tri-state -1, 0, 1.
Change-Id: I8bd1fcd94e07dbffc650f414ebc19f30236378bd
Reviewed-on: https://boringssl-review.googlesource.com/15667
Reviewed-by: Adam Langley <agl@google.com>
This CL adds utility code to process NIST CAVP test vectors using the
existing FileTest code.
Also add binaries for processing AESAVS (AES) and GCMVS (AES-GCM) vector
files.
Change-Id: I8e5ebf751d7d4b5504bbb52f3e087b0065babbe0
Reviewed-on: https://boringssl-review.googlesource.com/15484
Reviewed-by: Adam Langley <agl@google.com>
When code wants to push a pointer from the GOT onto the stack, we don't
have any registers to play with. We do, however, know that the stack is
viable and thankfully Intel has an “xchg” instruction that avoids the
need for an intermediate register.
Change-Id: Iba7e4f0f4c9b43b3d994cf6cfc92837b312c7728
Reviewed-on: https://boringssl-review.googlesource.com/15625
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 doesn't actually measure what we need(*) and, because of that, it's
way more noisy than expected.
(*) We want to know whether the pool has been initialised, not whether
it currently thinks it has a lot of bits, but we can't get what we want
without getrandom() support in the kernel.
Change-Id: I20accb99a592739c786a25c1656aeea050ae81a3
Reviewed-on: https://boringssl-review.googlesource.com/15624
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_ia32cap_addr avoids any relocations within the module, at the
cost of a runtime TEXTREL, which causes problems in some cases.
(Notably, if someone links us into a binary which uses the GCC "ifunc"
attribute, the loader crashes.)
We add a OPENSSL_ia32cap_addr_delta symbol (which is reachable
relocation-free from the module) stores the difference between
OPENSSL_ia32cap_P and its own address. Next, reference
OPENSSL_ia32cap_P in code as usual, but always doing LEAQ (or the
equivalent GOTPCREL MOVQ) into a register first. This pattern we can
then transform into a LEAQ and ADDQ on OPENSSL_ia32cap_addr_delta.
ADDQ modifies the FLAGS register, so this is only a safe transformation
if we safe and restore flags first. That, in turn, is only a safe
transformation if code always uses %rsp as a stack pointer (specifically
everything below the stack must be fair game for scribbling over). Linux
delivers signals on %rsp, so this should already be an ABI requirement.
Further, we must clear the red zone (using LEAQ to avoid touching FLAGS)
which signal handlers may not scribble over.
This also fixes the GOTTPOFF logic to clear the red zone.
Change-Id: I4ca6133ab936d5a13d5c8ef265a12ab6bd0073c9
Reviewed-on: https://boringssl-review.googlesource.com/15545
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>
Past the first word, the remaining arguments are usually separated by
commas. This avoids some of the awkward fixing up needed to extract
target registers, etc.
Change-Id: Id99b99e5160abf80e60afea96f2b46b53b55c9c5
Reviewed-on: https://boringssl-review.googlesource.com/15544
Reviewed-by: Adam Langley <agl@google.com>
OPENSSL_ia32cap_addr avoids any relocations within the module, at the
cost of a runtime TEXTREL, which causes problems in some cases.
(Notably, if someone links us into a binary which uses the GCC "ifunc"
attribute, the loader crashes.)
Fix C references of OPENSSL_ia32cap_addr with a function. This is
analogous to the BSS getters. A follow-up commit will fix perlasm with a
different scheme which avoids calling into a function (clobbering
registers and complicating unwind directives.)
Change-Id: I09d6cda4cec35b693e16b5387611167da8c7a6de
Reviewed-on: https://boringssl-review.googlesource.com/15525
Reviewed-by: Adam Langley <agl@google.com>
The test takes a little long to run. I've chopped it to primes up to
20,000. This ensures we still test some values out of range of the table
in crypto/bn/prime.c.
Also remove false comment in crypto/bn/prime.c.
Change-Id: I910015af9570b2f9f1c6c82dc61a0dbdfd24840b
Reviewed-on: https://boringssl-review.googlesource.com/15604
Reviewed-by: Adam Langley <agl@google.com>
We only ever compute it for odd (actually, prime) modulus as part of
BN_mod_sqrt.
If we cared, we could probably drop this from most binaries. This is
used to when modular square root needs Tonelli-Shanks. Modular square
root is only used for compressed coordinates. Of our supported curves
(I'm handwaiving away EC_GROUP_new_curve_GFp here[*]), only P-224 needs
the full Tonelli-Shanks algorithm (p is 1 mod 8). That computes the
Legendre symbol a bunch to find a non-square mod p. But p is known at
compile-time, so we can just hard-code a sample non-square.
Sadly, BN_mod_sqrt has some callers outside of crypto/ec, so there's
also that. Anyway, it's also not that large of a function.
[*] Glancing through SEC 2 and Brainpool, secp224r1 is the only curve
listed in either document whose prime is not either 3 mod 4 or 5 mod 8.
Even 5 mod 8 is rare: only secp224k1. It's unlikely anyone would notice
if we broke annoying primes. Though OpenSSL does support "WTLS" curves
which has an additional 1 mod 8 case.
Change-Id: If36aa78c0d41253ec024f2d90692949515356cd1
Reviewed-on: https://boringssl-review.googlesource.com/15425
Reviewed-by: Adam Langley <agl@google.com>
Not requiring the list of assembly sources to be comma-separated is
helpful to environments where the list would more naturally be
treated as a list.
Change-Id: I43b18cdbeed1dc7ad217ff61557ac55860f40733
Reviewed-on: https://boringssl-review.googlesource.com/15585
Reviewed-by: Matt Braithwaite <mab@google.com>
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>
Chromium's test infrastruction does not actually support GTest. It
requires a custom test runner in //base. Split gtest_main.cc up into a
gtest_main.h which defines a support function we maintain and a default
runner. Chromium's build will swap that file out for a custom one.
BUG=129
Change-Id: I3e39fe3a931b3051a61d5f8eef514ca6a504f11c
Reviewed-on: https://boringssl-review.googlesource.com/15564
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>
Looks like this file was forgotten when the old suites were removed.
Change-Id: Ied8d82e23ae5db0257add3c18eee46ee1a366637
Reviewed-on: https://boringssl-review.googlesource.com/15444
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>
Change-Id: I4e34dabe302f7dacdf04a89052ad9fe9254a1b81
Reviewed-on: https://boringssl-review.googlesource.com/15404
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>
Whether UCHAR_MAX expands to a signed or unsigned number appears to be a
matter of some debate. Or the Android headers are wrong. Just add a cast
and not think about it too hard.
Change-Id: I84e928bdfe459a9129cde276c82b60a318533552
Reviewed-on: https://boringssl-review.googlesource.com/15385
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This can just be a bog-standard loop. The compiler should be plenty
smart enough to transform it into whatever it likes.
Change-Id: I7b782dd2a11902f6e5c9902d9624be26eee5c959
Reviewed-on: https://boringssl-review.googlesource.com/15366
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>
CRYPTO_gcm128_init is currently assuming that it gets passed in
aesni_encrypt whenever it selects the AVX implementation. This is true,
but we can easily avoid this assumption by adding an extra boolean
input.
Change-Id: Ie7888323f0c93ff9df8f1cf3ba784fb35bb07076
Reviewed-on: https://boringssl-review.googlesource.com/15370
Reviewed-by: Adam Langley <agl@google.com>
These modes do internal random IV generation and are unsuitable for
non-testing purposes.
Change-Id: I14b98af8f6cf43b4fc835a2b04a9b0425b7651b7
Reviewed-on: https://boringssl-review.googlesource.com/15244
Reviewed-by: Adam Langley <agl@google.com>
This also fixes the comments regarding BN_prime_checks to match the
security level guarantees provided by BN_prime_checks.
Change-Id: I8032e88680bf51e8876e134b4253ed26c2072617
Reviewed-on: https://boringssl-review.googlesource.com/15304
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
C leaves some details as implementation-defined. We only support
implementations which define things consistent with the obvious
representation on current machines.
Change-Id: I255a2e54a86c8a7d987dea51ea7168ad66ad9ddd
Reviewed-on: https://boringssl-review.googlesource.com/15305
Reviewed-by: Adam Langley <agl@google.com>
CMake loves making archives, but that's not universal.
Change-Id: I5356b4701982748a46817e0094ad838605dcada6
Reviewed-on: https://boringssl-review.googlesource.com/15144
Reviewed-by: Adam Langley <agl@google.com>
Using |size_t| was correct, except for NaCl, which is a 64-bit build
with 32-bit pointers. In that configuration, |size_t| is smaller than
the native word size.
This change adds |crypto_word_t|, an unsigned type with native size and
switches constant-time functions to using it.
Change-Id: Ib275127063d5edbb7c55d413132711b7c74206b0
Reviewed-on: https://boringssl-review.googlesource.com/15325
Reviewed-by: Adam Langley <agl@google.com>
In some modes the compiler will emit a section for BSS symbols and
construct the values with labels, alignment and data instructions. This
change parses these sections and emits the local versions of each symbol
needed to make this work.
Change-Id: I8d43ffe4b5b734950aa4287a3dd7c0d2f191f2e4
Reviewed-on: https://boringssl-review.googlesource.com/15206
Reviewed-by: Adam Langley <agl@google.com>
We might want to back off on this in the future so that we don't upset
future compiler work but, for now, it's useful to know when we hit
something that we don't understand.
Change-Id: I763830b0ddcf5da20061fad673265d4a5855479c
Reviewed-on: https://boringssl-review.googlesource.com/15205
Reviewed-by: Adam Langley <agl@google.com>
In order to better handle BSS sections, rather than having a single loop
over the lines and state flags, pull lines as needed. This means that
subfunctions can process sections of the input.
Also, stop bothering to move the init_array to the end, it's already put
into its own section.
Change-Id: I0e62930c65d29baecb39ba0d8bbc21f2da3bde56
Reviewed-on: https://boringssl-review.googlesource.com/15204
Reviewed-by: Adam Langley <agl@google.com>
Support for platforms that we don't support FIPS on doesn't need to be
in the module. Also, functions for dealing with whether fork-unsafe
buffering is enabled are left out because they aren't implementing any
cryptography and they use global r/w state, making their inclusion
painful.
Change-Id: I71a0123db6f5449e9dfc7ec7dea0944428e661aa
Reviewed-on: https://boringssl-review.googlesource.com/15084
Reviewed-by: Adam Langley <agl@google.com>
With some optimisation settings, Clang was loading
BORINGSSL_bcm_text_hash with AVX2 instructions, which weren't getting
translated correctly. This seems to work and is less fragile.
The compiler just emits an leaq here. This is because it knows the
symbol is hidden (in the shared library sense), so it needn't go through
GOTPCREL. The assembler would have added a relocation, were the symbol
left undefined, but since we define the symbol later on, it all works
out without a relocation.
Were the symbol not hidden, the compiler would have emitted a movq by
way of GOTPCREL, but we can now translate those away anyway.
Change-Id: I442a22f4f8afaadaacbab7044f946a963ebfc46c
Reviewed-on: https://boringssl-review.googlesource.com/15384
Reviewed-by: Adam Langley <agl@google.com>
We BN_cmp with 1 at the top, so the absolute value code never runs.
This simplifies the BN_CTX business considerably. Also add a test for
negative prime numbers.
Change-Id: I500a56bc285c2f75576947cfb518e75c9e6861ce
Reviewed-on: https://boringssl-review.googlesource.com/15367
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>
Thanks to Rob Sloan for clearing out Android's uses of these functions.
I forgot we can hide these now.
BUG=97
Change-Id: I9bc7bf5ca379d3345743151e606f3e911367b4ed
Reviewed-on: https://boringssl-review.googlesource.com/15364
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Robert Sloan <varomodt@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: Ibd6b9b12b3b622f67f69da5c2add8b1b040882f1
Reviewed-on: https://boringssl-review.googlesource.com/15344
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 changes to delocate.go are needed because modes/ does things like
return the address of a module function. Both of these need to be
changed from referencing the GOT to using local symbols.
Rather than testing whether |ghash| is |gcm_ghash_avx|, we can just keep
that information in a flag.
The test for |aesni_ctr32_encrypt_blocks| is more problematic, but I
believe that it's superfluous and can be dropped: if you passed in a
stream function that was semantically different from
|aesni_ctr32_encrypt_blocks| you would already have a bug because
|CRYPTO_gcm128_[en|de]crypt_ctr32| will handle a block at the end
themselves, and assume a big-endian, 32-bit counter anyway.
Change-Id: I68a84ebdab6c6006e11e9467e3362d7585461385
Reviewed-on: https://boringssl-review.googlesource.com/15064
Reviewed-by: Adam Langley <agl@google.com>
SP 800-89 5.3.3 references FIPS 186 for the bounds on e. I /think/
that's section B.3.1 which says:
(b) The exponent e shall be an odd positive integer such that 2¹⁶ < e < 2²⁵⁶.
But that means that e has to be at least 17 bits. The check for
BN_is_odd ensures that 2¹⁶ itself is rejected.
Change-Id: Ib39f9d43032cbfe33317651c7b6eceb41b123291
Reviewed-on: https://boringssl-review.googlesource.com/15324
Reviewed-by: Steven Valdez <svaldez@google.com>
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>
Firstly, FIPS 186-4 C.3.2 is broken for w=3. In step 4.1 it generates a
random, 2-bit number but in step 4.2 it rejects all four possible values
and loops forever.
Secondly, BN_is_prime_fasttext_ex is broken when trial division is
requested and the prime is small. It finds that the prime is a multiple
of a known prime and rejects it. We inherited this from OpenSSL.
Thirdly, we were missing a BN_CTX_start/end in
BN_enhanced_miller_rabin_primality_test, which didn't matter but could
have mattered in the future.
Change-Id: Ie988e37b14bb22acb005fc0652860be6bbd2a55f
Reviewed-on: https://boringssl-review.googlesource.com/15264
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>
If all the inputs are given as assembly files then we can skip rewriting
symbols for the first file. If this file is bcm.s (i.e. the large
compiler output), this can save a few seconds of build time.
Change-Id: I4e4ea114acb86cd93e831b23b58f8c3401bc711c
Reviewed-on: https://boringssl-review.googlesource.com/15149
Reviewed-by: Adam Langley <agl@google.com>
delocate.go was adding redirector functions for the “_bss_get”
functions. (And they were going via the PLT too.)
Change-Id: I86bc9f0516a128a769068182cc280499f89b6c29
Reviewed-on: https://boringssl-review.googlesource.com/15148
Reviewed-by: Adam Langley <agl@google.com>
These relocations can be emitted for thread-local data. BoringSSL itself
doesn't include any thread-local variables that need linker support, but
ASAN and MSAN may inject these references in order to handle their own
bookkeeping.
Change-Id: I0c6e61d244be84d6bee5ccbf7c4ff4ea0f0b90fd
Reviewed-on: https://boringssl-review.googlesource.com/15147
Reviewed-by: Adam Langley <agl@google.com>
This is a version of PKCS7_get_certificates but does not require
crypto/x509.
BUG=54
Change-Id: I20152a8d1f3ed866d47e41fe576ea9f442490224
Reviewed-on: https://boringssl-review.googlesource.com/15129
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>
A follow-up change will add a CRYPTO_BUFFER variant. This makes the
naming match the header and doesn't require including x509.h. (Though
like ssl.h and pkcs8.h, some of the functions are implemented with code
that depends on crypto/x509.)
Change-Id: I5a7de209f4f775fe0027893f711326d89699ca1f
Reviewed-on: https://boringssl-review.googlesource.com/15128
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>
BUG=76
Change-Id: I8b754ba17b3e0beee425929e4b53785b2e95f0ae
Reviewed-on: https://boringssl-review.googlesource.com/15164
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>
I had a brain-fart and had in mind that strings.Index(x[i:], _) would
return a value relative to the beginning of |x|, which is impossible.
Change-Id: I905ea1fa3469ea13f2e3b782c4baf2431b615a2f
Reviewed-on: https://boringssl-review.googlesource.com/15146
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 also fixes TestGetUint to actually test CBS_get_last_u8's behavior.
Right now it can't distinguish CBS_get_last_u8 and CBS_get_u8.
BUG=129
Change-Id: Ie431bb1a828f1c6877938ba7e75c82305b54cf13
Reviewed-on: https://boringssl-review.googlesource.com/15007
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>
BUG=129
Change-Id: If91d97ea653177d55d5c703f091366ddce24da60
Reviewed-on: https://boringssl-review.googlesource.com/15006
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 is not actually sensible, but it seemed really funny. PEM files
sometimes carry private keys so, in principle, we'd probably prefer not
to leak the contents when we encode or decode them?
Change-Id: I7b056612bd7f22c28853bc89f56aee1f5103b8fb
Reviewed-on: https://boringssl-review.googlesource.com/15047
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>
FIPS 186-4 prescribes a particular ECDSA nonce selection algorithm,
implemented by BN_range_range_ex. Recast our nonce hardening mechanism
as additional data to be passed into the RBG during that algorithm.
Change-Id: Ic16a10cd58fd7deb7461f0c109a698ea80faff00
Reviewed-on: https://boringssl-review.googlesource.com/15046
Reviewed-by: Adam Langley <agl@google.com>
Rather than comparing against both min and max, FIPS prefers comparing
with max - min and adding min. It also does not believe in using
3*range. Align with it, though our old algorithm trivially produces the
same probability distribution on values.
Change-Id: I447cc3608b92ba93706489d702b8d6a68047f491
Reviewed-on: https://boringssl-review.googlesource.com/15045
Reviewed-by: Adam Langley <agl@google.com>
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>
FIPS requires that the output of the entropy source be checked to ensure
that no two n-bit blocks are equal.
Change-Id: Ia086ca5c888770e0fd71ee052278f77b544b9983
Reviewed-on: https://boringssl-review.googlesource.com/14926
Reviewed-by: Adam Langley <agl@google.com>
We already do this in the case that getrandom is supported. This change
adds a polling loop for the case where we are using /dev/urandom.
This makes FIPS imply Linux, which I think is fine for the time being.
Change-Id: I9bf5c0f51a908621655cbcc47fc86b0366168b97
Reviewed-on: https://boringssl-review.googlesource.com/14925
Reviewed-by: Adam Langley <agl@google.com>
Fork-unsafe buffering was a mode that could be enabled by applications
that were sure that they didn't need to worry about state duplication.
It saved reads to urandom.
Since everything is now going through the CTR-DRBG, we can get the same
effect by simply not reading additional data from urandom in this case.
This change drops the buffering from urandom.c and, instead, implements
fork-unsafe buffering as a mode that skips reading additional data from
urandom, which only happened when RDRAND wasn't available anyway.
Since we expect the power-on self-tests to call into the PRNG, this
change also makes the flag capable of changing at any point by using a
mutex rather than a once. This is split into a separate file so that it
doesn't have to go into the FIPS module—since it uses r/w data that
would be a pain.
Change-Id: I5fd0ead0422e770e35758f080bb1cffa70d0c8da
Reviewed-on: https://boringssl-review.googlesource.com/14924
Reviewed-by: Adam Langley <agl@google.com>
This isn't actually used yet, but implements CTR-DRBG from SP 800-90Ar1.
Specifically, it always uses AES-256 and no derivation function.
Change-Id: Ie82b829590226addd7c165eac410a5d584858bfd
Reviewed-on: https://boringssl-review.googlesource.com/14891
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I73213b5d9f3ac67bab70e3d9a36a4b67c558f3f5
Reviewed-on: https://boringssl-review.googlesource.com/15044
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>
Otherwise the order changes each time, which will make the build
egregiously non-deterministic.
Change-Id: Idd501ecd118c61a27566eafc61157715e48758bc
Reviewed-on: https://boringssl-review.googlesource.com/15026
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>
References to global symbols generate relocations, which breaks the
integrity check.
Change-Id: If6fa06d5d924294ab496c32e7f082a1ae60fdb24
Reviewed-on: https://boringssl-review.googlesource.com/15025
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>
Some assembly code references “OPENSSL_ia32cap_P+4(%rip)” etc, which
slipped by the previous check.
Change-Id: I22c3fbf9883aea695e8584857bf9c0e3113f9a77
Reviewed-on: https://boringssl-review.googlesource.com/15024
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>
Since only the consumers knows whether an EC key will be used for
ECDSA or ECDHE, it is part of the FIPS policy for the consumer to
check the validity of the generated key before signing with it.
Change-Id: Ie250f655c8fcb6a59cc7210def1e87eb958e9349
Reviewed-on: https://boringssl-review.googlesource.com/14745
Reviewed-by: Adam Langley <agl@google.com>
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>
This moves the kinv computation next to k generation and adds a check for group
size as per 186-4 B.5.2.
Change-Id: I8744080d3961bc9e29054985280fc835e3f1e25c
Reviewed-on: https://boringssl-review.googlesource.com/14944
Reviewed-by: Adam Langley <agl@google.com>
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>
It's not obvious how to make ASAN happy with the integrity test but this
will let us test FIPS-only code with ASAN at least.
Change-Id: Iac983787e04cb86a158e4416c410d9b2d1e5e03f
Reviewed-on: https://boringssl-review.googlesource.com/14965
Reviewed-by: Adam Langley <agl@google.com>
FIPS is not compatible with multiprime RSA. Any multiprime RSA private
keys will fail to parse after this change.
Change-Id: I8d969d668bf0be4f66c66a30e56f0e7f6795f3e9
Reviewed-on: https://boringssl-review.googlesource.com/14984
Reviewed-by: Adam Langley <agl@google.com>
FIPS prescribes a slightly different key generation algorithm than we
use. Specifically:
- Rather than using BN_RAND_TOP_TWO (so using 1.5 as an upper bound for
sqrt(2)), it prescribes using sqrt(2) itself. To avoid unnecessary
squaring, we do a comparison against a hard-coded approximation for
sqrt(2) good enough for the largest FIPS key size. I went ahead and
made it constant-time since it was easy, but all this is far from
constant-time.
- FIPS requires a check that |p-q| is sufficiently large.
- FIPS requires a check that d is sufficiently large.
- BN_generate_prime_ex adds some delta to clear a table of prime
numbers. FIPS does not specify any of that, so implement a separate
routine here.
The primality test itself will be aligned in a follow-up. For now, it is
left unchanged, except that trial division is turned back on. That makes
things faster and is analogous the original algorithm's delta-munging
logic.
Change-Id: If32f0635bfb67a8c4740dedd7781d00647bbf60b
Reviewed-on: https://boringssl-review.googlesource.com/14948
Reviewed-by: Adam Langley <agl@google.com>
Previously, inject-hash would run the FIPS module in order to trigger a
failure and then extract the calculated hash value from the output. This
makes cross-compiling difficult because the build process needs to run a
binary for the target platform.
This change drops this step. Instead, inject-hash.go parses the object
file itself and calculates the hash without needing to run the module.
Change-Id: I2593daa03094b0a17b498c2e8be6915370669596
Reviewed-on: https://boringssl-review.googlesource.com/14964
Commit-Queue: Adam Langley <agl@google.com>
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>
The FIPS RSA generation algorithm is unkind to keys with funny bit
sizes. Odd numbers of bits are especially inconvenient, but the sqrt(2)
bound is much simpler if the key size is a multiple of 128 (thus giving
prime sizes a multiple of 64, so the sqrt(2) bound is easier to work
with).
Also impose a minimum RSA key size. 255-bit RSA is far too small as it
is and gives small enough primes that the p-q FIPS bound (2^(n/2-100))
starts risking underflow.
Change-Id: I4583c90b67385e53641ccee9b29044e79e94c920
Reviewed-on: https://boringssl-review.googlesource.com/14947
Reviewed-by: Adam Langley <agl@google.com>
This is just some idle cleanup. The padding functions already must
handle size checks. Swap out the error code in the low-level portions to
keep that unchanged.
Also remove an old TODO(fork) about constant-time-ness. Signature
verification padding checks don't need to be constant time, and
decryption ones should be resolved now.
Change-Id: I20e7affdb7f2dce167a304afe707bfd537dd412a
Reviewed-on: https://boringssl-review.googlesource.com/14946
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I9f7f1dd609c38d1f4be536daff94a4ba002582d0
Reviewed-on: https://boringssl-review.googlesource.com/14888
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>
