Check the second ClientHello's PSK binder on resumption.
We perform all our negotiation based on the first ClientHello (for
consistency with what |select_certificate_cb| observed), which is in the
transcript, so we can ignore most of the second one.
However, we ought to check the second PSK binder. That covers the client
key share, which we do consume. In particular, we'll want to check if it
we ever send half-RTT data on these connections (we do not currently do
this). It is also a tricky computation, so we enforce the peer handled
it correctly.
Tested that both Chrome and Firefox continue to interop with this check,
when configuring uncommon curve preferences that trigger HRR. (Normally
neither browser sees HRRs against BoringSSL servers.)
Update-Note: This does enforce some client behavior that we hadn't been
enforcing previously. However, it only figures into TLS 1.3 (not many
implementations yet), and only clients which hit HelloRetryRequest
(rare), so this should be low risk.
Change-Id: I42126585ec0685d009542094192e674cbd22520d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/37124
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index c1c41a8..52cea6c 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -1901,7 +1901,17 @@
bool ssl_ext_pre_shared_key_parse_clienthello(
SSL_HANDSHAKE *hs, CBS *out_ticket, CBS *out_binders,
- uint32_t *out_obfuscated_ticket_age, uint8_t *out_alert, CBS *contents) {
+ uint32_t *out_obfuscated_ticket_age, uint8_t *out_alert,
+ const SSL_CLIENT_HELLO *client_hello, CBS *contents) {
+ // Verify that the pre_shared_key extension is the last extension in
+ // ClientHello.
+ if (CBS_data(contents) + CBS_len(contents) !=
+ client_hello->extensions + client_hello->extensions_len) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_PRE_SHARED_KEY_MUST_BE_LAST);
+ *out_alert = SSL_AD_ILLEGAL_PARAMETER;
+ return false;
+ }
+
// We only process the first PSK identity since we don't support pure PSK.
CBS identities, binders;
if (!CBS_get_u16_length_prefixed(contents, &identities) ||
