Record whether dummy PQ padding was used.

On reflection, I think we'll need to note whether dummy PQ padding was
echoed on a given connection. Otherwise measurements in Chrome will be
mixed with cases where people have MITM proxies that ignored the
extension, or possibly Google frontends that haven't been updated.

Therefore this change will be used to filter latency measurements in
Chrome to only include those where the extension was echoed and we'll
measure at levels of 1 byte (for control), 400 bytes, and 1100 bytes.

This also makes it an error if the server didn't echo an extension of
the same length as was sent.

Change-Id: Ib2a0b29cfb8719a75a28f3cf96710c57d88eaa68
Reviewed-on: https://boringssl-review.googlesource.com/26284
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>

include/openssl/ssl.h

@@ -2952,6 +2952,11 @@ OPENSSL_EXPORT const char *SSL_get_psk_identity(const SSL *ssl);
 // phase of the experiment. It returns one for success and zero otherwise.
 OPENSSL_EXPORT int SSL_set_dummy_pq_padding_size(SSL *ssl, size_t num_bytes);
 
+// SSL_dummy_pq_padding_used returns one if the server echoed a dummy PQ padding
+// extension and zero otherwise. It may only be called on a client connection
+// once the handshake has completed, otherwise it'll return zero.
+OPENSSL_EXPORT int SSL_dummy_pq_padding_used(SSL *ssl);
+
 
 // QUIC Transport Parameters.
 //

ssl/internal.h

@@ -1515,8 +1515,9 @@ struct SSL_HANDSHAKE {
   // |grease_seeded| is true.
   uint8_t grease_seed[ssl_grease_last_index + 1] = {0};
 
-  // dummy_pq_padding_len is the length of the extension that should be echoed
-  // in a ServerHello, or zero if no extension should be echoed.
+  // dummy_pq_padding_len, in a server, is the length of the extension that
+  // should be echoed in a ServerHello, or zero if no extension should be
+  // echoed.
   uint16_t dummy_pq_padding_len = 0;
 };
 
@@ -2674,6 +2675,11 @@ struct SSLConnection {
   // returns |SSL_HANDOFF|. This is copied in |SSL_new| from the |SSL_CTX|
   // element of the same name and may be cleared if the handoff is declined.
   bool handoff:1;
+
+  // did_dummy_pq_padding is only valid for a client. In that context, it is
+  // true iff the client observed the server echoing a dummy PQ padding
+  // extension.
+  bool did_dummy_pq_padding:1;
 };
 
 // From draft-ietf-tls-tls13-18, used in determining PSK modes.

ssl/ssl_lib.cc

@@ -2440,6 +2440,14 @@ int SSL_set_dummy_pq_padding_size(SSL *ssl, size_t num_bytes) {
   return 1;
 }
 
+int SSL_dummy_pq_padding_used(SSL *ssl) {
+  if (ssl->server || !ssl->s3->initial_handshake_complete) {
+    return 0;
+  }
+
+  return ssl->did_dummy_pq_padding;
+}
+
 void SSL_CTX_set_msg_callback(SSL_CTX *ctx,
                               void (*cb)(int write_p, int version,
                                          int content_type, const void *buf,

ssl/t1_lib.cc

@@ -557,11 +557,6 @@ static bool ignore_parse_clienthello(SSL_HANDSHAKE *hs, uint8_t *out_alert,
   return true;
 }
 
-static bool ignore_parse_serverhello(SSL_HANDSHAKE *hs, uint8_t *out_alert,
-                                     CBS *contents) {
-  return true;
-}
-
 static bool dont_add_serverhello(SSL_HANDSHAKE *hs, CBB *out) {
   return true;
 }
@@ -2355,6 +2350,21 @@ static bool ext_dummy_pq_padding_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
   return ext_dummy_pq_padding_add(out, len);
 }
 
+static bool ext_dummy_pq_padding_parse_serverhello(SSL_HANDSHAKE *hs,
+                                                   uint8_t *out_alert,
+                                                   CBS *contents) {
+  if (contents == nullptr) {
+    return true;
+  }
+
+  if (CBS_len(contents) != hs->ssl->dummy_pq_padding_len) {
+    return false;
+  }
+
+  hs->ssl->did_dummy_pq_padding = true;
+  return true;
+}
+
 static bool ext_dummy_pq_padding_parse_clienthello(SSL_HANDSHAKE *hs,
                                                    uint8_t *out_alert,
                                                    CBS *contents) {
@@ -2816,7 +2826,7 @@ static const struct tls_extension kExtensions[] = {
     TLSEXT_TYPE_dummy_pq_padding,
     NULL,
     ext_dummy_pq_padding_add_clienthello,
-    ignore_parse_serverhello,
+    ext_dummy_pq_padding_parse_serverhello,
     ext_dummy_pq_padding_parse_clienthello,
     ext_dummy_pq_padding_add_serverhello,
   },

ssl/test/bssl_shim.cc

@@ -1862,6 +1862,15 @@ static bool CheckHandshakeProperties(SSL *ssl, bool is_resume,
   if (config->expect_draft_downgrade != !!SSL_is_draft_downgrade(ssl)) {
     fprintf(stderr, "Got %sdraft downgrade signal, but wanted the opposite.\n",
             SSL_is_draft_downgrade(ssl) ? "" : "no ");
+    return false;
+  }
+
+  const bool did_dummy_pq_padding = !!SSL_dummy_pq_padding_used(ssl);
+  if (config->expect_dummy_pq_padding != did_dummy_pq_padding) {
+    fprintf(stderr,
+            "Dummy PQ padding %s observed, but expected the opposite.\n",
+            did_dummy_pq_padding ? "was" : "was not");
+    return false;
   }
 
   return true;

ssl/test/runner/handshake_server.go

@@ -1386,6 +1386,10 @@ func (hs *serverHandshakeState) processClientExtensions(serverExtensions *server
 		return errors.New("tls: no GREASE extension found")
 	}
 
+	if l := hs.clientHello.dummyPQPaddingLen; l != 0 {
+		serverExtensions.dummyPQPaddingLen = l
+	}
+
 	serverExtensions.serverNameAck = c.config.Bugs.SendServerNameAck
 
 	return nil

ssl/test/runner/runner.go

@@ -7358,6 +7358,7 @@ func addExtensionTests() {
 				flags: []string{
 					"-max-version", version.shimFlag(tls),
 					"-dummy-pq-padding-len", strconv.Itoa(paddingLen),
+					"-expect-dummy-pq-padding",
 				},
 			})
 

ssl/test/test_config.cc

@@ -132,6 +132,7 @@ const Flag<bool> kBoolFlags[] = {
     &TestConfig::allow_false_start_without_alpn },
   { "-expect-draft-downgrade", &TestConfig::expect_draft_downgrade },
   { "-handoff", &TestConfig::handoff },
+  { "-expect-dummy-pq-padding", &TestConfig::expect_dummy_pq_padding },
 };
 
 const Flag<std::string> kStringFlags[] = {

ssl/test/test_config.h

@@ -153,6 +153,7 @@ struct TestConfig {
   bool expect_draft_downgrade = false;
   int dummy_pq_padding_len = 0;
   bool handoff = false;
+  bool expect_dummy_pq_padding = false;
 };
 
 bool ParseConfig(int argc, char **argv, TestConfig *out_initial,