SSL_SIGN_RSA_PKCS1_MD5_SHA1 does not really exist, but is a private use
value we allocated to internally represent the TLS 1.0/1.1 RSA signature
algorithm. (Unlike the TLS 1.0/1.1 ECDSA signature algorithm, which is
the same as SSL_SIGN_ECDSA_SHA1, the RSA one is a bespoke MD5+SHA1
concatenation which never appears in TLS 1.2 and up.)
Although documented that you're not to use it with
SSL_CTX_set_verify_algorithm_prefs and
SSL_CTX_set_signing_algorithm_prefs (it only exists for
SSL_PRIVATE_KEY_METHOD), there's nothing stopping a caller from passing
it in.
Were you to do so anyway, we'd get confused and sign or verify it at TLS
1.2. This CL is the first half of a fix: since we already have
pkey_supports_algorithm that checks a (version, sigalg, key) tuple, that
function should just know this is not a 1.2-compatible algorithm.
A subsequent CL will also fix those APIs to not accept invalid values
from the caller, since these invalid calls will still, e.g., dump
garbage values on the wire.
Change-Id: I119503f9742a17952ed08e5815fb3d1419fd4a12
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55445
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Bug: 516
Change-Id: Iba2014da414658c08e42e0993912fa73848832d3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54945
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The cookie is only needed in SSL_HANDSHAKE, so there's no need to retain
it for the lifetime of the connection. (SSL_HANDSHAKE is released after
the handshake completes.)
Back when DTLS1_COOKIE_LENGTH was 32, storing it inline made some sense.
Now that RFC 6347 increased the maximum to 255 bytes, just indirect it
with an Array<uint8_t>. Along the way, remove the DTLS1_COOKIE_LENGTH
checks. The new limit is the largest that fits in the length prefix, so
it's always redundant. In fact, the constant was one higher was allowed
anyway. Add some tests for the maximum length, as well as zero-length
cookies.
I considered just repurposing the plain cookie field, used in
HelloRetryRequest (as opposed to HelloVerifyRequest), as they're
mutually exclusive, even in DTLS 1.3. But, when we get to DTLS 1.3,
that'll get a little hairy because ssl_write_client_hello will need
extra checks to know whether hs->cookie is meant to go in the
ClientHello directly or in extensions.
Change-Id: I1afedc7ce31414879545701bf8fe4658657ba66f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54466
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This runs through much the same code as the TLS 1.3 bits, though we use
a different hint field to avoid mixups between the fields. (Otherwise
the receiver may misinterpret a decryptPSK hint as the result of
decrypting the session_ticket extension, or vice versa. This could
happen if a ClientHello contains both a PSK and a session ticket.)
Bug: 504
Change-Id: I968bafe12120938e6e46e52536efd552b12c66a0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53805
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
The check finds implicit conversions of integer literals to bools:
bool b1 = 1;
bool b2 = static_cast<bool>(1);
and transforms them to:
bool b1 = true;
bool b2 = true;
Bug: chromium:1290142
Change-Id: I15579e28f544d07b331a230b70a8278e0651150d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51085
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Later CLs will clean up the ClientHello construction a bit (draft-12
avoids computing ClientHelloOuter twice). I suspect the transcript
handling on the client can also be simpler, but I'll see what's
convenient after I've changed how ClientHelloOuter is constructed.
Changes of note between draft-10 and draft-13:
- There is now an ECH confirmation signal in both HRR and SH. We don't
actually make much use of this in our client right now, but it
resolves a bunch of weird issues around HRR, including edge cases if
HRR applies to one ClientHello but not the other.
- The confirmation signal no longer depends on key_share and PSK, so we
don't have to work around a weird ordering issue.
- ech_is_inner is now folded into the main encrypted_client_hello code
point. This works better with some stuff around HRR.
- Padding is moved from the padding extension, computed with
ClientHelloInner, to something we fill in afterwards. This makes it
easier to pad up the whole thing to a multiple of 32. I've accordingly
updated to the latest recommended padding construction, and updated
the GREASE logic to match.
- ech_outer_extensions is much easier to process because the order is
required to be consistent. We were doing that anyway, and now a simple
linear scan works.
- ClientHelloOuterAAD now uses an all zero placeholder payload of the
same length. This lets us simplify the server code, but, for now, I've
kept the client code the same. I'll follow this up with a CL to avoid
computing ClientHelloOuter twice.
- ClientHelloOuterAAD is allowed to contain a placeholder PSK. I haven't
filled that in and will do it in a follow-up CL.
Bug: 275
Change-Id: I7464345125c53968b2fe692f9268e392120fc2eb
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48912
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We do this enough that it's worth extracting a common parser. And this
gives a struct we can pass around. Note this moves the server extensions
block parsing out of ssl_scan_serverhello_tlsext.
I've also consolidated a few error conditions to tighten the code up a
bit: the TLS 1.2 code distinguishes unknown from unadvertised cipher,
while the TLS 1.3 code didn't. And seeing the wrong legacy version
number in TLS 1.3 is really just a syntax error since it's not the
version field anymore. (RFC8446 specifies the value.)
Change-Id: Ia2f44ff9a3899b5a594569f1b258f2b487930496
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48908
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We fill in placeholder values of all zeros fairly often in TLS now,
as workarounds for messages being constructed in the wrong order.
draft-12 of ECH adds even more of these. Add a helper so we don't need
to interrupt an || chain with a memset.
Change-Id: Id4f9d988ee67598645a01637cc9515b475c1aec2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48909
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
absl::Span, base::span, and std::span have first() and last() methods
which give prefixes and suffixes. first() just saves 5 characters, but
last() is nicer to write than subspan() for suffixes.
Unlike subspan(), they also do not have clipping behavior, so we're
guaranteed the length is correct. The clipping behavior comes from
absl::Span::subspan() and is not present in std::span or base::span.
I've left it in, in case we switch to absl::Span in the future, but I
imagine absl::Span will need to migrate this at some point.
Change-Id: I042dd6c566b6d753ec6de9d84e8c09ac7c270267
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48905
Reviewed-by: Adam Langley <agl@google.com>
If a client offers ECH, but the server rejects it, the client completes
the handshake with ClientHelloOuter in order to authenticate retry keys.
Implement this flow. This is largely allowing the existing handshake to
proceed, but with some changes:
- Certificate verification uses the other name. This CL routes this up to
the built-in verifier and adds SSL_get0_ech_name_override for the
callback.
- We need to disable False Start to pick up server Finished in TLS 1.2.
- Client certificates, notably in TLS 1.3 where they're encrypted,
should only be revealed to the true server. Fortunately, not sending
client certs is always an option, so do that.
Channel ID has a similar issue. I've just omitted the extension in
ClientHelloOuter because it's deprecated and is unlikely to be used
with ECH at this point. ALPS may be worth some pondering but, the way
it's currently used, is not sensitive.
(Possibly we should change the draft to terminate the handshake before
even sending that flight...)
- The session is never offered in ClientHelloOuter, but our internal
book-keeping doesn't quite notice.
I had to replace ech_accept with a tri-state ech_status to correctly
handle an edge case in SSL_get0_ech_name_override: when ECH + 0-RTT +
reverify_on_resume are all enabled, the first certificate verification
is for the 0-RTT session and should be against the true name, yet we
have selected_ech_config && !ech_accept. A tri-state tracks when ECH is
actually rejected. I've maintained this on the server as well, though
the server never actually cares.
Bug: 275
Change-Id: Ie55966ca3dc4ffcc8c381479f0fe9bcacd34d0f8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48135
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This is a bit more self-explanatory, especially now that TLS 1.0 is the
minimum version we implement anyway.
Change-Id: Ic65e9f90bb5cd747328bd9e30b976d1e124c7764
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48130
Reviewed-by: Adam Langley <agl@google.com>
Although not permitted by the TLS specification, systems sometimes
ossify TLS extension order, or byte offsets of various fields. To
keep the ecosystem healthy, add an API to reorder ClientHello
extensions.
Since ECH, HelloRetryRequest, and HelloVerifyRequest are sensitive to
extension order, I've implemented this by per-connection permutation of
the indices in the kExtensions structure. This ensures that all
ClientHellos within a connection are consistently ordered. As follow-up
work, permuting the other messages would also be nice, though any server
messages would need to be incorporated in handshake hints.
Change-Id: I18ce39b4df5ee376c654943f07ec26a50e0923a9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48045
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Based on an initial implementation by Dan McArdle at
https://boringssl-review.googlesource.com/c/boringssl/+/46784
This CL contains most of a client implementation for
draft-ietf-tls-esni-10. The pieces missing so far, which will be done in
follow-up CLs are:
1. While the ClientHelloInner is padded, the server Certificate message
is not. I'll add that once we resolve the spec discussions on how to
do that. (We were originally going to use TLS record-level padding,
but that doesn't work well with QUIC.)
2. The client should check the public name is a valid DNS name before
copying it into ClientHelloOuter.server_name.
3. The ClientHelloOuter handshake flow is not yet implemented. This CL
can detect when the server selects ClientHelloOuter, but for now the
handshake immediately fails. A follow-up CL will remove that logic
and instead add the APIs and extra checks needed.
Otherwise, this should be complete, including padding and compression.
The main interesting point design-wise is that we run through
ClientHello construction multiple times. We need to construct
ClientHelloInner and ClientHelloOuter. Then each of those has slight
variants: EncodedClientHelloInner is the compressed form, and
ClientHelloOuterAAD just has the ECH extension erased to avoid a
circular dependency.
I've computed ClientHelloInner and EncodedClientHelloInner concurrently
because the compression scheme requires shifting the extensions around
to be contiguous. However, I've computed ClientHelloOuterAAD and
ClientHelloOuter by running through the logic twice. This probably can
be done better, but the next draft revises the construction anyway, so
I'm thinking I'll rework it then. (In the next draft, we use a
placeholder payload of the same length, so we can construct the
ClientHello once and fill in the payload.)
Additionally, now that we have a client available in ssl_test, this adds
a threading test to confirm that SSL_CTX_set1_ech_keys is properly
synchronized. (Confirmed that, if I drop the lock in
SSL_CTX_set1_ech_keys, TSan notices.)
Change-Id: Icaff68b595035bdcc73c468ff638e67c84239ef4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48004
Reviewed-by: Adam Langley <agl@google.com>
While the previous CL fixed a bug in computing this padding, we don't
actually need to pad the second (cleartext) ClientHello anyway. This
padding is to work around bugs in old F5 and WebSphere servers, which do
not speak TLS 1.3. Save a few bytes.
Change-Id: I9b5d9bb1c0d880f1b1c9182667a9d3d82588c04c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47999
Reviewed-by: Adam Langley <agl@google.com>
If we're dropping the PSK extension due to an HRR with mismatched hash
(looking back at that, we could easily have avoided that nuisance...
I've filed [0] on rfc8446bis), we don't predict the length correctly.
The consequence is we don't pad the second ClientHello to the desired
range. Fix this and add an assert.
[0] https://github.com/tlswg/tls13-spec/issues/1227
Change-Id: I47d880b6bdafa95840f7513b6b7718851f22554d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47998
Reviewed-by: Adam Langley <agl@google.com>
This makes calls to ssl_add_clienthello_tlsext a hair easier. Also we
only apply the [256, 511) compatibility hack to TLS, so we can just use
a constant.
Bug: 275
Change-Id: Ia2b5192aeef0cd8848ecfa1ea3b89a0a7382ff1a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47996
Reviewed-by: Adam Langley <agl@google.com>
ssl_add_clienthello_tlsext is about to get kinda messy with ECH. Move
the padding and GREASE extensions into a few helpers.
Bug: 275
Change-Id: I3bb702fb79dce4be68490c4a8fd889121ecdae58
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47995
Reviewed-by: Adam Langley <agl@google.com>
For TLS 1.3, since the bulk of extensions move to EncryptedExtensions,
we made the extension callbacks apply to EncryptedExtensions and pulled
the few ServerHello extensions out of the callback system. This means
the ServerHello naming is a little confusing.
We probably should rename these callbacks, though parse_server is a bit
ambiguous as to whether this is "parse the extension from the server" or
"parse as a server". For now, add a comment.
Change-Id: If1aa0846426de2cc8dcb2253695a8dd3285f7b76
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47994
Reviewed-by: Adam Langley <agl@google.com>
May not be strictly necessary, but similarly easier to reason about when
we need to interweave multiple ClientHellos.
Bug: 275
Change-Id: I9f85787860f3e8ce1653331ce52343d5bf5def23
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47992
Reviewed-by: Adam Langley <agl@google.com>
This is less effective than it seems because both
((const SSL_HANDSHAKE*)hs)->ssl and ((const SSL*)ssl)->s3 are both
non-const pointers. I considered moving hs->ssl to hs->ssl_ and adding
const-overloaded accessors, but I'd need to repeat the same with
ssl->s3, at which point this seemed not worth it yet. Maybe later if we
rewrite everything to more idiomatic C++.
Bug: 275
Change-Id: I9912a3df205916fdf2191a3687013d717528038d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47991
Reviewed-by: Adam Langley <agl@google.com>
This is kinda annoying and, like the grease_seed, demonstrates a
shortcoming with the idea of making add_clienthello completely const.
Strictly speaking, we only need idempotence. But I think this is okay:
const is much easier to enforce than idempotence, and we'll likely need
to do this anyway:
- While not in the current draft, I expect the draft's eventual HRR
resolution to retain the ECH extension, GREASE or not, on ECH reject.
Things are somewhat violating RFC8446 HRR rules right now. That means
we'll need to stash the ECH payload regardless.
- ECH binds all the other extensions in the outer ClientHello, so
computing the payload will need to move outside the callback system
anyway.
In some sense, all this is shifting our ClientHello output from the
"immediate mode" end of the spectrum (as we usually use) to the
"retained mode" end, which I suppose makes sense as this message becomes
more intricate.
Bug: 275
Change-Id: I9eb8cd1cde2ce264345b6ed3ee526d4eab81e911
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47990
Reviewed-by: Adam Langley <agl@google.com>
This is now never used. Instead, we rely on each renegotiation creating
a new handshake structure with fresh state. This simplifies things for
ECH.
(We probably could make an init hook work with ECH's two-ClientHello
scheme by either maintaining separate state per ClientHello or calling
init once for both ClientHellos. But the few uses of init were
removable, so this is easier.)
Bug: 275
Change-Id: Ie5e132fe072e5ea8db21ca16aa53fcd0895d8e48
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47987
Reviewed-by: Adam Langley <agl@google.com>
Like the early_data CL, this does shift a bit of logic that was
previously hidden away in the callbacks. For key_share, this is probably
a good move independent of ECH. The logic around HRR, etc., was a little
messy.
Bug: 275
Change-Id: Iafbcebdf66ce1f7957d798a98ee6b996fff24639
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47986
Reviewed-by: Adam Langley <agl@google.com>
Also add ECH GREASE state into the mix. Clearing this isn't critical,
especially now that we have an SSL_HANDSHAKE structure, but it's easy
enough.
Bug: 275
Change-Id: If1aa8d5c0c8fdb5af710852778ce452c507a2524
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47985
Reviewed-by: Adam Langley <agl@google.com>
ECH requires that we construct two ClientHellos. That means our
add_clienthello callbacks will need to be called multiple times and
should be const. (They already are called multiple times for
HelloRetryRequest, but we currently thread that through the callbacks a
bit. With ECH, I think we need to make them pure serialization.)
Bug: 275
Change-Id: I11f8195fd2ec4b8639f0a2af01a24d4974445580
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47984
Reviewed-by: Adam Langley <agl@google.com>
The first thing any deployment will want to monitor is whether ECH was
actually used. Also it's useful if the command-line tool can output
this. (The alert is how the client signals it discarded the connection
due to ECH reject.)
This also disables ECH with the handoff mechanism for now. (The
immediate cause being that ech_accept isn't serialized.) We'll probably
need to make some decisions around the ordering here, since ECH affects
where the true ClientHello is available.
Bug: 275
Change-Id: Ie4559733290e653a514fcd94431090bf86bc3172
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47911
Reviewed-by: Adam Langley <agl@google.com>
The ECH server extension is defined for TLS 1.3 EncryptedExtensions, not
TLS 1.2 ServerHello.
Bug: 275
Change-Id: Ie6e76c238075d70e6a0694ec0192df07da3457d1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47910
Reviewed-by: Adam Langley <agl@google.com>
The remaining remnants of Channel ID all configure the private key ahead
of time. Unwind the callback machinery, which cuts down on async points
and the cases we need to test.
This also unwinds some odd interaction between the callback and
SSL_set_tls_channel_id_enabled: If a client uses
SSL_set_tls_channel_id_enabled but doesn't set a callback, the handshake
would still pause at SSL_ERROR_WANT_CHANNEL_ID_LOOKUP. This is now
removed, so SSL_set_tls_channel_id_enabled only affects the server and
SSL_CTX_set1_tls_channel_id only affects the client.
Update-Note: SSL_CTX_set_channel_id_cb is removed.
SSL_set_tls_channel_id_enabled no longer enables Channel ID as a client,
only as a server.
Change-Id: I89ded99ca65e1c61b1bc4e009ca0bdca0b807359
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47907
Reviewed-by: Adam Langley <agl@google.com>
The channel_id_valid bit is both used for whether channel_id is filled
in (SSL_get_tls_channel_id), and whether this particular handshake will
eventually negotiate Channel ID.
The former means that, if SSL_get_tls_channel_id is called on the
client, we'll return all zeros. Apparently we never fill in channel_id
on the client at all. The latter means the state needs to be reset on
renegotiation because we do not currently forbid renegotiation with
Channel ID (we probably should...), which is the last use of the init
callback for extensions.
Instead, split this into a bit for the handshake and a bit for the
connection. Note this means we actually do not expose or even retain
whether Channel ID was used on the client.
This requires a tweak to the handoff logic, but it should be compatible.
The serialized ssl->s3->channel_id was always a no-op: the handback
happens before the ChannelID message, except in RSA key exchange. But we
forbid Channel ID in RSA key exchange anyway.
Update-Note: SSL_get_tls_channel_id will no longer return all zeros
during the handshake or on the client. I did not find any callers
relying on this.
Change-Id: Icd4b78dd3f311d1c7dfc1cae7d2b86dc7e327a99
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47906
Reviewed-by: Adam Langley <agl@google.com>
This avoids needing to worry about the interaction with renegotiation
which, in turn, means we can drop the init callback. (If we did support
DTLS renegotiation, we'd probably want to forbid the parameter from
changing anyway. Changing your SRTP parameters partway through will
likely confuse the RTP half of the application anyway.)
Change-Id: Ifef1e9479d9df296b69b0d296f6bef57b13da68e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47905
Reviewed-by: Adam Langley <agl@google.com>
Also now that it's finalized, flip the default for
SSL_set_quic_use_legacy_codepoint.
Update-Note: QUIC APIs now default to the standard code point rather
than the draft one. QUICHE has already been calling
SSL_set_quic_use_legacy_codepoint, so this should not affect them. Once
callers implementing the draft versions cycle out, we can then drop
SSL_set_quic_use_legacy_codepoint altogether. I've also bumped
BORINGSSL_API_VERSION in case we end up needing an ifdef.
Change-Id: Id2cab66215f4ad4c1e31503d329c0febfdb4603e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47864
Reviewed-by: David Schinazi <dschinazi@google.com>
Reviewed-by: Adam Langley <agl@google.com>
When decrypting a ticket we would copy the client's session ID into the
session and then copy the session's ID into the ServerHello (if
resuming). That seems icky. Instead install the same placeholder on the
server as we do on the client.
Change-Id: Icb50a3be2f05e6428f1b286c8c09015f7bb4af16
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47784
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This is part of a very deep dependency chain. I'm sniffing at making all
the add_clienthello callbacks const. Between HelloVerifyRequest,
HelloRetryRequest, and soon ECH, we're creating lots of ClientHellos per
connection. That's probably easiest to manage if constructing a
ClientHello had no side effects.
Update-Note: The change to the return type isn't quite compatible, but I
only found one caller of this function, which has since been fixed. (If
we need to return a non-const value for compatibility, we can do that
and document that the caller should not mutate the output.)
Change-Id: I21f18f7438920a5b03d874fa548f054af3a42c4a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47664
Reviewed-by: Adam Langley <agl@google.com>
We misread (or maybe it changed?) the draft padding scheme. The current
text does not round the whole payload to a multiple of 32, just the
server name as a fallback. Switch the GREASE size selection to match.
Although, we may want to change the draft here. See also
https://github.com/tlswg/draft-ietf-tls-esni/issues/433
While I'm here, update some references from draft-09 to draft-10. Also
make the comment less verbose.
Bug: 275
Change-Id: I3c9f34159890bc3b7d71f6877f34b895bc7f9b17
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47644
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We didn't end up deploying this. We also never implemented the final
RFC, so what we do have isn't useful for someone who wishes to deploy
it anyway.
Update-Note: Token binding APIs are removed.
Change-Id: Iecea7c3dcf9d3e2644a3b7afaf61511310b45d5f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47584
Reviewed-by: Adam Langley <agl@google.com>
This introduces an EVP_HPKE_KEM, to capture the KEM choice, and
EVP_HPKE_KEY, to capture the key import (and thus avoids asking
receivers to pass in the full keypair). It is a bit more wordy now, but
we'll be in a better place when some non-TLS user inevitably asks for a
P-256 version.
Bug: 410
Change-Id: Icb9cc8b028e6d1f86e6d8adb31ebf1f975181675
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47329
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This replaces the ID-based API with one that is more static linker
friendly. For ECH, it doesn't make a difference because we currently
pull in all the options we've implemented. But this means other HPKE
uses need not pull in everything ECH needs and vice versa.
Along the way, fix an inconsistency: we prefixed all the AEAD constants
with "AEAD", but not the others. Since the rest of the name already
determines everything, go with the shorter version.
Bug: 410
Change-Id: I56e46c13b43c97e15eeb45204cde7019dd21e250
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47327
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Bug: 275
Change-Id: I8096070386af7d2b5020875ea09bcc0c04ebc8cd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47245
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
See go/handshake-hints (internal).
CL originally by Bin Wu <wub@google.com>. I just reworked the tests and
tidied it up a bit. This is the start of a replacement for the split
handshakes API. For now, only TLS 1.3 is supported. It starts with an
initial set of hints, but we can add more later. (In particular, we
should probably apply the remote handshaker's extension order to avoid
needing to capability protect such changes.)
Change-Id: I7b6a6dfaa84c6c6e3436d2a4026c3652b8a79f0f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46535
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
See also 86a90dc749af91f8a7b8da6628c9ffca2bae3009 from upstream. This
differs from upstream's which treats {NULL, 2} as a valid way to spell
the empty list. (I think this is a mistake and have asked them about
it.)
Upstream's CL also, for them, newly makes the empty list disable ALPN,
when previously they'd disable it but misread it as a malloc failure.
For us, we'd already fixed the misreading due to our switch to
bssl::Array and bssl::Span, but the documentation was odd. This CL
preserves that behavior, but updates the documentation and writes a
test.
Update-Note: SSL_CTX_set_alpn_protos and SSL_set_alpn_protos will now
reject invalud inputs. Previously, they would accept them, but silently
send an invalid ALPN extension which the server would almost certainly
error on.
Change-Id: Id5830b2d8c3a5cee4712878fe92ee350c4914367
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46804
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We currently determine whether we need HelloRetryRequest at the same
time as resolving key share machinery. That is a little too late for
early data negotiation, so we end up accepting early data and then
clearing it later on in the function. This works but is easy to mess up,
given the preceding CL. There's also some ALPS logic that got this
wrong, but I believe it didn't result in any incorrect behavior.
Instead, this pulls secret computation out of the key_share helper
function, which now just finds the matching key share. We then check
early whether we need HRR, before deciding on early data.
Change-Id: I108865da08addfefed4a7db73c60e11cf4335093
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46765
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This CL adds an initial implementation of the ECH server, with pieces of
the client in BoGo as necessary for testing. In particular, the server
supports ClientHelloInner compression with ech_outer_extensions. When
ECH decryption fails, it can send retry_configs back to the client.
This server passes the "ech-accept" and "ech-reject" test cases in
tls-interop-runner[0] when tested against both the cloudflare-go and nss
clients. For reproducibility, I started with the main branch at commit
707604c262d8bcf3e944ed1d5a675077304732ce and updated the endpoint's
script to pass the server's ECHConfig and private key to the boringssl
tool.
Follow-up CLs will update HPKE to the latest draft and catch us up to
draft-10.
[0]: https://github.com/xvzcf/tls-interop-runner
Bug: 275
Change-Id: I49be35af46d1fd5dd9c62252f07d0bae179381ab
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45285
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
AES_128_GCM is more common than AES_GCM_128 and matches the
specification.
Bug: 275
Change-Id: If3446a38f7bfbe0250d9646e363db29b93e4d231
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46666
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Dan McArdle <dmcardle@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This aligns with OpenSSL's behavior. RFC7301 says servers should return
no_application_protocol if the client supported ALPN but no common
protocol was found. We currently interpret all values as
SSL_TLSEXT_ERR_NOACK. Instead, implement both modes and give guidance on
whne to use each. (NOACK is still useful because the callback may be
shared across multiple configurations, some of which don't support ALPN
at all. Those would want to return NOACK to ignore the list.)
To match upstream, I've also switched SSL_R_MISSING_ALPN, added for
QUIC, to SSL_R_NO_APPLICATION_PROTOCOL.
Update-Note: Callers that return SSL_TLSEXT_ERR_ALERT_FATAL from the
ALPN callback will change behavior. The old behavior may be restored by
returning SSL_TLSEXT_ERR_NOACK, though see the documentation for new
recommendations on return values.
Change-Id: Ib7917b5f8a098571bed764c79aa7a4ce0f728297
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45504
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
David Benjamin
Anton Bikineev
Adam Langley
Steven Valdez
Daniel McArdle