Commit Graph

5150 Commits

Author SHA1 Message Date
David Benjamin
855dabc9df Add an accessor for session->certs.
Chromium has some code which reaches into this field for memory
accounting.

This fixes a bug in doc.go where this line-wrapping confuses it. doc.go
needs a bit of a rewrite, but this is a bit better.

Change-Id: Ic9cc2c2fe9329d7bc366ccf91e0c9a92eae08ed2
Reviewed-on: https://boringssl-review.googlesource.com/27764
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-04-27 17:14:38 +00:00
David Benjamin
bf4bcdf16e Fix some stuttering.
Pointed out by Brian in
https://boringssl-review.googlesource.com/c/boringssl/+/15325/11/crypto/internal.h#203.

Change-Id: Ic8d8672202f862e984e4503467d725ba030d5440
Reviewed-on: https://boringssl-review.googlesource.com/27804
Reviewed-by: Adam Langley <agl@google.com>
2018-04-27 15:56:57 +00:00
David Benjamin
2d10c3688c Check in a copy of Project Wycheproof test vectors.
This is just a pristine copy of the JSON files for now. It's not hooked
up to anything yet.

Change-Id: I608b4b0368578f159cad23950d70578ff4c23da3
Reviewed-on: https://boringssl-review.googlesource.com/27784
Reviewed-by: Adam Langley <agl@google.com>
2018-04-26 23:07:29 +00:00
Joshua Liebow-Feeser
b8546dd8a9 Update location of root certificates on Fuchsia
Change-Id: I156552df15de5941be99736cca694db4677e2b2a
Reviewed-on: https://boringssl-review.googlesource.com/27744
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-04-25 21:32:20 +00:00
Adam Langley
cece32610b Add SHA256_TransformBlocks.
Rather than expose a (potentially) assembly function directly, wrap it
in a C function to make visibility control easier.

Change-Id: I4a2dfeb8999ff021b2e10fbc54850eeadabbefff
Reviewed-on: https://boringssl-review.googlesource.com/27724
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-04-25 17:51:50 +00:00
David Benjamin
ec4f0ddafc EC_GROUP_dup cannot fail.
We've since ref-counted it.

Change-Id: I5589e79f5bbba35b02ae659c7aa6ac76ba0082a3
Reviewed-on: https://boringssl-review.googlesource.com/27669
Reviewed-by: Adam Langley <agl@google.com>
2018-04-25 16:43:19 +00:00
David Benjamin
32e0d10069 Add EC_FELEM for EC_POINTs and related temporaries.
This introduces EC_FELEM, which is analogous to EC_SCALAR. It is used
for EC_POINT's representation in the generic EC_METHOD, as well as
random operations on tuned EC_METHODs that still are implemented
genericly.

Unlike EC_SCALAR, EC_FELEM's exact representation is awkwardly specific
to the EC_METHOD, analogous to how the old values were BIGNUMs but may
or may not have been in Montgomery form. This is kind of a nuisance, but
no more than before. (If p224-64.c were easily convertable to Montgomery
form, we could say |EC_FELEM| is always in Montgomery form. If we
exposed the internal add and double implementations in each of the
curves, we could give |EC_POINT| an |EC_METHOD|-specific representation
and |EC_FELEM| is purely a |EC_GFp_mont_method| type. I'll leave this
for later.)

