Commit Graph

5430 Commits

Author SHA1 Message Date
Adam Langley
03de6813d8 Write error messages in the FIPS module to stderr.
Previously, delocate.go couldn't handle GOT references and so |stderr|
was a problematic symbol. We can cope with them now, so write FIPS
power-on test and urandom errors to stderr rather than stdout.

Change-Id: If6d7c19ee5f22dcbd74fb01c231500c2e130e6f7
Update-note: resolves internal bug 110102292.
Reviewed-on: https://boringssl-review.googlesource.com/29244
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-06-25 10:30:42 +00:00
Adam Langley
bcfb49914b Add special AES-GCM AEAD for TLS 1.3.
This change adds an AES-GCM AEAD that enforces nonce uniqueness inside
the FIPS module, like we have for TLS 1.2. While TLS 1.3 has not yet
been mentioned in the FIPS 140 IG, we expect it to be in the next ~12
months and so are preparing for that.

Change-Id: I65a7d8196b08dc0033bdde5c844a73059da13d9e
Reviewed-on: https://boringssl-review.googlesource.com/29224
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-06-25 10:23:22 +00:00
David Benjamin
954eefae58 Actually add AES-192-OFB.
I forgot about this file.

Change-Id: Icb98ffe3ed682a80d7a809a4585a5537fed0ba1c
Reviewed-on: https://boringssl-review.googlesource.com/29284
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-06-21 22:03:34 +00:00
Adam Langley
0080d83b9f Implement the client side of certificate compression.
Change-Id: I0aced480af98276ebfe0970b4afb9aa957ee07cb
Reviewed-on: https://boringssl-review.googlesource.com/29024
Reviewed-by: Adam Langley <agl@google.com>
2018-06-18 22:16:11 +00:00
David Benjamin
f6e5d0d5a1 Add AES-192-OFB.
cryptography.io gets offended if the library supports some OFB sizes but
not others.

Change-Id: I7fc7b12e7820547a82aae84d9418457389a482fe
Reviewed-on: https://boringssl-review.googlesource.com/29204
Reviewed-by: Adam Langley <agl@google.com>
2018-06-18 21:58:46 +00:00
David Benjamin
7139f755b6 Fix some timing leaks in the DSA code.
The DSA code is deprecated and will, hopefully, be removed in the future.
Nonetheless, this is easy enough to fix. It's the analog of the work we'd
already done for ECDSA.

- Document more clearly that we don't care about the DSA code.

- Use the existing constant-time modular addition function rather than
  the ad-hoc code.

- Reduce the digest to satisfy modular operations' invariants. (The
  underlying algorithms could accept looser bounds, but we reduce for
  simplicity.) There's no particular reason to do this in constant time,
  but we have the code for it, so we may as well.

- This additionally adds a missing check that num_bits(q) is a multiple
  of 8. We otherwise don't compute the right answer. Verification
  already rejected all 160-, 224-, and 256-bit keys, and we only
  generate DSA parameters where the length of q matches some hash
  function's length, so this is unlikely to cause anyone trouble.

- Use Montgomery reduction to perform the modular multiplication. This
  could be optimized to save a couple Montgomery reductions as in ECDSA,
  but DSA is deprecated, so I haven't bothered optimizing this.

- The reduction from g^k (mod p) to r = g^k (mod p) (mod q) is left
  in variable time, but reversing it would require a discrete log
  anyway. (The corresponding ECDSA operation is much easier to make
  constant-time due to Hasse's theorem, though that's actually still a
  TODO. I need to finish lifting EC_FELEM up the stack.)

Thanks to Keegan Ryan from NCC Group for reporting the modular addition issue
(CVE-2018-0495). The remainder is stuff I noticed along the way.

Update-Note: See the num_bits(q) change.

Change-Id: I4f032b041e2aeb09f9737a39f178c24e6a7fa1cb
Reviewed-on: https://boringssl-review.googlesource.com/29145
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-06-15 02:37:45 +00:00
David Benjamin
9f9c938af0 Revert "Reland "Revert "Add other Windows configurations to the CQ."""
This reverts commit 43eb0af5f1.

