This is a fairly timid, first step at trying to pack common structures a
little better.
This change reorders a couple of structures a little and turns some
variables into bit-fields. Much more can still be done.
Change-Id: Idbe0f54d66559c0ad654bf7e8dea277a771a568f
Reviewed-on: https://boringssl-review.googlesource.com/6394
Reviewed-by: Adam Langley <agl@google.com>
This came up and I wasn't sure which it was without source-diving.
Change-Id: Ie659096e0f42a7448f81dfb1006c125d292fd7fd
Reviewed-on: https://boringssl-review.googlesource.com/6354
Reviewed-by: Adam Langley <alangley@gmail.com>
Right whether NPN is advertised can only be configured globally on the SSL_CTX.
Rather than adding two pointers to each SSL*, add an options bit to disable it
so we may plumb in a field trial to disable NPN.
Chromium wants to be able to route a bit in to disable NPN, but it uses SSL_CTX
incorrectly and has a global one, so it can't disconnect the callback. (That
really needs to get fixed. Although it's not clear this necessarily wants to be
lifted up to SSL_CTX as far as Chromium's SSLClientSocket is concerned since
NPN doesn't interact with the session cache.)
BUG=526713
Change-Id: I49c86828b963eb341c6ea6a442557b7dfa190ed3
Reviewed-on: https://boringssl-review.googlesource.com/6351
Reviewed-by: Adam Langley <alangley@gmail.com>
The internal session cache is keyed on session ID, so this is completely
useless for clients (indeed we never look it up internally). Along the way,
tidy up ssl_update_cache to be more readable. The slight behavior change is
that SSL_CTX_add_session's return code no longer controls the external
callback. It's not clear to me what that could have accomplished. (It can only
fail on allocation error. We only call it for new sessions, so the duplicate
case is impossible.)
The one thing of value the internal cache might have provided is managing the
timeout. The SSL_CTX_flush_sessions logic would flip the not_resumable bit and
cause us not to offer expired sessions (modulo SSL_CTX_flush_sessions's delay
and any discrepancies between the two caches). Instead, just check expiration
when deciding whether or not to offer a session.
This way clients that set SSL_SESS_CACHE_CLIENT blindly don't accidentally
consume gobs of memory.
BUG=531194
Change-Id: If97485beab21874f37737edc44df24e61ce23705
Reviewed-on: https://boringssl-review.googlesource.com/6321
Reviewed-by: Adam Langley <alangley@gmail.com>
In doing so, fix the documentation for SSL_CTX_add_session and
SSL_CTX_remove_session. I misread the code and documented the behavior
on session ID collision wrong.
Change-Id: I6f364305e1f092b9eb0b1402962fd04577269d30
Reviewed-on: https://boringssl-review.googlesource.com/6319
Reviewed-by: Adam Langley <alangley@gmail.com>
A random 32-byte (so 256-bit) session ID is never going to collide with
an existing one. (And, if it does, SSL_CTX_add_session does account for
this, so the server won't explode. Just attempting to resume some
session will fail.)
That logic didn't completely work anyway as it didn't account for
external session caches or multiple connections picking the same ID in
parallel (generation and insertion happen at different times) or
multiple servers sharing one cache. In theory one could fix this by
passing in a sufficiently clever generate_session_id, but no one does
that.
I found no callers of these functions, so just remove them altogether.
Change-Id: I8500c592cf4676de6d7194d611b99e9e76f150a7
Reviewed-on: https://boringssl-review.googlesource.com/6318
Reviewed-by: Adam Langley <alangley@gmail.com>
Private structs shouldn't be shown. Also there's a few sections that are
really more implementation details than anything else.
Change-Id: Ibc5a23ba818ab0531d9c68e7ce348f1eabbcd19a
Reviewed-on: https://boringssl-review.googlesource.com/6313
Reviewed-by: Adam Langley <alangley@gmail.com>
Although Chromium actually uses SSL_(get_)state as part of its fallback
reason heuristic, that function really should go in the deprecated
bucket. I kept SSL_state_string_long since having a human-readable
string is probably useful for logging.
SSL_set_SSL_CTX was only half-documented as the behavior of this
function is very weird. This warrants further investigation and
rethinking.
SSL_set_shutdown is absurd. I added an assert to trip up clearing bits
and set it to a bitwise OR since clearing bits may mess up the state
machine. Otherwise there's enough consumers and it's not quite the same
as SSL_CTX_set_quiet_shutdown that I've left it alone for now.
Change-Id: Ie35850529373a5a795f6eb04222668ff76d84aaa
Reviewed-on: https://boringssl-review.googlesource.com/6312
Reviewed-by: Adam Langley <alangley@gmail.com>
It just calls CRYPTO_library_init and doesn't do anything else. If
anything, I'd like to make CRYPTO_library_init completely go away too.
We have CRYPTO_once now, so I think it's safe to assume that, if ssl/
ever grows initialization needs beyond that of crypto/, we can hide it
behind a CRYPTO_once and not burden callers.
Change-Id: I63dc362e0e9e98deec5516f4620d1672151a91b6
Reviewed-on: https://boringssl-review.googlesource.com/6311
Reviewed-by: Adam Langley <alangley@gmail.com>
SSL_in_connect_init and SSL_in_accept_init are removed as they're unused
both within the library and externally. They're also kind of silly.
Expand on how False Start works at the API level in doing so.
Change-Id: Id2a8e34b5bb8f28329e3b87b4c64d41be3f72410
Reviewed-on: https://boringssl-review.googlesource.com/6310
Reviewed-by: Adam Langley <alangley@gmail.com>
They were since added to crypto.h and implemented in the library proper.
Change-Id: Idaa2fe2d9b213e67cf7ef61ff8bfc636dfa1ef1f
Reviewed-on: https://boringssl-review.googlesource.com/6309
Reviewed-by: Adam Langley <alangley@gmail.com>
They're really not all that helpful, considering they're each used
exactly once. They're also confusing as it is ALMOST the case that
SSL_TXT_FOO expands to "FOO", but SSL_TXT_AES_GCM expand "AESGCM" and
the protocol versions have lowercase v's and dots.
Change-Id: If78ad8edb0c024819219f61675c60c2a7f3a36b0
Reviewed-on: https://boringssl-review.googlesource.com/6307
Reviewed-by: Adam Langley <alangley@gmail.com>
This callback is some combination of arguably useful stuff (bracket
handshakes, alerts) and completely insane things (find out when the
state machine advances). Deprecate the latter.
Change-Id: Ibea5b32cb360b767b0f45b302fd5f1fe17850593
Reviewed-on: https://boringssl-review.googlesource.com/6305
Reviewed-by: Adam Langley <alangley@gmail.com>
Also clean up the code slightly.
Change-Id: I066a389242c46cdc7d41b1ae9537c4b7716c92a2
Reviewed-on: https://boringssl-review.googlesource.com/6302
Reviewed-by: Adam Langley <alangley@gmail.com>
Like tls1.h, ssl3.h is now just a bundle of protocol constants.
Hopefully we can opaquify this struct in due time, but for now it's
still public.
Change-Id: I68366eb233702e149c92e21297f70f8a4a45f060
Reviewed-on: https://boringssl-review.googlesource.com/6300
Reviewed-by: Adam Langley <alangley@gmail.com>
Now tls1.h is just a pile of protocol constants with no more circular
dependency problem.
I've preserved SSL_get_servername's behavior where it's simultaneously a
lookup of handshake state and local configuration. I've removed it from
SSL_get_servername_type. It got the logic wrong anyway with the order of
the s->session check.
(Searching through code, neither is used on the client, but the
SSL_get_servername one is easy.)
Change-Id: I61bb8fb0858b07d76a7835bffa6dc793812fb027
Reviewed-on: https://boringssl-review.googlesource.com/6298
Reviewed-by: Adam Langley <alangley@gmail.com>
SSL_alert_desc_string_long was kept in the undeprecated bucket and one missing
alert was added. We have some uses and it's not completely ridiculous for
logging purposes.
The two-character one is ridiculous though and gets turned into a stub
that returns a constant string ("!" or "!!") because M2Crypto expects
it.
Change-Id: Iaf8794b5d953630216278536236c7113655180af
Reviewed-on: https://boringssl-review.googlesource.com/6297
Reviewed-by: Adam Langley <alangley@gmail.com>
(Documentation/deprecation will come in later commits.)
Change-Id: I3aba26e32b2e47a1afb5cedd44d09115fc193bce
Reviewed-on: https://boringssl-review.googlesource.com/6296
Reviewed-by: Adam Langley <alangley@gmail.com>
The only reason you'd want it is to tls_unique, and we have a better API
for that. (It has one caller and that is indeed what that caller uses it
for.)
Change-Id: I39f8e353f56f18becb63dd6f7205ad31f4192bfd
Reviewed-on: https://boringssl-review.googlesource.com/6295
Reviewed-by: Adam Langley <alangley@gmail.com>
This is redundant with SSL_get_error. Neither is very good API, but
SSL_get_error is more common. SSL_get_error also takes a return code
which makes it harder to accidentally call it at some a point other than
immediately after an operation. (Any other point is confusing since you
can have SSL_read and SSL_write operations going on in parallel and
they'll get mixed up.)
Change-Id: I5818527c30daac28edb552c6c550c05c8580292d
Reviewed-on: https://boringssl-review.googlesource.com/6294
Reviewed-by: Adam Langley <alangley@gmail.com>
It's pretty clearly pointless to put in the public header.
Change-Id: I9527aba09b618f957618e653c4f2ae379ddd0fdb
Reviewed-on: https://boringssl-review.googlesource.com/6293
Reviewed-by: Adam Langley <alangley@gmail.com>
Also added a SSL_CTX_set_select_certificate_cb setter for
select_certificate_cb so code needn't access SSL_CTX directly. Plus it
serves as a convenient anchor for the documentation.
Change-Id: I23755b910e1d77d4bea7bb9103961181dd3c5efe
Reviewed-on: https://boringssl-review.googlesource.com/6291
Reviewed-by: Adam Langley <alangley@gmail.com>
These are theh two remaining quirks (SSL_OP_LEGACY_SERVER_CONNECT
aside). Add counters so we can determine whether there are still clients
that trip up these cases.
Change-Id: I7e92f42f3830c1df675445ec15a852e5659eb499
Reviewed-on: https://boringssl-review.googlesource.com/6290
Reviewed-by: Adam Langley <alangley@gmail.com>
It's missing fields and no one ever calls it.
Change-Id: I450edc1e29bb48edffb5fd3df8da19a03e4185ce
Reviewed-on: https://boringssl-review.googlesource.com/5821
Reviewed-by: Adam Langley <agl@google.com>
See also upstream's bf0fc41266f17311c5db1e0541d3dd12eb27deb6.
Change-Id: Ib692b0ad608f2e3291f2aeab2ad98a7e177d5851
Reviewed-on: https://boringssl-review.googlesource.com/6150
Reviewed-by: Adam Langley <agl@google.com>
Grouping along two axes is weird. Doesn't hugely matter which one, but
we should be consistent.
Change-Id: I80fb04d3eff739c08fda29515ce81d101d8542cb
Reviewed-on: https://boringssl-review.googlesource.com/6120
Reviewed-by: Adam Langley <agl@google.com>
The caller obligations for retransmit are messy, so I've peppered a few
other functions with mentions of it. There's only three functions, so
they're lumped in with the other core functions. They're irrelevant for
TLS, but important for DTLS.
Change-Id: Ifc995390952eef81370b58276915dcbe4fc7e3b5
Reviewed-on: https://boringssl-review.googlesource.com/6093
Reviewed-by: Adam Langley <agl@google.com>
Deprecate the client_cert_cb variant since you can't really configure
intermediates with it. (You might be able to by configuring the
intermediates without the leaf or key and leaving the SSL stack to
configure those, but that's really weird. cert_cb is simpler.)
Also document the two functions the callbacks may use to query the
CertificateRequest on the client.
Change-Id: Iad6076266fd798cd74ea4e09978e7f5df5c8a670
Reviewed-on: https://boringssl-review.googlesource.com/6092
Reviewed-by: Adam Langley <agl@google.com>
Add a slightly richer API. Notably, one can configure ssl_renegotiate_once to
only accept the first renego.
Also, this API doesn't repeat the mistake I made with
SSL_set_reject_peer_renegotiations which is super-confusing with the negation.
Change-Id: I7eb5d534e3e6c553b641793f4677fe5a56451c71
Reviewed-on: https://boringssl-review.googlesource.com/6221
Reviewed-by: Adam Langley <agl@google.com>
I put an extra space in there. Also document ownership and return value.
Change-Id: I0635423be7774a7db54dbf638cc548d291121529
Reviewed-on: https://boringssl-review.googlesource.com/6010
Reviewed-by: Adam Langley <agl@google.com>
Putting it at the top was probably a mistake? Even though SSL_CIPHER
(like SSL_SESSION) doesn't depend on SSL, if you're reading through the
header, SSL_CTX and SSL are the most important types. You could even use
the library without touch cipher suite configs if you don't care since
the default is decently reasonable, though it does include a lot of
ciphers. (Hard to change that if we wanted to because DEFAULT is often
used somewhat like ALL and then people subtract from it.)
Change-Id: Ic9ddfc921858f7a4c141972fe0d1e465ca196b9d
Reviewed-on: https://boringssl-review.googlesource.com/5963
Reviewed-by: Adam Langley <agl@google.com>
The cipher suite rules could also be anchored on SSL_TXT_* if desired. I
currently documented them in prose largely because SSL_TXT_* also
defines protocol version strings and those are weird; SSL_TXT_TLSV1_1
isn't even a cipher rule. (And, in fact, those are the only SSL_TXT_*
macros that we can't blindly remove. I found some code that #ifdef's the
version SSL_TXT_* macros to decide if version-locked SSL_METHODs are
available.)
Also they clutter the header. I was thinking maybe we should dump a lot
of the random constants into a separate undocumented header or perhaps
just unexport them.
I'm slightly torn on this though and could easily be convinced in the
other direction. (Playing devil's advocate, anchoring on SSL_TXT_* means
we're less likely to forget to document one so long as adding a
SSL_TXT_* macro is the convention.)
Change-Id: Ide2ae44db9d6d8f29c24943090c210da0108dc37
Reviewed-on: https://boringssl-review.googlesource.com/5962
Reviewed-by: Adam Langley <agl@google.com>
This mirrors how the server halves fall under configuring certificates.
Change-Id: I9bde85eecfaff6487eeb887c88cb8bb0c36b83d8
Reviewed-on: https://boringssl-review.googlesource.com/5961
Reviewed-by: Adam Langley <agl@google.com>
∙ Some comments had the wrong function name at the beginning.
∙ Some ARM asm ended up with two #if defined(__arm__) lines – one from
the .pl file and one inserted by the translation script.
Change-Id: Ia8032cd09f06a899bf205feebc2d535a5078b521
Reviewed-on: https://boringssl-review.googlesource.com/6000
Reviewed-by: Adam Langley <agl@google.com>
Or at least group them together and make a passing attempt to document
them. The legacy X.509 stack itself remains largely untouched and most
of the parameters have to do with it.
Change-Id: I9e11e2ad1bbeef53478c787344398c0d8d1b3876
Reviewed-on: https://boringssl-review.googlesource.com/5942
Reviewed-by: Adam Langley <agl@google.com>
Allow configuring digest preferences for the private key. Some
smartcards have limited support for signing digests, notably Windows
CAPI keys and old Estonian smartcards. Chromium used the supports_digest
hook in SSL_PRIVATE_KEY_METHOD to limit such keys to SHA1. However,
detecting those keys was a heuristic, so some SHA256-capable keys
authenticating to SHA256-only servers regressed in the switch to
BoringSSL. Replace this mechanism with an API to configure digest
preference order. This way heuristically-detected SHA1-only keys may be
configured by Chromium as SHA1-preferring rather than SHA1-requiring.
In doing so, clean up the shared_sigalgs machinery somewhat.
BUG=468076
Change-Id: I996a2df213ae4d8b4062f0ab85b15262ca26f3c6
Reviewed-on: https://boringssl-review.googlesource.com/5755
Reviewed-by: Adam Langley <agl@google.com>
We wish to be able to detect the use of RC4 so that we can flag it and
investigate before it's disabled.
Change-Id: I6dc3a5d2211b281097531a43fadf08edb5a09646
Reviewed-on: https://boringssl-review.googlesource.com/5930
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Get them out of the way when reading through the header.
Change-Id: Ied3f3601262e74570769cb7f858dcff4eff44813
Reviewed-on: https://boringssl-review.googlesource.com/5898
Reviewed-by: Adam Langley <agl@google.com>
