Add a CBB_add_zeros helper.

We fill in placeholder values of all zeros fairly often in TLS now,
as workarounds for messages being constructed in the wrong order.
draft-12 of ECH adds even more of these. Add a helper so we don't need
to interrupt an || chain with a memset.

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

crypto/bytestring/bytestring_test.cc

@@ -322,11 +322,11 @@ TEST(CBBTest, InitUninitialized) {
 }
 
 TEST(CBBTest, Basic) {
-  static const uint8_t kExpected[] = {1,   2,    3,    4,    5,    6,   7,
-                                      8,   9,    0xa,  0xb,  0xc,  0xd, 0xe,
-                                      0xf, 0x10, 0x11, 0x12, 0x13, 0x14, 3, 2,
-                                      10,  9,    8,    7,    0x12, 0x11, 0x10,
-                                      0xf, 0xe,  0xd,  0xc,  0xb};
+  static const uint8_t kExpected[] = {
+      0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a,
+      0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14,
+      0x03, 0x02, 0x0a, 0x09, 0x08, 0x07, 0x12, 0x11, 0x10, 0x0f,
+      0x0e, 0x0d, 0x0c, 0x0b, 0x00, 0x00, 0x00, 0x00};
   uint8_t *buf;
   size_t buf_len;
 
@@ -335,6 +335,7 @@ TEST(CBBTest, Basic) {
   cbb.Reset();
 
   ASSERT_TRUE(CBB_init(cbb.get(), 0));
+  ASSERT_TRUE(CBB_add_zeros(cbb.get(), 0));
   ASSERT_TRUE(CBB_add_u8(cbb.get(), 1));
   ASSERT_TRUE(CBB_add_u16(cbb.get(), 0x203));
   ASSERT_TRUE(CBB_add_u24(cbb.get(), 0x40506));
@@ -344,6 +345,7 @@ TEST(CBBTest, Basic) {
   ASSERT_TRUE(CBB_add_u16le(cbb.get(), 0x203));
   ASSERT_TRUE(CBB_add_u32le(cbb.get(), 0x708090a));
   ASSERT_TRUE(CBB_add_u64le(cbb.get(), 0xb0c0d0e0f101112));
+  ASSERT_TRUE(CBB_add_zeros(cbb.get(), 4));
   ASSERT_TRUE(CBB_finish(cbb.get(), &buf, &buf_len));
 
   bssl::UniquePtr<uint8_t> scoper(buf);

crypto/bytestring/cbb.c

@@ -404,6 +404,15 @@ int CBB_add_bytes(CBB *cbb, const uint8_t *data, size_t len) {
   return 1;
 }
 
+int CBB_add_zeros(CBB *cbb, size_t len) {
+  uint8_t *out;
+  if (!CBB_add_space(cbb, &out, len)) {
+    return 0;
+  }
+  OPENSSL_memset(out, 0, len);
+  return 1;
+}
+
 int CBB_add_space(CBB *cbb, uint8_t **out_data, size_t len) {
   if (!CBB_flush(cbb) ||
       !cbb_buffer_add(cbb->base, out_data, len)) {

include/openssl/bytestring.h

@@ -463,6 +463,10 @@ OPENSSL_EXPORT int CBB_add_asn1(CBB *cbb, CBB *out_contents, unsigned tag);
 // success and zero otherwise.
 OPENSSL_EXPORT int CBB_add_bytes(CBB *cbb, const uint8_t *data, size_t len);
 
+// CBB_add_zeros append |len| bytes with value zero to |cbb|. It returns one on
+// success and zero otherwise.
+OPENSSL_EXPORT int CBB_add_zeros(CBB *cbb, size_t len);
+
 // CBB_add_space appends |len| bytes to |cbb| and sets |*out_data| to point to
 // the beginning of that space. The caller must then write |len| bytes of
 // actual contents to |*out_data|. It returns one on success and zero

ssl/extensions.cc

@@ -1991,7 +1991,6 @@ static bool ext_pre_shared_key_add_clienthello(const SSL_HANDSHAKE *hs,
 
   // Fill in a placeholder zero binder of the appropriate length. It will be
   // computed and filled in later after length prefixes are computed.
-  uint8_t zero_binder[EVP_MAX_MD_SIZE] = {0};
   size_t binder_len = EVP_MD_size(ssl_session_get_digest(ssl->session.get()));
 
   CBB contents, identity, ticket, binders, binder;
@@ -2004,7 +2003,7 @@ static bool ext_pre_shared_key_add_clienthello(const SSL_HANDSHAKE *hs,
       !CBB_add_u32(&identity, obfuscated_ticket_age) ||
       !CBB_add_u16_length_prefixed(&contents, &binders) ||
       !CBB_add_u8_length_prefixed(&binders, &binder) ||
-      !CBB_add_bytes(&binder, zero_binder, binder_len)) {
+      !CBB_add_zeros(&binder, binder_len)) {
     return false;
   }
 
@@ -3324,14 +3323,12 @@ static const struct tls_extension *tls_extension_find(uint32_t *out_index,
 
 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)) {
+      !CBB_add_zeros(&child, len)) {
     OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
     return false;
   }
-  OPENSSL_memset(ptr, 0, len);
   return CBB_flush(cbb);
 }