This is analogous to the Go stack's handshakeErr field. Since it's quite
common for callers to run two I/O operations in parallel[*] like
SSL_read and SSL_write (or SSL_read and SSL_do_handshake for client
0-RTT). Accordingly, the new handshake state machine jams itself up on
handshake error, but to fully work with such callers, we should also
replay the error state.
This doesn't yet catch all cases (there are some parts of the read flow
which need to be fixed). Those will be resolved in later changes.
[*] Not actually in parallel, of course, but logically in parallel on a
non-blocking socket.
Bug: 206
Change-Id: I5a4d37a258b9e3fc555b732938b0528b839650f8
Reviewed-on: https://boringssl-review.googlesource.com/21285
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>
SSL_CTX_sessions is the only think making us expose LHASH as public API
and nothing uses it. Nothing can use it anyway as it's not thread-safe.
I haven't actually removed it yet since SSL_CTX is public, but once the
types are opaque, we could trim the number of symbols ssl.h pulls in
with some work.
Relatedly, fix thread safety of SSL_CTX_sess_number.
Change-Id: I75a6c93509d462cd5ed3ce76c587f0d1e7cd0797
Reviewed-on: https://boringssl-review.googlesource.com/20804
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>
The Java client implementation of the 3SHAKE mitigation incorrectly
rejects initial handshakes when all of the following are true:
1. The ClientHello offered a session.
2. The session was successfully resumed previously.
3. The server declines the session.
4. The server sends a certificate with a different SAN list than in the
previous session.
(Note the 3SHAKE mitigation is to reject certificates changes on
renegotiation, while Java's logic applies to initial handshakes as
well.)
The end result is long-lived Java clients break on some certificate
rotations. Fingerprint Java clients and decline all offered sessions.
This avoids (2) while still introducing new sessions to clear any
existing problematic sessions.
See also b/65323005.
Change-Id: Ib2b84c69b5ecba285ffb8c4d03de5626838d794e
Reviewed-on: https://boringssl-review.googlesource.com/20184
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>
Right now we report the per-connection value during the handshake and
the per-session value after the handshake. This also trims our tickets
slightly by removing a largely unused field from SSL_SESSION.
Putting it on SSL_HANDSHAKE would be better, but sadly a number of
bindings-type APIs expose it after the handshake.
Change-Id: I6a1383f95da9b1b141b9d6adadc05ee1e458a326
Reviewed-on: https://boringssl-review.googlesource.com/20064
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>
The ticket encryption key is rotated automatically once every 24 hours,
unless a key has been configured manually (i.e. using
|SSL_CTX_set_tlsext_ticket_keys|) or one of the custom ticket encryption
methods is used.
Change-Id: I0dfff28b33e58e96b3bbf7f94dcd6d2642f37aec
Reviewed-on: https://boringssl-review.googlesource.com/18924
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This allows us to fix another consumer that directly accesses SSL_CTX.
I've made ssl_test use it for test coverage, though we're okay with
ssl_test depending on ssl/internal.h.
Bug: 6
Change-Id: I464325e3faa9f0194bbd357fbb31a996afc0c2e1
Reviewed-on: https://boringssl-review.googlesource.com/18964
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Similarly, add EVP_AEAD_CTX_tag_len which computes the exact tag length
for required by EVP_AEAD_CTX_seal_scatter.
Change-Id: I069b0ad16fab314fd42f6048a3c1dc45e8376f7f
Reviewed-on: https://boringssl-review.googlesource.com/18324
Reviewed-by: Adam Langley <agl@google.com>
OpenSSL allows spaces, commas and semi-colons to be used as separators
in cipher strings, in addition to the usual colons.
This change documents that spaces cannot be used in equal-preference
groups and forbids these alternative separators in strict mode.
Change-Id: I3879e25aed54539c281511627e6a282e9463bdc3
Reviewed-on: https://boringssl-review.googlesource.com/18424
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
SSL_get0_peer_certificates is documented to return NULL if the peer was
anonymous, but it actually returns a non-NULL empty list (except in SSL
3.0 where the Certificate message and thus ssl_parse_cert_chain is
skipped).
Make the implementation match the documentation.
Change-Id: Ib3e25d2155f316cc5e9eb3ab7f74b78e08b8a86b
Reviewed-on: https://boringssl-review.googlesource.com/18226
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This is a C++ interface for encrypting and decrypting TLS application
data records in-place, wrapping the existing C API in tls_record.cc.
Also add bssl::Span, a non-owning reference to a contiguous array of
elements which can be used as a common interface over contiguous
container types (like std::vector), pointer-length-pairs, arrays, etc.
Change-Id: Iaa2ca4957cde511cb734b997db38f54e103b0d92
Reviewed-on: https://boringssl-review.googlesource.com/18104
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The previous attempt around the 'struct ssl_st' compatibility mess
offended OSS-Fuzz and UBSan because one compilation unit passed a
function pointer with ssl_st* and another called it with
bssl::SSLConnection*.
Linkers don't retain such types, of course, but to silence this alert,
instead make C-visible types be separate from the implementation and
subclass the public type. This does mean we risk polluting the symbol
namespace, but hopefully the compiler is smart enough to inline the
visible struct's constructor and destructor.
Bug: 132
Change-Id: Ia75a89b3a22a202883ad671a630b72d0aeef680e
Reviewed-on: https://boringssl-review.googlesource.com/18224
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
This is horrible, but everything else I tried was worse. The goal with
this CL is to take the extern "C" out of ssl/internal.h and move most
symbols to namespace bssl, so we can start using C++ helpers and
destructors without worry.
Complications:
- Public API functions must be extern "C" and match their declaration in
ssl.h, which is unnamespaced. C++ really does not want you to
interleave namespaced and unnamespaced things. One can actually write
a namespaced extern "C" function, but this means, from C++'s
perspective, the function is namespaced. Trying to namespace the
public header would worked but ended up too deep a rabbithole.
- Our STACK_OF macros do not work right in namespaces.
- The typedefs for our exposed but opaque types are visible in the
header files and copied into consuming projects as forward
declarations. We ultimately want to give SSL a destructor, but
clobbering an unnamespaced ssl_st::~ssl_st seems bad manners.
- MSVC complains about ambiguous names if one typedefs SSL to bssl::SSL.
This CL opts for:
- ssl/*.cc must begin with #define BORINGSSL_INTERNAL_CXX_TYPES. This
informs the public headers to create forward declarations which are
compatible with our namespaces.
- For now, C++-defined type FOO ends up at bssl::FOO with a typedef
outside. Later I imagine we'll rename many of them.
- Internal functions get namespace bssl, so we stop worrying about
stomping the tls1_prf symbol. Exported C functions are stuck as they
are. Rather than try anything weird, bite the bullet and reorder files
which have a mix of public and private functions. I expect that over
time, the public functions will become fairly small as we move logic
to more idiomatic C++.
Files without any public C functions can just be written normally.
- To avoid MSVC troubles, some bssl types are renamed to CPlusPlusStyle
in advance of them being made idiomatic C++.
Bug: 132
Change-Id: Ic931895e117c38b14ff8d6e5a273e868796c7581
Reviewed-on: https://boringssl-review.googlesource.com/18124
Reviewed-by: David Benjamin <davidben@google.com>
This is needed to switch Chromium's SSLServerSocket and parts of
Conscrypt to CRYPTO_BUFFER.
Bug: 54
Change-Id: Iacd417970607bc1a162057676b576956a3bdfa3f
Reviewed-on: https://boringssl-review.googlesource.com/17965
Reviewed-by: Adam Langley <agl@google.com>
This also serves as a certificate verification callback for
CRYPTO_BUFFER-based consumers. Remove the silly
SSL_CTX_i_promise_to_verify_certs_after_the_handshake placeholder.
Bug: 54, chromium:347402
Change-Id: I4c6b445cb9cd7204218acb2e5d1625e6f37aff6f
Reviewed-on: https://boringssl-review.googlesource.com/17964
Reviewed-by: David Benjamin <davidben@google.com>
Both Conscrypt and Netty have a lot of logic to map between the two
kinds of names. WebRTC needed an SSL_CIPHER_get_rfc_name for something.
Just have both in the library. Also deprecate SSL_CIPHER_get_rfc_name
in favor of SSL_CIPHER_standard_name, which matches upstream if built
with enable-ssl-trace. And, unlike SSL_CIPHER_get_rfc_name, this does
not require dealing with the malloc.
(Strangely this decreases bssl's binary size, even though we're carrying
more strings around. It seems the old SSL_CIPHER_get_rfc_name was
somewhat large in comparison. Regardless, a consumer that disliked 30
short strings probably also disliked the OpenSSL names. That would be
better solved by opaquifying SSL_CIPHER and adding a less stringy API
for configuring cipher lists. That's something we can explore later if
needed.)
I also made the command-line tool print out the standard names since
they're more standard. May as well push folks towards those going
forward.
Change-Id: Ieeb3d63e67ef4da87458e68d130166a4c1090596
Reviewed-on: https://boringssl-review.googlesource.com/17324
Reviewed-by: Robert Sloan <varomodt@google.com>
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>
As a precursor to removing the code entirely later, disable the protocol
by default. Callers must use SSL_CTX_set_min_version to enable it.
This change also makes SSLv3_method *not* enable SSL 3.0. Normally
version-specific methods set the minimum and maximum version to their
version. SSLv3_method leaves the minimum at the default, so we will
treat it as all versions disabled. To help debugging, the error code is
switched from WRONG_SSL_VERSION to a new NO_SUPPORTED_VERSIONS_ENABLED.
This also defines OPENSSL_NO_SSL3 and OPENSSL_NO_SSL3_METHOD to kick in
any no-ssl3 build paths in consumers which should provide a convenient
hook for any upstreaming changes that may be needed. (OPENSSL_NO_SSL3
existed in older versions of OpenSSL, so in principle one may encounter
an OpenSSL with the same settings.)
Change-Id: I96a8f2f568eb77b2537b3a774b2f7108bd67dd0c
Reviewed-on: https://boringssl-review.googlesource.com/14031
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The |select_certificate_cb| return values are somewhat confusing due
to the fact that they don't match the |cert_cb| ones, despite the
similarities between the two callbacks (they both have "certificate" in
the name! well, sort of).
This also documents the error return value (-1) which was previously
undocumented, and it expands the |SSL_CTX_set_select_certificate_cb|
documentation regarding retrial (by shamelessly copying from
|SSL_CTX_set_ticket_aead_method|).
Also updates other scattered documentation that was missed by previous
changes.
Change-Id: Ib962b31d08e6475e09954cbc3c939988b0ba13f7
Reviewed-on: https://boringssl-review.googlesource.com/14245
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This shuffles a bit of the code around session resumption in TLS 1.3 to
make the async point cleaner to inject. It also fills in cipher and
tlsext_hostname more uniformly.
Filling in the cipher on resumption is a no-op as SSL_SESSION_dup
already copies it, but avoids confusion should we ever implement TLS
1.3's laxer cipher matching on the server. Not filling in
tlsext_hostname on resumption was an oversight; the relevant check isn't
whether we are resuming but whether we have a fresh SSL_SESSION to fill
things into.
Change-Id: Ic02eb079ff228ce4a4d3e0de7445e18cd367e8b2
Reviewed-on: https://boringssl-review.googlesource.com/14205
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This change adds support for setting an |SSL_TICKET_AEAD_METHOD| which
allows a caller to control ticket encryption and decryption to a greater
extent than previously possible and also permits asynchronous ticket
decryption.
This change only includes partial support: TLS 1.3 work remains to be
done.
Change-Id: Ia2e10ebb3257e1a119630c463b6bf389cf20ef18
Reviewed-on: https://boringssl-review.googlesource.com/14144
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This change converts the CA names that are parsed from a server's
CertificateRequest, as well as the CA names that are configured for
sending to clients in the same, to use |CRYPTO_BUFFER|.
The |X509_NAME|-based interfaces are turned into compatibility wrappers.
Change-Id: I95304ecc988ee39320499739a0866c7f8ff5ed98
Reviewed-on: https://boringssl-review.googlesource.com/13585
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The new APIs are SSL_CTX_set_strict_cipher_list() and
SSL_set_strict_cipher_list(). They have two motivations:
First, typos in cipher lists can go undetected for a long time, and
can have surprising consequences when silently ignored.
Second, there is a tendency to use superstition in the construction of
cipher lists, for example by "turning off" things that do not actually
exist. This leads to the corrosive belief that DEFAULT and ALL ought
not to be trusted. This belief is false.
Change-Id: I42909b69186e0b4cf45457e5c0bc968f6bbf231a
Reviewed-on: https://boringssl-review.googlesource.com/13925
Commit-Queue: Matt Braithwaite <mab@google.com>
Reviewed-by: Matt Braithwaite <mab@google.com>
Recent changes added SSL-level setters to these APIs. Unfortunately,
this has the side effect of breaking SSL_set_SSL_CTX, which is how SNI
is typically handled. SSL_set_SSL_CTX is kind of a weird function in
that it's very sensitive to which of the hodge-podge of config styles is
in use. I previously listed out all the config styles here, but it was
long and unhelpful. (I counted up to 7.)
Of the various SSL_set_SSL_CTX-visible config styles, the sanest seems
to be to move it to CERT. In this case, it's actually quite reasonable
since they're very certificate-related.
Later we may wish to think about whether we can cut down all 7 kinds of
config styles because this is kinda nuts. I'm wondering we should do
CERT => SSL_CONFIG, move everything there, and make that be the same
structure that is dropped post-handshake (supposing the caller has
disavowed SSL_clear and renego). Fruit for later thought. (Note though
that comes with a behavior change for all the existing config.)
Change-Id: I9aa47d8bd37bf2847869e0b577739d4d579ee4ae
Reviewed-on: https://boringssl-review.googlesource.com/13864
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
|SSL_SESSION_from_bytes| now takes an |SSL_CTX*|, from which it uses the
|X509_METHOD| and buffer pool. This is our API so we can do this.
This also requires adding an |SSL_CTX*| argument to |SSL_SESSION_new|
for the same reason. However, |SSL_SESSION_new| already has very few
callers (and none in third-party code that I can see) so I think we can
get away with this.
Change-Id: I1337cd2bd8cff03d4b9405ea3146b3b59584aa72
Reviewed-on: https://boringssl-review.googlesource.com/13584
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
David Benjamin