Commit Graph

5120 Commits

Author SHA1 Message Date
David Benjamin
cba958f406 Make RSA_check_key constant-time and more meaningful.
Rather than recompute values the same as in key generation, where
possible, we check differently. In particular, most RSA values are
modular inverses of some value. Check each of them by multiplying and
using our naive constant-time division function.

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

Bug: 238
Change-Id: Iaca19f12c045457013def844a17bf502ed09136e
Reviewed-on: https://boringssl-review.googlesource.com/26373
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-03-30 19:54:00 +00:00
David Benjamin
c4e4757b63 Make RSA key generation constant-time.
This leaves RSA_check_key, which will be fixed in subsequent commits.

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

Bug: 238
Change-Id: I325f23fcc59302e68570908e5427b65471b799f6
Reviewed-on: https://boringssl-review.googlesource.com/26371
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-03-30 19:53:52 +00:00
David Benjamin
a44dae7fd3 Add a constant-time generic modular inverse function.
This uses the full binary GCD algorithm, where all four of A, B, C, and
D must be retained. (BN_mod_inverse_odd implements the odd number
version which only needs A and C.) It is patterned after the version
in the Handbook of Applied Cryptography, but tweaked so the coefficients
are non-negative and bounded.

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

Bug: 238
Change-Id: I6dc13524ea7c8ac1072592857880ddf141d87526
Reviewed-on: https://boringssl-review.googlesource.com/26370
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-03-30 19:53:44 +00:00
David Benjamin
1044553d6d Add new GCD and related primitives.
RSA key generation requires computing a GCD (p-1 and q-1 are relatively
prime with e) and an LCM (the Carmichael totient). I haven't made BN_gcd
itself constant-time here to save having to implement
bn_lshift_secret_shift, since the two necessary operations can be served
by bn_rshift_secret_shift, already added for Rabin-Miller. However, the
guts of BN_gcd are replaced. Otherwise, the new functions are only
connected to tests for now, they'll be used in subsequent CLs.

To support LCM, there is also now a constant-time division function.
This does not replace BN_div because bn_div_consttime is some 40x slower
than BN_div. That penalty is fine for RSA keygen because that operation
is not bottlenecked on division, so we prefer simplicity over
performance.

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

Bug: 238
Change-Id: Idbfbfa6e7f5a3b8782ce227fa130417b3702cf97
Reviewed-on: https://boringssl-review.googlesource.com/26369
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-03-30 19:53:36 +00:00
David Benjamin
23af438ccd Compute p - q in constant time.
Expose the constant-time abs_sub functions from the fixed Karatsuba code
in BIGNUM form for RSA to call into. RSA key generation involves
checking if |p - q| is above some lower bound.

BN_sub internally branches on which of p or q is bigger. For any given
iteration, this is not secret---one of p or q is necessarily the larger,
and whether we happened to pick the larger or smaller first is
irrelevant. Accordingly, there is no need to perform the p/q swap at the
end in constant-time.

However, this stage of the algorithm picks p first, sticks with it, and
then computes |p - q| for various q candidates. The distribution of
comparisons leaks information about p. The leak is unlikely to be
problematic, but plug it anyway.

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

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

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

Bug: 238
Change-Id: Ic56f92f0dcf04da1f542290a7e8cdab8036699ed
Reviewed-on: https://boringssl-review.googlesource.com/26367
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-03-30 19:53:18 +00:00
David Benjamin
97ac45e2f7 Change the order of GCD and trial division.
RSA key generation currently does the GCD check before the primality
test, in hopes of discarding things invalid by other means before
running the expensive primality check.

However, GCD is about to get a bit more expensive to clear the timing
leak, and the trial division part of primality testing is quite fast.
Thus, split that portion out via a new bn_is_obviously_composite and
call it before GCD.

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

