Correctly handle max_early_data_size over 2^16 Although we never set a max early data size greater than 2^16, the protocol allows it, so we need to track 32 bits worth of early data written, not 16. (With the size increase, move the field with other 4-byte-aligned values to make better use of struct padding.) The practical impact of this is minor. If a server sent a 0-RTT ticket that allowed > 65K of early data, and if somehow we managed to send that much data before getting the ServerHello, we might exceed the server's limit. The server would then reject the connection. Change-Id: Iee6f562555c062a0b3ded73b98027c3a13609e45 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/95267 Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Lily Chen <chlily@google.com> Presubmit-BoringSSL-Verified: boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/ssl/internal.h b/ssl/internal.h index 2e349b6..2bea8f6 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -1827,6 +1827,10 @@ // the ClientHelloInner. uint32_t inner_extensions_sent = 0; + // early_data_written is the amount of early data that has been written by the + // record layer. + uint32_t early_data_written = 0; + // error, if |wait| is |ssl_hs_error|, is the error the handshake failed on. UniquePtr<ERR_SAVE_STATE> error; @@ -2104,10 +2108,6 @@ // record layer. uint16_t early_data_read = 0; - // early_data_written is the amount of early data that has been written by the - // record layer. - uint16_t early_data_written = 0; - // signature_algorithm is the signature algorithm to be used in signing with // the selected credential, or zero if not applicable or not yet selected. uint16_t signature_algorithm = 0;
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc index 62f3d9d..9f3baa9 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc
@@ -1166,7 +1166,6 @@ } } } else { - static const char kInitialWrite[] = "hello"; bool pending_initial_write = false; if (config->read_with_unfinished_write) { if (!config->async) { @@ -1181,15 +1180,19 @@ // Let only one byte of the record through. AsyncBioAllowWrite(test_state->async_bio, 1); - int write_ret = SSL_write(ssl, kInitialWrite, strlen(kInitialWrite)); + int write_ret = SSL_write(ssl, config->shim_initial_write.data(), + config->shim_initial_write.size()); if (SSL_get_error(ssl, write_ret) != SSL_ERROR_WANT_WRITE) { fprintf(stderr, "Failed to leave unfinished write.\n"); return false; } pending_initial_write = true; } else if (config->shim_writes_first) { - if (WriteAll(ssl, kInitialWrite, strlen(kInitialWrite)) < 0) { - return false; + for (int i = 0; i < config->repeat_shim_initial_write; i++) { + if (WriteAll(ssl, config->shim_initial_write.data(), + config->shim_initial_write.size()) < 0) { + return false; + } } } if (!config->shim_shuts_down) { @@ -1247,7 +1250,8 @@ // Clear the initial write, if unfinished. if (pending_initial_write) { - if (WriteAll(ssl, kInitialWrite, strlen(kInitialWrite)) < 0) { + if (WriteAll(ssl, config->shim_initial_write.data(), + config->shim_initial_write.size()) < 0) { return false; } pending_initial_write = false;
diff --git a/ssl/test/runner/state_machine_tests.go b/ssl/test/runner/state_machine_tests.go index ebdca5e..c241695 100644 --- a/ssl/test/runner/state_machine_tests.go +++ b/ssl/test/runner/state_machine_tests.go
@@ -17,7 +17,9 @@ import ( "bytes" "fmt" + "slices" "strconv" + "strings" ) type stateMachineTestConfig struct { @@ -233,6 +235,33 @@ earlyData: true, }) + // Test handling of maximum early data that exceeds 2^16. + largeInitialWrite := strings.Repeat("0123456789abcdef", 32) + const largeInitialWriteRepeat = 256 + tests = append(tests, testCase{ + testType: clientTest, + name: "TLS13-EarlyData-TooMuchData-LargeLimit-Client", + config: Config{ + MaxVersion: VersionTLS13, + MinVersion: VersionTLS13, + MaxEarlyDataSize: uint32(len(largeInitialWrite) * largeInitialWriteRepeat), + }, + resumeConfig: &Config{ + MaxVersion: VersionTLS13, + MinVersion: VersionTLS13, + MaxEarlyDataSize: uint32(len(largeInitialWrite) * largeInitialWriteRepeat), + Bugs: ProtocolBugs{ + ExpectEarlyData: slices.Repeat([][]byte{[]byte(largeInitialWrite)}, largeInitialWriteRepeat), + }, + }, + resumeShimPrefix: largeInitialWrite, + resumeSession: true, + earlyData: true, + flags: []string{ + "-on-resume-shim-initial-write", largeInitialWrite, + "-on-resume-repeat-shim-initial-write", strconv.Itoa(largeInitialWriteRepeat + 1)}, + }) + // Unfinished writes can only be tested when operations are async. EarlyData // can't be tested as part of an ImplicitHandshake in this case since // otherwise the early data will be sent as normal data.
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index 54e1a92..4333d91 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc
@@ -390,6 +390,9 @@ BoolFlag("-enable-channel-id", &TestConfig::enable_channel_id), StringFlag("-send-channel-id", &TestConfig::send_channel_id), BoolFlag("-shim-writes-first", &TestConfig::shim_writes_first), + StringFlag("-shim-initial-write", &TestConfig::shim_initial_write), + IntFlag("-repeat-shim-initial-write", + &TestConfig::repeat_shim_initial_write), StringFlag("-host-name", &TestConfig::host_name), StringFlag("-advertise-alpn", &TestConfig::advertise_alpn), StringFlag("-expect-alpn", &TestConfig::expect_alpn),
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index 4b82cd3..8c81309 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h
@@ -110,6 +110,8 @@ bool enable_channel_id = false; std::string send_channel_id; bool shim_writes_first = false; + std::string shim_initial_write = "hello"; + int repeat_shim_initial_write = 1; std::string host_name; std::string advertise_alpn; std::string expect_alpn;