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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Change-Id: I48885402b88309bb514554d209e1827d31738756
Reviewed-on: https://boringssl-review.googlesource.com/6211
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <alangley@gmail.com>
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>
OpenSSL's BIO_get_fd returns the fd or -1, not a boolean.
Change-Id: I12a3429c71bb9c9064f9f91329a88923025f1fb5
Reviewed-on: https://boringssl-review.googlesource.com/6080
Reviewed-by: Adam Langley <agl@google.com>
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>
The goto always jumps into the loop so the for's initialisation
expression can never be executed. Clang warns about this.
Change-Id: I3c3d4b8430754099e9ca6fd20101868c40165245
This imports the Google-authored P-224 implementation by Emilia Käsper
and Bodo Möller that is also in upstream OpenSSL.
Change-Id: I16005c74a2a3e374fb136d36f3f6569dab9d8919
Reviewed-on: https://boringssl-review.googlesource.com/6145
Reviewed-by: Adam Langley <agl@google.com>
BUF_memdup tries to avoid mallocing zero bytes (and thus unduly
returning an error for a NULL return value) by testing whether the input
buffer is NULL. This goes back to the original OpenSSL code.
However, when |ext_npn_parse_serverhello| tries to use |BUF_memdup| to
copy an NPN value returned by a callback, some callbacks just set the
output /length/ to zero to indicate an empty value. Thus, when
|BUF_memdup| tests the pointer, it's an uninitialised value and MSan
throws an error.
Since passing a NULL pointer to |BUF_memdup| better imply that the
length is zero, while the reverse empirically isn't true, testing the
length seems safer.
Change-Id: I06626f7dfb761de631fd997bda60057b76b8da94
Bazel on Mac requires some alterations to the generated build files.
This change updates generate_build_files.py to emit suitable Bazel
files. This will require some tweaks to projects that build with Bazel.
Change-Id: I3d68ec754b8abaa41a348f86c32434477f2c5e1c
Reviewed-on: https://boringssl-review.googlesource.com/6146
Reviewed-by: Adam Langley <agl@google.com>
Previously a value of 0 would be accepted and intepreted as equivalent
to 1. This contradicts RFC 2898 which defines:
iterationCount INTEGER (1..MAX),
BUG=https://crbug.com/534961
Change-Id: I89623980f99fde3ca3780880d311955d3f6fe0b5
Reviewed-on: https://boringssl-review.googlesource.com/5971
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
