Shift some complexity out of ssl_add_clienthello_tlsext.

ssl_add_clienthello_tlsext is about to get kinda messy with ECH. Move
the padding and GREASE extensions into a few helpers.

Bug: 275
Change-Id: I3bb702fb79dce4be68490c4a8fd889121ecdae58
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47995
Reviewed-by: Adam Langley <agl@google.com>

ssl/handshake.cc

@@ -439,7 +439,7 @@ enum ssl_verify_result_t ssl_reverify_peer_cert(SSL_HANDSHAKE *hs,
   return ret;
 }
 
-uint16_t ssl_get_grease_value(const SSL_HANDSHAKE *hs,
+static uint16_t grease_index_to_value(const SSL_HANDSHAKE *hs,
                                       enum ssl_grease_index_t index) {
   // This generates a random value of the form 0xωaωa, for all 0 ≤ ω < 16.
   uint16_t ret = hs->grease_seed[index];
@@ -448,6 +448,19 @@ uint16_t ssl_get_grease_value(const SSL_HANDSHAKE *hs,
   return ret;
 }
 
+uint16_t ssl_get_grease_value(const SSL_HANDSHAKE *hs,
+                              enum ssl_grease_index_t index) {
+  uint16_t ret = grease_index_to_value(hs, index);
+  if (index == ssl_grease_extension2 &&
+      ret == grease_index_to_value(hs, ssl_grease_extension1)) {
+    // The two fake extensions must not have the same value. GREASE values are
+    // of the form 0x1a1a, 0x2a2a, 0x3a3a, etc., so XOR to generate a different
+    // one.
+    ret ^= 0x1010;
+  }
+  return ret;
+}
+
 enum ssl_hs_wait_t ssl_get_finished(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
   SSLMessage msg;

ssl/t1_lib.cc

@@ -3247,6 +3247,19 @@ static const struct tls_extension *tls_extension_find(uint32_t *out_index,
   return NULL;
 }
 
+static bool add_padding_extension(CBB *cbb, uint16_t ext, size_t len) {
+  CBB child;
+  uint8_t *ptr;
+  if (!CBB_add_u16(cbb, ext) ||  //
+      !CBB_add_u16_length_prefixed(cbb, &child) ||
+      !CBB_add_space(&child, &ptr, len)) {
+    OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+    return false;
+  }
+  OPENSSL_memset(ptr, 0, len);
+  return CBB_flush(cbb);
+}
+
 bool ssl_add_clienthello_tlsext(SSL_HANDSHAKE *hs, CBB *out,
                                 bool *out_needs_psk_binder, size_t header_len) {
   SSL *const ssl = hs->ssl;
@@ -3262,16 +3275,12 @@ bool ssl_add_clienthello_tlsext(SSL_HANDSHAKE *hs, CBB *out,
   // important to reset this value.
   hs->extensions.sent = 0;
 
-  uint16_t grease_ext1 = 0;
-  if (ssl->ctx->grease_enabled) {
   // Add a fake empty extension. See RFC 8701.
-    grease_ext1 = ssl_get_grease_value(hs, ssl_grease_extension1);
+  if (ssl->ctx->grease_enabled &&
-    if (!CBB_add_u16(&extensions, grease_ext1) ||
+      !add_padding_extension(
-        !CBB_add_u16(&extensions, 0 /* zero length */)) {
+          &extensions, ssl_get_grease_value(hs, ssl_grease_extension1), 0)) {
-      OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
     return false;
   }
-  }
 
   bool last_was_empty = false;
   for (size_t i = 0; i < kNumExtensions; i++) {
@@ -3293,22 +3302,10 @@ bool ssl_add_clienthello_tlsext(SSL_HANDSHAKE *hs, CBB *out,
 
   if (ssl->ctx->grease_enabled) {
     // Add a fake non-empty extension. See RFC 8701.
-    uint16_t grease_ext2 = ssl_get_grease_value(hs, ssl_grease_extension2);
+    if (!add_padding_extension(
-
+            &extensions, ssl_get_grease_value(hs, ssl_grease_extension2), 1)) {
-    // The two fake extensions must not have the same value. GREASE values are
-    // of the form 0x1a1a, 0x2a2a, 0x3a3a, etc., so XOR to generate a different
-    // one.
-    if (grease_ext1 == grease_ext2) {
-      grease_ext2 ^= 0x1010;
-    }
-
-    if (!CBB_add_u16(&extensions, grease_ext2) ||
-        !CBB_add_u16(&extensions, 1 /* one byte length */) ||
-        !CBB_add_u8(&extensions, 0 /* single zero byte as contents */)) {
-      OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
       return false;
     }
-
     last_was_empty = false;
   }
 
@@ -3348,17 +3345,10 @@ bool ssl_add_clienthello_tlsext(SSL_HANDSHAKE *hs, CBB *out,
       }
     }
 
-    if (padding_len != 0) {
+    if (padding_len != 0 &&
-      uint8_t *padding_bytes;
+        !add_padding_extension(&extensions, TLSEXT_TYPE_padding, padding_len)) {
-      if (!CBB_add_u16(&extensions, TLSEXT_TYPE_padding) ||
-          !CBB_add_u16(&extensions, padding_len) ||
-          !CBB_add_space(&extensions, &padding_bytes, padding_len)) {
-        OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
       return false;
     }
-
-      OPENSSL_memset(padding_bytes, 0, padding_len);
-    }
   }
 
   // The PSK extension must be last, including after the padding.