From 4749d8fb89ba5d765e0afa1799805b5096cb337b Mon Sep 17 00:00:00 2001 From: Dan McArdle Date: Thu, 20 May 2021 12:59:56 -0400 Subject: [PATCH] Implement fuzzer mode for ECH server. Now skipping over HPKE decryption in |ssl_client_hello_decrypt| when fuzzer mode is enabled. To improve code coverage, this fuzzer-only logic also also has the ability to simulate a failed decryption. As a result of mostly skipping the decryption, we now have to exclude "*-ECH-Server-Decline*" tests from running in fuzzer mode. These tests rely on the now-broken assumption that decryption will fail when the client used an ECHConfig unknown to the server. Bug: 275 Change-Id: I759a79c8596897cdd3d3a37e05f2973d47346ef9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47624 Commit-Queue: David Benjamin Reviewed-by: David Benjamin --- ssl/encrypted_client_hello.cc | 15 +++++++++++++ ssl/test/fuzzer.h | 33 +++++++++++++++++++++++++++++ ssl/test/fuzzer_tags.h | 2 +- ssl/test/runner/fuzzer_mode.json | 4 +++- ssl/test/runner/handshake_client.go | 18 ++++++++++++++-- 5 files changed, 68 insertions(+), 4 deletions(-) diff --git a/ssl/encrypted_client_hello.cc b/ssl/encrypted_client_hello.cc index 5aeb1b46a..785166758 100644 --- a/ssl/encrypted_client_hello.cc +++ b/ssl/encrypted_client_hello.cc @@ -316,6 +316,20 @@ bool ssl_client_hello_decrypt( return false; } +#if defined(BORINGSSL_UNSAFE_FUZZER_MODE) + // In fuzzer mode, disable encryption to improve coverage. We reserve a short + // input to signal decryption failure, so the fuzzer can explore fallback to + // ClientHelloOuter. + const uint8_t kBadPayload[] = {0xff}; + if (payload == kBadPayload) { + *out_is_decrypt_error = true; + OPENSSL_PUT_ERROR(SSL, SSL_R_DECRYPTION_FAILED); + return false; + } + if (!out_encoded_client_hello_inner->CopyFrom(payload)) { + return false; + } +#else // Attempt to decrypt into |out_encoded_client_hello_inner|. if (!out_encoded_client_hello_inner->Init(payload.size())) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); @@ -332,6 +346,7 @@ bool ssl_client_hello_decrypt( return false; } out_encoded_client_hello_inner->Shrink(encoded_client_hello_inner_len); +#endif return true; } diff --git a/ssl/test/fuzzer.h b/ssl/test/fuzzer.h index 8f7a35508..839560efa 100644 --- a/ssl/test/fuzzer.h +++ b/ssl/test/fuzzer.h @@ -230,6 +230,22 @@ const uint8_t kALPNProtocols[] = { 0x01, 'a', 0x02, 'a', 'a', 0x03, 'a', 'a', 'a', }; +const uint8_t kECHServerConfig[] = { + 0xfe, 0x0a, 0x00, 0x47, 0x2a, 0x00, 0x20, 0x00, 0x20, 0x6c, 0x55, + 0x96, 0x41, 0x3d, 0x12, 0x4e, 0x63, 0x3d, 0x39, 0x7a, 0xe9, 0xbc, + 0xec, 0xb2, 0x55, 0xd0, 0xe6, 0xaa, 0xbd, 0xa9, 0x79, 0xb8, 0x86, + 0x9a, 0x13, 0x61, 0xc6, 0x69, 0xac, 0xb4, 0x21, 0x00, 0x0c, 0x00, + 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x02, 0x00, 0x01, 0x00, 0x03, + 0x00, 0x10, 0x00, 0x0e, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x2e, + 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x00, 0x00, +}; + +const uint8_t kECHServerConfigPrivateKey[] = { + 0x35, 0x6d, 0x45, 0x06, 0xb3, 0x88, 0x89, 0x2e, 0xd6, 0x87, 0x84, + 0xd2, 0x2d, 0x6f, 0x83, 0x48, 0xad, 0xf2, 0xfd, 0x08, 0x51, 0x73, + 0x10, 0xa0, 0xb8, 0xdd, 0xe9, 0x96, 0x6a, 0xde, 0xbc, 0x82, +}; + int ALPNSelectCallback(SSL *ssl, const uint8_t **out, uint8_t *out_len, const uint8_t *in, unsigned in_len, void *arg) { static const uint8_t kProtocol[] = {'a', 'a'}; @@ -438,6 +454,23 @@ class TLSFuzzer { } SSL_CTX_set_tls_channel_id_enabled(ctx_.get(), 1); + if (role_ == kServer) { + bssl::UniquePtr config_list( + SSL_ECH_SERVER_CONFIG_LIST_new()); + if (!config_list) { + return false; + } + if (!SSL_ECH_SERVER_CONFIG_LIST_add( + config_list.get(), /*is_retry_config=*/true, kECHServerConfig, + sizeof(kECHServerConfig), kECHServerConfigPrivateKey, + sizeof(kECHServerConfigPrivateKey))) { + return false; + } + if (!SSL_CTX_set1_ech_server_config_list(ctx_.get(), config_list.get())) { + return false; + } + } + return true; } diff --git a/ssl/test/fuzzer_tags.h b/ssl/test/fuzzer_tags.h index 3946df797..b61222222 100644 --- a/ssl/test/fuzzer_tags.h +++ b/ssl/test/fuzzer_tags.h @@ -23,7 +23,7 @@ // The TLS client and server fuzzers coordinate with bssl_shim on a common // format to encode configuration parameters in a fuzzer file. To add a new // configuration, define a tag, update |SetupTest| in fuzzer.h to parse it, and -// update |WriteSettings| in bssl_shim to serialize it. Finally, record +// update |SettingsWriter| in bssl_shim to serialize it. Finally, record // transcripts from a test run, and use the BORINGSSL_FUZZER_DEBUG environment // variable to confirm the transcripts are compatible. diff --git a/ssl/test/runner/fuzzer_mode.json b/ssl/test/runner/fuzzer_mode.json index f2dc3fdcb..e7c8ad7fd 100644 --- a/ssl/test/runner/fuzzer_mode.json +++ b/ssl/test/runner/fuzzer_mode.json @@ -51,6 +51,8 @@ "Renegotiate-Client-BadExt*": "Fuzzer mode does not check renegotiation_info.", - "CBCRecordSplitting*": "Fuzzer mode does not implement record-splitting." + "CBCRecordSplitting*": "Fuzzer mode does not implement record-splitting.", + + "*-ECH-Server-Decline*": "Encryption with wrong ECHConfig will not fail because fuzzer mode skips HPKE decryption." } } diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index 7d32c7471..3451137cf 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go @@ -23,6 +23,8 @@ import ( "boringssl.googlesource.com/boringssl/ssl/test/runner/hpke" ) +const echBadPayloadByte = 0xff + type clientHandshakeState struct { c *Conn serverHello *serverHelloMsg @@ -826,7 +828,11 @@ NextCipherSuite: return nil, err } if c.config.Bugs.CorruptEncryptedClientHello { - hello.clientECH.payload[0] ^= 1 + if c.config.Bugs.NullAllCiphers { + hello.clientECH.payload = []byte{echBadPayloadByte} + } else { + hello.clientECH.payload[0] ^= 1 + } } } @@ -884,6 +890,10 @@ func (hs *clientHandshakeState) encryptClientHello(hello, innerHello *clientHell encodedInner := innerHello.marshalForEncodedInner() payload := hs.echHPKEContext.Seal(encodedInner, aad.finish()) + if c.config.Bugs.NullAllCiphers { + payload = encodedInner + } + // Place the ECH extension in the outer CH. hello.clientECH = &clientECH{ hpkeKDF: hs.echHPKEContext.KDF(), @@ -1534,7 +1544,11 @@ func (hs *clientHandshakeState) applyHelloRetryRequest(helloRetryRequest *helloR return err } if c.config.Bugs.CorruptSecondEncryptedClientHello { - hello.clientECH.payload[0] ^= 1 + if c.config.Bugs.NullAllCiphers { + hello.clientECH.payload = []byte{echBadPayloadByte} + } else { + hello.clientECH.payload[0] ^= 1 + } } } }