Commit Graph

63 Commits

Author SHA1 Message Date
David Benjamin
415660b26b Tidy up SSL_CTX_add_session.
The original logic was rather confusing.

Change-Id: I097e57817ea8ec2dd65a413c8751fba1682e928b
Reviewed-on: https://boringssl-review.googlesource.com/6320
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 19:22:40 +00:00
David Benjamin
0f653957c1 Add tests for the internal session cache behavior.
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>
2015-10-26 19:18:44 +00:00
David Benjamin
dc2aea2231 Remove all the logic around custom session IDs and retrying on collisions.
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>
2015-10-26 19:00:14 +00:00
David Benjamin
82170248e7 Document the info callback.
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>
2015-10-26 18:12:22 +00:00
David Benjamin
fd8e69f26d Better document the callbacks around client certificates.
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>
2015-10-13 18:18:40 +00:00
David Benjamin
c7ce977fb9 Ignore all extensions but renegotiation_info in SSL 3.0.
SSL 3.0 used to have a nice and simple rule around extensions. They don't
exist. And then RFC 5746 came along and made this all extremely confusing.

In an SSL 3.0 server, rather than blocking ServerHello extension
emission when renegotiation_info is missing, ignore all ClientHello
extensions but renegotiation_info. This avoids a mismatch between local
state and the extensions with emit.

Notably if, for some reason, a ClientHello includes the session_ticket
extension, does NOT include renegotiation_info or the SCSV, and yet the
client or server are decrepit enough to negotiate SSL 3.0, the
connection will fail due to unexpected NewSessionTicket message.

See https://crbug.com/425979#c9 for a discussion of something similar
that came up in diagnosing https://poodle.io/'s buggy POODLE check.
This is analogous to upstream's
5a3d8eebb7667b32af0ccc3f12f314df6809d32d.

(Not supporting renego as a server in any form anyway, we may as well
completely ignore extensions, but then our extensions callbacks can't
assume the parse hooks are always called. This way the various NULL
handlers still function.)

Change-Id: Ie689a0e9ffb0369ef7a20ab4231005e87f32d5f8
Reviewed-on: https://boringssl-review.googlesource.com/6180
Reviewed-by: Adam Langley <agl@google.com>
2015-10-11 20:47:19 +00:00
David Benjamin
20c0e90d11 Allow NULL inputs in SSL_SESSION_get_time.
Some code relies on OpenSSL's behavior where it allowed for NULL. But this time
add a comment so people don't think this is the convention for new functions.

BUG=538292

Change-Id: I66566e0e24566fafe17e05369276248be3b05591
Reviewed-on: https://boringssl-review.googlesource.com/6070
Reviewed-by: Adam Langley <agl@google.com>
2015-10-01 20:16:49 +00:00
David Benjamin
9e4e01ee14 Align the SSL stack on #include style.
ssl.h should be first. Also two lines after includes and the rest of the
file.

Change-Id: Icb7586e00a3e64170082c96cf3f8bfbb2b7e1611
Reviewed-on: https://boringssl-review.googlesource.com/5892
Reviewed-by: Adam Langley <agl@google.com>
2015-09-15 23:32:07 +00:00
David Benjamin
dafbdd49c7 Document session cache functions.
Also switch to the new variable names (SSL_CTX *ctx, SSL *ssl,
SSL_SESSION *session) for all documented functions.

Change-Id: I15e15a703b96af1727601108223c7ce3b0691f1d
Reviewed-on: https://boringssl-review.googlesource.com/5882
Reviewed-by: Adam Langley <agl@google.com>
2015-09-14 23:58:48 +00:00
David Benjamin
14e2b5070b const-correct a few SSL_SESSION functions.
Change-Id: I64d1e29d1e414add4ae522fd9603c550d426a557
Reviewed-on: https://boringssl-review.googlesource.com/5881
Reviewed-by: Adam Langley <agl@google.com>
2015-09-14 23:41:36 +00:00
David Benjamin
abb6c1c191 Reorder ssl_session.c slightly to match the header.
Change-Id: I43138057689a479ff7aba1543118af4b52bb821b
Reviewed-on: https://boringssl-review.googlesource.com/5880
Reviewed-by: Adam Langley <agl@google.com>
2015-09-14 23:40:55 +00:00
David Benjamin
190ab7f179 Move ex_data functions in ssl.h to a section.
Change-Id: Ie3b7c1956cc9d62091f649d0e621d84fce1d26ec
Reviewed-on: https://boringssl-review.googlesource.com/5879
Reviewed-by: Adam Langley <agl@google.com>
2015-09-14 23:40:34 +00:00
David Benjamin
8f3cc26812 Rename ssl_sess.c to ssl_session.c.
Change-Id: Ib730ffe49b30c79b3f30acae4070f6055081a0a3
Reviewed-on: https://boringssl-review.googlesource.com/5878
Reviewed-by: Adam Langley <agl@google.com>
2015-09-14 23:37:52 +00:00