Having to lazily create it is a little wordy, and we append to it in
three places now. V2ClientHello makes this slightly finicky, but I think
this is still clearer.
Change-Id: If931db0b56efd7f0728c0b7d119886864dd7933a
Reviewed-on: https://boringssl-review.googlesource.com/c/32824
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>
A split SSL handshake may involve 2 binaries, potentially built at
different versions: call them the "handoff/handback" binary and the
"handshake" binary. We would like to guarantee that the
handoff/handback binary does not make any promises that the handshake
binary cannot keep.
d2ed382 serialized |kCiphers|; this commit extends the same approach
to |kNamedGroups|.
Change-Id: Idb13e54e9b189236309f6054a36872c5a4d96985
Reviewed-on: https://boringssl-review.googlesource.com/c/32825
Reviewed-by: David Benjamin <davidben@google.com>
A split SSL handshake may involve 2 binaries, potentially built at
different versions: call them the "handoff/handback" binary and the
"handshake" binary. We would like to guarantee that the
handoff/handback binary does not make any promises that the handshake
binary cannot keep.
As a start, this commit serializes |kCiphers| to the handoff message.
When the handoff message is applied to an |SSL|, any configured
ciphers not listed in the handoff message will be removed, in order to
prevent them from being negotiated.
Subsequent commits will apply the same approach to other lists of features.
Change-Id: Idf6dbeadb750c076ab0509c09b9d3f22eb162b9c
Reviewed-on: https://boringssl-review.googlesource.com/c/29264
Reviewed-by: Matt Braithwaite <mab@google.com>
0-RTT support and APIs to consume NewSessionTicket will be added in a
follow-up.
Change-Id: Ib2b2c6b618b3e33a74355fb53fdbd2ffafcc5c56
Reviewed-on: https://boringssl-review.googlesource.com/c/31744
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>
Reviewed-by: David Benjamin <davidben@google.com>
As with sk_*, this. This doesn't fix the function pointer casts. Those
will be done in a follow-up change. Also add a test for lh_*_doall so we
cover both function pointer shapes.
Update-Note: This reworks how LHASH_OF(T) is implemented and also only
pulls in the definitions where used, but LHASH_OF(T) is never used
externally, so I wouldn't expect this to affect things.
Change-Id: I7970ce8c41b8589d6672b71dd03658d0e3bd89a7
Reviewed-on: https://boringssl-review.googlesource.com/c/32119
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>
Due to non-compliant middleboxes, it is possible we'll need to do some
surgery to this mechanism. Making it per-SSL is a little more flexible
and also eases some tests in Chromium until we get its SSL_CTX usage
fixed up.
Also fix up BoringSSL tests. We forgot to test it at TLS 1.0 and use the
-expect-tls13-downgrade flag.
Bug: 226
Change-Id: Ib39227e74e2d6f5e1fbc1ebcc091e751471b3cdc
Reviewed-on: https://boringssl-review.googlesource.com/c/32424
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>
This was added to support the no_certificate warning alert in SSLv3. That has
since been removed. In the long run, I would like for ssl_send_alert to go
through a flow similar to add_alert so the BIO-free APIs work right and avoid a
host of strangeness surrounding wpend_buf. For now, remove the unused hook.
Change-Id: I1995028b8af4ffa836028794e6b33b2cd1b2435b
Reviewed-on: https://boringssl-review.googlesource.com/31984
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>
- In base.h, if BORINGSSL_PREFIX is defined, include
boringssl_prefix_symbols.h
- In all .S files, if BORINGSSL_PREFIX is defined, include
boringssl_prefix_symbols_asm.h
- In base.h, BSSL_NAMESPACE_BEGIN and BSSL_NAMESPACE_END are
defined with appropriate values depending on whether
BORINGSSL_PREFIX is defined; these macros are used in place
of 'namespace bssl {' and '}'
- Add util/make_prefix_headers.go, which takes a list of symbols
and auto-generates the header files mentioned above
- In CMakeLists.txt, if BORINGSSL_PREFIX and BORINGSSL_PREFIX_SYMBOLS
are defined, run util/make_prefix_headers.go to generate header
files
- In various CMakeLists.txt files, add "global_target" that all
targets depend on to give us a place to hook logic that must run
before all other targets (in particular, the header file generation
logic)
- Document this in BUILDING.md, including the fact that it is
the caller's responsibility to provide the symbol list and keep it
up to date
- Note that this scheme has not been tested on Windows, and likely
does not work on it; Windows support will need to be added in a
future commit
Change-Id: If66a7157f46b5b66230ef91e15826b910cf979a2
Reviewed-on: https://boringssl-review.googlesource.com/31364
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>
Update-Note: If not explicitly configured to use tls13_all, callers that enable
TLS 1.3 will now only enable the final standard version.
Change-Id: Ifcfc65a9d8782c983df6e002925e8f77f45b6e53
Reviewed-on: https://boringssl-review.googlesource.com/31384
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>
Change-Id: I2d1671a4f21a602191fd0c9b932244a376ac5713
Reviewed-on: https://boringssl-review.googlesource.com/31104
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>
This isn't actually shorter, but there is a bunch of slicing up of the ticket,
which Span makes a little easier to follow.
Change-Id: I7ea4dfe025641a3b88e2c9b8e34246fefc23412f
Reviewed-on: https://boringssl-review.googlesource.com/29865
Reviewed-by: Adam Langley <agl@google.com>
Fixed up the comment about ssl_version. There's one line which I
manually edited:
int (*check_client_CA_list)(STACK_OF(CRYPTO_BUFFER) *names);
where clang-format puts spaces on both sides of the second *.
Change-Id: Id1c0bd02f664df14b1e5bbd8abaf2687fb9179db
Reviewed-on: https://boringssl-review.googlesource.com/31004
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>
Works in the 1.3 and 1.2 client handshakes, not implemented on the
server for now.
Creates an SSL_CTX option to reverify the server certificate on session
resumption. Reverification only runs the client's certificate verify callback.
Adds new states to the client handshakes: state_reverify_server_certificate in
TLS 1.2, and state_server_certificate_reverify in TLS 1.3.
Adds a negative test to make sure that by default we don't verify the
certificate on resumption, and positive tests that make sure we do when the
new option is set.
Change-Id: I3a47ff3eacb3099df4db4c5bc57f7c801ceea8f1
Bug: chromium:347402
Reviewed-on: https://boringssl-review.googlesource.com/29984
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>
These functions can be used to configure the signature algorithms. One
of them is a string mini-languaging parsing function, which we generally
dislike because it defeats static analysis. However, some dependent
projects (in this case TensorFlow) need it and we also dislike making
people patch.
Change-Id: I13f990c896a7f7332d78b1c351357d418ade8d11
Reviewed-on: https://boringssl-review.googlesource.com/30304
Reviewed-by: Steven Valdez <svaldez@google.com>
This helps with creating a separate binary to perform split
handshakes, in that the test state must be communicated to, and
retrieved from, the handshaker binary using a socket.
Change-Id: I9d70a9bb3d97dd339aab4f51c6de75f71e4fe72d
Reviewed-on: https://boringssl-review.googlesource.com/29704
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>
Mostly in comments, but there is one special-case around renegotiation_info
that can now be removed.
Change-Id: I2a9114cbff05e0cfff95fe93270fe42379728012
Reviewed-on: https://boringssl-review.googlesource.com/29824
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
With SSL 3.0 gone, there's no need to split up MD5 and SHA-1.
Change-Id: Ia4236c738dfa6743f1028c2d53761c95cba96288
Reviewed-on: https://boringssl-review.googlesource.com/29744
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>
While I'm here, remove the silly "tlsext_" prefix. At this point it's no
longer novel that a feature is encoded in an extension.
Change-Id: Ib5fbd2121333a213bdda0332885a8c90036ebc4d
Reviewed-on: https://boringssl-review.googlesource.com/29592
Reviewed-by: Adam Langley <agl@google.com>
This doesn't actually make use of much of C++ yet. (SSL_CTX and
SSL/SSL_CONFIG carry analogous versions of a number of fields. It's
difficult to switch them to UniquePtr separately.)
Change-Id: Ia948f539c5c90e2d8301193f719604a31be17fc4
Reviewed-on: https://boringssl-review.googlesource.com/29589
Reviewed-by: Adam Langley <agl@google.com>
This doesn't give them a destructor yet, just shifts things around. In
doing so, it reveals that we inconsistently allowed internal code, but
not external code, to call functions like bssl::SSL_CTX_set_handoff_mode
without a namespace because of ADL. External code doesn't get to do
this because it doesn't see that ssl_ctx_st has a base class in
namespace bssl.
Change-Id: I2ab3b00fff2d6369e850606eed63017e4f8cf8c4
Reviewed-on: https://boringssl-review.googlesource.com/29588
Reviewed-by: Adam Langley <agl@google.com>
It's 2018, but passing STL objects across the API boundary turns out to
still be more bother than it's worth. Since we're dropping UniquePtr in
the API anyway, go the whole way and make it a plain-C API.
Change-Id: Ic0202012e5d81afe62d71b3fb57e6a27a8f63c65
Update-note: this will need corresponding changes to the internal use of SSL_CTX_add_cert_compression_alg.
Reviewed-on: https://boringssl-review.googlesource.com/29564
Reviewed-by: David Benjamin <davidben@google.com>
Previously we'd partially attempted the ssl_st / bssl::SSLConnection
subclassing split, but that gets messy when we actually try to add a
destructor, because CRYPTO_EX_DATA's cleanup function needs an ssl_st*,
not a bssl::SSLConnection*. Downcasting is technically undefined at this
point and will likely offend some CFI-like check.
Moreover, it appears that even with today's subclassing split,
New<SSL>() emits symbols like:
W ssl_st*& std::forward<ssl_st*&>(std::remove_reference<ssl_st*&>::type&)
The compiler does not bother emitting them in optimized builds, but it
does suggest we can't really avoid claiming the ssl_st type name at the
symbol level, short of doing reinterpret_casts at all API boundaries.
And, of course, we've already long claimed it at the #include level.
So I've just left this defining directly on ssl_session_st. The cost is
we need to write some silly "bssl::" prefixes in the headers, but so it
goes. In the likely event we change our minds again, we can always
revise this.
Change-Id: Ieb429e8eaabe7c2961ef7f8d9234fb71f19a5e2a
Reviewed-on: https://boringssl-review.googlesource.com/29587
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>
lh_FOO_retrieve is often called with a dummy instance of FOO that has
only a few fields filled in. This works fine for C, but a C++
SSL_SESSION with destructors is a bit more of a nuisance here.
Instead, teach LHASH to allow queries by some external key type. This
avoids stack-allocating SSL_SESSION. Along the way, fix the
make_macros.sh script.
Change-Id: Ie0b482d4ffe1027049d49db63274c7c17f9398fa
Reviewed-on: https://boringssl-review.googlesource.com/29586
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>
Update-Note: SSL_CTX_set_min_proto_version(SSL3_VERSION) now fails.
SSL_OP_NO_SSLv3 is now zero. Internal SSL3-specific "AEAD"s are gone.
Change-Id: I34edb160be40a5eea3e2e0fdea562c6e2adda229
Reviewed-on: https://boringssl-review.googlesource.com/29444
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>
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>
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>
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>
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>
Previously, we'd omitted OpenSSL's OCSP APIs because they depend on a
complex OCSP mechanism and encourage the the unreliable server behavior
that hampers using OCSP stapling to fix revocation today. (OCSP
responses should not be fetched on-demand on a callback. They should be
managed like other server credentials and refreshed eagerly, so
temporary CA outage does not translate to loss of OCSP.)
But most of the APIs are byte-oriented anyway, so they're easy to
support. Intentionally omit the one that takes a bunch of OCSP_RESPIDs.
The callback is benign on the client (an artifact of OpenSSL reading
OCSP and verifying certificates in the wrong order). On the server, it
encourages unreliability, but pyOpenSSL/cryptography.io depends on this.
Dcument that this is only for compatibility with legacy software.
Also tweak a few things for compatilibility. cryptography.io expects
SSL_CTX_set_read_ahead to return something, SSL_get_server_tmp_key's
signature was wrong, and cryptography.io tries to redefine
SSL_get_server_tmp_key if SSL_CTRL_GET_SERVER_TMP_KEY is missing.
Change-Id: I2f99711783456bfb7324e9ad972510be8a95e845
Reviewed-on: https://boringssl-review.googlesource.com/28404
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>
Along the way, check the version against the cipher to make sure the
combination is possible.
(Found by fuzzing: a bad version trips an assert.)
Change-Id: Ib0a284fd5fd9b7ba5ceba63aa6224966282a2cb7
Reviewed-on: https://boringssl-review.googlesource.com/27265
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>
These are also not needed after the handshake.
Change-Id: I5de2d5cf18a3783a6c04c0a8fe311069fb51b939
Reviewed-on: https://boringssl-review.googlesource.com/27986
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The TLS 1.3 client logic used ctx instead. This is all moot as
SSL_set_SSL_CTX on a client really wouldn't work, but we should be
consistent. Unfortunately, this moves moving the pointer back to SSL
from SSL_CONFIG.
Change-Id: I45f8241e16f499ad416afd5eceb52dc82af9c4f4
Reviewed-on: https://boringssl-review.googlesource.com/27985
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
All CBC ciphers in TLS are broken and insecure. TLS 1.2 introduced
AEAD-based ciphers which avoid their many problems. It also introduced
new CBC ciphers based on HMAC-SHA256 and HMAC-SHA384 that share the same
flaws as the original HMAC-SHA1 ones. These serve no purpose. Old
clients don't support them, they have the highest overhead of all TLS
ciphers, and new clients can use AEADs anyway.
Remove them from libssl. This is the smaller, more easily reverted
portion of the removal. If it survives a week or so, we can unwind a lot
more code elsewhere in libcrypto. This removal will allow us to clear
some indirect calls from crypto/cipher_extra/tls_cbc.c, aligning with
the recommendations here:
https://github.com/HACS-workshop/spectre-mitigations/blob/master/crypto_guidelines.md#2-avoid-indirect-branches-in-constant-time-code
Update-Note: The following cipher suites are removed:
- TLS_RSA_WITH_AES_128_CBC_SHA256
- TLS_RSA_WITH_AES_256_CBC_SHA256
- TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256
- TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384
- TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256
- TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384
Change-Id: I7ade0fc1fa2464626560d156659893899aab6f77
Reviewed-on: https://boringssl-review.googlesource.com/27944
Reviewed-by: Adam Langley <agl@google.com>
|SSL_CONFIG| is a container for bits of configuration that are
unneeded after the handshake completes. By default it is retained for
the life of the |SSL|, but it may be shed at the caller's option by
calling SSL_set_shed_handshake_config(). This is incompatible with
renegotiation, and with SSL_clear().
|SSL_CONFIG| is reachable by |ssl->config| and by |hs->config|. The
latter is always non-NULL. To avoid null checks, I've changed the
signature of a number of functions from |SSL*| arguments to
|SSL_HANDSHAKE*| arguments.
When configuration has been shed, setters that touch |SSL_CONFIG|
return an error value if that is possible. Setters that return |void|
do nothing.
Getters that request |SSL_CONFIG| values will fail with an |assert| if
the configuration has been shed. When asserts are compiled out, they
will return an error value.
The aim of this commit is to simplify analysis of split-handshakes by
making it obvious that some bits of state have no effects beyond the
handshake. It also cuts down on memory usage.
Of note: |SSL_CTX| is still reachable after the configuration has been
shed, and a couple things need to be retained only for the sake of
post-handshake hooks. Perhaps these can be fixed in time.
Change-Id: Idf09642e0518945b81a1e9fcd7331cc9cf7cc2d6
Bug: 123
Reviewed-on: https://boringssl-review.googlesource.com/27644
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>
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>
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>
