We need EnableAllCiphers to make progress so, temporarily, defer the PSK
error. Also flip a true/false bug in the OCSP stapling logic.
Change-Id: Iad597c84393e1400c42b8b290eedc16f73f5ed30
Reviewed-on: https://boringssl-review.googlesource.com/8766
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
deriveTrafficAEAD gets confused by the EnableAllCiphers bug. As a hack,
just return the nil cipher. We only need to progress far enough to read
the shim's error code.
Change-Id: I72d25ac463a03a0e99dd08c38a1a7daef1f94311
Reviewed-on: https://boringssl-review.googlesource.com/8763
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
We'll enable it again later, but the initial land of the 1.3 handshake
will not do resumption. In preparation, turn those off.
Change-Id: I5f98b6a9422eb96be26c4ec41ca7ecde5f592da7
Reviewed-on: https://boringssl-review.googlesource.com/8765
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
In preparation for getting the tests going.
Change-Id: Ifd2ab09e6ce91f99abde759d5db8dc6554521572
Reviewed-on: https://boringssl-review.googlesource.com/8764
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Otherwise adding it to the handshake hash doesn't work right.
Change-Id: I2fabae72e8b088a5df26bbeac946f19144d58733
Reviewed-on: https://boringssl-review.googlesource.com/8762
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We'll enable them once we've gotten it working. For now, our TLS 1.3
believes there is no PSK.
Change-Id: I5ae51266927c8469c671844da9a0f7387c297050
Reviewed-on: https://boringssl-review.googlesource.com/8760
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
RSASSA-PSS with SHA-512 is slightly too large for 1024-bit RSA. One
should not be using 1024-bit RSA, but it's common enough for tests
(including our own in runner before they were regenerated), that we
should probably do the size check and avoid unnecessary turbulence to
everyone else's test setups.
Change-Id: If0c7e401d7d05404755cba4cbff76de3bc65c138
Reviewed-on: https://boringssl-review.googlesource.com/8746
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Tested by having client and server talk to each other. This adds the
certificate_extensions field to CertificateRequest which I'd previously
missed. (We completely ignore the field, with the expectation that the C
code won't have anything useful to do with it either.)
Change-Id: I74f96acd36747d4b6a6f533535e36ea8e94d2be8
Reviewed-on: https://boringssl-review.googlesource.com/8710
Reviewed-by: David Benjamin <davidben@google.com>
[Originally written by nharper, revised by davidben.]
Change-Id: If1d45c33994476f4bc9cd69831b6bbed40f792d0
Reviewed-on: https://boringssl-review.googlesource.com/8599
Reviewed-by: David Benjamin <davidben@google.com>
For now, skip the 1.2 -> 1.1 signal since that will affect shipping
code. We may as well enable it too, but wait until things have settled
down. This implements the version in draft-14 since draft-13's isn't
backwards-compatible.
Change-Id: I46be43e6f4c5203eb4ae006d1c6a2fe7d7a949ec
Reviewed-on: https://boringssl-review.googlesource.com/8724
Reviewed-by: David Benjamin <davidben@google.com>
Now that the odd client/server split (a remnant from the original
crypto/tls code not handling signing-hash/PRF mismatches) is gone, it
can just be pulled from the config.
Change-Id: Idb46c026d6529a2afc2b43d4afedc0aa950614db
Reviewed-on: https://boringssl-review.googlesource.com/8723
Reviewed-by: David Benjamin <davidben@google.com>
Saves worrying about forgetting it. (And indeed I forgot it in the TLS
1.3 code.)
Change-Id: Ibb55a83eddba675da64b7cf2c45eac6348c97784
Reviewed-on: https://boringssl-review.googlesource.com/8722
Reviewed-by: David Benjamin <davidben@google.com>
This way we can test failing client auth without having to worry about
first getting through server auth.
Change-Id: Iaf996d87ac3df702a17e76c26006ca9b2a5bdd1f
Reviewed-on: https://boringssl-review.googlesource.com/8721
Reviewed-by: David Benjamin <davidben@google.com>
[Rebased and tests added by davidben.]
In doing so, regenerate the test RSA certificate to be 2048-bit RSA.
RSA-PSS with SHA-512 is actually too large for 1024-bit RSA. Also make
the sigalg test loop test versions that do and don't work which subsumes
the ecdsa_sha1 TLS 1.3 test.
For now, RSA-PKCS1 is still allowed because NSS has yet to implement
RSA-PSS and we'd like to avoid complicated interop testing.
Change-Id: I686b003ef7042ff757bdaab8d5838b7a4d6edd87
Reviewed-on: https://boringssl-review.googlesource.com/8613
Reviewed-by: David Benjamin <davidben@google.com>
(Of course, it's still signing ServerKeyExchange messages since the
handshake's the old one.)
Change-Id: I35844a329d983f61ed0b5be20b333487406fe7e4
Reviewed-on: https://boringssl-review.googlesource.com/8614
Reviewed-by: David Benjamin <davidben@google.com>
Implement in both C and Go. To test this, route config into all the
sign.go functions so we can expose bugs to skip the check.
Unfortunately, custom private keys are going to be a little weird since
we can't check their curve type. We may need to muse on what to do here.
Perhaps the key type bit should return an enum that includes the curve?
It's weird because, going forward, hopefully all new key types have
exactly one kind of signature so key type == sig alg == sig alg prefs.
Change-Id: I1f487ec143512ead931e3392e8be2a3172abe3d2
Reviewed-on: https://boringssl-review.googlesource.com/8701
Reviewed-by: David Benjamin <davidben@google.com>
That instead happens via signature algorithms, which will be done in a
follow-up commit.
Change-Id: I97bc4646319dddbff62552244b0dd7e9bb2650ef
Reviewed-on: https://boringssl-review.googlesource.com/8700
Reviewed-by: David Benjamin <davidben@google.com>
This is in preparation for TLS 1.3 enforcing curve matches in signature
algorithms.
Change-Id: I82c3a1862703a15e4e36ceb7ec40e27235b620c3
Reviewed-on: https://boringssl-review.googlesource.com/8699
Reviewed-by: David Benjamin <davidben@google.com>
ssl_verify_* already ought to be checking this, so there's only a need
to check against the configured preferences.
Change-Id: I79bc771969c57f953278e622084641e6e20108e3
Reviewed-on: https://boringssl-review.googlesource.com/8698
Reviewed-by: David Benjamin <davidben@google.com>
{sha256,ecdsa} should not be silently accepted for an RSA key.
Change-Id: I0c0eea5071f7a59f2707ca0ea023a16cc4126d6a
Reviewed-on: https://boringssl-review.googlesource.com/8697
Reviewed-by: David Benjamin <davidben@google.com>
TLS 1.3 also forbids signing SHA-1 digests, but this will be done as a
consequence of forbidding PKCS#1 in 1.3 altogether (rsa_sign_sha1) and
requiring a curve match in ECDSA (ecdsa_sha1).
Change-Id: I665971139ccef9e270fd5796c5e6a814a8f663b1
Reviewed-on: https://boringssl-review.googlesource.com/8696
Reviewed-by: David Benjamin <davidben@google.com>
This is a lot more verbose and looks the same between RSA and ECDSA for
now, but it gives us room to implement the various algorithm-specific
checks. ECDSA algorithms must match the curve, PKCS#1 is forbidden in
TLS 1.3, etc.
Change-Id: I348cfae664d7b08195a2ab1190820b410e74c5e9
Reviewed-on: https://boringssl-review.googlesource.com/8694
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This allows us to specify client-only and unused callbacks without
needing to include empty wrappers, and allows us to continue using the
default ext_*_parse_clienthello function for early parsing.
Change-Id: I4104e22a0a6dd6b02f9a5605e9866f6b3de6a097
Reviewed-on: https://boringssl-review.googlesource.com/8743
Reviewed-by: David Benjamin <davidben@google.com>
Upstream have added |EVP_PKEY_up_ref|, but their version returns an int.
Having this function with a different signature like that is dangerous
so this change aligns BoringSSL with upstream. Users of this function in
Chromium and internally should already have been updated.
Change-Id: I0a7aeaf1a1ca3b0f0c635e2ee3826aa100b18157
Reviewed-on: https://boringssl-review.googlesource.com/8736
Reviewed-by: David Benjamin <davidben@google.com>
Rather than blindly select SHA-1 if we can't find a matching one, act as
if the peer advertised rsa_pkcs1_sha1 and ecdsa_sha1. This means that we
will fail the handshake if no common algorithm may be found.
This is done in preparation for removing the SHA-1 default in TLS 1.3.
Change-Id: I3584947909d3d6988b940f9404044cace265b20d
Reviewed-on: https://boringssl-review.googlesource.com/8695
Reviewed-by: David Benjamin <davidben@google.com>
Instead, in SSL_set_private_key_digest_prefs, convert the NID list to a
sigalgs list. We'll need to add a new API later when custom key callers
are ready to start advertising RSA-PSS.
This removes all callers of tls12_get_hash except inside the signing and
verifying functions.
Change-Id: Ie534f3b736c6ac6ebeb0d7770d489f72e3321865
Reviewed-on: https://boringssl-review.googlesource.com/8693
Reviewed-by: David Benjamin <davidben@google.com>
Instead have ssl3_cert_verify_hash output the hash, since it already
knows it. Also add a missing EVP_PKEY_CTX_set_signature_md call on the
client half. (Although, the call isn't actually necessary.)
Also remove now unnecessary static assert. Since EVP_md5_sha1 is an
EVP_MD itself, EVP_MAX_MD_SIZE is required to fit it already.
Change-Id: Ief74fdbdf08e9f124679475bafba2f6f1d8fc687
Reviewed-on: https://boringssl-review.googlesource.com/8692
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This reverts commits:
8d79ed674019fdcb52348d79ed6740
Because WebRTC (at least) includes our headers in an extern "C" block,
which precludes having any C++ in them.
Change-Id: Ia849f43795a40034cbd45b22ea680b51aab28b2d
For when the PackHandshakeFlight tests get enabled.
Change-Id: Iee20fd27d88ed58f59af3b7e2dd92235d35af9ce
Reviewed-on: https://boringssl-review.googlesource.com/8663
Reviewed-by: David Benjamin <davidben@google.com>
This change scatters the contents of the two scoped_types.h files into
the headers for each of the areas of the code. The types are now in the
|bssl| namespace.
Change-Id: I802b8de68fba4786b6a0ac1bacd11d81d5842423
Reviewed-on: https://boringssl-review.googlesource.com/8731
Reviewed-by: Adam Langley <agl@google.com>
We currently have the situation where the |tool| and |bssl_shim| code
includes scoped_types.h from crypto/test and ssl/test. That's weird and
shouldn't happen. Also, our C++ consumers might quite like to have
access to the scoped types.
Thus this change moves some of the template code to base.h and puts it
all in a |bssl| namespace to prepare for scattering these types into
their respective headers. In order that all the existing test code be
able to access these types, it's all moved into the same namespace.
Change-Id: I3207e29474dc5fcc344ace43119df26dae04eabb
Reviewed-on: https://boringssl-review.googlesource.com/8730
Reviewed-by: David Benjamin <davidben@google.com>
It still places the current message all over the place, but remove the
bizarre init_num/error/ok split. Now callers get the message length out
of init_num, which mirrors init_msg. Also fix some signedness.
Change-Id: Ic2e97b6b99e234926504ff217b8aedae85ba6596
Reviewed-on: https://boringssl-review.googlesource.com/8690
Reviewed-by: David Benjamin <davidben@google.com>
This machinery is so different between TLS and DTLS that there is no
sense in having them share structures. This switches us to maintaining
the full reassembled message in hm_fragment and get_message just lets
the caller read out of that when ready.
This removes the last direct handshake dependency on init_buf,
ssl3_hash_message.
Change-Id: I4eccfb6e6021116255daead5359a0aa3f4d5be7b
Reviewed-on: https://boringssl-review.googlesource.com/8667
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Since they include an ECDHE exchange in them, they are equally-well
suited to False Start.
Change-Id: I75d31493a614a78ccbf337574c359271831d654d
Reviewed-on: https://boringssl-review.googlesource.com/8732
Reviewed-by: David Benjamin <davidben@google.com>
Chromium no longer uses it.
Change-Id: I50cc55bad4124305686d299032a2e8ed2cb9d0d7
Reviewed-on: https://boringssl-review.googlesource.com/8691
Reviewed-by: David Benjamin <davidben@google.com>
Both DTLS and TLS still use it, but that will change in the following
commit. This also removes the handshake's knowledge of the
dtls_clear_incoming_messages function.
(It's possible we'll want to get rid of begin_handshake in favor of
allocating it lazily depending on how TLS 1.3 post-handshake messages
end up working out. But this should work for now.)
Change-Id: I0f512788bbc330ab2c947890939c73e0a1aca18b
Reviewed-on: https://boringssl-review.googlesource.com/8666
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
These are where the DTLS- and TLS-specific transport layer hooks will be
defined. Later we can probably move much of the implementations of these
hooks into these files so those functions can be static.
While I'm here, fix up the naming of some constants.
Change-Id: I1009dd9fdc3cc4fd49fbff0802f6289931abec3d
Reviewed-on: https://boringssl-review.googlesource.com/8665
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
[Originally written by nharper, revised by davidben.]
When we add this in the real code, this will want ample tests and hooks
for bugs, but get the core logic in to start with.
Change-Id: I86cf0b6416c9077dbb6471a1802ae984b8fa6c72
Reviewed-on: https://boringssl-review.googlesource.com/8598
Reviewed-by: David Benjamin <davidben@google.com>
V2ClientHello is going to be ugly wherever we do it, but this hides it
behind the transport method table. It removes a place where the
handshake state machine reaches into ssl3_get_message's internal state.
ssl3_get_message will now silently translate V2ClientHellos into true
ClientHellos and manage the handshake hash appropriately.
Now the only accesses of init_buf from the handshake state machines are
to create and destroy the buffer.
Change-Id: I81467a038f6ac472a465eec7486a443fe50a98e1
Reviewed-on: https://boringssl-review.googlesource.com/8641
Reviewed-by: Adam Langley <agl@google.com>
TLS 1.3 will use a different function from processClientHello.
Change-Id: I8b26a601cf553834b508feab051927d5986091ca
Reviewed-on: https://boringssl-review.googlesource.com/8597
Reviewed-by: David Benjamin <davidben@google.com>
As with the client, the logic around extensions in 1.3 will want to be
tweaked. readClientHello will probably shrink a bit. (We could probably
stuff 1.3 into the existing parameter negotiation logic, but I expect
it'll get a bit unwieldy once HelloRetryRequest, PSK resumption, and
0-RTT get in there, so I think it's best we leave them separate.)
Change-Id: Id8c323a06a1def6857a59accd9f87fb0b088385a
Reviewed-on: https://boringssl-review.googlesource.com/8596
Reviewed-by: David Benjamin <davidben@google.com>
While the random connection property extensions like ALPN and SRTP
remain largely unchanged in TLS 1.3 (but for interaction with 0-RTT),
authentication-related extensions change significantly and need
dedicated logic.
Change-Id: I2588935c2563a22e9879fb81478b8df5168b43de
Reviewed-on: https://boringssl-review.googlesource.com/8602
Reviewed-by: David Benjamin <davidben@google.com>
Test with and without PackHandshakeFlight enabled to cover when the
early post-CCS fragment will get packed into one of the pre-CCS
handshake records. Also test the resumption cases too to cover more
state transitions.
The various CCS-related tests (since CCS is kind of a mess) are pulled
into their own group.
Change-Id: I6384f2fb28d9885cd2b06d59e765e080e3822d8a
Reviewed-on: https://boringssl-review.googlesource.com/8661
Reviewed-by: David Benjamin <davidben@google.com>
[Originally written by nharper and then revised by davidben.]
Most features are missing, but it works for a start. To avoid breaking
the fake TLS 1.3 tests while the C code is still not landed, all the
logic is gated on a global boolean. When the C code gets in, we'll
set it to true and remove this boolean.
Change-Id: I6b3a369890864c26203fc9cda37c8250024ce91b
Reviewed-on: https://boringssl-review.googlesource.com/8601
Reviewed-by: David Benjamin <davidben@google.com>
I'm surprised we'd never tested this. In addition to splitting handshake
records up, one may pack multiple handshakes into a single record, as
they fit. Generalize the DTLS handshake flush hook to do this in TLS as
well.
Change-Id: Ia546d18c7c56ba45e50f489c5b53e1fcd6404f51
Reviewed-on: https://boringssl-review.googlesource.com/8650
Reviewed-by: David Benjamin <davidben@google.com>
TLS 1.2 and 1.3 will process more-or-less the same server extensions,
but at slightly different points in the handshake. In preparation for
that, split this out into its own function.
Change-Id: I5494dee4724295794dfd13c5e9f9f83eade6b20a
Reviewed-on: https://boringssl-review.googlesource.com/8586
Reviewed-by: Adam Langley <agl@google.com>
[Originally written by nharper, tweaked by davidben.]
For now, ignore them completely.
Change-Id: I28602f219d210a857aa80d6e735557b8d2d1c590
Reviewed-on: https://boringssl-review.googlesource.com/8585
Reviewed-by: Adam Langley <agl@google.com>
Rather than have a separate codepath, just skip the message_complete
logic and parse what's in the buffer. This also cuts down on one input
to setting up a reuse_message; message_type is now only written to in
the get_message implementation.
Change-Id: I96689b5957a3f2548af9099ec4e53cabacdc395a
Reviewed-on: https://boringssl-review.googlesource.com/8640
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Also move them with the other version negotiation tests.
Change-Id: I8ea5777c131f8ab618de3c6d02038e802bd34dd0
Reviewed-on: https://boringssl-review.googlesource.com/8550
Reviewed-by: David Benjamin <davidben@google.com>
TLS 1.2 and 1.3 will both need to call it at different points.
Change-Id: Id62ec289213aa6c06ebe5fe65a57ca6c2b53d538
Reviewed-on: https://boringssl-review.googlesource.com/8600
Reviewed-by: David Benjamin <davidben@google.com>
TLS 1.3 will need to call it under different circumstances. We will also
wish to test TLS 1.3 post-handshake auth, so this function must work
without being passed handshake state.
In doing so, implement matching based on signature algorithms as 1.3
does away with the certificate type list.
Change-Id: Ibdee44bbbb589686fcbcd7412432100279bfac63
Reviewed-on: https://boringssl-review.googlesource.com/8589
Reviewed-by: David Benjamin <davidben@google.com>
[Originally written by nharper and then tweaked by davidben.]
TLS 1.3 tweaks them slightly, so being able to write them in one pass
rather than two will be somewhat more convenient.
Change-Id: Ib7e2d63e28cbae025c840bbb34e9e9c295b44dc6
Reviewed-on: https://boringssl-review.googlesource.com/8588
Reviewed-by: David Benjamin <davidben@google.com>
[Originally written by nharper. Test added by davidben.]
Test vectors taken from hkdf_test.c.
Change-Id: I214bcae325e9c7c242632a169ab5cf80a3178989
Reviewed-on: https://boringssl-review.googlesource.com/8587
Reviewed-by: David Benjamin <davidben@google.com>
[Originally written by nharper, tweaked by davidben.]
In TLS 1.3, every extension the server previously sent gets moved to a
separate EncryptedExtensions message. To be able to share code between
the two, parse those extensions separately. For now, the handshake reads
from serverHello.extensions.foo, though later much of the extensions
logic will probably handle serverExtensions independent of whether it
resides in ServerHello or EncryptedExtensions.
Change-Id: I07aaae6df3ef6fbac49e64661d14078d0dbeafb0
Reviewed-on: https://boringssl-review.googlesource.com/8584
Reviewed-by: David Benjamin <davidben@google.com>
[Originally written by nharper and tweaked by davidben.]
This will end up being split in two with most of the ServerHello
extensions being serializable in both ServerHello and
EncryptedExtensions depending on version.
Change-Id: Ida5876d55fbafb982bc2e5fdaf82872e733d6536
Reviewed-on: https://boringssl-review.googlesource.com/8580
Reviewed-by: David Benjamin <davidben@google.com>
[Originally written by nharper and then slightly tweaked by davidben.]
Between the new deeply nested extension (KeyShare) and most of
ServerHello extensions moving to a separate message, this is probably
long overdue.
Change-Id: Ia86e30f56b597471bb7e27d726a9ec92687b4d10
Reviewed-on: https://boringssl-review.googlesource.com/8569
Reviewed-by: David Benjamin <davidben@google.com>
To match the Go side. That message will never be used for anything else,
so there's not much need to give it such a long name.
Change-Id: I3396c9d513d02d873e59cd8e81ee64005c5c706c
Reviewed-on: https://boringssl-review.googlesource.com/8620
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
TLS 1.3 defines its own EncryptedExtensions message. The existing one is
for Channel ID which probably should not have tried to generalize
itself.
Change-Id: I4f48bece98510eb54e64fbf3df6c2a7332bc0261
Reviewed-on: https://boringssl-review.googlesource.com/8566
Reviewed-by: David Benjamin <davidben@google.com>
Right now I believe we are testing against DTLS 1.3 ClientHellos. Fix
this in preparation for making VersionTLS13 go elsewhere in the Go code.
Unfortunately, I made the mistake of mapping DTLS 1.0 to TLS 1.0 rather
than 1.1 in Go. This does mean the names of the tests naturally work out
correctly, but we have to deal with this awkward DTLS-1.1-shaped hole in
our logic.
Change-Id: I8715582ed90acc1f08197831cae6de8d5442d028
Reviewed-on: https://boringssl-review.googlesource.com/8562
Reviewed-by: David Benjamin <davidben@google.com>
In preparation for TLS 1.3 using its actual handshake, switch most tests
to TLS 1.3 and add liberal TODOs for the tests which will need TLS 1.3
variants.
In doing so, move a few tests from basic tests into one of the groups.
Also rename BadECDSACurve to BadECDHECurve (it was never ECDSA) and add
a test to make sure FALLBACK_SCSV is correctly sensitive to the maximum
version.
Change-Id: Ifca6cf8f7a48d6f069483c0aab192ae691b1dd8e
Reviewed-on: https://boringssl-review.googlesource.com/8560
Reviewed-by: David Benjamin <davidben@google.com>
TLS 1.3 defines a new SignatureScheme uint16 enum that is backwards
compatible on the wire with TLS1.2's SignatureAndHashAlgorithm. This
change updates the go testing code to use a single signatureAlgorithm
enum (instead of 2 separate signature and hash enums) in preparation for
TLS 1.3. It also unifies all the signing around this new scheme,
effectively backporting the change to TLS 1.2.
For now, it does not distinguish signature algorithms between 1.2 and
1.3 (RSA-PSS instead of RSA-PKCS1, ECDSA must match curve types). When
the C code is ready make a similar change, the Go code will be updated
to match.
[Originally written by nharper, tweaked significantly by davidben.]
Change-Id: If9a315c4670755089ac061e4ec254ef3457a00de
Reviewed-on: https://boringssl-review.googlesource.com/8450
Reviewed-by: David Benjamin <davidben@google.com>
Upstream added this in a18a31e49d266. The various *_up_ref functions
return a variety of types, but this one returns int because upstream
appears to be trying to unify around that. (See upstream's c5ebfcab713.)
Change-Id: I7e1cfe78c3a32f5a85b1b3c14428bd91548aba6d
Reviewed-on: https://boringssl-review.googlesource.com/8581
Reviewed-by: Adam Langley <alangley@gmail.com>
In order to delay the digest of the handshake transcript and unify
around message-based signing callbacks, a copy of the transcript is kept
around until we are sure there is no certificate authentication.
This removes support for SSL_PRIVATE_KEY_METHOD as a client in SSL 3.0.
Change-Id: If8999a19ca021b4ff439319ab91e2cd2103caa64
Reviewed-on: https://boringssl-review.googlesource.com/8561
Reviewed-by: David Benjamin <davidben@google.com>
This replaces the old key_exchange_info APIs and does not require the
caller be aware of the mess around SSL_SESSION management. They
currently have the same bugs around renegotiation as before, but later
work to fix up SSL_SESSION tracking will fix their internals.
For consistency with the existing functions, I've kept the public API at
'curve' rather than 'group' for now. I think it's probably better to
have only one name with a single explanation in the section header
rather than half and half. (I also wouldn't be surprised if the IETF
ends up renaming 'group' again to 'key exchange' at some point. We'll
see what happens.)
Change-Id: I8e90a503bc4045d12f30835c86de64ef9f2d07c8
Reviewed-on: https://boringssl-review.googlesource.com/8565
Reviewed-by: Adam Langley <agl@google.com>
We ended up switching this from a curve to a cipher suite, so the group
ID isn't used. This is in preparation for adding an API for the curve
ID, at which point leaving the protocol constants undefined seems
somewhat bad manners.
Change-Id: Icb8bf4594879dbbc24177551868ecfe89bc2f8c3
Reviewed-on: https://boringssl-review.googlesource.com/8563
Reviewed-by: Adam Langley <agl@google.com>
This isn't filled in on the client and Chromium no longer uses it for
plain RSA. It's redundant with existing APIs. This is part of removing
the need for callers to call SSL_get_session where possible.
SSL_get_session is ambiguous when it comes to renego. Some code wants
the current connection state which should not include the pending
handshake and some code wants the handshake scratch space which should.
Renego doesn't exist in TLS 1.3, but TLS 1.3 makes NewSessionTicket a
post-handshake message, so SSL_get_session is somewhat silly of an API
there too.
SSL_SESSION_get_key_exchange_info is a BoringSSL-only API, so we can
freely change it and replace it with APIs keyed on SSL. In doing so, I
think it is better to provide APIs like "SSL_get_dhe_group_size" and
"SSL_get_curve_id" rather than make the caller do the multi-step
SSL_get_current_cipher / SSL_CIPHER_is_ECDHE dance. To that end, RSA
key_exchange_info is pointless as it can already be determined from the
peer certificate.
Change-Id: Ie90523083d8649701c17934b7be0383502a0caa3
Reviewed-on: https://boringssl-review.googlesource.com/8564
Reviewed-by: Adam Langley <agl@google.com>
QUIC, in particular, will set min_version to TLS 1.3 and has no need to send
any legacy ciphers.
Note this requires changing some test expectations. Removing all of TLS 1.1 and
below's ciphers in TLS 1.3 has consequences for how a tripped minimum version
reads.
BUG=66
Change-Id: I695440ae78b95d9c7b5b921c3cb2eb43ea4cc50f
Reviewed-on: https://boringssl-review.googlesource.com/8514
Reviewed-by: David Benjamin <davidben@google.com>
OpenSSL's SSL_OP_NO_* flags allow discontinuous version ranges. This is a
nuisance for two reasons. First it makes it unnecessarily difficult to answer
"are any versions below TLS 1.3 enabled?". Second the protocol does not allow
discontinuous version ranges on the client anyway. OpenSSL instead picks the
first continous range of enabled versions on the client, but not the server.
This is bizarrely inconsistent. It also doesn't quite do this as the
ClientHello sending logic does this, but not the ServerHello processing logic.
So we actually break some invariants slightly. The logic is also cumbersome in
DTLS which kindly inverts the comparison logic.
First, switch min_version/max_version's storage to normalized versions. Next
replace all the ad-hoc version-related functions with a single
ssl_get_version_range function. Client and server now consistently pick a
contiguous range of versions. Note this is a slight behavior change for
servers. Version-range-sensitive logic is rewritten to use this new function.
BUG=66
Change-Id: Iad0d64f2b7a917603fc7da54c9fc6656c5fbdb24
Reviewed-on: https://boringssl-review.googlesource.com/8513
Reviewed-by: David Benjamin <davidben@google.com>
The signing logic itself still depends on pre-hashed messages and will be fixed
in later commits.
Change-Id: I901b0d99917c311653d44efa34a044bbb9f11e57
Reviewed-on: https://boringssl-review.googlesource.com/8545
Reviewed-by: David Benjamin <davidben@google.com>
They're not necessary.
Change-Id: Ifeb3fae73a8b22f88019e6ef9f9ba5e64ed3cfab
Reviewed-on: https://boringssl-review.googlesource.com/8543
Reviewed-by: David Benjamin <davidben@google.com>
As part of the SignatureAlgorithm change in the TLS 1.3 specification,
the existing signature/hash combinations are replaced with a combined
signature algorithm identifier. This change maintains the existing APIs
while fixing the internal representations. The signing code currently
still treats the SignatureAlgorithm as a decomposed value, which will be
fixed as part of a separate CL.
Change-Id: I0cd1660d74ad9bcf55ce5da4449bf2922660be36
Reviewed-on: https://boringssl-review.googlesource.com/8480
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This file is still kind of a mess, but put the two halves together at least.
Change-Id: Ib21d9c4a7f4864cf80e521f7d0ebec029e5955a1
Reviewed-on: https://boringssl-review.googlesource.com/8502
Reviewed-by: Adam Langley <agl@google.com>
MSVC 2015 seems to support it just fine.
Change-Id: I9c91c18c260031e6024480d1f57bbb334ed7118c
Reviewed-on: https://boringssl-review.googlesource.com/8501
Reviewed-by: Adam Langley <agl@google.com>
buffer buffer buffer buffer buffer. At some point, words lose their meaning if
they're used too many times. Notably, the DTLS code can't decide whether a
"buffered message" is an incoming message to be reassembled or an outgoing
message to be (re)transmitted.
Change-Id: Ibdde5c00abb062c603d21be97aff49e1c422c755
Reviewed-on: https://boringssl-review.googlesource.com/8500
Reviewed-by: Adam Langley <agl@google.com>
This allows us to use CBB for all handshake messages. Now, SSL_PROTOCOL_METHOD
is responsible for implementing a trio of CBB-related hooks to assemble
handshake messages.
Change-Id: I144d3cac4f05b6637bf45d3f838673fc5c854405
Reviewed-on: https://boringssl-review.googlesource.com/8440
Reviewed-by: Adam Langley <agl@google.com>
It has size 7. There's no need for a priority queue structure, especially one
that's O(N^2) anyway.
Change-Id: I7609794aac1925c9bbf3015744cae266dcb79bff
Reviewed-on: https://boringssl-review.googlesource.com/8437
Reviewed-by: Adam Langley <agl@google.com>
The pair was a remnant of some weird statefulness and also ChangeCipherSpec
having a "sequence number" to make the pqueue turn into an array.
Change-Id: Iffd82594314df43934073bd141faee0fc167ed5f
Reviewed-on: https://boringssl-review.googlesource.com/8436
Reviewed-by: Adam Langley <agl@google.com>
Now that retransitting is a lot less stateful, a lot of surrounding code can
lose statefulness too. Rather than this overcomplicated pqueue structure,
hardcode that a handshake flight is capped at 7 messages (actually, DTLS can
only get up to 6 because we don't support NPN or Channel ID in DTLS) and used a
fixed size array.
This also resolves several TODOs.
Change-Id: I2b54c3441577a75ad5ca411d872b807d69aa08eb
Reviewed-on: https://boringssl-review.googlesource.com/8435
Reviewed-by: Adam Langley <agl@google.com>
Post-handshake retransmit in DTLS no longer needs that scratch space.
Change-Id: I2f070675d72426e61b19dab5bcac40bf62b8fd8d
Reviewed-on: https://boringssl-review.googlesource.com/8434
Reviewed-by: Adam Langley <agl@google.com>
Now dtls1_do_handshake_write takes in a serialized form of the full message and
writes it. It's a little weird to serialize and deserialize the header a bunch,
but msg_callback requires that we keep the full one around in memory anyway.
Between that and the handshake hash definition, DTLS really wants messages to
mean the assembled header, redundancies and all, so we'll just put together
messages that way.
This also fixes a bug where ssl_do_msg_callback would get passed in garbage
where the header was supposed to be. The buffered messages get sampled before
writing the fragment rather than after.
Change-Id: I4e3b8ce4aab4c4ab4502d5428dfb8f3f729c6ef9
Reviewed-on: https://boringssl-review.googlesource.com/8433
Reviewed-by: Adam Langley <agl@google.com>
It is an explicit copy of something, but it's a lot easier to reason about than
the init_buf/init_num gynmastics we were previously doing. This is along the
way to getting init_buf out of here.
Change-Id: Ia1819ba9db60ef6db09dd60d208dbc95fcfb4bd2
Reviewed-on: https://boringssl-review.googlesource.com/8432
Reviewed-by: Adam Langley <agl@google.com>
The only thing we've written before the signature is the hash. We can just
choose it anew. This is along the way to getting init_buf out of the handshake
output side. (init_buf is kind of a mess since it doesn't integrate nicely with
a top-level CBB. Some of the logic hasn't been converted to CBB because they're
interspersed with a BUF_MEM_grow.)
Change-Id: I693e834b5a03849bebb04f3f6b81f81fb04e2530
Reviewed-on: https://boringssl-review.googlesource.com/8431
Reviewed-by: Adam Langley <agl@google.com>
It doesn't really convey anything useful. Leave ssl_get_message alone for now
since it's called everywhere in the handshake and I'm about to tweak it
further.
Change-Id: I6f3a74c170e818f624be8fbe5cf6b796353406df
Reviewed-on: https://boringssl-review.googlesource.com/8430
Reviewed-by: Adam Langley <agl@google.com>
Saves us some mess if they're never zero. This also fixes a bug in
ssl3_get_max_client_version where it didn't account for all versions being
disabled properly.
Change-Id: I4c95ff57cf8953cb4a528263b252379f252f3e01
Reviewed-on: https://boringssl-review.googlesource.com/8512
Reviewed-by: David Benjamin <davidben@google.com>
Otherwise if the client's ClientHello logic is messed up and ServerHello is
fine, we won't notice.
Change-Id: I7f983cca45f7da1113ad4a72de1f991115e1b29a
Reviewed-on: https://boringssl-review.googlesource.com/8511
Reviewed-by: David Benjamin <davidben@google.com>
This also adds a missing check to the C half to ensure fake record types are
always correct, to keep implementations honest.
Change-Id: I1d65272e647ffa67018c721d52c639f8ba47d647
Reviewed-on: https://boringssl-review.googlesource.com/8510
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
We were missing this case. It is possible to receive an early unencrypted
ChangeCipherSpec alert in DTLS because they aren't ordered relative to the
handshake. Test this case. (ChangeCipherSpec in DTLS is kind of pointless.)
Change-Id: I84268bc1821734f606fb20bfbeda91abf372f32c
Reviewed-on: https://boringssl-review.googlesource.com/8460
Reviewed-by: Adam Langley <agl@google.com>
The payload comments aren't necessary now that our parsing code is readable in
itself. The check is impossible to hit.
Change-Id: Ib41ad606babda903a9fab50de3189f97e99cac2f
Reviewed-on: https://boringssl-review.googlesource.com/8248
Reviewed-by: Adam Langley <agl@google.com>
That both exist with nearly the same name is unfortunate. This also does away
with cert_req being unnecessarily tri-state.
Change-Id: Id83e13d0249b80700d9258b363d43b15d22898d8
Reviewed-on: https://boringssl-review.googlesource.com/8247
Reviewed-by: Adam Langley <agl@google.com>
TLS 1.2 has a long series of optional messages within a flight. We really
should send and process these synchronously. In the meantime, the 'skip'
pattern is probably the best we can get away with. Otherwise we have too many
state transitions to think about. (The business with CCS, NPN, and ChannelID is
particularly a headache. Session tickets aren't great either.)
Change-Id: I84e391a6410046372cf9c6989be056a27606ad19
Reviewed-on: https://boringssl-review.googlesource.com/8246
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
This is the only codepath where ssl->version can get a garbage value, which is
a little concerning. Since, in all these cases, the peer is failing to connect
and speaks so low a version we don't even accept it anymore, there is probably
not much value in letting them distinguish protocol_version from a record-layer
version number mismatch, where enforced (which will give a version-related
error anyway).
Should we get a decode_error or so just before version negotiation, we'd have
this behavior already.
Change-Id: I9b3e5685ab9c9ad32a7b7e3129363cd1d4cdaaf4
Reviewed-on: https://boringssl-review.googlesource.com/8420
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This implements the cipher suite constraints in "fake TLS 1.3". It also makes
bssl_shim and runner enable it by default so we can start adding MaxVersion:
VersionTLS12 markers to tests as 1.2 vs. 1.3 differences begin to take effect.
Change-Id: If1caf6e43938c8d15b0a0f39f40963b8199dcef5
Reviewed-on: https://boringssl-review.googlesource.com/8340
Reviewed-by: David Benjamin <davidben@google.com>
