Commit Graph

2462 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
60be027625 Style: fix some header guards
Change-Id: I86c30c7fe489c720f83f744696df0a0a20268531
Reviewed-on: https://boringssl-review.googlesource.com/6317
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 18:47:51 +00:00
David Benjamin
f91fa5cfc6 Documentation typo.
Change-Id: Iedcba0ac15bc14def9c2dc2407ed29d130133c0c
Reviewed-on: https://boringssl-review.googlesource.com/6315
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 18:47:31 +00:00
David Benjamin
dfa9c4a074 Linkify pipe words.
This required switching anchors from <a name> to id attributes, which
also works. HTML gets unhappy when you nest <a> tags inside each other
and tagging the elements is somewhat tidier.

Change-Id: I64094d35a0e820e37be9e5dc8db013a50774190f
Reviewed-on: https://boringssl-review.googlesource.com/6314
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 18:46:53 +00:00
David Benjamin
5ef619ef2a Hide some sections from the docs.
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>
2015-10-26 18:44:27 +00:00
David Benjamin
63006a913b Document the rest of ssl.h.
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>
2015-10-26 18:43:38 +00:00
David Benjamin
7a1eefd3cd Deprecate SSL_library_init.
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>
2015-10-26 18:36:23 +00:00
David Benjamin
5d8b128095 Document the (formerly) SSL_state wrapper macros.
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>
2015-10-26 18:35:01 +00:00
David Benjamin
449c3c7b7b Take some definitions out of the Android compatibility layer.
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>
2015-10-26 18:33:10 +00:00
David Benjamin
6e0c17aa3a Private (and deprecated) types.
Change-Id: Ia66e485cb2de45c9fb0a1ecd9a703863ad24d9c9
Reviewed-on: https://boringssl-review.googlesource.com/6308
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 18:32:35 +00:00
David Benjamin
da86cccaf6 Deprecate all the string macros.
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>
2015-10-26 18:15:33 +00:00
David Benjamin
6d5ea9225d Private constants are private.
Change-Id: Id20fcf357d4a0fc28734a7f2ea1fe077d4b34f1e
Reviewed-on: https://boringssl-review.googlesource.com/6306
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 18:12:57 +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
1b92f64b71 Fix comment style in crypto/rand/rand.c.
This compiled, so I guess everything we care about can do C++-style
comments, but better be uniform.

Change-Id: I9950c2df93cd81bb2bddb3a1e14e2de02c7e4807
Reviewed-on: https://boringssl-review.googlesource.com/6304
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 17:58:08 +00:00
David Benjamin
7227990ef1 More SSL_SESSION serialization functions.
Change-Id: I2dd8d073521a230b2b0c4e74ec3d6eeb4899623e
Reviewed-on: https://boringssl-review.googlesource.com/6303
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 17:57:50 +00:00
David Benjamin
066fe0a679 Document fd-based SSL APIs.
Also clean up the code slightly.

Change-Id: I066a389242c46cdc7d41b1ae9537c4b7716c92a2
Reviewed-on: https://boringssl-review.googlesource.com/6302
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 17:56:25 +00:00
David Benjamin
cef1eb4c1c Put renego functions together.
Change-Id: I3bfbf90a790a10e4464e0e39bbd7c0c2bee9fe35
Reviewed-on: https://boringssl-review.googlesource.com/6301
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 17:52:22 +00:00
David Benjamin
d5635d476c Fix ssl3.h / ssl.h circular dependency.
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>
2015-10-26 17:50:32 +00:00
David Benjamin
8370bfd6d1 Remove unhelpful warning about changing state numbers.
This dates all the way to SSLeay 0.9.0b. At this point the
application/handshake interleave logic in ssl3_read_bytes was already
present:

