commit 72cff81939dca644188558e08852bf04e77c727e
author Nick Harper <nharper@chromium.org> Thu Mar 26 18:06:16 2020 -0700
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Fri Apr 03 21:52:34 2020 +0000
tree 43b6b04af0a3593a1f269632b88de68cfce05f7c
parent ee2660203b6a95d866b1ab3935e154cf8a88dec8

Require QUIC method with Transport Parameters and vice versa

Bug: 296, 322

Change-Id: I297f53674ee7177f61d75695f47b5caeed78bd17
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/40384
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>

ssl/t1_lib.cc

diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index d04c874..dad0fcf 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -2547,10 +2547,17 @@
 
 static bool ext_quic_transport_params_add_clienthello(SSL_HANDSHAKE *hs,
                                                       CBB *out) {
-  if (hs->config->quic_transport_params.empty() ||
-      hs->max_version <= TLS1_2_VERSION) {
+  if (hs->config->quic_transport_params.empty() && !hs->ssl->quic_method) {
     return true;
   }
+  if (hs->config->quic_transport_params.empty() || !hs->ssl->quic_method) {
+    // QUIC Transport Parameters must be sent over QUIC, and they must not be
+    // sent over non-QUIC transports. If transport params are set, then
+    // SSL(_CTX)_set_quic_method must also be called.
+    OPENSSL_PUT_ERROR(SSL, SSL_R_QUIC_TRANSPORT_PARAMETERS_MISCONFIGURED);
+    return false;
+  }
+  assert(hs->min_version > TLS1_2_VERSION);
 
   CBB contents;
   if (!CBB_add_u16(out, TLSEXT_TYPE_quic_transport_parameters) ||
@@ -2568,13 +2575,19 @@
                                                         CBS *contents) {
   SSL *const ssl = hs->ssl;
   if (contents == nullptr) {
-    return true;
+    if (!ssl->quic_method) {
+      return true;
+    }
+    assert(ssl->quic_method);
+    *out_alert = SSL_AD_MISSING_EXTENSION;
+    return false;
   }
-  // QUIC requires TLS 1.3.
-  if (ssl_protocol_version(ssl) < TLS1_3_VERSION) {
+  if (!ssl->quic_method) {
     *out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
     return false;
   }
+  // QUIC requires TLS 1.3.
+  assert(ssl_protocol_version(ssl) == TLS1_3_VERSION);
 
   return ssl->s3->peer_quic_transport_params.CopyFrom(*contents);
 }
@@ -2583,8 +2596,22 @@
                                                         uint8_t *out_alert,
                                                         CBS *contents) {
   SSL *const ssl = hs->ssl;
-  if (!contents || !ssl->quic_method) {
-    return true;
+  if (!contents) {
+    if (!ssl->quic_method) {
+      if (hs->config->quic_transport_params.empty()) {
+        return true;
+      }
+      // QUIC transport parameters must not be set if |ssl| is not configured
+      // for QUIC.
+      OPENSSL_PUT_ERROR(SSL, SSL_R_QUIC_TRANSPORT_PARAMETERS_MISCONFIGURED);
+      *out_alert = SSL_AD_INTERNAL_ERROR;
+    }
+    *out_alert = SSL_AD_MISSING_EXTENSION;
+    return false;
+  }
+  if (!ssl->quic_method) {
+    *out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
+    return false;
   }
   assert(ssl_protocol_version(ssl) == TLS1_3_VERSION);
   return ssl->s3->peer_quic_transport_params.CopyFrom(*contents);
@@ -2592,8 +2619,11 @@
 
 static bool ext_quic_transport_params_add_serverhello(SSL_HANDSHAKE *hs,
                                                       CBB *out) {
+  assert(hs->ssl->quic_method != nullptr);
   if (hs->config->quic_transport_params.empty()) {
-    return true;
+    // Transport parameters must be set when using QUIC.
+    OPENSSL_PUT_ERROR(SSL, SSL_R_QUIC_TRANSPORT_PARAMETERS_MISCONFIGURED);
+    return false;
   }
 
   CBB contents;