It doesn't depend on the cipher now that export ciphers are gone. It need only
be called once. Also remove the valid bit; nothing ever reads it. Its output is
also only used within a function, so make mask_k and mask_a local variables.
So all the configuration-based checks are in one place, change the input
parameter from CERT to SSL and move the PSK and ECDHE checks to the mask
computation. This avoids having to evaluate the temporary EC key for each
cipher.
The remaining uses are on the client which uses them differently (disabled
features rather than enabled ones). Those too may as well be local variables,
so leave a TODO.
Change-Id: Ibcb574341795d4016ea749f0290a793eed798874
Reviewed-on: https://boringssl-review.googlesource.com/2287
Reviewed-by: Adam Langley <agl@google.com>
With this, `git cl upload` will do the right thing for boringssl.
BUG=none
Change-Id: Icc83144ef39bc71c795c411e608e0b8c009f3a04
Reviewed-on: https://boringssl-review.googlesource.com/2331
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Many are now unused. Only two are currently considered in cipher selection:
CERT_PKEY_VALID and CERT_PKEY_SIGN. (As per previous commits, this is either
bizarre due to limited slots or redundant with ssl_early_callback_ctx. We can
probably prune this too.)
This also fixes a bug where DTLS 1.0 went through a TLS 1.2 codepath. As the
DTLS code is currently arranged, all version comparisons must be done via
macros like SSL_USE_SIGALGS. (Probably we should add functions to map from DTLS
to TLS versions and slowly move the library to using the TLS version as
in-memory representation.)
Change-Id: I89bcf5b7b9ea5cdecf54f4445156586377328fe0
Reviewed-on: https://boringssl-review.googlesource.com/2286
Reviewed-by: Adam Langley <agl@google.com>
It's new in OpenSSL 1.0.2 so it's never set by existing code. This removes gobs
and gobs of complexity from tls1_check_chain. It only checks the local
certificate, not the peer certificate. The uses appear to be:
- Sanity-check configuration. Not worth the complexity.
- Guide in selecting ciphers based on ClientHello parameters and which
certificates in the CERT_PKEY are compatible. This isn't very useful one its
own since the CERT_PKEY array only stores one slot per type (e.g. you cannot
configure RSA/SHA-1 and RSA/SHA-256).
- For the (currently removed) SSL_check_chain to return more information based
on ClientHello parameters and guide selecting a certificate. This is
potentially useful but, as noted in the commit which removed it, redundant
with ssl_early_callback_ctx.
This CL is largely mechanical removing of dead codepaths. The follow-up will
clean up the now unnecessary parts of this function.
Change-Id: I2ebfa17e4f73e59aa1ee9e4ae7f615af2c6cf590
Reviewed-on: https://boringssl-review.googlesource.com/2285
Reviewed-by: Adam Langley <agl@google.com>
Get rid of now dead codepaths.
Change-Id: I3b5d49097cba70c5698a230cc6c1d79bdd0f0880
Reviewed-on: https://boringssl-review.googlesource.com/2284
Reviewed-by: Adam Langley <agl@google.com>
Both of these are newly-exported in OpenSSL 1.0.2, so they cannot be used by
current consumers.
This was added in upstream's 18d7158809 to
support custom selection of certificates. The intent seems to be that you
listen to cert_cb and use SSL_check_chain to lean on OpenSSL to process
signature algorithms list for you.
Unfortunately, the implementation is slightly suspect: it uses the same
function as the codepath which mutates and refers to the CERT_PKEY of the
matching type. Some access was guarded by check_flags, but this is too
complex. Part of it is also because the matching digest is selected early and
we intend to connect this to EVP_PKEY_supports_digest so it is no longer a
property of just the key type.
Let's remove the hook for now, to unblock removing a lot of complexity. After
cleaning up this area, a function like this could be cleaner to support, but
we already have a version of this: select_certificate_cb and
ssl_early_callback_ctx.
Change-Id: I3add425b3996e5e32d4a88e14cc607b4fdaa5aec
Reviewed-on: https://boringssl-review.googlesource.com/2283
Reviewed-by: Adam Langley <agl@google.com>
This is maintained just to distinguish whether the digest was negotiated or we
simply fell back to assuming SHA-1 support. No code is sensitive to this flag
and it adds complexity because it is set at a different time, for now, from the
rest of valid_flags.
The flag is new in OpenSSL 1.0.2, so nothing external could be sensitive to it.
Change-Id: I9304e358d56f44d912d78beabf14316d456bf389
Reviewed-on: https://boringssl-review.googlesource.com/2282
Reviewed-by: Adam Langley <agl@google.com>
This is new in OpenSSL 1.0.2 so it isn't used anywhere. Cuts down slightly on
connection-global state associated with signature algorithm processing.
Repurposing the digest field to mean both "the digest we choose to sign with
this key" and "the digest the last signature we saw happened to use" is
confusing.
Change-Id: Iec4d5078c33e271c8c7b0ab221c356ee8480b89d
Reviewed-on: https://boringssl-review.googlesource.com/2281
Reviewed-by: Adam Langley <agl@google.com>
This is intended for TLS client auth with Windows CAPI- and CNG-backed keys
which implement sign over sign_raw and do not support all hash functions. Only
plumbed through RSA for now.
Change-Id: Ica42e7fb026840f817a169da9372dda226f7d6fd
Reviewed-on: https://boringssl-review.googlesource.com/2250
Reviewed-by: Adam Langley <agl@google.com>
Just the negotiation portion as everything else is external. This feature is
used in WebRTC.
Change-Id: Iccc3983ea99e7d054b59010182f9a56a8099e116
Reviewed-on: https://boringssl-review.googlesource.com/2310
Reviewed-by: Adam Langley <agl@google.com>
Also add functionality for setting external buffers to give the
caller better control of the buffers. This is typical needed if OS
sockets can outlive the bio pair.
Change-Id: I500f0c522011ce76e9a9bce5d7b43c93d9d11457
Prior to this change, BoringSSL maintained a 2-byte buffer for alerts,
and would support reassembly of fragmented alerts.
NSS does not support fragmented alerts, nor would any reasonable
implementation produce them. Remove fragmented alert handling and
produce an error if a fragmented alert has ever been encountered.
Change-Id: I31530ac372e8a90b47cf89404630c1c207cfb048
Reviewed-on: https://boringssl-review.googlesource.com/2125
Reviewed-by: Adam Langley <agl@google.com>
All of NSS, upstream OpenSSL, SChannel, and Secure Transport require, on the
client, that the ServerHello version match the session's version on resumption.
OpenSSL's current behavior is incompatible with all of these. Fall back to a
full handshake on the server instead of mismatch.
Add a comment on the client for why we are, as of
30ddb434bfb845356fbacb6b2bd51f8814c7043c, not currently enforcing the same in
the client.
Change-Id: I60aec972d81368c4ec30e2fd515dabd69401d175
Reviewed-on: https://boringssl-review.googlesource.com/2244
Reviewed-by: Adam Langley <agl@google.com>
No need to include unistd.h. (Though it probably should include string.h for
memcmp and strcmp.)
Change-Id: Ib09d2da4f7079c9d87338df75ec3560f4f203764
Reviewed-on: https://boringssl-review.googlesource.com/2260
Reviewed-by: Adam Langley <agl@google.com>
Clients all consistently reject mismatches. If a different version was
negotiated, a server should ignore the resumption. This doesn't actually affect
current tests. We really want to be making this change in BoringSSL (and then
upstream), but get the Go half into shape first.
Change-Id: Ieee7e141331d9e08573592e661889bd756dccfa9
Reviewed-on: https://boringssl-review.googlesource.com/2243
Reviewed-by: Adam Langley <agl@google.com>
The limit increased from 32 to 255 between DTLS 1.0 and DTLS 1.2.
Change-Id: I329a59f9ba2bccc70282e2b47679c57b67e5ed43
Reviewed-on: https://boringssl-review.googlesource.com/2242
Reviewed-by: Adam Langley <agl@google.com>
These'll get removed once most of renego support is gone, but this is to prove
removing the warning alert from the previous commit still prevents legacy
renegotiations.
Change-Id: I7d9d95e1d4c5d23d3b6d170938a5499a65f2d5ea
Reviewed-on: https://boringssl-review.googlesource.com/2236
Reviewed-by: Adam Langley <agl@google.com>
Ensure that the client rejects it with UNEXPECTED_MESSAGE, not by attempting to
decode it.
Change-Id: Ifc5613cf1152e0f7dcbee73e05df1ef367dfbfd5
Reviewed-on: https://boringssl-review.googlesource.com/2232
Reviewed-by: Adam Langley <agl@google.com>
There's not much point in retaining the identity hint in the SSL_SESSION. This
avoids the complexity around setting psk_identity hint on either the SSL or the
SSL_SESSION. Introduce a peer_psk_identity_hint for the client to store the one
received from the server.
This changes the semantics of SSL_get_psk_identity_hint; it now only returns
the value configured for the server. The client learns the hint through the
callback. This is compatible with the one use of this API in conscrypt (it
pulls the hint back out to pass to a callback).
Change-Id: I6d9131636b47f13ac5800b4451436a057021054a
Reviewed-on: https://boringssl-review.googlesource.com/2213
Reviewed-by: Adam Langley <agl@google.com>
This is an experimental flag that dates back to SSLeay 0.8.1b or earlier. It's
never set internally and never set in consumers.
Change-Id: I922583635c9f3d8d93f08f1707531ad22a26ae6a
Reviewed-on: https://boringssl-review.googlesource.com/2214
Reviewed-by: Adam Langley <agl@google.com>
At the record layer, DTLS maintains a window of seen sequence numbers to detect
replays. Add tests to cover that case. Test both repeated sequence numbers
within the window and sequence numbers past the window's left edge. Also test
receiving sequence numbers far past the window's right edge.
Change-Id: If6a7a24869db37fdd8fb3c4b3521b730e31f8f86
Reviewed-on: https://boringssl-review.googlesource.com/2221
Reviewed-by: Adam Langley <agl@google.com>
The error queue should only take ownership of the data if ERR_get_* is called,
not ERR_peek_*. Add a test for ERR_peek_error_line_data.
Change-Id: I976fc90fb54437dff723418ef3afd94f1c967922
Reviewed-on: https://boringssl-review.googlesource.com/2237
Reviewed-by: Adam Langley <agl@google.com>
Saves doing it ad-hoc all the time.
Change-Id: Ic1a1180f56eec37c19799649bb8f18237bd617f8
Reviewed-on: https://boringssl-review.googlesource.com/2241
Reviewed-by: Adam Langley <agl@google.com>
r and s are scalars, not EC coordinates.
Change-Id: I46a20215d3c602559c18c74a1da9a91543ea73ca
Reviewed-on: https://boringssl-review.googlesource.com/2240
Reviewed-by: Adam Langley <agl@google.com>
If psk_len were 0, it would already have been an error earlier. The PSK cipher
suites don't lose the other_secret || psk construction if the PSK happens to be
empty.
Change-Id: I1917236720d0862658562bc8f014cb827ee9aed5
Reviewed-on: https://boringssl-review.googlesource.com/2233
Reviewed-by: Adam Langley <agl@google.com>
Server-side support was removed in 77a942b7fedb58ae1afee042255b4b9267abebca,
but client-side support was retained as it appeared NSS supported this.
However, this is not the case: ssl3_HandleServerKeyExchange only allows a
ServerKeyExchange message if hs.ws is in an appropriate state.
ssl3_AuthCertificate only sets it to allow ServerKeyExchange if it is a key
exchange that normally uses it or if is_limited is set. is_limited is only set
for the export cipher suites.
Thus we can safely remove this without waiting on gathering UMA data.
BUG=chromium:400587
Change-Id: I9aefb742dbb2d99c13340ab48017e1ceee04bc2f
Reviewed-on: https://boringssl-review.googlesource.com/2230
Reviewed-by: Adam Langley <agl@google.com>
This was added in upstream's 82e610e2cf. The
commit message cites draft-ietf-tls-renegotiation which was on
draft-ietf-tls-renegotiation-01 at the time. The text in question (6.2 Server
Considerations) is no longer in RFC 5746. The RFC now recommends terminating
the connection which is much simpler.
It also was wrong anyway as it checked s->ctx->options instead of s->options
for SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION.
Removing that block will result in the connection being terminated in
ssl_scan_clienthello_tlsext.
Change-Id: Ie222c78babd3654c5023ad07ac0d8e0adde68698
Reviewed-on: https://boringssl-review.googlesource.com/2235
Reviewed-by: Adam Langley <agl@google.com>
Parameters like these should not change between 32-bit and 64-bit. 64 is also
the value recommended in RFC 6347, section 4.1.2.6. Document those fields while
I'm here.
Change-Id: I8481ee0765ff3d261a96a2e1a53b6ad6695b2d42
Reviewed-on: https://boringssl-review.googlesource.com/2222
Reviewed-by: Adam Langley <agl@google.com>
This was added in http://rt.openssl.org/Ticket/Display.html?id=2033 to support
a mode where a DTLS socket would statelessly perform the ClientHello /
HelloVerifyRequest portion of the handshake, to be handed off to a socket
specific to this peer address.
This is not used by WebRTC or other current consumers. If we need to support
something like this, it would be cleaner to do the listen portion (cookieless
ClientHello + HelloVerifyRequest) externally and then spin up an SSL instance
on receipt of a cookied ClientHello. This would require a slightly more complex
BIO to replay the second ClientHello but would avoid peppering the DTLS
handshake state with a special short-circuiting mode.
Change-Id: I7a413932edfb62f8b9368912a9a0621d4155f1aa
Reviewed-on: https://boringssl-review.googlesource.com/2220
Reviewed-by: Adam Langley <agl@google.com>
One of them was never implemented upstream or downstream. The other no longer
works in BoringSSL. They're not used within BoringSSL (this still compiles),
even in X509_INFO, and do not appear to be used by consumers. If they were, we
would like to know via a compile failure.
This removes the last consumer within BoringSSL of the ASN.1 parsing macros.
Change-Id: Ifb72b1fcd0a4f7b3e6b081486f8638110872334b
Reviewed-on: https://boringssl-review.googlesource.com/2203
Reviewed-by: Adam Langley <agl@google.com>
Imported from upstream's 9bed73adaa.
Upstream's commit appears to have been based on BoringSSL's commits to
improve the constant-time behaviour of RSA padding checks and thus I've
not tried to import those bits of the change.
Change-Id: I0ea5775b0f1e18741bbbc9f792a6af0d3d2a4caf
When calling X509_set_version to set v1 certificate, that should mean
that the version number field is omitted.
(Imported from upstream's 8c0d19d857)
Change-Id: If433fda7b6ccbd899f3379a38581c351cf4a82da
Can't really happen, but the flow of control isn't obvious. Add an
initializer.
(Imported from upstream's fa2ae04c40)
Change-Id: If393687bca9f505b825feffaf2a63895a0ea5b6a
Pull constant-time methods out to a separate header, add tests.
(Imported from upstream's 9a9b0c0401 and
27739e9265)
Change-Id: Id570f5c531aca791112929e6258989f43c8a78d7
The old code implicitly relies on the ASN.1 code returning a \0-prefixed
buffer when the buffer length is 0. Change this to verify explicitly
that the ASN.1 string has positive length.
(Imported from upstream's 7f7c05ca63)
Change-Id: Icc6c44b874bdcb02374016a36d209830d6162a8a
When d2i_ECPrivateKey reads a private key with a missing (optional)
public key, generate one automatically from the group and private key.
(Imported from upstream's 2083f7c465)
Change-Id: I9e5090de87cf846ab92e4be5b6bf64e6091d02e4
State hanging off the SSL gets freed in two places.
Change-Id: I41a8d2a7cab35f0098396006e1f6380038ec471a
Reviewed-on: https://boringssl-review.googlesource.com/2212
Reviewed-by: Adam Langley <agl@google.com>
David Benjamin
Nico Weber
Håvard Molland
Alex Chernyakhovsky
Adam Langley