Bug: 238
Change-Id: I3999771fb73cca16797cab9332d14c4ebeb02046
Reviewed-on: https://boringssl-review.googlesource.com/26366
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-03-30 19:53:06 +00:00
David Benjamin
40729e374d Revert "Update SDE to 8.16.0."
This reverts commit 21ef155063. Doesn't
look like I succeeded in uploading that. Will sort that out later.

Change-Id: Ic5395abe46b2b99aaffd254afcd97157518c8ba8
Reviewed-on: https://boringssl-review.googlesource.com/26886
Reviewed-by: David Benjamin <davidben@google.com>
2018-03-30 17:59:40 +00:00
David Benjamin
21ef155063 Update SDE to 8.16.0.
Change-Id: If6891b9338352d4f1dc9f902d4e32ca358764675
Reviewed-on: https://boringssl-review.googlesource.com/26885
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>
2018-03-30 17:52:56 +00:00
David Benjamin
365e48c104 Update tools.
Change-Id: Ibbba2e2fa81b5f8b3a25ebadc50297f50dcb610e
Reviewed-on: https://boringssl-review.googlesource.com/26884
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>
2018-03-30 17:51:56 +00:00
Adam Langley
1902d818ac Tighten and test name-checking functions.
This change follows up from e759a9cd with more extensive changes and
tests:

If a name checking function (like |X509_VERIFY_PARAM_set1_host|) fails,
it now poisons the |X509_VERIFY_PARAM| so that all verifications will
fail. This is because we have observed that some callers are not
checking the return value of these functions.

Using a length of zero for a hostname to mean |strlen| is now an error.
It also an error for email addresses and IP addresses now, and doesn't
end up trying to call |strlen| on a (binary) IP address.

Setting an email address with embedded NULs now fails. So does trying to
configure an empty hostname or email with (NULL, 0).

|X509_check_*| functions in BoringSSL don't accept zero lengths (unlike
OpenSSL). It's now tested that such calls always fail.

Change-Id: I4484176f2aae74e502a09081c7e912c85e8d090b
Update-Note: several behaviour changes. See change description.
Reviewed-on: https://boringssl-review.googlesource.com/26764
Reviewed-by: David Benjamin <davidben@google.com>
2018-03-30 16:50:11 +00:00
David Benjamin
56f5eb9ffd Name constant-time functions more consistently.
I'm not sure why I separated "fixed" and "quick_ctx" names. That's
annoying and doesn't generalize well to, say, adding a bn_div_consttime
function for RSA keygen.

Change-Id: I751d52b30e079de2f0d37a952de380fbf2c1e6b7
Reviewed-on: https://boringssl-review.googlesource.com/26364
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-03-29 23:30:55 +00:00
David Benjamin
e6f46e2563 Blind the range check for finding a Rabin-Miller witness.
Rabin-Miller requires selecting a random number from 2 to |w|-1.
This is done by picking an N-bit number and discarding out-of-range
values. This leaks information about |w|, so apply blinding. Rather than
discard bad values, adjust them to be in range.
Though not uniformly selected, these adjusted values
are still usable as Rabin-Miller checks.

Rabin-Miller is already probabilistic, so we could reach the desired
confidence levels by just suitably increasing the iteration count.
However, to align with FIPS 186-4, we use a more pessimal analysis: we
do not count the non-uniform values towards the iteration count. As a
result, this function is more complex and has more timing risk than
necessary.

We count both total iterations and uniform ones and iterate until we've
reached at least |BN_PRIME_CHECKS_BLINDED| and |iterations|,
respectively.  If the latter is large enough, it will be the limiting
factor with high probability and we won't leak information.

Note this blinding does not impact most calls when picking primes
because composites are rejected early. Only the two secret primes see
extra work.  So while this does make the BNTest.PrimeChecking test take
about 2x longer to run on debug mode, RSA key generation time is fine.

