ssl_cipher_list_to_bytes is client-only, so s->renegotiate worked, but
the only reason the other two worked is because s->renegotiate isn't a
lie on the server before ServerHello.
BUG=429450
Change-Id: If68a986c6ec4a0f16e57a6187238e05b50ecedfc
Reviewed-on: https://boringssl-review.googlesource.com/4822
Reviewed-by: Adam Langley <agl@google.com>
tls1_enc is now SSL_AEAD_CTX_{open,seal}. This starts tidying up a bit
of the record-layer logic. This removes rr->input, as encrypting and
decrypting records no longer refers to various globals. It also removes
wrec altogether. SSL3_RECORD is now only used to maintain state about
the current incoming record. Outgoing records go straight to the write
buffer.
This also removes the outgoing alignment memcpy and simply calls
SSL_AEAD_CTX_seal with the parameters as appropriate. From bssl speed
tests, this seems to be faster on non-ARM and a bit of a wash on ARM.
Later it may be worth recasting these open/seal functions to write into
a CBB (tweaked so it can be malloc-averse), but for now they take an
out/out_len/max_out trio like their EVP_AEAD counterparts.
BUG=468889
Change-Id: Ie9266a818cc053f695d35ef611fd74c5d4def6c3
Reviewed-on: https://boringssl-review.googlesource.com/4792
Reviewed-by: Adam Langley <agl@google.com>
This cuts down on one config knob as well as one case in the renego
combinatorial explosion. Since the only case we care about with renego
is the client auth hack, there's no reason to ever do resumption.
Especially since, no matter what's in the session cache:
- OpenSSL will only ever offer the session it just established,
whether or not a newer one with client auth was since established.
- Chrome will never cache sessions created on a renegotiation, so
such a session would never make it to the session cache.
- The new_session + SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION
logic had a bug where it would unconditionally never offer tickets
(but would advertise support) on renego, so any server doing renego
resumption against an OpenSSL-derived client must not support
session tickets.
This also gets rid of s->new_session which is now pointless.
BUG=429450
Change-Id: I884bdcdc80bff45935b2c429b4bbc9c16b2288f8
Reviewed-on: https://boringssl-review.googlesource.com/4732
Reviewed-by: Adam Langley <agl@google.com>
There's multiple different versions of this check, between
s->s3->have_version (only works at some points), s->new_session (really
weird and not actually right), s->renegotiate (fails on the server
because it's always 2 after ClientHello), and s->s3->tmp.finish_md_len
(super confusing). Add an explicit bit with clear meaning. We'll prune
some of the others later; notably s->renegotiate can go away when
initiating renegotiation is removed.
This also tidies up the extensions to be consistent about whether
they're allowed during renego:
- ALPN failed to condition when accepting from the server, so even
if the client didn't advertise, the server could.
- SCTs now *are* allowed during renego. I think forbidding it was a
stray copy-paste. It wasn't consistently enforced in both ClientHello
and ServerHello, so the server could still supply it. Moreover, SCTs
are part of the certificate, so we should accept it wherever we accept
certificates, otherwise that session's state becomes incomplete. This
matches OCSP stapling. (NB: Chrome will never insert a session created
on renego into the session cache and won't accept a certificate
change, so this is moot anyway.)
Change-Id: Ic9bd1ebe2a2dbe75930ed0213bf3c8ed8170e251
Reviewed-on: https://boringssl-review.googlesource.com/4730
Reviewed-by: Adam Langley <agl@google.com>
When tlsext_ticket_key_cb is used, the full bounds aren't known until
after the callback has returned.
Change-Id: I9e89ffae6944c74c4ca04e6aa28afd3ec80aa1d4
Reviewed-on: https://boringssl-review.googlesource.com/4552
Reviewed-by: Adam Langley <agl@google.com>
It's unused, but for some old #ifdef branch in wpa_supplicant's EAP-FAST
hack, before SSL_set_session_ticket_ext_cb existed.
Change-Id: Ifc11fea2f6434354f756e04e5fc3ed5f1692025e
Reviewed-on: https://boringssl-review.googlesource.com/4550
Reviewed-by: Adam Langley <agl@google.com>
This is a really dumb API wart. Now that we have a limited set of curves that
are all reasonable, the automatic logic should just always kick in. This makes
set_ecdh_auto a no-op and, instead of making it the first choice, uses it as
the fallback behavior should none of the older curve selection APIs be used.
Currently, by default, server sockets can only use the plain RSA key exchange.
BUG=481139
Change-Id: Iaabc82de766cd00968844a71aaac29bd59841cd4
Reviewed-on: https://boringssl-review.googlesource.com/4531
Reviewed-by: Adam Langley <agl@google.com>
See upstream's bd891f098bdfcaa285c073ce556d0f5e27ec3a10. It honestly seems
kinda dumb for a client to do this, but apparently the spec allows this.
Judging by code inspection, OpenSSL 1.0.1 also allowed this, so this avoids a
behavior change when switching from 1.0.1 to BoringSSL.
Add a test for this, which revealed that, unlike upstream's version, this
actually works with ecdh_auto since tls1_get_shared_curve also needs updating.
(To be mentioned in newsletter.)
Change-Id: Ie622700f17835965457034393b90f346740cfca8
Reviewed-on: https://boringssl-review.googlesource.com/4464
Reviewed-by: Adam Langley <agl@google.com>
These are all masks of some sort (except id which is a combined version and
cipher), so they should use fixed-size unsigned integers.
Change-Id: I058dd8ad231ee747df4b4fb17d9c1e2cbee21918
Reviewed-on: https://boringssl-review.googlesource.com/4283
Reviewed-by: Adam Langley <agl@google.com>
The rest of ssl/ still includes things everywhere, but this at least fixes the
includes that were implicit from ssl/internal.h.
Change-Id: I7ed22590aca0fe78af84fd99a3e557f4b05f6782
Reviewed-on: https://boringssl-review.googlesource.com/4281
Reviewed-by: Adam Langley <agl@google.com>
Match the other internal headers.
Change-Id: Iff7e2dd06a1a7bf993053d0464cc15638ace3aaa
Reviewed-on: https://boringssl-review.googlesource.com/4280
Reviewed-by: Adam Langley <agl@google.com>
There's multiple sets of APIs for selecting the curve. Fold away
SSL_OP_SINGLE_ECDH_USE as failing to set it is either a no-op or a bug. With
that gone, the consumer only needs to control the selection of a curve, with
key generation from then on being uniform. Also clean up the interaction
between the three API modes in s3_srvr.c; they were already mutually exclusive
due to tls1_check_ec_tmp_key.
This also removes all callers of EC_KEY_dup (and thus CRYPTO_dup_ex_data)
within the library.
Change-Id: I477b13bd9e77eb03d944ef631dd521639968dc8c
Reviewed-on: https://boringssl-review.googlesource.com/4200
Reviewed-by: Adam Langley <agl@google.com>
Quite a few functions reported wrong function names when pushing
to the error stack.
Change-Id: I84d89dbefd2ecdc89ffb09799e673bae17be0e0f
Reviewed-on: https://boringssl-review.googlesource.com/4080
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Align with upstream's renames from a while ago. These names are considerably
more standard. This also aligns with upstream in that both "ECDHE" and "EECDH"
are now accepted in the various cipher string parsing bits.
Change-Id: I84c3daeacf806f79f12bc661c314941828656b04
Reviewed-on: https://boringssl-review.googlesource.com/4053
Reviewed-by: Adam Langley <agl@google.com>
See also upstream's 34e3edbf3a10953cb407288101fd56a629af22f9. This fixes
CVE-2015-0291. Also bubble up malloc failures in tls1_set_shared_sigalgs. Tidy
up style a bit and remove unnecessary check (it actually is unnecessary; see
https://boringssl-review.googlesource.com/4042).
Change-Id: Idfb31a90fb3e56ef6fe7701464748a5c1603f064
Reviewed-on: https://boringssl-review.googlesource.com/4047
Reviewed-by: Adam Langley <agl@google.com>
Some things were misindented in the reformatting.
Change-Id: I97642000452ce4d5b4c8a39b794cec13097d8760
Reviewed-on: https://boringssl-review.googlesource.com/3870
Reviewed-by: Adam Langley <agl@google.com>
None of these are version-specific. SSL_PROTOCOL_METHOD's interface will change
later, but this gets us closer to folding away SSL3_ENC_METHOD.
Change-Id: Ib427cdff32d0701a18fe42a52cdbf798f82ba956
Reviewed-on: https://boringssl-review.googlesource.com/3769
Reviewed-by: Adam Langley <agl@google.com>
Found while diagnosing some crashes and hangs in the malloc tests. This (and
the follow-up) get us further but does not quite let the malloc tests pass
quietly, even without valgrind. DTLS silently ignores some malloc failures
(confusion with silently dropping bad packets) which then translate to hangs.
Change-Id: Ief06a671e0973d09d2883432b89a86259e346653
Reviewed-on: https://boringssl-review.googlesource.com/3482
Reviewed-by: Adam Langley <agl@google.com>
They're not in the duplicated handshake state machines anyway. But we still
shouldn't negotiate them. d1_pkt.c assumes Finished is the only post-CCS
handshake message. An unexpected handshake message in the current epoch may
either be a retransmit/out-of-order message from the previous handshake, or a
message from the next handshake (also potentially out-of-order). In the former
case, we shouldn't spin up another handshake state machine instance.
(This assumption is required due to a protocol bug. DTLS resets sequence
numbers after a handshake, so it is necessary to categorize handshake fragments
by pre-CCS and post-CCS to distinguish between retransmit and renego.)
Change-Id: Ib3c1c7085c729e36a40f7ff14494733156924a24
Reviewed-on: https://boringssl-review.googlesource.com/3028
Reviewed-by: Adam Langley <agl@google.com>
The under 32 constraint is silly; it's to check for duplicate curves in
library-supplied configuration. That API is new as of 1.0.2. It doesn't seem
worth bothering; if the caller supplies a repeated value, may as well emit a
repeated one and so be it. (Probably no one will ever call that function
outside of maybe test code anyway.)
While I'm here, remove the 0 constraint too. It's not likely to change, but
removing the return value overload seems easier than keeping comments about it
comments about it.
Change-Id: I01d36dba1855873875bb5a0ec84b040199e0e9bc
Reviewed-on: https://boringssl-review.googlesource.com/2844
Reviewed-by: Adam Langley <agl@google.com>
We only implement four curves (P-224, P-256, P-384, and P-521) and only
advertise the latter three by default. Don't maintain entries corresponding to
all the unimplemented curves.
Change-Id: I1816a10c6f849ca1d9d896bc6f4b64cd6b329481
Reviewed-on: https://boringssl-review.googlesource.com/2843
Reviewed-by: Adam Langley <agl@google.com>
They both happen to be zero, but OBJ_undef is a type error; OBJ_foo expands to
a comma-separated list of integers.
Change-Id: Ia5907dd3bc83240b7cc98af6456115d2efb48687
Reviewed-on: https://boringssl-review.googlesource.com/2842
Reviewed-by: Adam Langley <agl@google.com>
When parsing ClientHello clear any existing extension state from
SRP login and SRTP profile.
(Imported from upstream's 4f605ccb779e32a770093d687e0554e0bbb137d3)
More state that should be systematically reset across handshakes. Add a reset
on the ServerHello end too since that was missed.
Change-Id: Ibb4549acddfd87caf7b6ff853e2adbfa4b7e7856
Reviewed-on: https://boringssl-review.googlesource.com/2838
Reviewed-by: Adam Langley <agl@google.com>
This lets us fold away the SSLv3-specific generate_master_secret. Once SSLv3
uses AEADs, others will fold away as well.
Change-Id: I27c1b75741823bc6db920d35f5dd5ce71b6fdbb3
Reviewed-on: https://boringssl-review.googlesource.com/2697
Reviewed-by: Adam Langley <agl@google.com>
This avoids needing a should_add_to_finished_hash boolean on do_write. The
logic in do_write was a little awkward because do_write would be called
multiple times if the write took several iterations. This also gets complex if
DTLS retransmits are involved. (At a glance, it's not obvious the
BIO_CTRL_DGRAM_MTU_EXCEEDED case actually works.)
Doing it as the handshake message is being prepared avoids this concern. It
also gives a natural point for the extended master secret logic which needs to
do work after the finished hash has been sampled.
As a bonus, we can remove s->d1->retransmitting which was only used to deal
with this issue.
Change-Id: Ifedf23ee4a6c5e08f960d296a6eb1f337a16dc7a
Reviewed-on: https://boringssl-review.googlesource.com/2604
Reviewed-by: Adam Langley <agl@google.com>
There's an undefined one not used anywhere. The others ought to be const. Also
move the forward declaration to ssl.h so we don't have to use the struct name.
Change-Id: I76684cf65255535c677ec19154cac74317c289ba
Reviewed-on: https://boringssl-review.googlesource.com/2561
Reviewed-by: Adam Langley <agl@google.com>
There's no need to make that conditional.
Change-Id: Idac1aba42b22e3fe8e7731ae4ecb5ebc4183336c
Reviewed-on: https://boringssl-review.googlesource.com/2550
Reviewed-by: Adam Langley <agl@google.com>
Some code predated the RFCs themselves, but the RFCs now exist. Also remove
now obsolete comments and some unused #defines.
See upstream's cffeacd91e70712c99c431bf32a655fa1b561482. (Though this predates
it; I just remembered I never uploaded it.)
Change-Id: I5e56f0ab6b7f558820f72e84dfdbc71a8c23cb91
Reviewed-on: https://boringssl-review.googlesource.com/2475
Reviewed-by: Adam Langley <agl@google.com>
The ClientHello record is padded to 1024 bytes when
fastradio_padding is enabled. As a result, the 3G cellular radio
is fast forwarded to DCH (high data rate) state. This mechanism
leads to a substantial redunction in terms of TLS handshake
latency, and benefits mobile apps that are running on top of TLS.
Change-Id: I3d55197b6d601761c94c0f22871774b5a3dad614
This is only used for EAP-FAST which we apparently don't need to support.
Remove it outright. We broke it in 9eaeef81fa by
failing to account for session misses.
If this changes and we need it later, we can resurrect it. Preferably
implemented differently: the current implementation is bolted badly onto the
handshake. Ideally use the supplied callbacks to fabricate an appropriate
SSL_SESSION and resume that with as much of the normal session ticket flow as
possible.
The one difference is that EAP-FAST seems to require the probing mechanism for
session tickets rather than the sane session ID echoing version. We can
reimplement that by asking the record layer to probe ahead for one byte.
Change-Id: I38304953cc36b2020611556a91e8ac091691edac
Reviewed-on: https://boringssl-review.googlesource.com/2360
Reviewed-by: Adam Langley <agl@google.com>
The ex_data index may fail to be allocated. Also don't leave a dangling pointer
in handshake_dgst if EVP_DigestInit_ex fails and check a few more init function
failures.
Change-Id: I2e99a89b2171c9d73ccc925a2f35651af34ac5fb
Reviewed-on: https://boringssl-review.googlesource.com/2342
Reviewed-by: Adam Langley <agl@google.com>
tls1_process_sigalgs now only determines the intersection between the peer
algorithms and those configured locally. That list is queried later to
determine the hash algorithm to use when signing CertificateVerify or
ServerKeyExchange.
This is needed to support client auth on Windows where smartcards or CAPI may
not support all hash functions.
As a bonus, this does away with more connection-global state. This avoids the
current situation where digests are chosen before keys are known (for
CertificateVerify) or for slots that don't exist.
Change-Id: Iec3619a103d691291d8ebe08ef77d574f2faf0e8
Reviewed-on: https://boringssl-review.googlesource.com/2280
Reviewed-by: Adam Langley <agl@google.com>
CERT_PKEY_SIGN isn't meaningful since, without strict mode, we always fall back
to SHA-1 anyway. So the digest is never NULL when CERT_PKEY_SIGN is computed.
The entire valid_flags is now back to it's pre-1.0.2 check of seeing if the
certificate and key are configured.
This finally removes the sensitivity between valid_flags and selecting the
digest, so we can defer choosing the digest all we like.
Change-Id: I9f9952498f512d7f0cc799497f7c5b52145a48af
Reviewed-on: https://boringssl-review.googlesource.com/2288
Reviewed-by: Adam Langley <agl@google.com>
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>
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 18d7158809c9722f4c6d2a8af7513577274f9b56 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>
There's no need to store them on the session. They're temporary handshake
state and weren't serialized in d2i_SSL_SESSION anyway.
Change-Id: I830d378ab49aaa4fc6c4c7a6a8c035e2263fb763
Reviewed-on: https://boringssl-review.googlesource.com/1990
Reviewed-by: Adam Langley <agl@google.com>
