Send ECH acceptance signal from backend server. This CL implements the backend server behavior described in Section 7.2 of draft-ietf-tls-esni-09. Bug: 275 Change-Id: I2e162673ce564db0cb75fc9b71ef11ed15037f4b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43924 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/ssl/handshake.cc b/ssl/handshake.cc index eb630a5..b38f96a 100644 --- a/ssl/handshake.cc +++ b/ssl/handshake.cc
@@ -126,6 +126,8 @@ SSL_HANDSHAKE::SSL_HANDSHAKE(SSL *ssl_arg) : ssl(ssl_arg), + ech_present(false), + ech_is_inner_present(false), scts_requested(false), needs_psk_binder(false), handshake_finalized(false),
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc index 22fa6a1..bc0a0d1 100644 --- a/ssl/handshake_server.cc +++ b/ssl/handshake_server.cc
@@ -644,6 +644,12 @@ return ssl_hs_error; } + if (hs->ech_present && hs->ech_is_inner_present) { + OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION); + ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); + return ssl_hs_error; + } + hs->state = state12_select_certificate; return ssl_hs_ok; }
diff --git a/ssl/internal.h b/ssl/internal.h index 5545bec..58a3769 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -1419,6 +1419,15 @@ const SSLMessage &msg, CBS *binders); +// Encrypted Client Hello. + +// tls13_ech_accept_confirmation computes the server's ECH acceptance signal, +// writing it to |out|. It returns true on success, and false on failure. +bool tls13_ech_accept_confirmation( + SSL_HANDSHAKE *hs, bssl::Span<uint8_t> out, + bssl::Span<const uint8_t> server_hello_ech_conf); + + // Handshake functions. enum ssl_hs_wait_t { @@ -1720,6 +1729,14 @@ // key_block is the record-layer key block for TLS 1.2 and earlier. Array<uint8_t> key_block; + // ech_present, on the server, indicates whether the ClientHello contained an + // encrypted_client_hello extension. + bool ech_present : 1; + + // ech_is_inner_present, on the server, indicates whether the ClientHello + // contained an ech_is_inner extension. + bool ech_is_inner_present : 1; + // scts_requested is true if the SCT extension is in the ClientHello. bool scts_requested : 1; @@ -1886,7 +1903,8 @@ bool ssl_ext_key_share_parse_clienthello(SSL_HANDSHAKE *hs, bool *out_found, Array<uint8_t> *out_secret, uint8_t *out_alert, CBS *contents); -bool ssl_ext_key_share_add_serverhello(SSL_HANDSHAKE *hs, CBB *out); +bool ssl_ext_key_share_add_serverhello(SSL_HANDSHAKE *hs, CBB *out, + bool dry_run); bool ssl_ext_pre_shared_key_parse_serverhello(SSL_HANDSHAKE *hs, uint8_t *out_alert,
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc index 7ec61fd..ce1a7a6 100644 --- a/ssl/t1_lib.cc +++ b/ssl/t1_lib.cc
@@ -739,6 +739,33 @@ return true; } +static bool ext_ech_parse_clienthello(SSL_HANDSHAKE *hs, uint8_t *out_alert, + CBS *contents) { + if (contents != nullptr) { + hs->ech_present = true; + return true; + } + return true; +} + +static bool ext_ech_is_inner_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) { + return true; +} + +static bool ext_ech_is_inner_parse_clienthello(SSL_HANDSHAKE *hs, + uint8_t *out_alert, + CBS *contents) { + if (contents == nullptr) { + return true; + } + if (CBS_len(contents) > 0) { + *out_alert = SSL_AD_ILLEGAL_PARAMETER; + return false; + } + hs->ech_is_inner_present = true; + return true; +} + // Renegotiation indication. // @@ -2435,7 +2462,8 @@ return true; } -bool ssl_ext_key_share_add_serverhello(SSL_HANDSHAKE *hs, CBB *out) { +bool ssl_ext_key_share_add_serverhello(SSL_HANDSHAKE *hs, CBB *out, + bool dry_run) { uint16_t group_id; CBB kse_bytes, public_key; if (!tls1_get_shared_group(hs, &group_id) || @@ -2448,10 +2476,10 @@ !CBB_flush(out)) { return false; } - - hs->ecdh_public_key.Reset(); - - hs->new_session->group_id = group_id; + if (!dry_run) { + hs->ecdh_public_key.Reset(); + hs->new_session->group_id = group_id; + } return true; } @@ -3128,7 +3156,15 @@ NULL, ext_ech_add_clienthello, ext_ech_parse_serverhello, - ignore_parse_clienthello, + ext_ech_parse_clienthello, + dont_add_serverhello, + }, + { + TLSEXT_TYPE_ech_is_inner, + NULL, + ext_ech_is_inner_add_clienthello, + forbid_parse_serverhello, + ext_ech_is_inner_parse_clienthello, dont_add_serverhello, }, {
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index b8e0eae..f1d4bfa 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go
@@ -128,6 +128,7 @@ extensionDelegatedCredentials uint16 = 0x22 // draft-ietf-tls-subcerts-06 extensionDuplicate uint16 = 0xffff // not IANA assigned extensionEncryptedClientHello uint16 = 0xfe09 // not IANA assigned + extensionECHIsInner uint16 = 0xda09 // not IANA assigned ) // TLS signaling cipher suite values @@ -799,10 +800,24 @@ // encrypted_client_hello extension containing a ClientECH structure. ExpectClientECH bool + // ExpectServerAcceptECH causes the client to expect that the server will + // indicate ECH acceptance in the ServerHello. + ExpectServerAcceptECH bool + // SendECHRetryConfigs, if not empty, contains the ECH server's serialized // retry configs. SendECHRetryConfigs []byte + // SendEncryptedClientHello, when true, causes the client to send a + // placeholder encrypted_client_hello extension on the ClientHelloOuter + // message. + SendPlaceholderEncryptedClientHello bool + + // SendECHIsInner, when non-nil, causes the client to send an ech_is_inner + // extension on the ClientHelloOuter message. When nil, the extension will + // be omitted. + SendECHIsInner []byte + // SwapNPNAndALPN switches the relative order between NPN and ALPN in // both ClientHello and ServerHello. SwapNPNAndALPN bool
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index 3ebc070..bf89c01 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go
@@ -19,6 +19,8 @@ "math/big" "net" "time" + + "boringssl.googlesource.com/boringssl/ssl/test/runner/hpke" ) type clientHandshakeState struct { @@ -122,6 +124,7 @@ ocspStapling: !c.config.Bugs.NoOCSPStapling, sctListSupported: !c.config.Bugs.NoSignedCertificateTimestamps, serverName: c.config.ServerName, + echIsInner: c.config.Bugs.SendECHIsInner, supportedCurves: c.config.curvePreferences(), supportedPoints: []uint8{pointFormatUncompressed}, nextProtoNeg: len(c.config.NextProtos) > 0, @@ -211,6 +214,16 @@ hello.alpsProtocols = append(hello.alpsProtocols, protocol) } + if c.config.Bugs.SendPlaceholderEncryptedClientHello { + hello.clientECH = &clientECH{ + hpkeKDF: hpke.HKDFSHA256, + hpkeAEAD: hpke.AES128GCM, + configID: []byte{0x02, 0x0d, 0xe8, 0xae, 0xf5, 0x8b, 0x59, 0xb5}, + enc: []byte{0xe2, 0xf0, 0x96, 0x64, 0x18, 0x35, 0x10, 0xb3}, + payload: []byte{0xa8, 0x53, 0x3a, 0x8d, 0xe8, 0x5f, 0x7c, 0xd7}, + } + } + var keyShares map[CurveID]ecdhCurve if maxVersion >= VersionTLS13 { keyShares = make(map[CurveID]ecdhCurve) @@ -740,13 +753,13 @@ hs.writeServerHash(helloRetryRequest.marshal()) hs.writeClientHash(secondHelloBytes) } - hs.writeServerHash(hs.serverHello.marshal()) if c.vers >= VersionTLS13 { if err := hs.doTLS13Handshake(); err != nil { return err } } else { + hs.writeServerHash(hs.serverHello.marshal()) if c.config.Bugs.EarlyChangeCipherSpec > 0 { hs.establishKeys() c.writeRecord(recordTypeChangeCipherSpec, []byte{1}) @@ -839,10 +852,6 @@ return errors.New("tls: session IDs did not match.") } - // Once the PRF hash is known, TLS 1.3 does not require a handshake - // buffer. - hs.finishedHash.discardHandshakeBuffer() - zeroSecret := hs.finishedHash.zeroSecret() // Resolve PSK and compute the early secret. @@ -891,6 +900,25 @@ hs.finishedHash.addEntropy(zeroSecret) } + // Determine whether the server indicated ECH acceptance. + + // Generate ServerHelloECHConf, which is identical to the ServerHello except + // that the last 8 bytes of the random value are zeroes. + echAcceptConfirmation := hs.finishedHash.deriveSecretPeek([]byte("ech accept confirmation"), hs.serverHello.marshalForECHConf()) + serverAcceptedECH := bytes.Equal(echAcceptConfirmation[:8], hs.serverHello.random[24:]) + if c.config.Bugs.ExpectServerAcceptECH && !serverAcceptedECH { + return errors.New("tls: server did not indicate ECH acceptance") + } + if !c.config.Bugs.ExpectServerAcceptECH && serverAcceptedECH { + return errors.New("tls: server indicated ECH acceptance") + } + + // Once the PRF hash is known, TLS 1.3 does not require a handshake + // buffer. + hs.finishedHash.discardHandshakeBuffer() + + hs.writeServerHash(hs.serverHello.marshal()) + // Derive handshake traffic keys and switch read key to handshake // traffic key. clientHandshakeTrafficSecret := hs.finishedHash.deriveSecret(clientHandshakeTrafficLabel)
diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go index 42b5eb5..a8ea67d 100644 --- a/ssl/test/runner/handshake_messages.go +++ b/ssl/test/runner/handshake_messages.go
@@ -303,6 +303,7 @@ nextProtoNeg bool serverName string clientECH *clientECH + echIsInner []byte ocspStapling bool supportedCurves []CurveID supportedPoints []uint8 @@ -422,7 +423,6 @@ }) } if m.clientECH != nil { - // https://tools.ietf.org/html/draft-ietf-tls-esni-09 body := newByteBuilder() body.addU16(m.clientECH.hpkeKDF) body.addU16(m.clientECH.hpkeAEAD) @@ -435,6 +435,12 @@ body: body.finish(), }) } + if m.echIsInner != nil { + extensions = append(extensions, extension{ + id: extensionECHIsInner, + body: m.echIsInner, + }) + } if m.ocspStapling { certificateStatusRequest := newByteBuilder() // RFC 4366, section 3.6 @@ -1269,6 +1275,16 @@ return true } +// marshalForECHConf marshals |m|, but zeroes out the last 8 bytes of the +// ServerHello.random. +func (m *serverHelloMsg) marshalForECHConf() []byte { + serverHelloECHConf := *m + serverHelloECHConf.raw = nil + serverHelloECHConf.random = make([]byte, 32) + copy(serverHelloECHConf.random[:24], m.random) + return serverHelloECHConf.marshal() +} + type encryptedExtensionsMsg struct { raw []byte extensions serverExtensions
diff --git a/ssl/test/runner/prf.go b/ssl/test/runner/prf.go index 73251c6..b27f038 100644 --- a/ssl/test/runner/prf.go +++ b/ssl/test/runner/prf.go
@@ -457,6 +457,16 @@ return hkdfExpandLabel(h.hash, h.secret, label, h.appendContextHashes(nil), h.hash.Size()) } +// deriveSecretPeek is the same as deriveSecret, but it enables the caller to +// tentatively append messages to the transcript. The |extraMessages| parameter +// contains the bytes of these tentative messages. +func (h *finishedHash) deriveSecretPeek(label []byte, extraMessages []byte) []byte { + hashPeek := h.hash.New() + hashPeek.Write(h.buffer) + hashPeek.Write(extraMessages) + return hkdfExpandLabel(h.hash, h.secret, label, hashPeek.Sum(nil), h.hash.Size()) +} + // The following are context strings for CertificateVerify in TLS 1.3. var ( clientCertificateVerifyContextTLS13 = []byte("TLS 1.3, client CertificateVerify")
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 3b0bd85..6f1f0ab 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -16246,6 +16246,95 @@ expectedLocalError: "remote error: error decoding message", expectedError: ":ERROR_PARSING_EXTENSION:", }) + + // Test that the server responds to an empty ECH extension with the acceptance + // confirmation. + testCases = append(testCases, testCase{ + testType: serverTest, + name: "ECH-Server-ECHIsInner", + config: Config{ + MinVersion: VersionTLS13, + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + SendECHIsInner: []byte{}, + ExpectServerAcceptECH: true, + }, + }, + }) + + // Test that server fails the handshake when it sees a nonempty ech_is_inner + // extension. + testCases = append(testCases, testCase{ + testType: serverTest, + name: "ECH-Server-ECHIsInner-NotEmpty", + config: Config{ + MinVersion: VersionTLS13, + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + SendECHIsInner: []byte{42, 42, 42}, + }, + }, + shouldFail: true, + expectedLocalError: "remote error: illegal parameter", + expectedError: ":ERROR_PARSING_EXTENSION:", + }) + + // When ech_is_inner extension is absent, the server should not accept ECH. + testCases = append(testCases, testCase{ + testType: serverTest, + name: "ECH-Server-ECHIsInner-Absent", + config: Config{ + MinVersion: VersionTLS13, + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + ExpectServerAcceptECH: false, + }, + }, + }) + + // Test that a TLS 1.3 server that receives an ech_is_inner extension can + // negotiate TLS 1.2 without clobbering the downgrade signal. + testCases = append(testCases, testCase{ + testType: serverTest, + name: "ECH-Server-ECHIsInner-Absent-TLS12", + config: Config{ + MinVersion: VersionTLS12, + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + // Omit supported_versions extension so the server negotiates + // TLS 1.2. + OmitSupportedVersions: true, + SendECHIsInner: []byte{}, + }, + }, + // Check that the client sees the TLS 1.3 downgrade signal in + // ServerHello.random. + shouldFail: true, + expectedLocalError: "tls: downgrade from TLS 1.3 detected", + }) + + // Test that the handshake fails when the client sends both + // encrypted_client_hello and ech_is_inner extensions. + // + // TODO(dmcardle) Replace this test once runner is capable of sending real + // ECH extensions. The equivalent test will send ech_is_inner and a real + // encrypted_client_hello extension derived from a key unknown to the + // server. + testCases = append(testCases, testCase{ + testType: serverTest, + name: "ECH-Server-EncryptedClientHello-ECHIsInner", + config: Config{ + MinVersion: VersionTLS13, + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + SendECHIsInner: []byte{}, + SendPlaceholderEncryptedClientHello: true, + }, + }, + shouldFail: true, + expectedLocalError: "remote error: illegal parameter", + expectedError: ":UNEXPECTED_EXTENSION:", + }) } func worker(statusChan chan statusMsg, c chan *testCase, shimPath string, wg *sync.WaitGroup) {
diff --git a/ssl/tls13_enc.cc b/ssl/tls13_enc.cc index 198adb6..cda53ec 100644 --- a/ssl/tls13_enc.cc +++ b/ssl/tls13_enc.cc
@@ -507,4 +507,40 @@ return true; } +bool tls13_ech_accept_confirmation( + SSL_HANDSHAKE *hs, bssl::Span<uint8_t> out, + bssl::Span<const uint8_t> server_hello_ech_conf) { + // Compute the hash of the transcript concatenated with + // |server_hello_ech_conf| without modifying |hs->transcript|. + uint8_t context_hash[EVP_MAX_MD_SIZE]; + unsigned context_hash_len; + ScopedEVP_MD_CTX ctx; + if (!hs->transcript.CopyToHashContext(ctx.get(), hs->transcript.Digest()) || + !EVP_DigestUpdate(ctx.get(), server_hello_ech_conf.data(), + server_hello_ech_conf.size()) || + !EVP_DigestFinal_ex(ctx.get(), context_hash, &context_hash_len)) { + return false; + } + + // Per draft-ietf-tls-esni-09, accept_confirmation is computed with + // Derive-Secret, which derives a secret of size Hash.length. That value is + // then truncated to the first 8 bytes. Note this differs from deriving an + // 8-byte secret because the target length is included in the derivation. + uint8_t accept_confirmation_buf[EVP_MAX_MD_SIZE]; + bssl::Span<uint8_t> accept_confirmation = + MakeSpan(accept_confirmation_buf, hs->transcript.DigestLen()); + if (!hkdf_expand_label(accept_confirmation, hs->transcript.Digest(), + hs->secret(), label_to_span("ech accept confirmation"), + MakeConstSpan(context_hash, context_hash_len))) { + return false; + } + + if (out.size() > accept_confirmation.size()) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return false; + } + OPENSSL_memcpy(out.data(), accept_confirmation.data(), out.size()); + return true; +} + BSSL_NAMESPACE_END
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc index 9dfe325..c072a6f 100644 --- a/ssl/tls13_server.cc +++ b/ssl/tls13_server.cc
@@ -636,20 +636,58 @@ static enum ssl_hs_wait_t do_send_server_hello(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; + Span<uint8_t> random(ssl->s3->server_random); + RAND_bytes(random.data(), random.size()); + + // If the ClientHello has an ech_is_inner extension, we must be the ECH + // backend server. In response to ech_is_inner, we will overwrite part of the + // ServerHello.random with the ECH acceptance confirmation. + if (hs->ech_is_inner_present) { + // Construct the ServerHelloECHConf message, which is the same as + // ServerHello, except the last 8 bytes of its random field are zeroed out. + Span<uint8_t> random_suffix = random.subspan(24); + OPENSSL_memset(random_suffix.data(), 0, random_suffix.size()); + + ScopedCBB cbb; + CBB body, extensions, session_id; + if (!ssl->method->init_message(ssl, cbb.get(), &body, + SSL3_MT_SERVER_HELLO) || + !CBB_add_u16(&body, TLS1_2_VERSION) || + !CBB_add_bytes(&body, random.data(), random.size()) || + !CBB_add_u8_length_prefixed(&body, &session_id) || + !CBB_add_bytes(&session_id, hs->session_id, hs->session_id_len) || + !CBB_add_u16(&body, SSL_CIPHER_get_protocol_id(hs->new_cipher)) || + !CBB_add_u8(&body, 0) || + !CBB_add_u16_length_prefixed(&body, &extensions) || + !ssl_ext_pre_shared_key_add_serverhello(hs, &extensions) || + !ssl_ext_key_share_add_serverhello(hs, &extensions, /*dry_run=*/true) || + !ssl_ext_supported_versions_add_serverhello(hs, &extensions) || + !CBB_flush(cbb.get())) { + return ssl_hs_error; + } + + // Note that |cbb| includes the message type and length fields, but not the + // record layer header. + if (!tls13_ech_accept_confirmation( + hs, random_suffix, + bssl::MakeConstSpan(CBB_data(cbb.get()), CBB_len(cbb.get())))) { + return ssl_hs_error; + } + } + // Send a ServerHello. ScopedCBB cbb; CBB body, extensions, session_id; if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_SERVER_HELLO) || !CBB_add_u16(&body, TLS1_2_VERSION) || - !RAND_bytes(ssl->s3->server_random, sizeof(ssl->s3->server_random)) || - !CBB_add_bytes(&body, ssl->s3->server_random, SSL3_RANDOM_SIZE) || + !CBB_add_bytes(&body, random.data(), random.size()) || !CBB_add_u8_length_prefixed(&body, &session_id) || !CBB_add_bytes(&session_id, hs->session_id, hs->session_id_len) || !CBB_add_u16(&body, SSL_CIPHER_get_protocol_id(hs->new_cipher)) || !CBB_add_u8(&body, 0) || !CBB_add_u16_length_prefixed(&body, &extensions) || !ssl_ext_pre_shared_key_add_serverhello(hs, &extensions) || - !ssl_ext_key_share_add_serverhello(hs, &extensions) || + !ssl_ext_key_share_add_serverhello(hs, &extensions, /*dry_run=*/false) || !ssl_ext_supported_versions_add_serverhello(hs, &extensions) || !ssl_add_message_cbb(ssl, cbb.get())) { return ssl_hs_error;