Reason for revert: Hopefully this is resolved by https://boringssl.googlesource.com/boringssl/+/7c8e725e55a72c914eb3a33af6cc65b4188102c6 ?

Original change's description:
> Reland "Revert "Add other Windows configurations to the CQ.""
> 
> This reverts commit 23e92d5d16.
> 
> Reason for revert: Nope. Still doesn't work. Back to poking infra
> about it...
> 
> Original change's description:
> > Revert "Revert "Add other Windows configurations to the CQ.""
> > 
> > This reverts commit 98831738f2.
> > 
> > Let's try this again. tandrii@ says this should be resolved as of
> > https://crbug.com/840505. (That was a while ago. I'd forgotten about
> > it.)
> > 
> > Change-Id: Ib49a629198a33d44ff1c3aa13af5825def1a5c4d
> > Reviewed-on: https://boringssl-review.googlesource.com/28924
> > 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>
> 
> TBR=davidben@google.com,svaldez@google.com
> 
> Change-Id: Iecd0710075f1fedc4dea69283d018042fb1a2490
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://boringssl-review.googlesource.com/29104
> 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>

TBR=davidben@google.com,svaldez@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I583641be42e6e6e93eb30adbe56ae20812608103
Reviewed-on: https://boringssl-review.googlesource.com/29184
Reviewed-by: David Benjamin <davidben@google.com>
2018-06-14 20:06:36 +00:00
David Benjamin
23aa4d228a Update tools.
Change-Id: I4cc9b9fc13a20080d78f9b9bca1e24571f46f117
Reviewed-on: https://boringssl-review.googlesource.com/29146
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-06-14 13:58:05 +00:00
David Benjamin
dd935202c9 Zero-initialize tmp in ec_GFp_simple_mul_single.
Although the original value of tmp does not matter, the selects
ultimately do bit operations on the uninitialized values and thus depend
on them behaving like *some* consistent concrete value. The C spec
appears to allow uninitialized values to resolve to trap
representations, which means this isn't quite valid..

(If I'm reading it wrong and the compiler must behave as if there were a
consistent value in there, it's probably fine, but there's no sense in
risking compiler bugs on a subtle corner of things.)

Change-Id: Id4547b0ec702414b387e906c4de55595e6214ddb
Reviewed-on: https://boringssl-review.googlesource.com/29124
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-06-13 19:58:24 +00:00
Adam Barth
6ff2ba80b7 [fuchsia] Update to zx_cprng_draw_new
This version doesn't have short reads. We'll eventually rename the
syscall back to zx_cprng_draw once all the clients have migrated to the
new semantics.

Change-Id: I7a7f6751e4d85dcc9b0a03a533dd93f3cbee277f
Reviewed-on: https://boringssl-review.googlesource.com/29084
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-06-12 14:58:43 +00:00
David Benjamin
43eb0af5f1 Reland "Revert "Add other Windows configurations to the CQ.""
This reverts commit 23e92d5d16.

Reason for revert: Nope. Still doesn't work. Back to poking infra
about it...

Original change's description:
> Revert "Revert "Add other Windows configurations to the CQ.""
> 
> This reverts commit 98831738f2.
> 
> Let's try this again. tandrii@ says this should be resolved as of
> https://crbug.com/840505. (That was a while ago. I'd forgotten about
> it.)
> 
> Change-Id: Ib49a629198a33d44ff1c3aa13af5825def1a5c4d
> Reviewed-on: https://boringssl-review.googlesource.com/28924
> 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>

TBR=davidben@google.com,svaldez@google.com

Change-Id: Iecd0710075f1fedc4dea69283d018042fb1a2490
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://boringssl-review.googlesource.com/29104
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-06-12 13:23:55 +00:00
David Benjamin
23e92d5d16 Revert "Revert "Add other Windows configurations to the CQ.""
This reverts commit 98831738f2.

Let's try this again. tandrii@ says this should be resolved as of
https://crbug.com/840505. (That was a while ago. I'd forgotten about
it.)