((
  (s->state & SSL_ST_CONNECT) &&
  (s->state >= SSL3_ST_CW_CLNT_HELLO_A) &&
  (s->state <= SSL3_ST_CR_SRVR_HELLO_A)
 ) || (
  (s->state & SSL_ST_ACCEPT) &&
  (s->state <= SSL3_ST_SW_HELLO_REQ_A) &&
  (s->state >= SSL3_ST_SR_CLNT_HELLO_A)
 )

The comment is attached to SSL3_ST_SR_CLNT_HELLO_A, so I suspect this is
what it was about. This logic is gone now, so let's remove that scary
warning.

Change-Id: I45f13b53b79e35d80e6074b0942600434deb0684
Reviewed-on: https://boringssl-review.googlesource.com/6299
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 17:48:57 +00:00
David Benjamin
9f6b5266d9 Fix typo.
(Imported from upstream's ec3a7c9b3729cd45c550222556100666aedc5bbc.)

Change-Id: I9f281fc03e6ece628d46344cf2c0850dd3bcd703
Reviewed-on: https://boringssl-review.googlesource.com/6343
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 17:48:24 +00:00
David Benjamin
036152e6a5 Fix incorrect error-handling in BN_div_recp.
See upstream's e90f1d9b74275c11e3492e521e46f4b1afa6f883.

Change-Id: I68470acb97dac59e586b1c72aad50de6bd0156cb
Reviewed-on: https://boringssl-review.googlesource.com/6342
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 17:48:10 +00:00
David Benjamin
0ea470fdb2 Fix self-signed handling.
Don't mark a certificate as self-signed if keyUsage is present and
certificate signing is not asserted.

PR#3979

(Imported from upstream's e272f8ef8f63298466494adcd29512797ab1eece.)

Change-Id: I3120832f32455e8e099708fa2491d85d3d4a3930
Reviewed-on: https://boringssl-review.googlesource.com/6341
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 17:46:22 +00:00
David Benjamin
dd6fed9704 Explicitly handle empty NewSessionTickets on the client.
RFC 5077 explicitly allows the server to change its mind and send no
ticket by sending an empty NewSessionTicket. See also upstream's
21b538d616b388fa0ce64ef54da3504253895cf8.

CBS_stow handles this case somewhat, so we won't get confused about
malloc(0) as upstream did. But we'll still fill in a bogus SHA-256
session ID, cache the session, and send a ClientHello with bogus session
ID but empty ticket extension. (The session ID field changes meaning
significantly when the ticket is or isn't empty. Non-empty means "ignore
the session ID, but echo if it resuming" while empty means "I support
tickets, but am offering this session ID".

The other behavior change is that a server which changes its mind on a
resumption handshake will no longer override the client's session cache
with a ticket-less session.

(This is kind of silly. Given that we don't get completely confused due
to CBS_stow, it might not be worth bothering with the rest. Mostly it
bugged me that we send an indicator session ID with no ticket.)

Change-Id: Id6b5bde1fe51aa3e1f453a948e59bfd1e2502db6
Reviewed-on: https://boringssl-review.googlesource.com/6340
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 17:44:54 +00:00
David Benjamin
07e138425d Move remaining functions out of tls1.h.
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>
2015-10-26 17:38:40 +00:00
Adam Langley
10a1a9d32e Update references to the padding draft.
The padding draft is now RFC 7685:
https://www.rfc-editor.org/rfc/rfc7685.txt

Change-Id: I26945b10e7923e75c016232c663baac11c4389ae
2015-10-21 14:49:23 -07:00
Adam Langley
6a7cfbe06a Allow ARM capabilities to be set at compile time.
Some ARM environments don't support |getauxval| or signals and need to
configure the capabilities of the chip at compile time. This change adds
defines that allow them to do so.

Change-Id: I4e6987f69dd13444029bc7ac7ed4dbf8fb1faa76
Reviewed-on: https://boringssl-review.googlesource.com/6280
Reviewed-by: Adam Langley <agl@google.com>
2015-10-20 22:40:15 +00:00
David Benjamin
c2ae53db6d Document alert handling.
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>
2015-10-20 19:03:24 +00:00
David Benjamin
b86847c24b Clean up ssl_stat.c slightly.
(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>
2015-10-20 18:49:51 +00:00
David Benjamin
1a1b34d759 Deprecate SSL_get_(peer_)finished.
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>
2015-10-20 18:48:41 +00:00
David Benjamin
93d17499e9 Deprecate SSL_want*.
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>
2015-10-20 18:42:15 +00:00
David Benjamin
7f393f72a5 Unexport SSL_SESSION_ASN1_VERSION.
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>
2015-10-20 18:41:08 +00:00
David Benjamin
79a0589dc4 Ditch remaining filename comments from public headers and ssl/
Change-Id: I8fc795d18aacb0c929b82e7d58514b22103e2106
Reviewed-on: https://boringssl-review.googlesource.com/6292
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-20 18:40:05 +00:00
David Benjamin
d4c2bceaab Document early callback functions.
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>
2015-10-20 18:29:33 +00:00
David Benjamin
cfdee213f8 Add use counters for SSL_OP_TLS_D5_BUG and SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER.
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>
2015-10-20 18:22:47 +00:00
David Benjamin
96e97b1bf1 Convert ssl3_send_channel_id to CBB.
In doing so, simplify the mess around serializing the public key.
Channel ID specifies that you write x and y concatenated. Rather than
using the X9.62 serialization and chopping bits off, get the affine
coordinates and write them out in the same way we write r and s.

Also unify the P-256 sanity check around SSL_set1_tls_channel_id and
actually check the curve NID.

BUG=468889

Change-Id: I228877b736c9722e368d315064ce3ae6893adfc0
Reviewed-on: https://boringssl-review.googlesource.com/6201
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-20 18:16:46 +00:00
David Benjamin
e97b96224c Convert ssl3_send_next_proto to CBB.
BUG=468889

Change-Id: I841b2816ba47c8c1129e333012272902a52cafd3
Reviewed-on: https://boringssl-review.googlesource.com/6200
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-20 18:08:28 +00:00
David Benjamin
5638046cd7 Convert ssl3_send_server_hello to CBB.
BUG=468889

Change-Id: I899d67addbff01c64175f47b19ca2b688626405b
Reviewed-on: https://boringssl-review.googlesource.com/6191
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-20 18:04:48 +00:00
David Benjamin
e8d53508ca Convert ssl3_send_client_hello to CBB.
Start converting the ones we can right now. Some of the messier ones
resize init_buf rather than assume the initial size is sufficient, so
those will probably wait until init_buf is gone and the handshake's
undergone some more invasive surgery. The async ones will also require
some thought. But some can be incrementally converted now.

BUG=468889

Change-Id: I0bc22e4dca37d9d671a488c42eba864c51933638
Reviewed-on: https://boringssl-review.googlesource.com/6190
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-20 17:56:19 +00:00
Matt Braithwaite
978f16ea08 size_t RSA functions.
This extends 79c59a30 to |RSA_public_encrypt|, |RSA_private_encrypt|,
and |RSA_public_decrypt|.  It benefits Conscrypt, which expects these
functions to have the same signature as |RSA_public_private_decrypt|.

Change-Id: Id1ce3118e8f20a9f43fd4f7bfc478c72a0c64e4b
Reviewed-on: https://boringssl-review.googlesource.com/6286
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-10-19 23:54:47 +00:00
Eric Roman
63fa118f3a Reject iterations=0 when calling PKCS5_PBKDF2_HMAC().
BUG=https://crbug.com/534961

Change-Id: I69e2434bf8d5564711863c393ee3bafe3763cf24
Reviewed-on: https://boringssl-review.googlesource.com/5932
Reviewed-by: Adam Langley <agl@google.com>
2015-10-13 19:40:55 +00:00
David Benjamin
c617413527 Remove SSL_SESSION_print*.
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>
2015-10-13 19:24:14 +00:00
Brian Smith
e5ae760a96 Silence MSVC warning C4210.
The warning is:

    C4210: nonstandard extension used : function given file scope.

It is caused by function declarations that aren't at the top level in a
file.

Change-Id: Ib1c2ae64e15e66eb0a7255a29c0e560fbf55c2b2
Reviewed-on: https://boringssl-review.googlesource.com/6210
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-10-13 18:20:29 +00:00
David Benjamin
b735b1b6e4 Document that SSL_CTX_sess_set_get_cb's id parameter should be const.
See also upstream's bf0fc41266f17311c5db1e0541d3dd12eb27deb6.

Change-Id: Ib692b0ad608f2e3291f2aeab2ad98a7e177d5851
Reviewed-on: https://boringssl-review.googlesource.com/6150
Reviewed-by: Adam Langley <agl@google.com>
2015-10-13 18:19:28 +00:00
David Benjamin
f88b81aa75 Put OCSP and SCT accessors with SSL_get_peer_certificate.
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>
2015-10-13 18:19:06 +00:00
David Benjamin
8ac00cafbf Document DTLS-specific retransmit and MTU functions.
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>
2015-10-13 18:18:54 +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
9f85949baf Document a few more miscellaneous things.
Change-Id: Ib4829bf9344341e3d4fe90c7cea66e217366fe97
Reviewed-on: https://boringssl-review.googlesource.com/6091
Reviewed-by: Adam Langley <agl@google.com>
2015-10-13 18:18:19 +00:00
David Benjamin
9f4913f4a8 Remove the ifdef around ec.h's ec_key.h compatibility include
It doesn't actually do anything.

Change-Id: I8a5748dc86b842406cc656a5b251e1a7c0092377
Reviewed-on: https://boringssl-review.googlesource.com/6090
Reviewed-by: Adam Langley <alangley@gmail.com>
Reviewed-by: Adam Langley <agl@google.com>
2015-10-13 18:14:52 +00:00