Fix ALPS state machine in QUIC servers.
The state machine around EndOfEarlyData is a bit messy, which caused a
problem introducing the new message in QUIC. We keep waffling on whether
that state junction should no-op the EndOfEarlyData state or skip it.
Since skipping it caused us to miss this spot, let's try no-op-ing it.
As a test, so this CL is easier to cherry-pick, I've just duplicated the
basic server test. Better, however, would be to run all the extensions
tests under QUIC. (In particular, this is missing 0-RTT coverage.) But
this is a large diff and requires improving the mock QUIC transport,
etc., in runner. That work is done in follow-up CLs, which replace this
duplicated test.
Change-Id: I25b6feabdc6e5393ba7f486651986a90e3721667
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44985
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 5dacff4..aca229a 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -7008,6 +7008,40 @@
},
})
+ // TODO(davidben): This is a copy of ALPS-Basic-Server to test a
+ // QUIC-specific ALPS issue. Remove this and instead run all the
+ // tests in this function at all protocols.
+ testCases = append(testCases, testCase{
+ protocol: quic,
+ testType: serverTest,
+ name: "ALPS-Basic-Server-QUIC-" + ver.name,
+ skipQUICALPNConfig: true,
+ config: Config{
+ MaxVersion: ver.version,
+ NextProtos: []string{"proto"},
+ ApplicationSettings: map[string][]byte{"proto": []byte("runner1")},
+ },
+ resumeConfig: &Config{
+ MaxVersion: ver.version,
+ NextProtos: []string{"proto"},
+ ApplicationSettings: map[string][]byte{"proto": []byte("runner2")},
+ },
+ resumeSession: true,
+ expectations: connectionExpectations{
+ peerApplicationSettings: []byte("shim1"),
+ },
+ resumeExpectations: &connectionExpectations{
+ peerApplicationSettings: []byte("shim2"),
+ },
+ flags: []string{
+ "-select-alpn", "proto",
+ "-on-initial-application-settings", "proto,shim1",
+ "-on-initial-expect-peer-application-settings", "runner1",
+ "-on-resume-application-settings", "proto,shim2",
+ "-on-resume-expect-peer-application-settings", "runner2",
+ },
+ })
+
// Test that the server can defer its ALPS configuration to the ALPN
// selection callback.
testCases = append(testCases, testCase{
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index c072a6f..6e9cd07 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -882,7 +882,7 @@
hs->client_handshake_secret())) {
return ssl_hs_error;
}
- hs->tls13_state = state13_read_client_certificate;
+ hs->tls13_state = state13_process_end_of_early_data;
return ssl->s3->early_data_accepted ? ssl_hs_early_return : ssl_hs_ok;
}
@@ -893,27 +893,31 @@
static enum ssl_hs_wait_t do_process_end_of_early_data(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
- // If early data was not accepted, the EndOfEarlyData will be in the discarded
- // early data.
- if (hs->ssl->s3->early_data_accepted) {
- SSLMessage msg;
- if (!ssl->method->get_message(ssl, &msg)) {
- return ssl_hs_read_message;
+ // In protocols that use EndOfEarlyData, we must consume the extra message and
+ // switch to client_handshake_secret after the early return.
+ if (ssl->quic_method == nullptr) {
+ // If early data was not accepted, the EndOfEarlyData will be in the
+ // discarded early data.
+ if (hs->ssl->s3->early_data_accepted) {
+ SSLMessage msg;
+ if (!ssl->method->get_message(ssl, &msg)) {
+ return ssl_hs_read_message;
+ }
+ if (!ssl_check_message_type(ssl, msg, SSL3_MT_END_OF_EARLY_DATA)) {
+ return ssl_hs_error;
+ }
+ if (CBS_len(&msg.body) != 0) {
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ return ssl_hs_error;
+ }
+ ssl->method->next_message(ssl);
}
- if (!ssl_check_message_type(ssl, msg, SSL3_MT_END_OF_EARLY_DATA)) {
+ if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_open,
+ hs->new_session.get(),
+ hs->client_handshake_secret())) {
return ssl_hs_error;
}
- if (CBS_len(&msg.body) != 0) {
- ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- return ssl_hs_error;
- }
- ssl->method->next_message(ssl);
- }
- if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_open,
- hs->new_session.get(),
- hs->client_handshake_secret())) {
- return ssl_hs_error;
}
hs->tls13_state = state13_read_client_encrypted_extensions;
return ssl_hs_ok;
