|SSL_CTX| and |X509_STORE| have grown their own locks. Several static
locks have been added to hack around not being able to use a
|CRYPTO_once_t| in public headers. Lastly, support for calling
|SSL_CTX_set_generate_session_id| concurrently with active connections
has been removed. No other property of an |SSL_CTX| works like that.
Change-Id: Iff5fe3ee3fdd6ea9c9daee96f850b107ad8a6bca
Reviewed-on: https://boringssl-review.googlesource.com/4775
Reviewed-by: Adam Langley <agl@google.com>
Convert reference counts in ssl/ to use |CRYPTO_refcount_t|.
Change-Id: I5d60f641b0c89b1ddfe38bfbd9d7285c60377f4c
Reviewed-on: https://boringssl-review.googlesource.com/4773
Reviewed-by: Adam Langley <agl@google.com>
SSL_get_current_cipher is documented by upstream to return the cipher actually
being used. However, because it reads s->session, it returns information
pertaining to the session to be offered if queried before ServerHello or early
in an abbreviated handshake.
Logic around s->session needs more comprehensive cleanup but for just this
function, defining it to be the current outgoing cipher is close to the current
semantics but for fixing the initial state (s->session->cipher is populated
when sending CCS). Store it in the SSL_AEAD_CTX which seems a natural place to
associate state pertaining to a connection half.
BUG=484744
Change-Id: Ife8db27a16615d0dbb2aec65359537243e08af7c
Reviewed-on: https://boringssl-review.googlesource.com/4733
Reviewed-by: Adam Langley <agl@google.com>
This cuts down on one config knob as well as one case in the renego
combinatorial explosion. Since the only case we care about with renego
is the client auth hack, there's no reason to ever do resumption.
Especially since, no matter what's in the session cache:
- OpenSSL will only ever offer the session it just established,
whether or not a newer one with client auth was since established.
- Chrome will never cache sessions created on a renegotiation, so
such a session would never make it to the session cache.
- The new_session + SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION
logic had a bug where it would unconditionally never offer tickets
(but would advertise support) on renego, so any server doing renego
resumption against an OpenSSL-derived client must not support
session tickets.
This also gets rid of s->new_session which is now pointless.
BUG=429450
Change-Id: I884bdcdc80bff45935b2c429b4bbc9c16b2288f8
Reviewed-on: https://boringssl-review.googlesource.com/4732
Reviewed-by: Adam Langley <agl@google.com>
We have a lot of options that don't do anything.
Change-Id: I1681fd07d1272547d4face87917ce41029bbf0de
Reviewed-on: https://boringssl-review.googlesource.com/4731
Reviewed-by: Adam Langley <agl@google.com>
As of crbug.com/484543, Chromium's SSLClientSocket is not sensitive to whether
renegotiation is enabled or not. Disable it by default and require consumers to
opt into enabling this protocol mistake.
BUG=429450
Change-Id: I2329068284dbb851da010ff1fd398df3d663bcc3
Reviewed-on: https://boringssl-review.googlesource.com/4723
Reviewed-by: Adam Langley <agl@google.com>
There's no real need to ever disable it, so this is one fewer configuration to
test. It's still disabled for DTLS, but a follow-up will resolve that.
Change-Id: Ia95ad8c17ae8236ada516b3968a81c684bf37fd9
Reviewed-on: https://boringssl-review.googlesource.com/4683
Reviewed-by: Adam Langley <agl@google.com>
With DTLSv1_get_timeout de-ctrl-ified, the type checker complains about
OPENSSL_timeval. Existing callers all use the real timeval.
Now that OPENSSL_timeval is not included in any public structs, simply
forward-declare timeval itself in ssl.h and pull in winsock2.h in internal
headers.
Change-Id: Ieaf110e141578488048c28cdadb14881301a2ce1
Reviewed-on: https://boringssl-review.googlesource.com/4682
Reviewed-by: Adam Langley <agl@google.com>
Nothing ever uses those structs. This to avoid having any structs in the
public header which use struct timeval.
In doing so, move the protocol version constants up to ssl.h so dtls1.h
may be empty. This also removes TLS1_get_version and TLS1_get_client_version
as they're unused and depend on TLS1_VERSION_MAJOR. This still lets tls1.h
be included independently from ssl.h (though I don't think anyone ever includes
it...).
Change-Id: Ieac8b90cf94f7f1e742a88bb75c0ee0aa4b1414c
Reviewed-on: https://boringssl-review.googlesource.com/4681
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>
It's only called for client certificates with NULL. The interaction with
extra_certs is more obvious if we handle that case externally. (We
shouldn't attach extra_certs if there is no leaf.)
Change-Id: I9dc26f32f582be8c48a4da9aae0ceee8741813dc
Reviewed-on: https://boringssl-review.googlesource.com/4613
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>
The API is unused and rather awkward (mixes output parameters with
return values, special-case for NULL).
Change-Id: I4396f98534bf1271e53642f255e235cf82c7615a
Reviewed-on: https://boringssl-review.googlesource.com/4560
Reviewed-by: Adam Langley <agl@google.com>
Also size them based on the limits in the quantities they control (after
checking bounds at the API boundary).
BUG=404754
Change-Id: Id56ba45465a473a1a793244904310ef747f29b63
Reviewed-on: https://boringssl-review.googlesource.com/4559
Reviewed-by: Adam Langley <agl@google.com>
Not going to bother adding the compatibility macros. If they get ifdef'd
out, all the better.
BUG=404754
Change-Id: I26414d2fb84ee1f0b15a3b96c871949fe2bb7fb1
Reviewed-on: https://boringssl-review.googlesource.com/4558
Reviewed-by: Adam Langley <agl@google.com>
This is a bitmask, so the number of bits available should be the same
across all platforms.
Change-Id: I98e8d375fc7d042aeae1270174bc8fc63fba5dfc
Reviewed-on: https://boringssl-review.googlesource.com/4556
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>
These are never used and no flags are defined anyway.
Change-Id: I206dc2838c5f68d87559a702dcb299b208cc7e1e
Reviewed-on: https://boringssl-review.googlesource.com/4493
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>
OpenSSH, especially, does some terrible things that mean that it needs
the EVP_CIPHER structure to be exposed ☹. Damian is open to a better API
to replace this, but only if OpenSSL agree too. Either way, it won't be
happening soon.
Change-Id: I393b7a6af6694d4d2fe9ebcccd40286eff4029bd
Reviewed-on: https://boringssl-review.googlesource.com/4330
Reviewed-by: Adam Langley <agl@google.com>
This causes any unexpected handshake records to be met with a fatal
no_renegotiation alert.
In addition, restore the redundant version sanity-checks in the handshake state
machines. Some code would zero the version field as a hacky way to break the
handshake on renego. Those will be removed when switching to this API.
The spec allows for a non-fatal no_renegotiation alert, but ssl3_read_bytes
makes it difficult to find the end of a ClientHello and skip it entirely. Given
that OpenSSL goes out of its way to map non-fatal no_renegotiation alerts to
fatal ones, this seems probably fine. This avoids needing to account for
another source of the library consuming an unbounded number of bytes without
returning data up.
Change-Id: Ie5050d9c9350c29cfe32d03a3c991bdc1da9e0e4
Reviewed-on: https://boringssl-review.googlesource.com/4300
Reviewed-by: Adam Langley <agl@google.com>
There's this giant "Underdocumented functions" section in the middle, but it
doesn't look too silly once the "Deprecated methods" section is merged in with
the other deprecated functions.
Change-Id: Ib97d88b0f915f60e9790264474a9e4aa3e115382
Reviewed-on: https://boringssl-review.googlesource.com/4291
Reviewed-by: Adam Langley <agl@google.com>
Just about everything depends on SSL_CIPHER. Move it to the top as the first
section in ssl.h. Match the header order and the source file order and document
everything. Also make a couple of minor style guide tweaks.
Change-Id: I6a810dbe79238278ac480e5ced1447055715a79f
Reviewed-on: https://boringssl-review.googlesource.com/4290
Reviewed-by: Adam Langley <agl@google.com>
Mostly stuff that doc.go was grumpy about. The main change is to move the
version-specific headers to the bottom. Injecting them in the middle makes it
seem as if the definitions above the #include and those below are somehow
different, but it compiles fine with them at the bottom. (They have to be at
the bottom because those headers depend on ssl.h.)
Change-Id: Iaa4139d2f157c7a3fd0ea609b78ff11d2edfc7b0
Reviewed-on: https://boringssl-review.googlesource.com/4289
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>
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>
We shouldn't be wrapping system headers.
Change-Id: I77498f4ec869797050b276eb764d892f73782f9f
Reviewed-on: https://boringssl-review.googlesource.com/4282
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>
After sharding the session cache for fallbacks, the numbers have been pretty
good; 0.03% on dev and 0.02% on canary. Stable is at 0.06% but does not have
the sharded session cache. Before sharding, stable, beta, and dev had been
fairly closely aligned. Between 0.03% being low and the fallback saving us in
all but extremely contrived cases, I think this should be fairly safe.
Add tests for both the cipher suite and protocol version mismatch checks.
BUG=441456
Change-Id: I2374bf64d0aee0119f293d207d45319c274d89ab
Reviewed-on: https://boringssl-review.googlesource.com/3972
Reviewed-by: Adam Langley <agl@google.com>
Also check for overflow, although it really shouldn't happen.
Change-Id: I34dfe8eaf635aeaa8bef2656fda3cd0bad7e1268
Reviewed-on: https://boringssl-review.googlesource.com/4235
Reviewed-by: Adam Langley <agl@google.com>
Separate actually writing the fragment to the network from assembling it so
there is no need for is_fragment. record_split_done also needn't be a global;
as of 7fdeaf11017b82368e0a97547fc491b90ad40f67, it is always reset to 0 whether
or not SSL3_WANT_WRITE occurred, despite the comment.
I believe this is sound, but the pre-7fdeaf1 logic wasn't quiiite right;
ssl3_write_pending allows a retry to supply *additional* data, so not all
plaintext had been commited to before the IV was randomized. We could fix this
by tracking how many bytes were committed to the last time we fragmented, but
this is purely an optimization and doesn't seem worth the complexity.
This also fixes the alignment computation in the record-splitting case. The
extra byte was wrong, as demonstrated by the assert.
Change-Id: Ia087a45a6622f4faad32e501942cc910eca1237b
Reviewed-on: https://boringssl-review.googlesource.com/4234
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>
Within the library, only ssl_update_cache read them, so add a dedicated field
to replace that use.
The APIs have a handful of uninteresting callers so I've left them in for now,
but they now always return zero.
Change-Id: Ie4e36fd4ab18f9bff544541d042bf3c098a46933
Reviewed-on: https://boringssl-review.googlesource.com/4101
Reviewed-by: Adam Langley <agl@google.com>
Adam Langley
David Benjamin
Matt Braithwaite
Adam Langley