Convert more things to Array.
This adds a CopyFrom companion to Init as a replacement for CBS_stow.
Bug: 132
Change-Id: I4d77291b07552bd2286a09f8ba33655d6d97c853
Reviewed-on: https://boringssl-review.googlesource.com/20670
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/handshake.cc b/ssl/handshake.cc
index dece144..d95677f 100644
--- a/ssl/handshake.cc
+++ b/ssl/handshake.cc
@@ -144,13 +144,10 @@
}
SSL_HANDSHAKE::~SSL_HANDSHAKE() {
- OPENSSL_free(cookie);
- OPENSSL_free(key_share_bytes);
OPENSSL_free(ecdh_public_key);
OPENSSL_free(peer_sigalgs);
OPENSSL_free(server_params);
ssl->ctx->x509_method->hs_flush_cached_ca_names(this);
- OPENSSL_free(certificate_types);
OPENSSL_free(key_block);
}
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index 62e15e2..9d2a1fb 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -976,13 +976,10 @@
// Initialize ECDH and save the peer public key for later.
hs->key_share = SSLKeyShare::Create(group_id);
- uint8_t *peer_key = nullptr;
- size_t peer_key_len = 0;
if (!hs->key_share ||
- !CBS_stow(&point, &peer_key, &peer_key_len)) {
+ !hs->peer_key.CopyFrom(point)) {
return ssl_hs_error;
}
- hs->peer_key.Reset(peer_key, peer_key_len);
} else if (!(alg_k & SSL_kPSK)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_MESSAGE);
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNEXPECTED_MESSAGE);
@@ -1109,8 +1106,7 @@
return ssl_hs_error;
}
- if (!CBS_stow(&certificate_types, &hs->certificate_types,
- &hs->num_certificate_types)) {
+ if (!hs->certificate_types.CopyFrom(certificate_types)) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
return ssl_hs_error;
}
diff --git a/ssl/internal.h b/ssl/internal.h
index 17fe602..31cbdeb 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -158,6 +158,8 @@
#include <openssl/span.h>
#include <openssl/stack.h>
+#include "../crypto/internal.h"
+
#if defined(OPENSSL_WINDOWS)
// Windows defines struct timeval in winsock2.h.
@@ -314,6 +316,16 @@
return true;
}
+ // CopyFrom replaces the array with a newly-allocated copy of |in|. It returns
+ // true on success and false on error.
+ bool CopyFrom(Span<const uint8_t> in) {
+ if (!Init(in.size())) {
+ return false;
+ }
+ OPENSSL_memcpy(data_, in.data(), in.size());
+ return true;
+ }
+
private:
T *data_ = nullptr;
size_t size_ = 0;
@@ -1284,13 +1296,11 @@
SSLTranscript transcript;
// cookie is the value of the cookie received from the server, if any.
- uint8_t *cookie = nullptr;
- size_t cookie_len = 0;
+ Array<uint8_t> cookie;
// key_share_bytes is the value of the previously sent KeyShare extension by
// the client in TLS 1.3.
- uint8_t *key_share_bytes = nullptr;
- size_t key_share_bytes_len = 0;
+ Array<uint8_t> key_share_bytes;
// ecdh_public_key, for servers, is the key share to be sent to the client in
// TLS 1.3.
@@ -1332,8 +1342,7 @@
// certificate_types, on the client, contains the set of certificate types
// received in a CertificateRequest message.
- uint8_t *certificate_types = nullptr;
- size_t num_certificate_types = 0;
+ Array<uint8_t> certificate_types;
// local_pubkey is the public key we are authenticating as.
UniquePtr<EVP_PKEY> local_pubkey;
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 7a75776..262f9b2 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -2022,8 +2022,8 @@
*out_types = NULL;
return 0;
}
- *out_types = ssl->s3->hs->certificate_types;
- return ssl->s3->hs->num_certificate_types;
+ *out_types = ssl->s3->hs->certificate_types.data();
+ return ssl->s3->hs->certificate_types.size();
}
EVP_PKEY *SSL_get_privatekey(const SSL *ssl) {
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index 6301505..7f3308b 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -149,19 +149,16 @@
// This function does an initial scan over the extensions block to filter those
// out.
static int tls1_check_duplicate_extensions(const CBS *cbs) {
- CBS extensions = *cbs;
- size_t num_extensions = 0, i = 0;
- uint16_t *extension_types = NULL;
- int ret = 0;
-
// First pass: count the extensions.
+ size_t num_extensions = 0;
+ CBS extensions = *cbs;
while (CBS_len(&extensions) > 0) {
uint16_t type;
CBS extension;
if (!CBS_get_u16(&extensions, &type) ||
!CBS_get_u16_length_prefixed(&extensions, &extension)) {
- goto done;
+ return 0;
}
num_extensions++;
@@ -171,39 +168,34 @@
return 1;
}
- extension_types =
- (uint16_t *)OPENSSL_malloc(sizeof(uint16_t) * num_extensions);
- if (extension_types == NULL) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
- goto done;
+ Array<uint16_t> extension_types;
+ if (!extension_types.Init(num_extensions)) {
+ return 0;
}
// Second pass: gather the extension types.
extensions = *cbs;
- for (i = 0; i < num_extensions; i++) {
+ for (size_t i = 0; i < extension_types.size(); i++) {
CBS extension;
if (!CBS_get_u16(&extensions, &extension_types[i]) ||
!CBS_get_u16_length_prefixed(&extensions, &extension)) {
// This should not happen.
- goto done;
+ return 0;
}
}
assert(CBS_len(&extensions) == 0);
// Sort the extensions and make sure there are no duplicates.
- qsort(extension_types, num_extensions, sizeof(uint16_t), compare_uint16_t);
- for (i = 1; i < num_extensions; i++) {
+ qsort(extension_types.data(), extension_types.size(), sizeof(uint16_t),
+ compare_uint16_t);
+ for (size_t i = 1; i < num_extensions; i++) {
if (extension_types[i - 1] == extension_types[i]) {
- goto done;
+ return 0;
}
}
- ret = 1;
-
-done:
- OPENSSL_free(extension_types);
- return ret;
+ return 1;
}
int ssl_client_hello_init(SSL *ssl, SSL_CLIENT_HELLO *out,
@@ -2102,13 +2094,11 @@
// We received a HelloRetryRequest without a new curve, so there is no new
// share to append. Leave |hs->key_share| as-is.
if (group_id == 0 &&
- !CBB_add_bytes(&kse_bytes, hs->key_share_bytes,
- hs->key_share_bytes_len)) {
+ !CBB_add_bytes(&kse_bytes, hs->key_share_bytes.data(),
+ hs->key_share_bytes.size())) {
return 0;
}
- OPENSSL_free(hs->key_share_bytes);
- hs->key_share_bytes = NULL;
- hs->key_share_bytes_len = 0;
+ hs->key_share_bytes.Reset();
if (group_id == 0) {
return CBB_flush(out);
}
@@ -2142,15 +2132,11 @@
return 0;
}
- if (!hs->received_hello_retry_request) {
- // Save the contents of the extension to repeat it in the second
- // ClientHello.
- hs->key_share_bytes_len = CBB_len(&kse_bytes);
- hs->key_share_bytes =
- (uint8_t *)BUF_memdup(CBB_data(&kse_bytes), CBB_len(&kse_bytes));
- if (hs->key_share_bytes == NULL) {
- return 0;
- }
+ // Save the contents of the extension to repeat it in the second ClientHello.
+ if (!hs->received_hello_retry_request &&
+ !hs->key_share_bytes.CopyFrom(
+ MakeConstSpan(CBB_data(&kse_bytes), CBB_len(&kse_bytes)))) {
+ return 0;
}
return CBB_flush(out);
@@ -2310,7 +2296,7 @@
// https://tools.ietf.org/html/draft-ietf-tls-tls13-16#section-4.2.2
static int ext_cookie_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
- if (hs->cookie == NULL) {
+ if (hs->cookie.size() == 0) {
return 1;
}
@@ -2318,15 +2304,13 @@
if (!CBB_add_u16(out, TLSEXT_TYPE_cookie) ||
!CBB_add_u16_length_prefixed(out, &contents) ||
!CBB_add_u16_length_prefixed(&contents, &cookie) ||
- !CBB_add_bytes(&cookie, hs->cookie, hs->cookie_len) ||
+ !CBB_add_bytes(&cookie, hs->cookie.data(), hs->cookie.size()) ||
!CBB_flush(out)) {
return 0;
}
// The cookie is no longer needed in memory.
- OPENSSL_free(hs->cookie);
- hs->cookie = NULL;
- hs->cookie_len = 0;
+ hs->cookie.Reset();
return 1;
}
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc
index 2ac6195..b6ff840 100644
--- a/ssl/tls13_client.cc
+++ b/ssl/tls13_client.cc
@@ -99,7 +99,7 @@
return ssl_hs_error;
}
- if (!CBS_stow(&cookie_value, &hs->cookie, &hs->cookie_len)) {
+ if (!hs->cookie.CopyFrom(cookie_value)) {
return ssl_hs_error;
}
}
@@ -841,10 +841,7 @@
void ssl_clear_tls13_state(SSL_HANDSHAKE *hs) {
hs->key_share.reset();
-
- OPENSSL_free(hs->key_share_bytes);
- hs->key_share_bytes = NULL;
- hs->key_share_bytes_len = 0;
+ hs->key_share_bytes.Reset();
}
} // namespace bssl