Change-Id: Ib49a629198a33d44ff1c3aa13af5825def1a5c4d
Reviewed-on: https://boringssl-review.googlesource.com/28924
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-06-11 20:01:42 +00:00
David Benjamin
4665da6e91 Add OFB ciphers to EVP_get_cipherbyname.
This is so they're exposed out of cryptography.io.

Change-Id: I225a35605ae8f3da091e95241ce072eeeabcd855
Reviewed-on: https://boringssl-review.googlesource.com/29044
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-06-11 19:46:43 +00:00
David Benjamin
3b2ff028c4 Add SSL_SESSION_get0_id_context.
This matches OpenSSL 1.1.0. Someone requested it.

Change-Id: I230bb9ec646cd32e71413a68e93058818c8f2aad
Reviewed-on: https://boringssl-review.googlesource.com/29004
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-06-11 14:25:23 +00:00
Loo Rong Jie
b570fd9fd6 Link advapi32.lib when linking crypto.
This is needed for RtlGenRandom [0] in crypto/rand_extra/windows.c [1].

Linker error actually shows "SystemFunction036" instead of "RtlGenRandom".

[0]: https://msdn.microsoft.com/en-us/library/windows/desktop/aa387694(v=vs.85).aspx
[1]: f21650709a/crypto/rand_extra/windows.c (L44)

Change-Id: Ie52e0a1d507e3598ba3ae3c449d0088981c06d8a
Reviewed-on: https://boringssl-review.googlesource.com/28884
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-06-07 19:05:59 +00:00
Adam Langley
070151c96f Update ECDH and EVP tests to accept latest Wycheproof vectors.
(This upstreams a change that was landed internally.)

Change-Id: Ic32793f8b1ae2d03e8ccbb0a9ac5f62add4c295b
Reviewed-on: https://boringssl-review.googlesource.com/28984
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-06-07 16:54:30 +00:00
David Benjamin
1c68fa2350 Hide SSL_SESSION.
The last libssl struct is now opaque! (Promote the SSL_MAX_* constants
as folks use them pretty frequently.)

Update-Note: SSL_SESSION is now opaque. I believe everything handles
this now.

Bug: 6
Change-Id: I8cd29d16173e4370f3341c0e6f0a56e00ea188e9
Reviewed-on: https://boringssl-review.googlesource.com/28964
Reviewed-by: Adam Langley <agl@google.com>
2018-06-07 02:58:27 +00:00
Matthew Braithwaite
3e2b3ee25f Hand back in-progress handshakes after a session resumption.
And since there are now 3 different points in the state machine where
a handback can occur, introduce an enum to describe them.

Change-Id: I41866214c39d27d1bbd965d28eb122c0e1f9902a
Reviewed-on: https://boringssl-review.googlesource.com/28344
Commit-Queue: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-06-06 21:17:36 +00:00
David Benjamin
5267ef7b4a Reject unexpected application data in bidirectional shutdown.
Update-Note: This tweaks the SSL_shutdown behavior. OpenSSL's original
SSL_shutdown behavior was an incoherent mix of discarding the record and
rejecting it (it would return SSL_ERROR_SYSCALL but retrying the
operation would discard it). SSLeay appears to have intended to discard
it, so we previously "fixed" it actually discard.

However, this behavior is somewhat bizarre and means we skip over
unbounded data, which we typically try to avoid. If you are trying to
cleanly shutdown the TLS portion of your protocol, surely it is at a
point where additional data is a syntax error. I suspect I originally
did not realize that, because the discarded record did not properly
continue the loop, SSL_shutdown would appear as if it rejected the data,
and so it's unlikely anyone was relying on that behavior.

Discussion in https://github.com/openssl/openssl/pull/6340 suggests
(some of) upstream also prefers rejecting.

Change-Id: Icde419049306ed17eb06ce1a7e1ff587901166f3
Reviewed-on: https://boringssl-review.googlesource.com/28864
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-06-04 21:39:58 +00:00
Adam Langley
a307cb7d58 Preliminary support for compressed certificates.
This change adds server-side support for compressed certificates.