The generic add and doubling formulas are aligned with the formulas
proved in fiat-crypto. Those only applied to a = -3, so I've proved a
generic one in https://github.com/mit-plv/fiat-crypto/pull/356, in case
someone uses a custom curve.  The new formulas are verified,
constant-time, and swap a multiply for a square. As expressed in
fiat-crypto they do use more temporaries, but this seems to be fine with
stack-allocated EC_FELEMs. (We can try to help the compiler later,
but benchamrks below suggest this isn't necessary.)

Unlike BIGNUM, EC_FELEM can be stack-allocated. It also captures the
bounds in the type system and, in particular, that the width is correct,
which will make it easier to select a point in constant-time in the
future. (Indeed the old code did not always have the correct width. Its
point formula involved halving and implemented this in variable time and
variable width.)

Before:
Did 77274 ECDH P-256 operations in 10046087us (7692.0 ops/sec)
Did 5959 ECDH P-384 operations in 10031701us (594.0 ops/sec)
Did 10815 ECDSA P-384 signing operations in 10087892us (1072.1 ops/sec)
Did 8976 ECDSA P-384 verify operations in 10071038us (891.3 ops/sec)
Did 2600 ECDH P-521 operations in 10091688us (257.6 ops/sec)
Did 4590 ECDSA P-521 signing operations in 10055195us (456.5 ops/sec)
Did 3811 ECDSA P-521 verify operations in 10003574us (381.0 ops/sec)

After:
Did 77736 ECDH P-256 operations in 10029858us (7750.5 ops/sec) [+0.8%]
Did 7519 ECDH P-384 operations in 10068076us (746.8 ops/sec) [+25.7%]
Did 13335 ECDSA P-384 signing operations in 10029962us (1329.5 ops/sec) [+24.0%]
Did 11021 ECDSA P-384 verify operations in 10088600us (1092.4 ops/sec) [+22.6%]
Did 2912 ECDH P-521 operations in 10001325us (291.2 ops/sec) [+13.0%]
Did 5150 ECDSA P-521 signing operations in 10027462us (513.6 ops/sec) [+12.5%]
Did 4264 ECDSA P-521 verify operations in 10069694us (423.4 ops/sec) [+11.1%]

This more than pays for removing points_make_affine previously and even
speeds up ECDH P-256 slightly. (The point-on-curve check uses the
generic code.)

Next is to push the stack-allocating up to ec_wNAF_mul, followed by a
constant-time single-point multiplication.

Bug: 239
Change-Id: I44a2dff7c52522e491d0f8cffff64c4ab5cd353c
Reviewed-on: https://boringssl-review.googlesource.com/27668
Reviewed-by: Adam Langley <agl@google.com>
2018-04-25 16:39:58 +00:00
David Benjamin
6a289b3ec4 Remove EC_POINTs_make_affine and related logic.
This does not appear to actually pull its weight. The purpose of this
logic is to switch some adds to the faster add_mixed in the wNAF code,
at the cost of a rather expensive inversion. This optimization kicks in
for generic curves, so P-384 and P-521:

With:
Did 32130 ECDSA P-384 signing operations in 30077563us (1068.2 ops/sec)
Did 27456 ECDSA P-384 verify operations in 30073086us (913.0 ops/sec)
Did 14122 ECDSA P-521 signing operations in 30077407us (469.5 ops/sec)
Did 11973 ECDSA P-521 verify operations in 30037330us (398.6 ops/sec)

Without:
Did 32445 ECDSA P-384 signing operations in 30069721us (1079.0 ops/sec)
Did 27056 ECDSA P-384 verify operations in 30032303us (900.9 ops/sec)
Did 13905 ECDSA P-521 signing operations in 30000430us (463.5 ops/sec)
Did 11433 ECDSA P-521 verify operations in 30021876us (380.8 ops/sec)

For single-point multiplication, the optimization is not useful. This
makes sense as we only have one table's worth of additions to convert
but still pay for the inversion. For double-point multiplication, it is
slightly useful for P-384 and very useful for P-521. However, the next
change to stack-allocate EC_FELEMs will more than compensate for
removing it.  (The immediate goal here is to simplify the EC_FELEM
story.)

Additionally, that this optimization was not useful for single-point
multiplication implies that, should we wish to recover this, a modest
8-entry pre-computed (affine) base point table should have the same
effect or better.

Update-Note: I do not believe anything was calling either of these
functions. (If necessary, we can always add no-op stubs as whether a
point is affine is not visible to external code. It previously kicked in
some optimizations, but those were removed for constant-time needs
anyway.)

Bug: 239
Change-Id: Ic9c51b001c45595cfe592274c7d5d652f4234839
Reviewed-on: https://boringssl-review.googlesource.com/27667
Reviewed-by: Adam Langley <agl@google.com>
2018-04-25 16:12:06 +00:00
David Benjamin
06c28d8e51 Simplify shim timeout logic.
I don't think this lock is actually needed. If the process exited by the
time we call shim.Process.Kill(), then the test ultimately finished. If
not, wait() will return that the process died by a signal.

Change-Id: I668a86583aba16fd00e0cd05071acc13059a2c42
Reviewed-on: https://boringssl-review.googlesource.com/27325
Reviewed-by: Adam Langley <agl@google.com>
2018-04-25 16:07:28 +00:00
David Benjamin
48b276db3d Give ssl_cipher_preference_list_st a destructor.
Change-Id: I578a284c6a8cae773a97d3d30ad8a5cd13f56164
Reviewed-on: https://boringssl-review.googlesource.com/27491
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>
2018-04-24 19:55:29 +00:00
David Benjamin
06d467c58a ghashv8-armx.pl: add Qualcomm Kryo results.
(Imported from upstream's 753316232243ccbf86b96c1c51ffcb41651d9ad5.)

Just to sync up a bit further.

Change-Id: I805150d0f0c10d68648fae83603b0d46231ae4ec
Reviewed-on: https://boringssl-review.googlesource.com/27685
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-04-24 19:48:59 +00:00
David Benjamin
a7c8f2b7b0 ghashv8-armvx.pl: Fix various typos.
(Imported from upstream's 46f4e1bec51dc96fa275c168752aa34359d9ee51.)

Change-Id: Ie9c1e9cfc38a3962e3674a68bc0174d064272fc2
Reviewed-on: https://boringssl-review.googlesource.com/27684
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-04-24 19:48:49 +00:00
David Benjamin
a63d0ad40d Require BN_mod_exp_mont* inputs be reduced.
If the caller asked for the base to be treated as secret, we should
provide that. Allowing unbounded inputs is not compatible with being
constant-time.

Additionally, this aligns with the guidance here:
https://github.com/HACS-workshop/spectre-mitigations/blob/master/crypto_guidelines.md#1-do-not-conditionally-choose-between-constant-and-non-constant-time

Update-Note: BN_mod_exp_mont_consttime and BN_mod_exp_mont now require
inputs be fully reduced. I believe current callers tolerate this.

Additionally, due to a quirk of how certain operations were ordered,
using (publicly) zero exponent tolerated a NULL BN_CTX while other
exponents required non-NULL BN_CTX. Non-NULL BN_CTX is now required
uniformly. This is unlikely to cause problems. Any call site where the
exponent is always zero should just be replaced with BN_value_one().

Change-Id: I7c941953ea05f36dc2754facb9f4cf83a6789c61
Reviewed-on: https://boringssl-review.googlesource.com/27665
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-04-24 18:29:29 +00:00
David Benjamin
52a68a9b43 Remove unused string.h include.
This is unused now that we use the silly memcpy, etc., wrappers to work
around the C NULL/0 language bug.

See https://android-review.googlesource.com/c/platform/external/boringssl/+/670794

Change-Id: I15c878cee6badb4551c8d5cfa1371a9bff4000fb
Reviewed-on: https://boringssl-review.googlesource.com/27666
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-04-24 17:42:39 +00:00
David Benjamin
5c0e0cec83 Remove Z = 1 special-case in generic point_get_affine.
As the point may be the output of some private key operation, whether Z
accidentally hit one is secret.

Bug: 239
Change-Id: I7db34cd3b5dd5ca4b96980e8993a9b4eda49eb88
Reviewed-on: https://boringssl-review.googlesource.com/27664
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-04-24 16:16:53 +00:00
David Benjamin
f5858ca008 Remove unnecessary endian flip in p224-64.c.
We have little-endian BIGNUM functions now.

Change-Id: Iffc46a14e75c6bba2e170b824b1a08c69d2e9d18
Reviewed-on: https://boringssl-review.googlesource.com/27594
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-04-24 16:15:28 +00:00
David Benjamin
b8f14b7d53 Add dedicated scalar inversion code to p256-x86_64.c.
This is adapted from upstream's
eb7916960bf50f436593abe3d5f2e0592d291017.

This gives a 22% win for ECDSA signing. (Upstream cites 30-40%, but they
are unnecessarily using BN_mod_exp_mont_consttime in their generic path.
The exponent is public. I expect part of their 30-40% is just offsetting
this.)

Did 506000 ECDSA P-256 signing operations in 25044595us (20204.0 ops/sec)
Did 170506 ECDSA P-256 verify operations in 25033567us (6811.1 ops/sec)

Did 618000 ECDSA P-256 signing operations in 25031294us (24689.1 ops/sec)
Did 182240 ECDSA P-256 verify operations in 25006918us (7287.6 ops/sec)

Most of the performance win appears to be from the assembly operations
and not the addition chain. I have a CL to graft the addition chain onto
the C implementation, but it did not show measurable improvement in
ECDSA verify. ECDSA sign gets 2-4% faster, but we're more concerned
about ECDSA verify in the OPENSSL_SMALL builds.

Change-Id: Ide166f98b146c025f7f80ed7906336c16818540a
Reviewed-on: https://boringssl-review.googlesource.com/27593
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-04-24 16:14:57 +00:00
David Benjamin
364a51ec3a Abstract scalar inversion in EC_METHOD.
This introduces a hook for the OpenSSL assembly.

Change-Id: I35e0588f0ed5bed375b12f738d16c9f46ceedeea
Reviewed-on: https://boringssl-review.googlesource.com/27592
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-04-24 16:13:24 +00:00
David Benjamin
b27b579fdd Add some tests for scalar operations.
Largely random data, but make it easy to add things in the future.

Change-Id: I30bee790bd9671b4d0327c2244fe5cd1a8954f90
Reviewed-on: https://boringssl-review.googlesource.com/27591
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-04-24 16:12:34 +00:00
David Benjamin
3861ae662a p256-x86_64-asm.pl: add .cfi and SEH handlers to new functions.
Imported from upstream's d5e11843fe430dfa89bdf83b6f7805c709dcdb41.

Change-Id: Ie6d64ef821b66531995b43d015ab2755558eaa57
Reviewed-on: https://boringssl-review.googlesource.com/27590
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-04-24 16:10:08 +00:00
David Benjamin
5c30dab835 Import P-256 scalar multiplication assembly from OpenSSL.
This imports the assembly portion of
eb7916960bf50f436593abe3d5f2e0592d291017 from upstream. Note the
OPENSSL_ia32cap_P bits were tweaked to be delocate-compatible. Those
should be reviewed against the original file.

Change-Id: I19eef722225bb7928275e3d93890f80aa2f8734d
Reviewed-on: https://boringssl-review.googlesource.com/27589
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-04-24 16:09:08 +00:00
David Benjamin
7121fe24e9 Align ECDSA sign/verify scalar inversions.
We were still using the allocating scalar inversion for ECDSA verify
because previously it seemed to be faster. It appears to have flipped
now, though probably was always just a wash.

While I'm here, save a multiplication by swapping the inversion and
Montgomery reduction.

Did 200000 ECDSA P-256 signing operations in 10025749us (19948.6 ops/sec)
Did 66234 ECDSA P-256 verify operations in 10061123us (6583.2 ops/sec)

Did 202000 ECDSA P-256 signing operations in 10020846us (20158.0 ops/sec)
Did 68052 ECDSA P-256 verify operations in 10020592us (6791.2 ops/sec)

The actual motivation is to get rid of the unchecked EC_SCALAR function
and align sign/verify in preparation for the assembly scalar ops.

Change-Id: I1bd3a5719a67966dc8edaa43535a3864b69f76d0
Reviewed-on: https://boringssl-review.googlesource.com/27588
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-04-24 16:00:12 +00:00
David Benjamin
941f535438 Abstract away EC_SCALAR operations.
Just a little bit cleaner.

Change-Id: I0ed192a531b5aa853ba082caa6088e838f12c863
Reviewed-on: https://boringssl-review.googlesource.com/27587
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-04-24 15:37:40 +00:00
David Benjamin
9291be5b27 Remove return values from bn_*_small.
No sense in adding impossible error cases we need to handle.
Additionally, tighten them a bit and require strong bounds. (I wasn't
sure what we'd need at first and made them unnecessarily general.)

Change-Id: I21a0afde90a55be2e9a0b8d7288f595252844f5f
Reviewed-on: https://boringssl-review.googlesource.com/27586
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-04-24 15:34:32 +00:00
David Benjamin
3f8074c2de Fix the error on overly large group orders.
Change-Id: I9b11fabb79b5dfe031ac5ea2f021b28b87262761
Reviewed-on: https://boringssl-review.googlesource.com/27585
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-04-24 15:27:17 +00:00
David Benjamin
cd01254900 Explicitly guarantee BN_MONT_CTX::{RR,N} have the same width.
This is so the *_small functions can assume somewhat more uniform
widths, to simplify their error-handling.

Change-Id: I0420cb237084b253e918c64b0c170a5dfd99ab40
Reviewed-on: https://boringssl-review.googlesource.com/27584
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-04-24 15:22:09 +00:00
Adam Langley
e3aba378c9 Fix typo in ssl_cert_cache_chain_certs.
After e325c3f471, this typo bites and
causes SSL_CTX_get_extra_chain_certs to return an empty stack.

Change-Id: I6aa7093d1ca4f3ba0f520a644b14de5b3a3ccaa6
Reviewed-on: https://boringssl-review.googlesource.com/27604
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-04-23 19:21:01 +00:00
David Benjamin
a2938719a4 Improve the RSA key generation failure probability.
The FIPS 186-4 algorithm we use includes a limit which hits a 2^-20
failure probability, assuming my math is right. We've observed roughly
2^-23. This is a little large at scale. (See b/77854769.)

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

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

Change-Id: I0e1da98413ebd4ffa65fb74c67a58a0e0cd570ff
Reviewed-on: https://boringssl-review.googlesource.com/27288
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-04-20 21:34:05 +00:00
David Benjamin
9af9b946d2 Restore the BN_mod codepath for public Montgomery moduli.
https://boringssl-review.googlesource.com/10520 and then later
https://boringssl-review.googlesource.com/25285 made BN_MONT_CTX_set
constant-time, which is necessary for RSA's mont_p and mont_q. However,
due to a typo in the benchmark, they did not correctly measure.

Split BN_MONT_CTX creation into a constant-time and variable-time one.
The constant-time one uses our current algorithm and the latter restores
the original BN_mod codepath.

Should we wish to avoid BN_mod, I have an alternate version lying
around:

First, BN_set_bit + bn_mod_lshift1_consttime as now to count up to 2*R.
Next, observe that 2*R = BN_to_montgomery(2) and R*R =
BN_to_montgomery(R) = BN_to_montgomery(2^r_bits) Also observe that
BN_mod_mul_montgomery only needs n0, not RR. Split the core of
BN_mod_exp_mont into its own function so the caller handles conversion.
Raise 2*R to the r_bits power to get 2^r_bits*R = R*R.

The advantage of that algorithm is that it is still constant-time, so we
only need one BN_MONT_CTX_new. Additionally, it avoids BN_mod which is
otherwise (almost, but the remaining links should be easy to cut) out of
the critical path for correctness. One less operation to worry about.

The disadvantage is that it is gives a 25% (RSA-2048) or 32% (RSA-4096)
slower RSA verification speed. I went with the BN_mod one for the time
being.

Before:
Did 9204 RSA 2048 signing operations in 10052053us (915.6 ops/sec)
Did 326000 RSA 2048 verify (same key) operations in 10028823us (32506.3 ops/sec)
Did 50830 RSA 2048 verify (fresh key) operations in 10033794us (5065.9 ops/sec)
Did 1269 RSA 4096 signing operations in 10019204us (126.7 ops/sec)
Did 88435 RSA 4096 verify (same key) operations in 10031129us (8816.1 ops/sec)
Did 14552 RSA 4096 verify (fresh key) operations in 10053411us (1447.5 ops/sec)

After:
Did 9150 RSA 2048 signing operations in 10022831us (912.9 ops/sec)
Did 322000 RSA 2048 verify (same key) operations in 10028604us (32108.2 ops/sec)
Did 289000 RSA 2048 verify (fresh key) operations in 10017205us (28850.4 ops/sec)
Did 1270 RSA 4096 signing operations in 10072950us (126.1 ops/sec)
Did 87480 RSA 4096 verify (same key) operations in 10036328us (8716.3 ops/sec)
Did 80730 RSA 4096 verify (fresh key) operations in 10073614us (8014.0 ops/sec)

Change-Id: Ie8916d1634ccf8513ceda458fa302f09f3e93c07
Reviewed-on: https://boringssl-review.googlesource.com/27287
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-04-20 20:50:15 +00:00
David Benjamin
7e2a8a34ba Speed up variable windowed exponentation a bit.
The first non-zero window (which we can condition on for public
exponents) always multiplies by one. This means we can cut out one
Montgomery multiplication. It also means we never actually need to
initialize r to one, saving another Montgomery multiplication for P-521.

This, in turn, means we don't need the bn_one_to_montgomery optimization
for the public-exponent exponentations, so we can delete
bn_one_to_montgomery_small. (The function does currently promise to
handle p = 0, but this is not actually reachable, so it can just do a
reduction on RR.)

For RSA, where we're not doing many multiplications to begin with,
saving one is noticeable.

Before:
Did 92000 RSA 2048 verify (same key) operations in 3002557us (30640.6 ops/sec)
Did 25165 RSA 4096 verify (same key) operations in 3045046us (8264.2 ops/sec)

After:
Did 100000 RSA 2048 verify (same key) operations in 3002483us (33305.8 ops/sec)
Did 26603 RSA 4096 verify (same key) operations in 3010942us (8835.4 ops/sec)

(Not looking at the fresh key number yet as that still needs to be
fixed.)

Change-Id: I81a025a68d9b0f8eb0f9c6c04ec4eedf0995a345
Reviewed-on: https://boringssl-review.googlesource.com/27286
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-04-20 20:37:45 +00:00
Jesse Selover
b1e6a85443 Change OPENSSL_cpuid_setup to reserve more extended feature space.
Copy of openssl change https://git.openssl.org/gitweb/?p=openssl.git;h=d6ee8f3dc4414cd97bd63b801f8644f0ff8a1f17

OPENSSL_ia32cap: reserve for new extensions.
Change-Id: I96b43c82ba6568bae848449972d3ad9d20f6d063
Reviewed-on: https://boringssl-review.googlesource.com/27564
Reviewed-by: David Benjamin <davidben@google.com>
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-04-19 20:48:58 +00:00
Jesse Selover
35e7c994be Remove files from Trusty which can't link because of Trusty libc.
Change-Id: If3d93648cf6561c02c208895526ae1f1cbfa2b51
Reviewed-on: https://boringssl-review.googlesource.com/27524
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-04-19 19:06:58 +00:00
Adam Langley
6f6a237d46 delocate: put emitted labels in a specific file.
Otherwise Clang has to assign a file entry to the label which conflicts with
later, explicit, file entries.

Change-Id: Ifc782821517aa7b48ba3ef304d4468f2bc850ac2
Reviewed-on: https://boringssl-review.googlesource.com/27544
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-04-19 17:54:31 +00:00
David Benjamin
56b1a8efa6 Test the high-order bit in X25519.
This schism came up in passing again, and I realized we never added a
TLS-level test for this. Fix that.

Change-Id: I10f910bb5a975d6b3b73d99e7412ade35654fddb
Reviewed-on: https://boringssl-review.googlesource.com/27224
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-04-19 00:56:35 +00:00
David Benjamin
56ea9e2769 Fix bn_mod_exp_mont_small when exponentiating to zero.
It's defined to return one in Montgomery form, not a normal one.

(Not that this matters. This function is only used to Fermat's Little
Theorem. Probably it should have been less general, though we'd need to
make new test vectors first.)

Change-Id: Ia8d7588e6a413b25f01280af9aacef0192283771
Reviewed-on: https://boringssl-review.googlesource.com/27285
Reviewed-by: Adam Langley <agl@google.com>
2018-04-18 22:13:16 +00:00
David Benjamin
e0ae249f03 Remove a = 0 special-case in BN_mod_exp_mont.
BN_mod_exp_mont is intended to protect the base, but not the exponent.
Accordingly, it shouldn't treat a base of zero as special.

Change-Id: Ib053e8ce65ab1741973a9f9bfeff8c353567439c
Reviewed-on: https://boringssl-review.googlesource.com/27284
Reviewed-by: Adam Langley <agl@google.com>
2018-04-18 22:03:16 +00:00
David Benjamin
d319205007 Deny CRT to unbalanced RSA keys.
Our technique to perform the reduction only works for balanced key
sizes. For unbalanced keys, we fall back to variable-time logic.
Instead, fall back earlier to the non-CRT codepath, which is still
secure, just slower. This also aligns with the advice here:

https://github.com/HACS-workshop/spectre-mitigations/blob/master/crypto_guidelines.md#1-do-not-conditionally-choose-between-constant-and-non-constant-time

Update-Note: This is a performance hit (some keys will run 3x slower),
but only for keys with different-sized primes. I believe the Windows
crypto APIs will not accept such keys at all. There are two scenarios to
be concerned with for RSA performance:

1. Performance of reasonably-generated keys. Keys that BoringSSL or
anyone else reasonable generates will all be balanced, so this change
does not affect them.

2. Worst-case performance for DoS purposes. This CL does not change the
worst-case performance for RSA at a given bit size. In fact, it improves
it slightly. A sufficiently unbalanced RSA key is as slow as not doing
CRT at all.

In both cases, this change does not affect performance. The affected
keys are pathologically-generated ones that were not quite pathological
enough.

Bug: 235
Change-Id: Ie298dabb549ab9108fa9374aa86ebffe8b6c6c88
Reviewed-on: https://boringssl-review.googlesource.com/27504
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-04-17 15:14:04 +00:00
David Benjamin
024f5df3c8 Avoid some divisions in Lucky 13 fix.
data_plus_mac_size is secret. Values derived from it cannot quite be
safely divided by md_block_size because SHA-384 ciphers prevent that
field from being constant. We know the value is a power of two, so do
the strength reduction by hand.

Change-Id: Id62ab9e646f4e21d507a7059cfe84d49bbb986e6
Reviewed-on: https://boringssl-review.googlesource.com/27505
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-04-17 15:13:55 +00:00
David Benjamin
e325c3f471 Give CERT a destructor.
Change-Id: I97f5290d908e59ece75fe5b8fa72d51c3cf62148
Reviewed-on: https://boringssl-review.googlesource.com/27489
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>
2018-04-16 20:25:23 +00:00
David Benjamin
fceca8e27b Move srtp_profile to ssl->s3.
This too is connection-level state to be reset on SSL_clear.

Change-Id: I071c9431c28a7d0ff3eb20c679784d4aa4c236a5
Reviewed-on: https://boringssl-review.googlesource.com/27490
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-04-16 20:07:43 +00:00
David Benjamin
e28552dec8 Add an API to disable RSA-PSS for certificates.
Chrome uses the platform certificate verifier and thus cannot reliably
expect PSS signatures to work in all configurations. Add an API for the
consumer to inform BoringSSL of this ability. We will then adjust our
advertisements accordingly.

Note that, because TLS 1.2 does not have the signature_algorithms_cert
extension, turning off TLS 1.3 and using this API will stop advertising
RSA-PSS. I believe this is the correct behavior given the semantics of
that code point.

The tests check the various combinations here, as well as checking that
the peer never sends signature_algorithms_cert identical to
signature_algorithms.

Bug: 229
Change-Id: I8c33a93efdc9252097e3899425b49548fc42a93a
Reviewed-on: https://boringssl-review.googlesource.com/27488
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-04-16 20:02:43 +00:00
David Benjamin
c977532240 Pretty-print TicketAEADMethod tests.
It's hard to diagnose "20".

Change-Id: I57e8d0fb6e4937ddeca45b3645463ca0dc872ea6
Reviewed-on: https://boringssl-review.googlesource.com/27487
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-04-16 19:11:33 +00:00
David Benjamin
6879e19362 Rename SSL_SIGN_RSA_PSS_SHA* constants.
This reflects the change to add the key type into the constant. The old
constants are left around for now as legacy aliases and will be removed
later.

Change-Id: I67f1b50c01fbe0ebf4a2e9e89d3e7d5ed5f5a9d7
Reviewed-on: https://boringssl-review.googlesource.com/27486
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-04-16 19:00:03 +00:00
David Benjamin
5ad94767ab Remove legacy SSL_CTX_sess_set_get_cb overload.
Update-Note: I believe everything relying on this overload has since
    been updated.

Change-Id: I7facf59cde56098e5e3c79470293b67abb715f4c
Reviewed-on: https://boringssl-review.googlesource.com/27485
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-04-16 18:50:33 +00:00
David Benjamin
68478b7e9b Add runtime bounds checks to bssl::Span.
Better safe than sorry.

Change-Id: Ia99fa59ef1345835e01c330d99707bc8899a33a1
Reviewed-on: https://boringssl-review.googlesource.com/27484
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-04-16 16:26:33 +00:00
David Benjamin
9f0e7cb314 Move TB state to ssl->s3.
These are connection state, so they should be reset on SSL_clear.

Change-Id: I861fe52578836615d2719c9e1ff0911c798f336e
Reviewed-on: https://boringssl-review.googlesource.com/27384
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-04-13 18:10:44 +00:00
David Benjamin
b8b1a9d8de Add SSL_SESSION_get0_cipher.
Conscrypt need this function right now. They ought to be fixed up to not
need this but, in the meantime, this API is also provided by OpenSSL and
will clear one most consumer reaching into SSL_SESSION.

Bumping the API since Conscrypt often involves multi-sided stuff.

Change-Id: I665ca6b6a17ef479133c29c23fc639f278128c69
Reviewed-on: https://boringssl-review.googlesource.com/27405
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-04-13 17:45:23 +00:00
Daniel Hirche
1414d86ff9 tool: Move the RSA specific code from |Speed| to |SpeedRSA|.
In addition, make use of bssl::ScopedEVP_MD_CTX in |SpeedHashChunk|,
otherwise the ctx doesn't get destroyed on failure.

Change-Id: I5828080cb9f4eb7c77cc2ff185d9aa8135311385
Reviewed-on: https://boringssl-review.googlesource.com/27464
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-04-13 17:35:13 +00:00
David Benjamin
27e4c3bab2 Add an OPENSSL_malloc_init stub.
OpenSSL 1.1.0 renamed that. Also clang-format wanted to smush it all
onto one line.

Change-Id: Icdaa0eefc503c4aab1b309ccb34625f5e811c537
Reviewed-on: https://boringssl-review.googlesource.com/27404
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-04-13 17:30:44 +00:00
Daniel Hirche
de20810fb4 Fix return value in speed tool.
Change-Id: Iceed87c194201d28c4a51b1c19a59fe2f20b6a5e
Reviewed-on: https://boringssl-review.googlesource.com/27444
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-04-13 16:36:27 +00:00