In 0RTT mode, reverify the server certificate before sending early data.

Bug: chromium:347402
Change-Id: I1442b595ed7296b9d9fe88357565f68e1ab80ffd
Reviewed-on: https://boringssl-review.googlesource.com/c/32644
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/handshake_client.cc

@@ -177,6 +177,7 @@ BSSL_NAMESPACE_BEGIN
 enum ssl_client_hs_state_t {
   state_start_connect = 0,
   state_enter_early_data,
+  state_early_reverify_server_certificate,
   state_read_hello_verify_request,
   state_read_server_hello,
   state_tls13,
@@ -466,10 +467,26 @@ static enum ssl_hs_wait_t do_enter_early_data(SSL_HANDSHAKE *hs) {
 
   // Stash the early data session, so connection properties may be queried out
   // of it.
-  hs->in_early_data = true;
   hs->early_session = UpRef(ssl->session);
-  hs->can_early_write = true;
+  hs->state = state_early_reverify_server_certificate;
+  return ssl_hs_ok;
+}
+
+static enum ssl_hs_wait_t do_early_reverify_server_certificate(SSL_HANDSHAKE *hs) {
+  if (hs->ssl->ctx->reverify_on_resume) {
+    switch (ssl_reverify_peer_cert(hs)) {
+    case ssl_verify_ok:
+      break;
+    case ssl_verify_invalid:
+      return ssl_hs_error;
+    case ssl_verify_retry:
+      hs->state = state_early_reverify_server_certificate;
+      return ssl_hs_certificate_verify;
+    }
+  }
 
+  hs->in_early_data = true;
+  hs->can_early_write = true;
   hs->state = state_read_server_hello;
   return ssl_hs_early_return;
 }
@@ -1692,6 +1709,9 @@ enum ssl_hs_wait_t ssl_client_handshake(SSL_HANDSHAKE *hs) {
       case state_enter_early_data:
         ret = do_enter_early_data(hs);
         break;
+      case state_early_reverify_server_certificate:
+        ret = do_early_reverify_server_certificate(hs);
+        break;
       case state_read_hello_verify_request:
         ret = do_read_hello_verify_request(hs);
         break;
@@ -1775,6 +1795,8 @@ const char *ssl_client_handshake_state(SSL_HANDSHAKE *hs) {
       return "TLS client start_connect";
     case state_enter_early_data:
       return "TLS client enter_early_data";
+    case state_early_reverify_server_certificate:
+      return "TLS client early_reverify_server_certificate";
     case state_read_hello_verify_request:
       return "TLS client read_hello_verify_request";
     case state_read_server_hello:

ssl/test/bssl_shim.cc

@@ -704,6 +704,7 @@ static bool DoConnection(bssl::UniquePtr<SSL_SESSION> *out_session,
 
     // Reset the connection and try again at 1-RTT.
     SSL_reset_early_data_reject(ssl.get());
+    GetTestState(ssl.get())->cert_verified = false;
 
     // After reseting, the socket should report it is no longer in an early data
     // state.

ssl/test/runner/fuzzer_mode.json

@@ -44,8 +44,7 @@
     "EarlyData-ALPNOmitted1-Server-*": "Trial decryption does not work with the NULL cipher.",
     "EarlyData-ALPNOmitted2-Server-*": "Trial decryption does not work with the NULL cipher.",
     "*-EarlyData-RejectUnfinishedWrite-Client-*": "Trial decryption does not work with the NULL cipher.",
-    "EarlyData-Reject-Client-*": "Trial decryption does not work with the NULL cipher.",
-    "EarlyData-RejectTicket-Client-*": "Trial decryption does not work with the NULL cipher.",
+    "EarlyData-Reject*-Client-*": "Trial decryption does not work with the NULL cipher.",
     "CustomExtensions-Server-EarlyDataOffered": "Trial decryption does not work with the NULL cipher.",
 
     "Renegotiate-Client-BadExt*": "Fuzzer mode does not check renegotiation_info.",

ssl/test/runner/runner.go

@@ -908,7 +908,7 @@ func doExchange(test *testCase, config *Config, conn net.Conn, isResume bool, tr
 
 		for i, v := range buf {
 			if v != testMessage[i]^0xff {
-				return fmt.Errorf("bad reply contents at byte %d", i)
+				return fmt.Errorf("bad reply contents at byte %d; got %q and wanted %q", i, buf, testMessage)
 			}
 		}
 	}
@@ -5016,6 +5016,165 @@ func addStateMachineCoverageTests(config stateMachineTestConfig) {
 						}, flags...),
 						resumeSession: true,
 					})
+					if vers.version >= VersionTLS13 {
+						tests = append(tests, testCase{
+							testType: testType,
+							name:     "EarlyData-RejectTicket-Client-Reverify" + suffix,
+							config: Config{
+								MaxVersion:       vers.version,
+								MaxEarlyDataSize: 16384,
+							},
+							resumeConfig: &Config{
+								MaxVersion:             vers.version,
+								MaxEarlyDataSize:       16384,
+								SessionTicketsDisabled: true,
+							},
+							tls13Variant:         vers.tls13Variant,
+							resumeSession:        true,
+							expectResumeRejected: true,
+							flags: append([]string{
+								"-enable-early-data",
+								"-expect-ticket-supports-early-data",
+								"-reverify-on-resume",
+								"-on-resume-shim-writes-first",
+								// Session tickets are disabled, so the runner will not send a ticket.
+								"-on-retry-expect-no-session",
+								"-expect-reject-early-data",
+							}, flags...),
+						})
+						tests = append(tests, testCase{
+							testType: testType,
+							name:     "EarlyData-Reject0RTT-Client-Reverify" + suffix,
+							config: Config{
+								MaxVersion:       vers.version,
+								MaxEarlyDataSize: 16384,
+							},
+							resumeConfig: &Config{
+								MaxVersion:             vers.version,
+								MaxEarlyDataSize:       16384,
+								Bugs: ProtocolBugs{
+									AlwaysRejectEarlyData: true,
+								},
+							},
+							tls13Variant:         vers.tls13Variant,
+							resumeSession:        true,
+							expectResumeRejected: false,
+							flags: append([]string{
+								"-enable-early-data",
+								"-expect-reject-early-data",
+								"-expect-ticket-supports-early-data",
+								"-reverify-on-resume",
+								"-on-resume-shim-writes-first",
+							}, flags...),
+						})
+						tests = append(tests, testCase{
+							testType: testType,
+							name:     "EarlyData-RejectTicket-Client-ReverifyFails" + suffix,
+							config: Config{
+								MaxVersion:       vers.version,
+								MaxEarlyDataSize: 16384,
+							},
+							resumeConfig: &Config{
+								MaxVersion:             vers.version,
+								MaxEarlyDataSize:       16384,
+								SessionTicketsDisabled: true,
+							},
+							tls13Variant:         vers.tls13Variant,
+							resumeSession:        true,
+							expectResumeRejected: true,
+							shouldFail: true,
+							expectedError: ":CERTIFICATE_VERIFY_FAILED:",
+							flags: append([]string{
+								"-enable-early-data",
+								"-expect-ticket-supports-early-data",
+								"-reverify-on-resume",
+								"-on-resume-shim-writes-first",
+								// Session tickets are disabled, so the runner will not send a ticket.
+								"-on-retry-expect-no-session",
+								"-on-retry-verify-fail",
+								"-expect-reject-early-data",
+							}, flags...),
+						})
+						tests = append(tests, testCase{
+							testType: testType,
+							name:     "EarlyData-Reject0RTT-Client-ReverifyFails" + suffix,
+							config: Config{
+								MaxVersion:       vers.version,
+								MaxEarlyDataSize: 16384,
+							},
+							resumeConfig: &Config{
+								MaxVersion:             vers.version,
+								MaxEarlyDataSize:       16384,
+								Bugs: ProtocolBugs{
+									AlwaysRejectEarlyData: true,
+								},
+							},
+							tls13Variant:         vers.tls13Variant,
+							resumeSession:        true,
+							expectResumeRejected: false,
+							shouldFail: true,
+							expectedError: ":CERTIFICATE_VERIFY_FAILED:",
+							flags: append([]string{
+								"-enable-early-data",
+								"-expect-reject-early-data",
+								"-expect-ticket-supports-early-data",
+								"-reverify-on-resume",
+								"-on-resume-shim-writes-first",
+								"-on-retry-verify-fail",
+							}, flags...),
+						})
+						// This tests that we only call the verify callback once.
+						tests = append(tests, testCase{
+							testType: testType,
+							name:     "EarlyData-Accept0RTT-Client-Reverify" + suffix,
+							config: Config{
+								MaxVersion:       vers.version,
+								MaxEarlyDataSize: 16384,
+							},
+							resumeConfig: &Config{
+								MaxVersion:             vers.version,
+								MaxEarlyDataSize:       16384,
+								Bugs: ProtocolBugs{
+									ExpectEarlyData: [][]byte{[]byte("hello")},
+								},
+							},
+							tls13Variant:         vers.tls13Variant,
+							resumeSession:        true,
+							expectResumeRejected: false,
+							flags: append([]string{
+								"-enable-early-data",
+								"-expect-ticket-supports-early-data",
+								"-reverify-on-resume",
+								"-on-resume-shim-writes-first",
+							}, flags...),
+						})
+						tests = append(tests, testCase{
+							testType: testType,
+							name:     "EarlyData-Accept0RTT-Client-ReverifyFails" + suffix,
+							config: Config{
+								MaxVersion:       vers.version,
+								MaxEarlyDataSize: 16384,
+							},
+							resumeConfig: &Config{
+								MaxVersion:             vers.version,
+								MaxEarlyDataSize:       16384,
+								Bugs: ProtocolBugs{
+									ExpectEarlyData: [][]byte{[]byte("hello")},
+								},
+							},
+							tls13Variant:         vers.tls13Variant,
+							resumeSession:        true,
+							shouldFail: true,
+							expectedError: ":CERTIFICATE_VERIFY_FAILED:",
+							flags: append([]string{
+								"-enable-early-data",
+								"-expect-ticket-supports-early-data",
+								"-reverify-on-resume",
+								"-on-resume-verify-fail",
+								"-on-resume-shim-writes-first",
+							}, flags...),
+						})
+					}
 				}
 			}
 		}

ssl/tls13_client.cc

@@ -465,7 +465,7 @@ static enum ssl_hs_wait_t do_read_certificate_request(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
   // CertificateRequest may only be sent in non-resumption handshakes.
   if (ssl->s3->session_reused) {
-    if (ssl->ctx->reverify_on_resume) {
+    if (ssl->ctx->reverify_on_resume && !ssl->s3->early_data_accepted) {
       hs->tls13_state = state_server_certificate_reverify;
       return ssl_hs_ok;
     }