Add CBB_zero to set a CBB to the zero state.

One tedious thing about using CBB is that you can't safely CBB_cleanup
until CBB_init is successful, which breaks the general 'goto err' style
of cleanup. This makes it possible:

  CBB_zero ~ EVP_MD_CTX_init
  CBB_init ~ EVP_DigestInit
  CBB_cleanup ~ EVP_MD_CTX_cleanup

Change-Id: I085ecc4405715368886dc4de02285a47e7fc4c52
Reviewed-on: https://boringssl-review.googlesource.com/5267
Reviewed-by: Adam Langley <agl@google.com>

crypto/bytestring/bytestring_test.cc

@@ -649,6 +649,14 @@ static bool TestASN1Uint64() {
   return true;
 }
 
+static int TestZero() {
+  CBB cbb;
+  CBB_zero(&cbb);
+  // Calling |CBB_cleanup| on a zero-state |CBB| must not crash.
+  CBB_cleanup(&cbb);
+  return 1;
+}
+
 int main(void) {
   CRYPTO_library_init();
 
@@ -665,7 +673,8 @@ int main(void) {
       !TestCBBASN1() ||
       !TestBerConvert() ||
       !TestASN1Uint64() ||
-      !TestGetOptionalASN1Bool()) {
+      !TestGetOptionalASN1Bool() ||
+      !TestZero()) {
     return 1;
   }
 

crypto/bytestring/cbb.c

@@ -20,6 +20,10 @@
 #include <openssl/mem.h>
 
 
+void CBB_zero(CBB *cbb) {
+  memset(cbb, 0, sizeof(CBB));
+}
+
 static int cbb_init(CBB *cbb, uint8_t *buf, size_t cap) {
   struct cbb_buffer_st *base;
 

include/openssl/bytestring.h

@@ -248,6 +248,12 @@ struct cbb_st {
   char is_top_level;
 };
 
+/* CBB_zero sets an uninitialised |cbb| to the zero state. It must be
+ * initialised with |CBB_init| or |CBB_init_fixed| before use, but it is safe to
+ * call |CBB_cleanup| without a successful |CBB_init|. This may be used for more
+ * uniform cleanup of a |CBB|. */
+OPENSSL_EXPORT void CBB_zero(CBB *cbb);
+
 /* CBB_init initialises |cbb| with |initial_capacity|. Since a |CBB| grows as
  * needed, the |initial_capacity| is just a hint. It returns one on success or
  * zero on error. */

ssl/d1_both.c

@@ -592,18 +592,15 @@ long dtls1_get_message(SSL *s, int st1, int stn, int msg_type, long max,
     goto err;
   }
 
+  /* Reconstruct the assembled message. */
+  size_t len;
   CBB cbb;
+  CBB_zero(&cbb);
   if (!BUF_MEM_grow(s->init_buf,
                     (size_t)frag->msg_header.msg_len +
                     DTLS1_HM_HEADER_LENGTH) ||
-      !CBB_init_fixed(&cbb, (uint8_t *)s->init_buf->data, s->init_buf->max)) {
-    OPENSSL_PUT_ERROR(SSL, dtls1_get_message, ERR_R_MALLOC_FAILURE);
-    goto err;
-  }
-
-  /* Reconstruct the assembled message. */
-  size_t len;
-  if (!CBB_add_u8(&cbb, frag->msg_header.type) ||
+      !CBB_init_fixed(&cbb, (uint8_t *)s->init_buf->data, s->init_buf->max) ||
+      !CBB_add_u8(&cbb, frag->msg_header.type) ||
       !CBB_add_u24(&cbb, frag->msg_header.msg_len) ||
       !CBB_add_u16(&cbb, frag->msg_header.seq) ||
       !CBB_add_u24(&cbb, 0 /* frag_off */) ||
@@ -611,7 +608,7 @@ long dtls1_get_message(SSL *s, int st1, int stn, int msg_type, long max,
       !CBB_add_bytes(&cbb, frag->fragment, frag->msg_header.msg_len) ||
       !CBB_finish(&cbb, NULL, &len)) {
     CBB_cleanup(&cbb);
-    OPENSSL_PUT_ERROR(SSL, dtls1_get_message, ERR_R_INTERNAL_ERROR);
+    OPENSSL_PUT_ERROR(SSL, dtls1_get_message, ERR_R_MALLOC_FAILURE);
     goto err;
   }
   assert(len == (size_t)frag->msg_header.msg_len + DTLS1_HM_HEADER_LENGTH);

ssl/s3_clnt.c

@@ -1963,19 +1963,16 @@ int ssl3_send_client_key_exchange(SSL *s) {
       uint8_t *new_pms;
       size_t new_pms_len;
 
-      if (!CBB_init(&cbb, 2 + psk_len + 2 + pms_len)) {
-        OPENSSL_PUT_ERROR(SSL, ssl3_send_client_key_exchange,
-                          ERR_R_MALLOC_FAILURE);
-        goto err;
-      }
-      if (!CBB_add_u16_length_prefixed(&cbb, &child) ||
+      CBB_zero(&cbb);
+      if (!CBB_init(&cbb, 2 + psk_len + 2 + pms_len) ||
+          !CBB_add_u16_length_prefixed(&cbb, &child) ||
           !CBB_add_bytes(&child, pms, pms_len) ||
           !CBB_add_u16_length_prefixed(&cbb, &child) ||
           !CBB_add_bytes(&child, psk, psk_len) ||
           !CBB_finish(&cbb, &new_pms, &new_pms_len)) {
         CBB_cleanup(&cbb);
         OPENSSL_PUT_ERROR(SSL, ssl3_send_client_key_exchange,
-                          ERR_R_INTERNAL_ERROR);
+                          ERR_R_MALLOC_FAILURE);
         goto err;
       }
       OPENSSL_cleanse(pms, pms_len);

ssl/s3_srvr.c

@@ -747,12 +747,10 @@ int ssl3_get_v2_client_hello(SSL *s) {
          rand_len);
 
   /* Write out an equivalent SSLv3 ClientHello. */
+  CBB_zero(&client_hello);
   if (!CBB_init_fixed(&client_hello, (uint8_t *)s->init_buf->data,
-                      s->init_buf->max)) {
-    OPENSSL_PUT_ERROR(SSL, ssl3_get_v2_client_hello, ERR_R_MALLOC_FAILURE);
-    return -1;
-  }
-  if (!CBB_add_u8(&client_hello, SSL3_MT_CLIENT_HELLO) ||
+                      s->init_buf->max) ||
+      !CBB_add_u8(&client_hello, SSL3_MT_CLIENT_HELLO) ||
       !CBB_add_u24_length_prefixed(&client_hello, &hello_body) ||
       !CBB_add_u16(&hello_body, version) ||
       !CBB_add_bytes(&hello_body, random, SSL3_RANDOM_SIZE) ||
@@ -760,7 +758,7 @@ int ssl3_get_v2_client_hello(SSL *s) {
       !CBB_add_u8(&hello_body, 0) ||
       !CBB_add_u16_length_prefixed(&hello_body, &cipher_suites)) {
     CBB_cleanup(&client_hello);
-    OPENSSL_PUT_ERROR(SSL, ssl3_get_v2_client_hello, ERR_R_INTERNAL_ERROR);
+    OPENSSL_PUT_ERROR(SSL, ssl3_get_v2_client_hello, ERR_R_MALLOC_FAILURE);
     return -1;
   }
 
@@ -1977,18 +1975,15 @@ int ssl3_get_client_key_exchange(SSL *s) {
     uint8_t *new_data;
     size_t new_len;
 
-    if (!CBB_init(&new_premaster, 2 + psk_len + 2 + premaster_secret_len)) {
-      OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange,
-                        ERR_R_MALLOC_FAILURE);
-      goto err;
-    }
-    if (!CBB_add_u16_length_prefixed(&new_premaster, &child) ||
+    CBB_zero(&new_premaster);
+    if (!CBB_init(&new_premaster, 2 + psk_len + 2 + premaster_secret_len) ||
+        !CBB_add_u16_length_prefixed(&new_premaster, &child) ||
         !CBB_add_bytes(&child, premaster_secret, premaster_secret_len) ||
         !CBB_add_u16_length_prefixed(&new_premaster, &child) ||
         !CBB_add_bytes(&child, psk, psk_len) ||
         !CBB_finish(&new_premaster, &new_data, &new_len)) {
       OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange,
-                        ERR_R_INTERNAL_ERROR);
+                        ERR_R_MALLOC_FAILURE);
       CBB_cleanup(&new_premaster);
       goto err;
     }

ssl/ssl_asn1.c

@@ -163,11 +163,9 @@ static int SSL_SESSION_to_bytes_full(SSL_SESSION *in, uint8_t **out_data,
     return 0;
   }
 
-  if (!CBB_init(&cbb, 0)) {
-    return 0;
-  }
-
-  if (!CBB_add_asn1(&cbb, &session, CBS_ASN1_SEQUENCE) ||
+  CBB_zero(&cbb);
+  if (!CBB_init(&cbb, 0) ||
+      !CBB_add_asn1(&cbb, &session, CBS_ASN1_SEQUENCE) ||
       !CBB_add_asn1_uint64(&session, SSL_SESSION_ASN1_VERSION) ||
       !CBB_add_asn1_uint64(&session, in->ssl_version) ||
       !CBB_add_asn1(&session, &child, CBS_ASN1_OCTETSTRING) ||

ssl/ssl_lib.c

@@ -2582,11 +2582,9 @@ int ssl_ctx_log_rsa_client_key_exchange(SSL_CTX *ctx,
     return 0;
   }
 
-  if (!CBB_init(&cbb, 4 + 16 + 1 + premaster_len * 2 + 1)) {
-    return 0;
-  }
-
-  if (!CBB_add_bytes(&cbb, (const uint8_t *)"RSA ", 4) ||
+  CBB_zero(&cbb);
+  if (!CBB_init(&cbb, 4 + 16 + 1 + premaster_len * 2 + 1) ||
+      !CBB_add_bytes(&cbb, (const uint8_t *)"RSA ", 4) ||
       /* Only the first 8 bytes of the encrypted premaster secret are
        * logged. */
       !cbb_add_hex(&cbb, encrypted_premaster, 8) ||
@@ -2624,11 +2622,9 @@ int ssl_ctx_log_master_secret(SSL_CTX *ctx, const uint8_t *client_random,
     return 0;
   }
 
-  if (!CBB_init(&cbb, 14 + 64 + 1 + master_len * 2 + 1)) {
-    return 0;
-  }
-
-  if (!CBB_add_bytes(&cbb, (const uint8_t *)"CLIENT_RANDOM ", 14) ||
+  CBB_zero(&cbb);
+  if (!CBB_init(&cbb, 14 + 64 + 1 + master_len * 2 + 1) ||
+      !CBB_add_bytes(&cbb, (const uint8_t *)"CLIENT_RANDOM ", 14) ||
       !cbb_add_hex(&cbb, client_random, 32) ||
       !CBB_add_bytes(&cbb, (const uint8_t *)" ", 1) ||
       !cbb_add_hex(&cbb, master, master_len) ||