Another, perhaps simpler, option here would have to run
bn_rand_range_words to the full 100 count, select an arbitrary
successful try, and declare failure of the entire keygen process (as we
do already) if all tries failed. I went with the option in this CL
because I happened to come up with it first, and because the failure
probability decreases much faster. Additionally, the option in this CL
does not affect composite numbers, while the alternate would. This gives
a smaller multiplier on our entropy draw. We also continue to use the
"wasted" work for stronger assurance on primality. FIPS' numbers are
remarkably low, considering the increase has negligible cost.

Thanks to Nathan Benjamin for helping me explore the failure rate as the
target count and blinding count change.

Now we're down to the rest of RSA keygen, which will require all the
operations we've traditionally just avoided in constant-time code!

Median of 29 RSA keygens: 0m0.169s -> 0m0.298s
(Accuracy beyond 0.1s is questionable. The runs at subsequent test- and
rename-only CLs were 0m0.217s, 0m0.245s, 0m0.244s, 0m0.247s.)

Bug: 238
Change-Id: Id6406c3020f2585b86946eb17df64ac42f30ebab
Reviewed-on: https://boringssl-review.googlesource.com/25890
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-03-29 22:02:24 +00:00
David Benjamin
8eadca50a2 Don't leak |a| in the primality test.
(This is actually slightly silly as |a|'s probability distribution falls
off exponentially, but it's easy enough to do right.)

Instead, we run the loop to the end. This is still performant because we
can, as before, return early on composite numbers. Only two calls
actually run to the end. Moreover, running to the end has comparable
cost to BN_mod_exp_mont_consttime.

Median time goes from 0.140s to 0.231s. That cost some, but we're still
faster than the original implementation.

We're down to one more leak, which is that the BN_rand_range_ex call
does not hide |w1|. That one may only be solved probabilistically...

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

Bug: 238
Change-Id: I4847cb0053118c572d2dd5f855388b5199fa6ce2
Reviewed-on: https://boringssl-review.googlesource.com/25888
Reviewed-by: Adam Langley <agl@google.com>
2018-03-28 01:44:31 +00:00
David Benjamin
9362ed9e14 Use a Barrett reduction variant for trial division.
Compilers use a variant of Barrett reduction to divide by constants,
which conveniently also avoids problematic operations on the secret
numerator. Implement the variant as described here:
http://ridiculousfish.com/blog/posts/labor-of-division-episode-i.html

Repurpose this to implement a constant-time BN_mod_word replacement.
It's even much faster! I've gone ahead and replaced the other
BN_mod_word calls on the primes table.

That should give plenty of budget for the other changes. (I am assuming
that a regression is okay, as RSA keygen is not performance-sensitive,
but that I should avoid anything too dramatic.)

Proof of correctness: https://github.com/davidben/fiat-crypto/blob/barrett/src/Arithmetic/BarrettReduction/RidiculousFish.v

Median of 29 RSA keygens: 0m0.621s -> 0m0.123s
(Accuracy beyond 0.1s is questionable, though this particular
improvement is quite solid.)

Bug: 238
Change-Id: I67fa36ffe522365b13feb503c687b20d91e72932
Reviewed-on: https://boringssl-review.googlesource.com/25887
Reviewed-by: Adam Langley <agl@google.com>
2018-03-28 01:42:18 +00:00
David Benjamin
232a6be6f1 Make primality testing mostly constant-time.
The extra details in Enhanced Rabin-Miller are only used in
RSA_check_key_fips, on the public RSA modulus, which the static linker
will drop in most of our consumers anyway. Implement normal Rabin-Miller
for RSA keygen and use Montgomery reduction so it runs in constant-time.

Note that we only need to avoid leaking information about the input if
it's a large prime. If the number ends up composite, or we find it in
our table of small primes, we can return immediately.

The leaks not addressed by this CL are:

- The difficulty of selecting |b| leaks information about |w|.
- The distribution of whether step 4.4 runs leaks information about w.
- We leak |a| (the largest power of two which divides w) everywhere.
- BN_mod_word in the trial division is not constant-time.

These will be resolved in follow-up changes.

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

Bug: 238
Change-Id: I0cf0ff22079732a0a3ababfe352bb4327e95b879
Reviewed-on: https://boringssl-review.googlesource.com/25886
Reviewed-by: Adam Langley <agl@google.com>
2018-03-28 01:42:06 +00:00
David Benjamin
50418afb7f Add some EC base point multiplication test vectors.
Probably worth having actual test vectors for these, rather than
checking our code against itself. Additionally, small negative numbers
have, in the past been valuable test vectors (see long comment in
point_add from OpenSSL's ecp_nistp521.c).

Change-Id: Ia5aa8a80eb5b6d0089c3601c5fec2364e699794d
Reviewed-on: https://boringssl-review.googlesource.com/26848
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>
2018-03-27 23:33:24 +00:00
David Benjamin
718c88c961 Fix a bug in p224-64.c.
p224_felem_neg does not produce an output within the tight bounds
suitable for p224_felem_contract. This was found by inspection of the
code.

This only affects the final y-coordinate output of arbitrary-point
multiplication, so it is a no-op for ECDH and ECDSA.

Change-Id: I1d929458d1f21d02cd8e745d2f0f7040a6bb0627
Reviewed-on: https://boringssl-review.googlesource.com/26847
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>
2018-03-27 18:03:14 +00:00
Adam Langley
72bc2328b1 Note licenses for support code in the top-level LICENSE file.
This keeps some scripts happy.

Change-Id: I79be4f3d014b72fbe3f0793759ad2b42329a550c
Reviewed-on: https://boringssl-review.googlesource.com/26824
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>
2018-03-27 17:03:47 +00:00
David Benjamin
2e16f6ba81 Add a test for CRYPTO_memcmp.
This test is written in honor of CVE-2018-0733.

Change-Id: I8a41f917b08496870037f745f19bdcdb65b3d623
Reviewed-on: https://boringssl-review.googlesource.com/26845
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>
2018-03-27 16:22:47 +00:00
David Benjamin
2a19a17ca7 Limit ASN.1 constructed types recursive definition depth
Constructed types with a recursive definition could eventually exceed
the stack given malicious input with excessive recursion. Therefore we
limit the stack depth.

CVE-2018-0739

Credit to OSSFuzz for finding this issue.

(Imported from upstream's 9310d45087ae546e27e61ddf8f6367f29848220d.)

BoringSSL does not contain any such structures, but import this anyway
with a test.

Change-Id: I0e84578ea795134f25dae2ac8b565f3c26ef3204
Reviewed-on: https://boringssl-review.googlesource.com/26844
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>
2018-03-27 15:40:37 +00:00
David Benjamin
0970d397c4 Make various BIGNUM comparisons constant-time.
Primality testing checks for small words in random places.

Median of 29 RSA keygens: 0m0.811s -> 0m0.521s
(Accuracy beyond 0.1s is questionable, and this "speed up" is certainly
noise.)

Bug: 238
Change-Id: Ie5efab7291302a42ac6e283d25da0c094d8577e7
Reviewed-on: https://boringssl-review.googlesource.com/25885
Reviewed-by: Adam Langley <agl@google.com>
2018-03-26 18:53:53 +00:00
David Benjamin
ad066861dd Add bn_usub_fixed.
There are a number of random subtractions in RSA key generation. Add a
fixed-width version.

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

Bug: 238
Change-Id: I9fa0771b95a438fd7d2635fd77a332146ccc96d9
Reviewed-on: https://boringssl-review.googlesource.com/25884
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2018-03-26 18:53:43 +00:00
Adam Langley
d89d65ba12 Add utility program for emitting P-256 x86-64 table.
No semantic change: the table is the same as before, but now with less
magic.

Change-Id: I351c2446e9765f25b7dfb901c9e98f12099a325c
Reviewed-on: https://boringssl-review.googlesource.com/26744
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>
2018-03-26 16:28:42 +00:00
David Benjamin
6ebef73213 Add bssl::UniquePtr<AUTHORITY_INFO_ACCESS>
Change-Id: I8a0c1196bd455a9193c411764f26e662f5b98649
Reviewed-on: https://boringssl-review.googlesource.com/26804
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>
2018-03-26 15:36:33 +00:00
David Benjamin
5fca613918 Fix typo in point_add.
Rather than writing the answer into the output, it wrote it into some
awkwardly-named temporaries. Thanks to Daniel Hirche for reporting this
issue!

Bug: chromium:825273
Change-Id: I5def4be045cd1925453c9873218e5449bf25e3f5
Reviewed-on: https://boringssl-review.googlesource.com/26785
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>
2018-03-23 21:12:29 +00:00
David Benjamin
6291af4e52 Add -DOPENSSL_SMALL to CMake.
Adding preprocessor flags requires a lot of typing in the CMake
command-line (-DCMAKE_C_FLAGS=-DOPENSSL_SMALL
-DCMAKE_CXX_FLAGS=-DOPENSSL_SMALL).

Change-Id: Ieafc4155d656306c1f22746f780faa5c1d3e27be
Reviewed-on: https://boringssl-review.googlesource.com/26784
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>
2018-03-23 21:07:48 +00:00
David Benjamin
441efad4d7 Add RSA_PSS_PARAMS to bssl::UniquePtr.
Change-Id: I471eb1c13aafb71ba5dc33f623811d5447cc85c6
Reviewed-on: https://boringssl-review.googlesource.com/26684
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-03-22 20:34:07 +00:00
Adam Langley
e759a9cd84 Support the OpenSSL “pass zero for strlen” when setting X.509 hostnames.
BoringSSL does not generally support this quirk but, in this case, we
didn't make it a fatal error and it's instead a silent omission of
hostname checking. This doesn't affect Chrome but, in case something is
using BoringSSL and using this trick, this change makes it safe.

BUG=chromium:824799

Change-Id: If417817b997b9faa9963c09dfc95d06a5d445e0b
Reviewed-on: https://boringssl-review.googlesource.com/26724
Commit-Queue: Adam Langley <alangley@gmail.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>
2018-03-22 17:19:07 +00:00
David Benjamin
d67e311ce4 Test BN_primality test with OEIS A014233 values .
These are composite numbers whose composite witnesses aren't in the
first however many prime numbers, so deterministically checking small
numbers may not work.

We don't check composite witnesses deterministically but these are
probably decent tests. (Not sure how else to find composites with
scarce witnesses, but these seemed decent candidates.)

Change-Id: I23dcb7ba603a64c1f7d1e9a16942e7c29c76da51
Reviewed-on: https://boringssl-review.googlesource.com/26645
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>
2018-03-22 16:26:37 +00:00
Adam Langley
88e6a05f46 Configure asmjs and wasm as generic, 32-bit machines.
Change-Id: Ia4fdd1eb848abacf43e18f6741ffa4ff79e40fd8
Reviewed-on: https://boringssl-review.googlesource.com/26664
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>
2018-03-20 23:24:06 +00:00
David Benjamin
d61334d187 Document preferences for EC_GROUP_new_by_curve_name.
Folks should use curve25519 or P-256 if in doubt.

Change-Id: Ie35381ef739744788a80345286f7b21e2bb67c88
Reviewed-on: https://boringssl-review.googlesource.com/26646
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>
2018-03-20 20:15:06 +00:00
Andrii Shyshkalov
433c0aab3b CQ: use new luci.boringssl.try bucket.
Should land after https://boringssl-review.googlesource.com/c/boringssl/+/26264
lands.

R=davidben@google.com

Bug: chromium:813228
Change-Id: I66cd792f9e90c3161b7886ed17d5057295d82abe
Reviewed-on: https://boringssl-review.googlesource.com/26604
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>
2018-03-20 16:31:15 +00:00
David Benjamin
ee764744e0 Add some BN_mod_inverse tests.
Generated randomly.

Change-Id: I51e6871ffddc4c5954a773db4473e944cb9818ed
Reviewed-on: https://boringssl-review.googlesource.com/26084
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>
2018-03-20 16:11:45 +00:00
David Benjamin
1bfb5c0f79 Add some tests for BN_gcd.
These were randomly generated.

Change-Id: I532afdaf469e6c80e518dae3a75547ff7cb0948f
Reviewed-on: https://boringssl-review.googlesource.com/26065
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>
2018-03-20 16:08:56 +00:00
David Benjamin
380fc326c3 Add RSA_check_key tests.
Change-Id: I5ac52de4217b32631b1d455f5d693d7b2aec665f
Reviewed-on: https://boringssl-review.googlesource.com/26372
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-03-19 22:29:40 +00:00
David Benjamin
ac97cc0e51 Fill in missing check_bn_tests.go features.
Change-Id: Ic0421b628212521d673cb7053b0fb278c827ebf5
Reviewed-on: https://boringssl-review.googlesource.com/26064
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>
2018-03-19 21:41:00 +00:00
David Benjamin
4b6055defb Add better tests for BN_rand.
Change-Id: Iefeeeb12c4a5a12e8dffc6817bb368d68a074cd0
Reviewed-on: https://boringssl-review.googlesource.com/25889
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>
2018-03-19 21:18:45 +00:00
David Benjamin
8a1a5daa49 Send the fake session ID in the test suite.
NSS only enables compatibility mode on the server if the client
requested it by way of the session ID. This is slightly off as a client
has no way not to request it when offering a TLS 1.2 session, but it is
in the spec.

So our tests are usable for other stacks, send a fake session ID in the
runner by default. The existing EmptySessionID-TLS13* test asserts that
BoringSSL behaves as we expect it to on empty session IDs too. The
intent is that NSS will disable that test but can otherwise leave the
rest enabled.

Change-Id: I370bf90aba1805c2f6970ceee0d29ecf199f437d
Reviewed-on: https://boringssl-review.googlesource.com/26504
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>
2018-03-19 21:06:05 +00:00
Adam Langley
d096c06b34 bytestring: document that |CBS_get_optional_asn1| can have a NULL output.
On the other hand, the type-specific
|CBS_get_optional_asn1_octet_string| must have a valid pointer and we
should check this in the “present” case or there could be a lucking
crash in some user waiting for an expected value to be missing.

Change-Id: Ida40e069ac7f0e50967e3f6c6b3fc01e49bd8894
Reviewed-on: https://boringssl-review.googlesource.com/26564
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>
2018-03-19 20:22:25 +00:00
sphawk
3ab1a69545 fix compilation error for non-english windows (like cjk)
add /utf-8 switch for msvc build. source code has several utf-8 characters
fix C2001 error. escape non-printable ascii code generated by embed_test_data.go
fix C4819 warning. add u8 keyword on utf-8 string literal (ripemd_test.cc)

Change-Id: I8c04dc7f0359e6ee27efada066863826d263d5cd
Reviewed-on: https://boringssl-review.googlesource.com/26484
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>
2018-03-15 17:52:23 +00:00
Adam Langley
fa3e9c3385 Add |SSL_COMP_get[0_name|_id]|.
These functions are needed by MySQL 8.0:
https://github.com/mysql/mysql-server/blob/8.0/vio/viossl.cc#L459

Change-Id: I4f13fa26cfe695229d6c8df80bcfc218408184da
Reviewed-on: https://boringssl-review.googlesource.com/26544
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>
2018-03-15 17:34:33 +00:00
David Benjamin
a0bc29a775 Remove remnants of the HRR message.
It has now been folded into ServerHello. Additionally, TLS 1.2 and TLS
1.3 ServerHellos are now more uniform, so we can avoid the extra
ServerHello parser.

Change-Id: I46641128c3f65fe37e7effca5bef4a76bf3ba84c
Reviewed-on: https://boringssl-review.googlesource.com/26524
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-03-13 21:10:03 +00:00
David Benjamin
10bfb89859 Fix 20-year-old typo in BN_mask_bits.
This clearly was supposed to be a return 1. See
https://github.com/openssl/openssl/issues/5537 for details.

(Additionally, now that our BIGNUMs may be non-minimal, this function
violates the rule that BIGNUM functions should not depend on widths. We
should use w >= bn_minimal_width(a) to retain the original behavior. But
the original behavior is nuts, so let's just fix it.)

Update-Note: BN_mask_bits no longer reports failure in some cases. These
    cases were platform-dependent and not useful, and code search confirms
    nothing was relying on it.

Change-Id: I31b1c2de6c5de9432c17ec3c714a5626594ee03c
Reviewed-on: https://boringssl-review.googlesource.com/26464
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>
2018-03-08 21:53:06 +00:00
David Benjamin
47d88415db Document that BN_bn2bin is not constant-time.
Change-Id: Id503850f92cc792229ed7558371e5038399c98d7
Reviewed-on: https://boringssl-review.googlesource.com/26385
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-03-07 21:44:51 +00:00
David Benjamin
3d2c6b0b0e Document EC_POINT_get_affine_coordinates_GFp allowing NULL x and y.
Change-Id: Iffc1f43afc0fed2166509775ac3c52f90eb7cddf
Reviewed-on: https://boringssl-review.googlesource.com/26384
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>
2018-03-07 21:32:51 +00:00
David Benjamin
a6bfc45b62 Store EC_KEY's private key as an EC_SCALAR.
This isn't strictly necessary now that BIGNUMs are safe, but we get to
rely on type-system annotations from EC_SCALAR. Additionally,
EC_POINT_mul depends on BN_div, while the EC_SCALAR version does not.

Change-Id: I75e6967f3d35aef17278b94862f4e506baff5c23
Reviewed-on: https://boringssl-review.googlesource.com/26424
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>
2018-03-07 21:17:31 +00:00
David Benjamin
d62fe6f3e8 Fold EC_KEY_copy into EC_KEY_dup.
EC_KEY_copy left unset fields alone, which meant it was possible to
create an EC_KEY with mismatched private key and group. Nothing was
using EC_KEY_copy anyway, and in keeping of us generally preferring
fresh objects over object reuse, remove it. EC_KEY_dup itself can also
be made simpler by using the very setters available.

Additionally, skip copying the method table. As of
https://boringssl-review.googlesource.com/16344, we no longer copy the
ex_data, so we probably shouldn't copy the method pointers either,
aligning with RSAPrivateKey_dup.

Update-Note: If I missed anything and someone uses EC_KEY_copy, it
   should be easy to port them to EC_KEY_dup.

Change-Id: Ibbdcea73345d91fa143fbe70a15bb527972693e8
Reviewed-on: https://boringssl-review.googlesource.com/26404
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>
2018-03-07 21:17:02 +00:00
Adam Langley
40cdb3b5da Don't test |initial_handshake_complete| for dummy PQ padding status.
Checking |initial_handshake_complete| was a mistake—it's not true for
False Start connections at the time when Chrome wants to measure whether
PQ padding was used or not.

Change-Id: I51757e00f3e02129666ee1ce31c30d63f1bcbe74
Reviewed-on: https://boringssl-review.googlesource.com/26444
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>
2018-03-07 20:27:11 +00:00
David Benjamin
ec55dc15d3 Update tools.
Also remove the -Wtautological-constant-compare logic. I believe Clang
has since removed that problematic warning from -Wall and that check was
causing problems when we were embedded as a subproject in a project that
didn't set CMP0025.

(In that case, by the time our build file ran, the compiler had already
been detected and the damage done. This unfortunately means the next
Clang version check will hit the same issue, but let's deal with that
when we get there.)

Change-Id: Iea5f262899b74c5b84f707f4cf4ac4b3540c4acb
Reviewed-on: https://boringssl-review.googlesource.com/26375
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>
2018-03-05 20:49:21 +00:00