Fix client 0-RTT handling with ALPS.

When offering 0-RTT, the client should check that all carried-over
values are consistent with its preferences. This ensures that parameter
negotiation happens independently of 0-RTT. The ALPS version of this
check was a tad too aggressive: a session without ALPS should be treated
as always compatible.

I'll follow this with a fix to the draft spec to clarify this.

Change-Id: Ia3c2a60449c555d1d91c4e528215f8e551a90a9f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45104
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>

ssl/t1_lib.cc

@@ -2180,14 +2180,17 @@ static bool ext_early_data_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
       return true;
     }
 
-    Span<const uint8_t> settings;
-    bool has_alps = ssl_get_local_application_settings(
-        hs, &settings, ssl->session->early_alpn);
-    if (has_alps != ssl->session->has_application_settings ||
-        settings != ssl->session->local_application_settings) {
-      // 0-RTT carries ALPS over, so we only offer it when the value matches.
-      ssl->s3->early_data_reason = ssl_early_data_alps_mismatch;
-      return true;
+    // If the previous connection negotiated ALPS, only offer 0-RTT when the
+    // local are settings are consistent with what we'd offer for this
+    // connection.
+    if (ssl->session->has_application_settings) {
+      Span<const uint8_t> settings;
+      if (!ssl_get_local_application_settings(hs, &settings,
+                                              ssl->session->early_alpn) ||
+          settings != ssl->session->local_application_settings) {
+        ssl->s3->early_data_reason = ssl_early_data_alps_mismatch;
+        return true;
+      }
     }
   }
 

ssl/test/runner/runner.go

@@ -7437,35 +7437,41 @@ func addExtensionTests() {
 						flags = append(flags, "-on-resume-expect-peer-application-settings", "runner")
 					}
 
-					// The client should not offer early data.
-					testCases = append(testCases, testCase{
-						protocol:           protocol,
-						testType:           clientTest,
-						name:               fmt.Sprintf("ALPS-EarlyData-Mismatch-%s-Client-%s", test.name, suffix),
-						skipQUICALPNConfig: true,
-						config: Config{
-							MaxVersion:          ver.version,
-							MaxEarlyDataSize:    16384,
-							NextProtos:          []string{"proto"},
-							ApplicationSettings: map[string][]byte{"proto": []byte("runner")},
-						},
-						resumeSession: true,
-						flags: append([]string{
-							"-enable-early-data",
-							"-expect-ticket-supports-early-data",
-							"-expect-no-offer-early-data",
-							"-advertise-alpn", "\x05proto",
-							"-expect-alpn", "proto",
-						}, flags...),
-						expectations: connectionExpectations{
-							peerApplicationSettings: test.initialSettings,
-						},
-						resumeExpectations: &connectionExpectations{
-							peerApplicationSettings: test.resumeSettings,
-						},
-					})
+					// The client should not offer early data if the session is
+					// inconsistent with the new configuration. Note that if
+					// the session did not negotiate ALPS (test.initialSettings
+					// is nil), the client always offers early data.
+					if test.initialSettings != nil {
+						testCases = append(testCases, testCase{
+							protocol:           protocol,
+							testType:           clientTest,
+							name:               fmt.Sprintf("ALPS-EarlyData-Mismatch-%s-Client-%s", test.name, suffix),
+							skipQUICALPNConfig: true,
+							config: Config{
+								MaxVersion:          ver.version,
+								MaxEarlyDataSize:    16384,
+								NextProtos:          []string{"proto"},
+								ApplicationSettings: map[string][]byte{"proto": []byte("runner")},
+							},
+							resumeSession: true,
+							flags: append([]string{
+								"-enable-early-data",
+								"-expect-ticket-supports-early-data",
+								"-expect-no-offer-early-data",
+								"-advertise-alpn", "\x05proto",
+								"-expect-alpn", "proto",
+							}, flags...),
+							expectations: connectionExpectations{
+								peerApplicationSettings: test.initialSettings,
+							},
+							resumeExpectations: &connectionExpectations{
+								peerApplicationSettings: test.resumeSettings,
+							},
+						})
+					}
 
-					// The server should reject early data.
+					// The server should reject early data if the session is
+					// inconsistent with the new selection.
 					testCases = append(testCases, testCase{
 						protocol:           protocol,
 						testType:           serverTest,
@@ -7490,6 +7496,42 @@ func addExtensionTests() {
 						},
 					})
 				}
+
+				// Test that 0-RTT continues working when the shim configures
+				// ALPS but the peer does not.
+				testCases = append(testCases, testCase{
+					protocol:           protocol,
+					testType:           clientTest,
+					name:               "ALPS-EarlyData-Client-ServerDecline-" + suffix,
+					skipQUICALPNConfig: true,
+					config: Config{
+						MaxVersion: ver.version,
+						NextProtos: []string{"proto"},
+					},
+					resumeSession: true,
+					earlyData:     true,
+					flags: []string{
+						"-advertise-alpn", "\x05proto",
+						"-expect-alpn", "proto",
+						"-application-settings", "proto,shim",
+					},
+				})
+				testCases = append(testCases, testCase{
+					protocol:           protocol,
+					testType:           serverTest,
+					name:               "ALPS-EarlyData-Server-ClientNoOffer-" + suffix,
+					skipQUICALPNConfig: true,
+					config: Config{
+						MaxVersion: ver.version,
+						NextProtos: []string{"proto"},
+					},
+					resumeSession: true,
+					earlyData:     true,
+					flags: []string{
+						"-select-alpn", "proto",
+						"-application-settings", "proto,shim",
+					},
+				})
 			} else {
 				// Test the client rejects the ALPS extension if the server
 				// negotiated TLS 1.2 or below.