With SSL2 gone, there's no need for this split between the abstract
cipher framework and ciphers. Put the cipher suite table in ssl_cipher.c
and move other SSL_CIPHER logic there. With that gone, prune the
cipher-related hooks in SSL_PROTOCOL_METHOD.
BUG=468889
Change-Id: I48579de8bc4c0ea52781ba1b7b57bc5b4919d21c
Reviewed-on: https://boringssl-review.googlesource.com/4961
Reviewed-by: Adam Langley <agl@google.com>
They're redundant with each other.
Change-Id: I17e7ff8c4e0b1486986dd866fd99673fa2aaa494
Reviewed-on: https://boringssl-review.googlesource.com/4959
Reviewed-by: Adam Langley <agl@google.com>
All ciphers are implemented by an EVP_AEAD.
Change-Id: Ifa754599a34e16bf97e1a4b84a271c6d45462c7c
Reviewed-on: https://boringssl-review.googlesource.com/4958
Reviewed-by: Adam Langley <agl@google.com>
The ctrl hooks are left alone since they should just go away.
Simplifying the cipher story will happen in the next CL.
BUG=468889
Change-Id: I979971c90f59c55cd5d17554f1253158b114f18b
Reviewed-on: https://boringssl-review.googlesource.com/4957
Reviewed-by: Adam Langley <agl@google.com>
This still needs significant work, especially the close_notify half, but
clarify the interface and get *_read_bytes out of SSL_PROTOCOL_METHOD.
read_bytes is an implementation detail of those two and get_message
rather than both an implementation detail of get_message for handshake
and a (wholly inappropriate) exposed interface for the other two.
BUG=468889
Change-Id: I7dd23869e0b7c3532ceb2e9dd31ca25ea31128e7
Reviewed-on: https://boringssl-review.googlesource.com/4956
Reviewed-by: Adam Langley <agl@google.com>
The SSL_PROTOCOL_METHOD table needs work, but this makes it clearer
exactly what the shared interface between the upper later and TLS/DTLS
is.
BUG=468889
Change-Id: I38931c484aa4ab3f77964d708d38bfd349fac293
Reviewed-on: https://boringssl-review.googlesource.com/4955
Reviewed-by: Adam Langley <agl@google.com>
When the peer or caller requests a renegotiation, OpenSSL doesn't
renegotiate immediately. It sets a flag to begin a renegotiation as soon
as record-layer read and write buffers are clear. One reason is that
OpenSSL's record layer cannot write a handshake record while an
application data record is being written. The buffer consistency checks
around partial writes will break.
None of these cases are relevant for the client auth hack. We already
require that renego come in at a quiescent part of the application
protocol by forbidding handshake/app_data interleave.
The new behavior is now: when a HelloRequest comes in, if the record
layer is not idle, the renegotiation is rejected as if
SSL_set_reject_peer_renegotiations were set. Otherwise we immediately
begin the new handshake. The server may not send any application data
between HelloRequest and completing the handshake. The HelloRequest may
not be consumed if an SSL_write is pending.
Note this does require that Chromium's HTTP stack not attempt to read
the HTTP response until the request has been written, but the
renegotiation logic already assumes it. Were Chromium to drive the
SSL_read state machine early and the server, say, sent a HelloRequest
after reading the request headers but before we've sent the whole POST
body, the SSL state machine may racily enter renegotiate early, block
writing the POST body on the new handshake, which would break Chromium's
ERR_SSL_CLIENT_AUTH_CERT_NEEDED plumbing.
BUG=429450
Change-Id: I6278240c3bceb5d2e1a2195bdb62dd9e0f4df718
Reviewed-on: https://boringssl-review.googlesource.com/4825
Reviewed-by: Adam Langley <agl@google.com>
tls1_enc is now SSL_AEAD_CTX_{open,seal}. This starts tidying up a bit
of the record-layer logic. This removes rr->input, as encrypting and
decrypting records no longer refers to various globals. It also removes
wrec altogether. SSL3_RECORD is now only used to maintain state about
the current incoming record. Outgoing records go straight to the write
buffer.
This also removes the outgoing alignment memcpy and simply calls
SSL_AEAD_CTX_seal with the parameters as appropriate. From bssl speed
tests, this seems to be faster on non-ARM and a bit of a wash on ARM.
Later it may be worth recasting these open/seal functions to write into
a CBB (tweaked so it can be malloc-averse), but for now they take an
out/out_len/max_out trio like their EVP_AEAD counterparts.
BUG=468889
Change-Id: Ie9266a818cc053f695d35ef611fd74c5d4def6c3
Reviewed-on: https://boringssl-review.googlesource.com/4792
Reviewed-by: Adam Langley <agl@google.com>
The only place using it is export keying material which can do the
version check inline.
Change-Id: I1893966c130aa43fa97a6116d91bb8b04f80c6fb
Reviewed-on: https://boringssl-review.googlesource.com/4615
Reviewed-by: Adam Langley <agl@google.com>
clang-format got a little confused there.
Change-Id: I46df523e8a7813a2b4e243da3df22851b3393873
Reviewed-on: https://boringssl-review.googlesource.com/4614
Reviewed-by: Adam Langley <agl@google.com>
Next batch. Mostly a bunch of deprecated things. This switches
SSL_CTX_set_tmp_rsa from always failing to always succeeding. The latter
is probably a safer behavior; a consumer may defensively set a temporary
RSA key. We'll successfully "set it" and just never use the result.
Change-Id: Idd3d6bf4fc1a20bc9a26605bb9c77c9f799f993c
Reviewed-on: https://boringssl-review.googlesource.com/4566
Reviewed-by: Adam Langley <agl@google.com>
This is an API wart that makes it easy to accidentally reuse the server
DHE half for every handshake. It's much simpler to have only one mode.
This mirrors the change made to the ECDHE code; align with that logic.
Change-Id: I47cccbb354d70127ab458f99a6d390b213e4e515
Reviewed-on: https://boringssl-review.googlesource.com/4565
Reviewed-by: Adam Langley <agl@google.com>
The only difference is SSL_clear_num_renegotiations which is never
called.
Change-Id: Id661c71e89d34d834349ad1f1a296e332606e6cc
Reviewed-on: https://boringssl-review.googlesource.com/4564
Reviewed-by: Adam Langley <agl@google.com>
Document them while I'm here. This adds a new 'preprocessor
compatibility section' to avoid breaking #ifdefs. The CTRL values
themselves are defined to 'doesnt_exist' to catch anything calling
SSL_ctrl directly until that function can be unexported completely.
BUG=404754
Change-Id: Ia157490ea8efe0215d4079556a0c7643273e7601
Reviewed-on: https://boringssl-review.googlesource.com/4553
Reviewed-by: Adam Langley <agl@google.com>
Probably we'll want some simpler server-side API later. But, as things
stand, all consumers of these functions are #ifdef'd out and have to be
because the requisite OCSP_RESPONSE types are gone.
Change-Id: Ic82b2ab3feca14c56656da3ceb3651819e3eb377
Reviewed-on: https://boringssl-review.googlesource.com/4551
Reviewed-by: Adam Langley <agl@google.com>
It's unused, but for some old #ifdef branch in wpa_supplicant's EAP-FAST
hack, before SSL_set_session_ticket_ext_cb existed.
Change-Id: Ifc11fea2f6434354f756e04e5fc3ed5f1692025e
Reviewed-on: https://boringssl-review.googlesource.com/4550
Reviewed-by: Adam Langley <agl@google.com>
SSL_get0_chain_certs calls a ctrl function with
SSL_CTRL_GET_CHAIN_CERTS. The switch failed to set a positive return
value and so the call always appeared to fail.
Change-Id: If40ca7840197a9748fd69b761fd905f44bb79835
Reviewed-on: https://boringssl-review.googlesource.com/4521
Reviewed-by: Adam Langley <agl@google.com>
This is a really dumb API wart. Now that we have a limited set of curves that
are all reasonable, the automatic logic should just always kick in. This makes
set_ecdh_auto a no-op and, instead of making it the first choice, uses it as
the fallback behavior should none of the older curve selection APIs be used.
Currently, by default, server sockets can only use the plain RSA key exchange.
BUG=481139
Change-Id: Iaabc82de766cd00968844a71aaac29bd59841cd4
Reviewed-on: https://boringssl-review.googlesource.com/4531
Reviewed-by: Adam Langley <agl@google.com>
It's no longer needed to distinguish ciphers from fake ciphers.
Change-Id: I1ad4990ba936b1059eb48f3d2f309eb832dd1cb5
Reviewed-on: https://boringssl-review.googlesource.com/4285
Reviewed-by: Adam Langley <agl@google.com>
Rather than shoehorn real ciphers and cipher aliases into the same type (that's
what cipher->valid is used for), treat them separately. Make
ssl_cipher_apply_rule match ciphers by cipher_id (the parameter was ignored and
we assumed that masks uniquely identify a cipher) and remove the special cases
around zero for all the masks. This requires us to remember which fields
default to 0 and which default to ~0u, but the logic is much clearer.
Finally, now that ciphers and cipher aliases are different, don't process rules
which sum together an actual cipher with cipher aliases. This would AND
together the masks for the alias with the values in the cipher and do something
weird around alg_ssl. (alg_ssl is just weird in general, as everyone trying to
disable SSLv3 in OpenSSL recently discovered.)
With all that, we can finally remove cipher->valid which was always one.
Change-Id: Iefcfe159bd6c22dbaea3a5f1517bd82f756dcfe1
Reviewed-on: https://boringssl-review.googlesource.com/4284
Reviewed-by: Adam Langley <agl@google.com>
These are all masks of some sort (except id which is a combined version and
cipher), so they should use fixed-size unsigned integers.
Change-Id: I058dd8ad231ee747df4b4fb17d9c1e2cbee21918
Reviewed-on: https://boringssl-review.googlesource.com/4283
Reviewed-by: Adam Langley <agl@google.com>
The rest of ssl/ still includes things everywhere, but this at least fixes the
includes that were implicit from ssl/internal.h.
Change-Id: I7ed22590aca0fe78af84fd99a3e557f4b05f6782
Reviewed-on: https://boringssl-review.googlesource.com/4281
Reviewed-by: Adam Langley <agl@google.com>
Match the other internal headers.
Change-Id: Iff7e2dd06a1a7bf993053d0464cc15638ace3aaa
Reviewed-on: https://boringssl-review.googlesource.com/4280
Reviewed-by: Adam Langley <agl@google.com>
These are the remaining untested cipher suites. Rather than add support in
runner.go, just remove them altogether. Grepping for this is a little tricky,
but nothing enables aNULL (all occurrences disable it), and all occurrences of
["ALL:] seem to be either unused or explicitly disable anonymous ciphers.
Change-Id: I4fd4b8dc6a273d6c04a26e93839641ddf738343f
Reviewed-on: https://boringssl-review.googlesource.com/4258
Reviewed-by: Adam Langley <agl@google.com>
There's multiple sets of APIs for selecting the curve. Fold away
SSL_OP_SINGLE_ECDH_USE as failing to set it is either a no-op or a bug. With
that gone, the consumer only needs to control the selection of a curve, with
key generation from then on being uniform. Also clean up the interaction
between the three API modes in s3_srvr.c; they were already mutually exclusive
due to tls1_check_ec_tmp_key.
This also removes all callers of EC_KEY_dup (and thus CRYPTO_dup_ex_data)
within the library.
Change-Id: I477b13bd9e77eb03d944ef631dd521639968dc8c
Reviewed-on: https://boringssl-review.googlesource.com/4200
Reviewed-by: Adam Langley <agl@google.com>
Align with upstream's renames from a while ago. These names are considerably
more standard. This also aligns with upstream in that both "ECDHE" and "EECDH"
are now accepted in the various cipher string parsing bits.
Change-Id: I84c3daeacf806f79f12bc661c314941828656b04
Reviewed-on: https://boringssl-review.googlesource.com/4053
Reviewed-by: Adam Langley <agl@google.com>
It created the cert structure in SSL_CTX or SSL if it was NULL, but they can
never be NULL as the comments already said.
(Imported from upstream's 2c3823491d8812560922a58677e3ad2db4b2ec8d.)
Change-Id: I97c7bb306d6f3c18597850db9f08023b2ef74839
Reviewed-on: https://boringssl-review.googlesource.com/4042
Reviewed-by: Adam Langley <agl@google.com>
None of these are version-specific. SSL_PROTOCOL_METHOD's interface will change
later, but this gets us closer to folding away SSL3_ENC_METHOD.
Change-Id: Ib427cdff32d0701a18fe42a52cdbf798f82ba956
Reviewed-on: https://boringssl-review.googlesource.com/3769
Reviewed-by: Adam Langley <agl@google.com>
Upstream settled in this API, and it's also the one that we expect
internally and that third_party code will expect.
Change-Id: Id7af68cf0af1f2e4d9defd37bda2218d70e2aa7b
Reviewed-on: https://boringssl-review.googlesource.com/3542
Reviewed-by: Adam Langley <agl@google.com>
It may fail because the BIO_write to the memory BIO can allocate.
Unfortunately, this bubbles up pretty far up now that we've moved the handshake
hash to ssl3_set_handshake_header.
Change-Id: I58884347a4456bb974ac4783078131522167e29d
Reviewed-on: https://boringssl-review.googlesource.com/3483
Reviewed-by: Adam Langley <agl@google.com>
OpenSSL's internal names for the ciphers are not the standard ones and are not
easy to consistently map to the standard ones. Add an API to get the real names
out. (WebRTC wants an API to get the standard names out.)
Also change some incorrect flags on SHA-256 TLS 1.2 ciphers;
SSL_HANDSHAKE_MAC_DEFAULT and SSL_HANDSHAKE_MAC_SHA256 are the same after TLS
1.2. A TLS 1.2 cipher should be tagged explicitly with SHA-256. (This avoids
tripping a check in SSL_CIPHER_get_rfc_name which asserts that default-hash
ciphers only ever use SHA-1 or MD5 for the bulk cipher MAC.)
Change-Id: Iaec2fd4aa97df29883094d3c2ae60f0ba003bf07
This makes the following changes:
- SSL_cutthrough_complete no longer rederives whether cutthrough happened and
just maintains a handshake bit.
- SSL_in_init no longer returns true if we are False Starting but haven't
completed the handshake. That logic was awkward as it depended on querying
in_read_app_data to force SSL_read to flush the entire handshake. Defaulting
SSL_in_init to continue querying the full handshake and special-casing
SSL_write is better. E.g. the check in bidirectional SSL_shutdown wants to know
if we're in a handshake. No internal consumer of
SSL_MODE_HANDSHAKE_CUTTHROUGH ever queries SSL_in_init directly.
- in_read_app_data is gone now that the final use is dead.
Change-Id: I05211a116d684054dfef53075cd277b1b30623b5
Reviewed-on: https://boringssl-review.googlesource.com/3336
Reviewed-by: Adam Langley <agl@google.com>
This is the source of much of renegotiation's complexity, and of OpenSSL's
implementation of it. In practice, we only care about renegotiation because of
the client auth hack. There, we can safely assume that no server will send
application data between sending the HelloRequest and completing the handshake.
BUG=429450
Change-Id: I37f5abea5fdedb1d53e24ceb11f71287c74bb777
Reviewed-on: https://boringssl-review.googlesource.com/3332
Reviewed-by: Adam Langley <agl@google.com>
They both happen to be zero, but OBJ_undef is a type error; OBJ_foo expands to
a comma-separated list of integers.
Change-Id: Ia5907dd3bc83240b7cc98af6456115d2efb48687
Reviewed-on: https://boringssl-review.googlesource.com/2842
Reviewed-by: Adam Langley <agl@google.com>
This introduces another knob into SSL_AEAD_CTX to omit the version from the ad
parameter. It also allows us to fold a few more SSL3_ENC_METHOD hooks together.
Change-Id: I6540d410d4722f734093554fb434dab6e5217d4f
Reviewed-on: https://boringssl-review.googlesource.com/2698
Reviewed-by: Adam Langley <agl@google.com>
This lets us fold away the SSLv3-specific generate_master_secret. Once SSLv3
uses AEADs, others will fold away as well.
Change-Id: I27c1b75741823bc6db920d35f5dd5ce71b6fdbb3
Reviewed-on: https://boringssl-review.googlesource.com/2697
Reviewed-by: Adam Langley <agl@google.com>
The EVP_CIPHER codepath should no longer be used with TLS. It still exists for
DTLS and SSLv3. The AEAD construction in TLS does not allow for
variable-overhead AEADs, so stateful AEADs do not include the length in the ad
parameter. Rather the AEADs internally append the unpadded length once it is
known. EVP_aead_rc4_md5_tls is modified to account for this.
Tests are added (and RC4-MD5's regenerated) for each of the new AEADs. The
cipher tests are all moved into crypto/cipher/test because there's now a lot of
them and they clutter the directory listing.
In ssl/, the stateful AEAD logic is also modified to account for stateful AEADs
with a fixed IV component, and for AEADs which use a random nonce (for the
explicit-IV CBC mode ciphers).
The new implementation fixes a bug/quirk in stateless CBC mode ciphers where
the fixed IV portion of the keyblock was generated regardless. This is at the
end, so it's only relevant for EAP-TLS which generates a MSK from the end of
the key block.
Change-Id: I2d8b8aa11deb43bde2fd733f4f90b5d5b8cb1334
Reviewed-on: https://boringssl-review.googlesource.com/2692
Reviewed-by: Adam Langley <agl@google.com>
State on s3 gets freed in both ssl3_clear and ssl3_free. Considate to just
ssl3_free. This replaces the (SSL,ssl,ssl3)_clear calls in (SSL,ssl,ssl3)_new
with the state that was initialized. This results in a little code duplication
between SSL_new and SSL_clear because state is on the wrong object. I've just
left TODOs for now; some of it will need disentangling.
We're far from it, but going forward, separate state between s and s->s3 as:
- s contains configuration state, DTLS or TLS. It is initialized from SSL_CTX,
configurable directly afterwards, and preserved across SSL_clear calls.
(Including when it's implicitly set as part of a handshake callback.)
- Connection state hangs off s->s3 (TLS) and s->d1 (DTLS). It is reset across
SSL_clear. This should happen naturally out of a ssl_free/ssl_new pair.
The goal is to avoid needing separate initialize and reset code for anything;
the point any particular state is reset is the point its owning context is
destroyed and recreated.
Change-Id: I5d779010778109f8c339c07433a0777feaf94d1f
Reviewed-on: https://boringssl-review.googlesource.com/2822
Reviewed-by: Adam Langley <agl@google.com>
Configuration data inherited from the ctx happens in SSL_new. (This also gets
in the way of using ssl3_free/ssl3_new to implement SSL_clear.)
Change-Id: I2773af91abf4e1edc0c1a324bc1e94088d7c2274
Reviewed-on: https://boringssl-review.googlesource.com/2821
Reviewed-by: Adam Langley <agl@google.com>
This avoids needing a should_add_to_finished_hash boolean on do_write. The
logic in do_write was a little awkward because do_write would be called
multiple times if the write took several iterations. This also gets complex if
DTLS retransmits are involved. (At a glance, it's not obvious the
BIO_CTRL_DGRAM_MTU_EXCEEDED case actually works.)
Doing it as the handshake message is being prepared avoids this concern. It
also gives a natural point for the extended master secret logic which needs to
do work after the finished hash has been sampled.
As a bonus, we can remove s->d1->retransmitting which was only used to deal
with this issue.
Change-Id: Ifedf23ee4a6c5e08f960d296a6eb1f337a16dc7a
Reviewed-on: https://boringssl-review.googlesource.com/2604
Reviewed-by: Adam Langley <agl@google.com>
David Benjamin
Adam Langley