Add a test for a subtle corner of 0-RTT and version tracking
This is a huge mess. I was trying to simplify our version state to be:
- Version not known
- Version not known but sending 0-RTT at V
- Version known to be V
But it turns out we can actually have both early version and true
versions existing and at different values. There are two cases:
1. We send TLS 1.3 early data and get back a TLS 1.2 ServerHello. We
will return an error but, at least right now, we continue to present
a self-consistent TLS 1.3 picture, even though we send the
protocol_version alert at TLS 1.2.
2. [Does not exist yet] We send TLS 1.4 early data and get back a TLS
1.3 ServerHello. This isn't even fatal to the connection, so we
especially should keep reporting the early session until
SSL_reset_early_Data_reject.
Case 2 is extra weird because we won't actually immediately flag the
mismatch as a 0-RTT reject! Right now we don't run the reject ceremony
until we read EncryptedExtensions, but at that point we'll have even
decrypted a record under the other version.
But this case also doesn't exist today. There is an analogous thing that
does exist today with the cipher suite. For both the version and the
cipher suite, we get around this right now by having the SSLAEADContext
hold on to this information, so we pull it out of there.
We could instead run the rejection ceremony at multiple points in the
handshake, as soon as we detect any kind of inconsistency. Not sure
which is best. Either way, let's test this case.
Change-Id: I333175b25ba35261e1dd7d02e9ec1d9f1d43ff73
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71531
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 37766b2..46d5af9 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -9750,5 +9750,65 @@
}
}
+TEST(SSLTest, EarlyDataVersionMismatch) {
+ bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
+ ASSERT_TRUE(client_ctx);
+ bssl::UniquePtr<SSL_CTX> server_ctx =
+ CreateContextWithTestCertificate(TLS_method());
+ ASSERT_TRUE(server_ctx);
+ SSL_CTX_set_early_data_enabled(client_ctx.get(), 1);
+ SSL_CTX_set_early_data_enabled(server_ctx.get(), 1);
+ SSL_CTX_set_session_cache_mode(client_ctx.get(), SSL_SESS_CACHE_BOTH);
+ SSL_CTX_set_session_cache_mode(server_ctx.get(), SSL_SESS_CACHE_BOTH);
+
+ bssl::UniquePtr<SSL_SESSION> session =
+ CreateClientSession(client_ctx.get(), server_ctx.get());
+ ASSERT_TRUE(session);
+ EXPECT_TRUE(SSL_SESSION_early_data_capable(session.get()));
+
+ // Turn off TLS 1.3 at the server.
+ SSL_CTX_set_max_proto_version(server_ctx.get(), TLS1_2_VERSION);
+ bssl::UniquePtr<SSL> client, server;
+ ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(),
+ server_ctx.get()));
+ SSL_set_session(client.get(), session.get());
+
+ // Send the ClientHello. The client should immediately treat the handshake as
+ // successful and offer early data.
+ EXPECT_EQ(1, SSL_do_handshake(client.get()));
+ EXPECT_TRUE(SSL_in_early_data(client.get()));
+
+ // In the early data state, we report the predicted version, so that callers
+ // see self-consistent connection properties.
+ EXPECT_EQ(SSL_version(client.get()), TLS1_3_VERSION);
+ EXPECT_NE(SSL_get0_peer_certificates(client.get()), nullptr);
+
+ // Read the ClientHello and send the ServerHello. The server will (implicitly
+ // by negotiating TLS 1.2) reject early data.
+ EXPECT_EQ(-1, SSL_do_handshake(server.get()));
+ EXPECT_EQ(SSL_ERROR_WANT_READ, SSL_get_error(server.get(), -1));
+
+ // Read the ServerHello. The client will now see the ServerHello and report a
+ // version mismatch. Unlike other 0-RTT rejections, this is fatal, because a
+ // TLS 1.2 server cannot recover from 0-RTT rejection.
+ EXPECT_EQ(-1, SSL_do_handshake(client.get()));
+ EXPECT_EQ(SSL_ERROR_SSL, SSL_get_error(client.get(), -1));
+ EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_SSL,
+ SSL_R_WRONG_VERSION_ON_EARLY_DATA));
+
+ // |SSL_version| should continue reporting self-consistent state until the
+ // caller calls |SSL_reset_early_data_reject|.
+ //
+ // TLS 1.3 to TLS 1.2 is not the most interesting version-related 0-RTT
+ // rejection because it is fatal to the connection anyway. Once there are two
+ // post-TLS-1.3 versions, or if we implement DTLS 1.3 0-RTT (where a DTLS 1.2
+ // server will skip over early data naturally), those will make for better
+ // tests. In particular, early_data accept is signaled in EncryptedExtensions,
+ // but the new version is learned at ServerHello. Though an implementation
+ // could already infer based on the version that early data will be rejected.
+ EXPECT_EQ(SSL_version(client.get()), TLS1_3_VERSION);
+ EXPECT_NE(SSL_get0_peer_certificates(client.get()), nullptr);
+}
+
} // namespace
BSSL_NAMESPACE_END
diff --git a/ssl/ssl_versions.cc b/ssl/ssl_versions.cc
index 13ad4d1..2f71978 100644
--- a/ssl/ssl_versions.cc
+++ b/ssl/ssl_versions.cc
@@ -250,7 +250,11 @@
}
static uint16_t ssl_version(const SSL *ssl) {
- // In early data, we report the predicted version.
+ // In early data, we report the predicted version. Note it is possible that we
+ // have a predicted version and a *different* true version. This means 0-RTT
+ // has been rejected, but until the reject has reported to the application and
+ // applied with |SSL_reset_early_data_reject|, we continue reporting a
+ // self-consistent connection.
if (SSL_in_early_data(ssl) && !ssl->server) {
return ssl->s3->hs->early_session->ssl_version;
}
