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>
diff --git a/crypto/bytestring/bytestring_test.cc b/crypto/bytestring/bytestring_test.cc
index eafb0de..484cd34 100644
--- a/crypto/bytestring/bytestring_test.cc
+++ b/crypto/bytestring/bytestring_test.cc
@@ -322,11 +322,11 @@
}
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 @@
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 @@
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);
diff --git a/crypto/bytestring/cbb.c b/crypto/bytestring/cbb.c
index efb89c7..12587cd 100644
--- a/crypto/bytestring/cbb.c
+++ b/crypto/bytestring/cbb.c
@@ -404,6 +404,15 @@
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)) {
diff --git a/include/openssl/bytestring.h b/include/openssl/bytestring.h
index 39b7fb2..180326d 100644
--- a/include/openssl/bytestring.h
+++ b/include/openssl/bytestring.h
@@ -463,6 +463,10 @@
// 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
diff --git a/ssl/extensions.cc b/ssl/extensions.cc
index 48fc5c5..9af928c 100644
--- a/ssl/extensions.cc
+++ b/ssl/extensions.cc
@@ -1991,7 +1991,6 @@
// 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 @@
!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 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);
}
