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>
These were added in an attempt to deal with the empty vs. NULL confusion
in PKCS#12. Instead, PKCS8_encrypt and PKCS8_decrypt already treated
NULL special. Since we're stuck with supporting APIs like those anyway,
Chromium has been converted to use that feature. This cuts down on the
number of APIs we need to decouple from crypto/asn1.
BUG=54
Change-Id: Ie2d4798d326c5171ea5d731da0a2c11278bc0241
Reviewed-on: https://boringssl-review.googlesource.com/13885
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 reduces us from seven different configuration patterns to six (see
comment #2 of linked bug). I do not believe there is any behavior change
here as SSL_set_SSL_CTX already manually copied the field. It now gives
us a nice invariant: SSL_set_SSL_CTX overrides all and only the
dual-SSL/SSL_CTX options hanging off of CERT.
BUG=123
Change-Id: I1ae06b791fb869917a6503cee41afb2d9be53d89
Reviewed-on: https://boringssl-review.googlesource.com/13865
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>
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>
Right now the only way to set an SCT list is the per-context function
SSL_CTX_set_signed_cert_timestamp_list. However this assumes that all the
SSLs generated from a SSL_CTX share the same SCT list, which is wrong.
In order to avoid memory duplication in case SSL_CTX has its own list, a
CRYPTO_BUFFER is used for both SSL_CTX and SSL.
Change-Id: Id20e6f128c33cf3e5bff1be390645441be6518c6
Reviewed-on: https://boringssl-review.googlesource.com/13642
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 version negotiation logic was a little bizarrely wedged in the
middle of the state machine. (We don't support server renegotiation, so
have_version is always false here.)
BUG=128
Change-Id: I9448dce374004b92e8bd5172c36a4e0eea51619c
Reviewed-on: https://boringssl-review.googlesource.com/13561
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
In TLS 1.2, resumption's benefits are more-or-less subsumed by False
Start. TLS 1.2 resumption lifetime is bounded by how much traffic we are
willing to encrypt without fresh key material, so the lifetime is short.
Renewal uses the same key, so we do not allow it to increase lifetimes.
In TLS 1.3, resumption unlocks 0-RTT. We do not implement psk_ke, so
resumption incorporates fresh key material into both encrypted traffic
(except for early data) and renewed tickets. Thus we are both more
willing to and more interested in longer lifetimes for tickets. Renewal
is also not useless. Thus in TLS 1.3, lifetime is bound separately by
the lifetime of a given secret as a psk_dhe_ke authenticator and the
lifetime of the online signature which authenticated the initial
handshake.
This change maintains two lifetimes on an SSL_SESSION: timeout which is
the renewable lifetime of this ticket, and auth_timeout which is the
non-renewable cliff. It also separates the TLS 1.2 and TLS 1.3 timeouts.
The old session timeout defaults and configuration apply to TLS 1.3, and
we define new ones for TLS 1.3.
Finally, this makes us honor the NewSessionTicket timeout in TLS 1.3.
It's no longer a "hint" in 1.3 and there's probably value in avoiding
known-useless 0-RTT offers.
BUG=120
Change-Id: Iac46d56e5a6a377d8b88b8fa31f492d534cb1b85
Reviewed-on: https://boringssl-review.googlesource.com/13503
Reviewed-by: Adam Langley <agl@google.com>
This special-case is almost unexposed (the timeout is initialized to the
default) except if the caller calls SSL_CTX_set_timeout(0). Preserve
that behavior by mapping 0 to SSL_DEFAULT_SESSION_TIMEOUT in
SSL_CTX_set_timeout but simplify the internal state.
Change-Id: Ice03a519c25284b925f1e0cf485f2d8c54dc5038
Reviewed-on: https://boringssl-review.googlesource.com/13502
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The TLS 1.2 state machine now looks actually much closer to the TLS 1.3
one on the write side. Although the write states still have a BIO-style
return, they don't actually send anything anymore. Only the BIO flush
state does. Reads are still integrated into the states themselves
though, so I haven't made it match TLS 1.3 yet.
BUG=72
Change-Id: I7708162efca13cd335723efa5080718a5f2808ab
Reviewed-on: https://boringssl-review.googlesource.com/13228
Reviewed-by: Adam Langley <agl@google.com>
On the TLS side, we introduce a running buffer of ciphertext. Queuing up
pending data consists of encrypting the record into the buffer. This
effectively reimplements what the buffer BIO was doing previously, but
this resizes to fit the whole flight.
As part of this, rename all the functions to add to the pending flight
to be more uniform. This CL proposes "add_foo" to add to the pending
flight and "flush_flight" to drain it.
We add an add_alert hook for alerts but, for now, only the SSL 3.0
warning alert (sent mid-handshake) uses this mechanism. Later work will
push this down to the rest of the write path so closure alerts use it
too, as in DTLS. The intended end state is that all the ssl_buffer.c and
wpend_ret logic will only be used for application data and eventually
optionally replaced by the in-place API, while all "incidental" data
will be handled internally.
For now, the two buffers are mutually exclusive. Moving closure alerts
to "incidentals" will change this, but flushing application data early
is tricky due to wpend_ret. (If we call ssl_write_buffer_flush,
do_ssl3_write doesn't realize it still has a wpend_ret to replay.) That
too is all left alone in this change.
To keep the diff down, write_message is retained for now and will be
removed from the state machines in a follow-up change.
BUG=72
Change-Id: Ibce882f5f7196880648f25d5005322ca4055c71d
Reviewed-on: https://boringssl-review.googlesource.com/13224
Reviewed-by: Adam Langley <agl@google.com>
Instead, "writing" a message merely adds it to the outgoing_messages
structure. The code to write the flight then loops over it all and now
shares code with retransmission. The verbs here are all a little odd,
but they'll be fixed in later commits.
In doing so, this fixes a slight miscalculation of the record-layer
overhead when retransmitting a flight that spans two epochs. (We'd use
the encrypted epoch's overhead for the unencrypted epoch.)
BUG=72
Change-Id: I8ac897c955cc74799f8b5ca6923906e97d6dad17
Reviewed-on: https://boringssl-review.googlesource.com/13223
Reviewed-by: Adam Langley <agl@google.com>
It's not completely clear to me why select_cetificate_cb behaves the way it
does, however not only is it confusing, but it makes assumptions about the
application using BoringSSL (it's not always possible to implement custom
logic outside of the callbacks provided by libssl), that make this callback
somewhat useless.
Case in point, the callback can be used for changing min/max protocol versions
based on per-site policies, and select_certificate_cb is the only place where
SSL_set_min/max_proto_version() can be used (e.g. you can't call them in
cert_cb because it's too late), but the decision on the specific versions to
use might depend on configuration that needs retrieving asynchronously from
over the network, which requires re-running the callback multiple times.
Change-Id: Ia8e151b163628545373e7fd1f327e9af207478a6
Reviewed-on: https://boringssl-review.googlesource.com/13000
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Before RFC 7539 we had a ChaCha20-Poly1305 cipher suite that had a 64/64
nonce/counter split (as DJB's original ChaCha20 did). RFC 7539 changed
that to 96/32 and we've supported both for some time.
This change removes the old version and the TLS cipher suites that used
it.
BUG=chromium:682816
Change-Id: I2345d6db83441691fe0c1ab6d7c6da4d24777849
Reviewed-on: https://boringssl-review.googlesource.com/13203
Reviewed-by: Adam Langley <agl@google.com>
TLS 1.3 forbids warning alerts, and sending these is a bad idea. Per RFC
6066:
If the server understood the ClientHello extension but
does not recognize the server name, the server SHOULD take one of two
actions: either abort the handshake by sending a fatal-level
unrecognized_name(112) alert or continue the handshake. It is NOT
RECOMMENDED to send a warning-level unrecognized_name(112) alert,
because the client's behavior in response to warning-level alerts is
unpredictable.
The motivation is to cut down on the number of places where we send
non-closing alerts. We can't remove them yet (SSL 3.0 and TLS 1.3 draft
18 need to go), but eventually this can be a simplifying assumption.
Already this means DTLS never sends warning alerts, which is good
because DTLS can't retransmit them.
Change-Id: I577a1eb9c23e66d28235c0fbe913f00965e19486
Reviewed-on: https://boringssl-review.googlesource.com/13221
Reviewed-by: Adam Langley <agl@google.com>
Before RFC 7539 we had a ChaCha20-Poly1305 cipher suite that had a 64/64
nonce/counter split (as DJB's original ChaCha20 did). RFC 7539 changed
that to 96/32 and we've supported both for some time.
This change removes the old version and the TLS cipher suites that used
it.
Change-Id: Icd9c2117c657f3aa6df55990c618d562194ef0e8
Reviewed-on: https://boringssl-review.googlesource.com/13201
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
BN_FLG_CONSTTIME is a ridiculous API and easy to mess up
(CVE-2016-2178). Instead, code that needs a particular algorithm which
preserves secrecy of some arguemnt should call into that algorithm
directly.
This is never set outside the library and is finally unused within the
library! Credit for all this goes almost entirely to Brian Smith. I just
took care of the last bits.
Note there was one BN_FLG_CONSTTIME check that was still reachable, the
BN_mod_inverse in RSA key generation. However, it used the same code in
both cases for even moduli and φ(n) is even if n is not a power of two.
Traditionally, RSA keys are not powers of two, even though it would make
the modular reductions a lot easier.
When reviewing, check that I didn't remove a BN_FLG_CONSTTIME that led
to a BN_mod_exp(_mont) or BN_mod_inverse call (with the exception of the
RSA one mentioned above). They should all go to functions for the
algorithms themselves like BN_mod_exp_mont_consttime.
This CL shows the checks are a no-op for all our tests:
https://boringssl-review.googlesource.com/c/12927/
BUG=125
Change-Id: I19cbb375cc75aac202bd76b51ca098841d84f337
Reviewed-on: https://boringssl-review.googlesource.com/12926
Reviewed-by: Adam Langley <alangley@gmail.com>
David Benjamin