(Although some definitions for client-side support are included in the
headers, there's no code behind them yet.)

Change-Id: I0f98abf0b782b7337ddd014c58e19e6b8cc5a3c2
Reviewed-on: https://boringssl-review.googlesource.com/27964
Reviewed-by: David Benjamin <davidben@google.com>
2018-06-04 21:24:20 +00:00
David Benjamin
c1e4f338b1 Use std::thread in thread_test.cc.
The STL already came up with a threading abstraction for us. If this
sticks, that also means we can more easily write tests elsewhere that
use threads. (A test that makes a bunch of TLS connections on a shared
SSL_CTX run under TSan would be nice. Likewise with some of the messy
RSA locking.)

Update-Note: This adds a dependency from crypto_test to C++11 threads.
Hopefully it doesn't cause issues.

Change-Id: I26f89f6b3b79240e516017877d06fd9a815fc315
Reviewed-on: https://boringssl-review.googlesource.com/28865
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-06-04 17:32:48 +00:00
Adam Langley
1627871d18 Include bn/internal.h for RSAZ code.
When building files separately, omitting this causes some #defines to be
missing.

Change-Id: I235231467d3f51ee0a53325698356aefa72c6a67
Reviewed-on: https://boringssl-review.googlesource.com/28944
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-06-04 17:26:29 +00:00
David Benjamin
7bf0bccd61 Add missing <condition_variable> include.
Change-Id: I07040cabcef191f0ab4a7b0e9bd4d46b37b09169
std::condition_variable has its own header to include.
Reviewed-on: https://boringssl-review.googlesource.com/28904
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-06-04 15:08:29 +00:00
David Benjamin
caf8ddd0ba Add SSL_SESSION_set1_id.
This matches the OpenSSL 1.1.0 spelling. I'd thought we could hide
SSL_SESSION this pass, but I missed one test that messed with session
IDs!

Bug: 6
Change-Id: I84ea113353eb0eaa2b06b68dec71cb9061c047ca
Reviewed-on: https://boringssl-review.googlesource.com/28866
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-06-04 14:25:28 +00:00
David Benjamin
81a6f6d8de Add a tool to check for filename collisions.
GN does not like multiple files in the same target that share a name, so
add a script to check for this. A follow-up changes will hook that up to
the builders, so we'll flag this in try jobs rather than when the change
trickles downstream.

Change-Id: Ic413dd9aeed6da54fc85dea07f80fe7084be9e9e
Reviewed-on: https://boringssl-review.googlesource.com/28844
Reviewed-by: Adam Langley <agl@google.com>
2018-05-31 19:02:43 +00:00
David Benjamin
fe7a17440f Fix typo.
Change-Id: Id7d8c8acf2f441dc34be7d363fb4dd2dfcb0e1c4
Reviewed-on: https://boringssl-review.googlesource.com/28804
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-05-30 15:42:35 +00:00
David Benjamin
a827d1809c Match OpenSSL's EVP_MD_CTX_reset return value.
In neither OpenSSL nor BoringSSL can this function actually fail, but
OpenSSL makes it return one anyway. Match them for compatibility.

Change-Id: I497437321ad9ccc5da738f06cd5b19c467167575
Reviewed-on: https://boringssl-review.googlesource.com/28784
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-05-29 17:07:16 +00:00
David Benjamin
9229b4fb86 Fix typo in build flags.
Change-Id: Ifa2f9f6ae00e6af5ce22254496ed37e2774b79e8
Reviewed-on: https://boringssl-review.googlesource.com/28747
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-05-29 14:33:16 +00:00
David Benjamin
700631bdf0 Pack encrypted handshake messages together.
We have a successful TLS 1.3 deployment, in spite of non-compliant
middleboxes everywhere, so now let's get this optimization in. It would
have been nice to test with this from the beginning, but sadly we forgot
about it. Ah well. This shaves 63 bytes off the server's first flight,
and then another 21 bytes off the pair of NewSessionTickets.

So we'll more easily notice in case of anything catastrophic, tie this
behavior to draft 28.

Update-Note: This slightly tweaks our draft-28 behavior.

Change-Id: I4f176a919bf7181239d6ebb31e7870f12364e0f9
Reviewed-on: https://boringssl-review.googlesource.com/28744
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-05-29 14:28:56 +00:00
David Benjamin
81d4a03bb0 Update tools.
Change-Id: I290af4599231040887f41c922550bb87dfb98cb0
Reviewed-on: https://boringssl-review.googlesource.com/28746
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-05-29 14:24:35 +00:00
David Benjamin
f86693dff7 Document the correct nonce length for AES-GCM.
It would be nice to restrict these, limiting the incorrect sizes to a
separate EVP_AEAD, but start by documenting this.

Bug: 34
Change-Id: I09845882f76a53a010355ceefd168d4fc10a0681
Reviewed-on: https://boringssl-review.googlesource.com/28745
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-05-24 22:13:07 +00:00
David Benjamin
5601bdac1a Rename crypto/rsa_extra/print.c.
It appears Chromium still gets upset when two files in a target share a
base name.

Change-Id: I9e6f182d97405e7e70b2bcf8ced7c80ba23edca1
Reviewed-on: https://boringssl-review.googlesource.com/28724
Reviewed-by: Adam Langley <agl@google.com>
2018-05-23 22:36:14 +00:00
Adam Langley
990a32327c Add --embed_test_data=false option to generate_build_files.py
This allows consumers not to use crypto_test_data.cc (which embeds all
the test files), although they'll have to provide their own
implementation of that functionality.

Change-Id: I309d5b3bd9495137e1df788b34048794b0072f3b
Reviewed-on: https://boringssl-review.googlesource.com/28706
Reviewed-by: David Benjamin <davidben@google.com>
2018-05-22 17:24:42 +00:00
Adam Langley
239c05a782 Allow convert_wycheproof.go to be used one file at a time.
Change-Id: I3c1d77ac9dea6faefc3711e84cf93191f35fe755
Reviewed-on: https://boringssl-review.googlesource.com/28705
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-05-22 17:19:45 +00:00
Adam Langley
576b637861 Move convert_wycheproof.go to util/
This file is not part of the Wycheproof project and consumers of
BoringSSL who wish to provide Wycheproof themselves (and not have
third_party/wycheproof_testvectors) need it in another location.

Change-Id: I730fe294f46a9aac77b858a91a03ee64fb8ea579
Reviewed-on: https://boringssl-review.googlesource.com/28704
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-05-22 17:16:36 +00:00
Adam Vartanian
189270cd19 Ignore Spectre mitigation warning.
VS2017 has added a new warning that indicates where Spectre mitigation
code would be inserted if /Qspectre were specified.

Change-Id: If80cd6a7d0c5a45313f4c3644b304cadecf465b0
Reviewed-on: https://boringssl-review.googlesource.com/28684
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-05-22 15:26:15 +00:00
David Benjamin
c93724b530 Benchmark TLS AES-CBC ciphers in both directions.
Between CBC being only parallelizable in one direction, bsaes vs vpaes,
and the Lucky 13 fix, seal and open look very different here. Benchmark
both directions.

Change-Id: I9266ab2800adc29dbeee0ca74502addb92409e23
Reviewed-on: https://boringssl-review.googlesource.com/28644
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-05-21 23:08:46 +00:00
Brian Smith
fee8709f69 Replace |alloca| in |BN_mod_exp_mont_consttime|.
|alloca| is dangerous and poorly specified, according to any
description of |alloca|. It's also hard for some analysis tools to
reason about.

The code here assumed |alloca| is a macro, which isn't a valid
assumption. Depending on what which headers are included and what
toolchain is being used, |alloca| may or may not be defined as a macro,
and this might change over time if/when toolchains are updated. Or, we
might be doing static analysis and/or dynamic analysis with a different
configuration w.r.t. the availability of |alloca| than production
builds use.

Regardless, the |alloca| code path only kicked in when the inputs are
840 bits or smaller. Since the multi-prime RSA support was removed, for
interesting RSA key sizes the input will be at least 1024 bits and this
code path won't be triggered since powerbufLen will be larger than 3072
bytes in those cases. ECC inversion via Fermat's Little Theorem has its
own constant-time exponentiation so there are no cases where smaller
inputs need to be fast.

The RSAZ code avoids the |OPENSSL_malloc| for 2048-bit RSA keys.
Increasingly the RSAZ code won't be used though, since it will be
skipped over on Broadwell+ CPUs. Generalize the RSAZ stack allocation
to work for non-RSAZ code paths. In order to ensure this doesn't cause
too much stack usage on platforms where RSAZ wasn't already being used,
only do so on x86-64, which already has this large stack size
requirement due to RSAZ.

This change will make it easier to refactor |BN_mod_exp_mont_consttime|
to do that more safely and in a way that's more compatible with various
analysis tools.

This is also a step towards eliminating the |uintptr_t|-based alignment
hack.

Since this change increases the number of times |OPENSSL_free| is
skipped, I've added an explicit |OPENSSL_cleanse| to ensure the
zeroization is done. This should be done regardless of the other changes
here.

Change-Id: I8a161ce2720a26127e85fff7513f394883e50b2e
Reviewed-on: https://boringssl-review.googlesource.com/28584
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-05-21 19:43:05 +00:00
Adam Langley
63e2a08123 Spell Falko Strenzke's name correctly.
Thanks to Brian Smith for pointing this out.

Change-Id: I27ae58df0028bc6aa3a11741acb5453369e202cc
Reviewed-on: https://boringssl-review.googlesource.com/28625
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-05-21 18:18:04 +00:00
David Benjamin
982279b366 Add a PKCS#12 fuzzer.
Change-Id: Iee3a3d46d283bd6cbb46940e630916aacdd71db6
Reviewed-on: https://boringssl-review.googlesource.com/28552
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-05-15 23:58:32 +00:00
David Benjamin
2f5100e629 More compatibility stuff.
cryptography.io wants things exposed out of EVP_get_cipherby* including,
sadly, ECB mode.

Change-Id: I9bac46f8ffad1a79d190cee3b0c0686bf540298e
Reviewed-on: https://boringssl-review.googlesource.com/28464
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-05-15 23:57:53 +00:00
David Benjamin
9b2c6a93e5 Extract friendly names attached to certificates.
OpenSSL staples each certificate's friendly name to the X509 with
X509_alias_set1. Mimic this. pyOpenSSL expects to find it there.

Update-Note: We actually parse some attributes now. PKCS#12 files with
malformed ones may not parse.

Change-Id: I3b78958eedf195509cd222ea4f0c884be3753770
Reviewed-on: https://boringssl-review.googlesource.com/28551
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-05-15 23:44:53 +00:00
David Benjamin
22ae0b8577 Try both null and empty passwords when decoding PKCS#12.
PKCS#12 encodes passwords as NUL-terminated UCS-2, so the empty password
is encoded as {0, 0}. Some implementations use the empty byte array for
"no password". OpenSSL considers a non-NULL password as {0, 0} and a
NULL password as {}. It then, in high-level PKCS#12 parsing code, tries
both options.

Match this behavior to appease pyOpenSSL's tests.

Change-Id: I07ef91d54454b6f2647f86b7eb9b13509b2876d3
Reviewed-on: https://boringssl-review.googlesource.com/28550
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-05-15 23:41:32 +00:00
David Benjamin
910320a3a0 Restore some revocation-related X.509 extensions.
These are tied to OPENSSL_NO_OCSP in upstream but do not actually depend
on most of the OCSP machinery. The CRL invdate extension, in particular,
isn't associated with OCSP at all. cryptography.io gets upset if these
two extensions aren't parseable, and they're tiny.

I do not believe this actually affects anything beyond functions like
X509_get_ext_d2i. In particular, the list of NIDs for the criticality
check is elsewhere.

Change-Id: I889f6ebf4ca4b34b1d9ff15f45e05878132826a1
Reviewed-on: https://boringssl-review.googlesource.com/28549
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2018-05-15 23:36:08 +00:00
David Benjamin
db196aab50 Distinguish unrecognized SPKI/PKCS8 key types from syntax errors.
Change-Id: Ia24aae31296772e2ddccf78f10a6640da459adf7
Reviewed-on: https://boringssl-review.googlesource.com/28548
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2018-05-15 23:36:02 +00:00
Adam Langley
91254c244c Rename |asm_AES_*| to |aes_nohw_*|.
Rather than have plain-C functions, asm functions, and accelerated
functions, just have accelerated and non-accelerated, where the latter
are either provided by assembly or by C code.

Pertinently, this allows Aarch64 to use hardware accel for the basic
|AES_*| functions.

Change-Id: I0003c0c7a43d85a3eee8c8f37697f61a3070dd40
Reviewed-on: https://boringssl-review.googlesource.com/28385
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-05-15 23:02:52 +00:00
David Benjamin
d12f2ba55e Tweak RSA errors for compatibility.
cryptography.io wants RSA_R_BLOCK_TYPE_IS_NOT_02, only used by the
ancient RSA_padding_check_SSLv23 function. Define it but never emit it.

Additionally, it's rather finicky about RSA_R_TOO_LARGE* errors. We
merged them in BoringSSL because having RSA_R_TOO_LARGE,
RSA_R_TOO_LARGE_FOR_MODULUS, and RSA_R_TOO_LARGE_FOR_KEY_SIZE is a
little silly. But since we don't expect well-behaved code to condition
on error codes anyway, perhaps that wasn't worth it.  Split them back
up.

Looking through OpenSSL, there is a vague semantic difference:

RSA_R_DIGEST_TOO_BIG_FOR_RSA_KEY - Specifically emitted if a digest is
too big for PKCS#1 signing with this key.

RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE - You asked me to sign or encrypt a
digest/plaintext, but it's too big for this key.

RSA_R_DATA_TOO_LARGE_FOR_MODULUS - You gave me an RSA ciphertext or
signature and it is not fully reduced modulo N.
-OR-
The padding functions produced something that isn't reduced, but I
believe this is unreachable outside of RSA_NO_PADDING.

RSA_R_DATA_TOO_LARGE - Some low-level padding function was told to copy
a digest/plaintext into some buffer, but the buffer was too small. I
think this is basically unreachable.
-OR-
You asked me to verify a PSS signature, but I didn't need to bother
because the digest/salt parameters you picked were too big.

Update-Note: This depends on cl/196566462.
Change-Id: I2e539e075eff8bfcd52ccde365e975ebcee72567
Reviewed-on: https://boringssl-review.googlesource.com/28547
Reviewed-by: Adam Langley <agl@google.com>
2018-05-15 23:02:49 +00:00
David Benjamin
fa544f1c05 Reject if the ALPN callback returned an empty protocol.
If the callback returns an empty ALPN, we forget we negotiated ALPN at
all (bssl::Array does not distinguish null and empty). Empty ALPN
protocols are forbidden anyway, so reject these ahead of time.

Change-Id: I42f1fc4c843bc865e23fb2a2e5d57424b569ee99
Reviewed-on: https://boringssl-review.googlesource.com/28546
Reviewed-by: Adam Langley <agl@google.com>
2018-05-15 23:02:39 +00:00
Daniel Hirche
e6737a8656 x509_test: Fix gcc-8 build
gcc-8 complains that struct Test shadows class Test from googletest.

Change-Id: Ie0c61eecebc726973c6aaa949e338da3d4474977
Reviewed-on: https://boringssl-review.googlesource.com/28524
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-05-15 22:58:22 +00:00
David Benjamin
911cc0a0aa The legacy client OCSP callback should run without server OCSP.
It's conditioned in OpenSSL on client offer, not server accept.

Change-Id: Iae5483a33d9365258446ce0ae34132aeb4a92c66
Reviewed-on: https://boringssl-review.googlesource.com/28545
Reviewed-by: Adam Langley <agl@google.com>
2018-05-15 22:57:41 +00:00