Add a bssl::InplaceVector to libssl
We're constantly hand-rolling these inlined bounded array structures.
Inspired by C++26's std::inplace_vector name, add something very basic.
This is still very barebones and probably can be improved. But it's
enough to be able to use spans more often.
Perhaps in a decade we'll be able to just use the real one. To keep our
size optimization, InplaceVector picks the minimal integer needed to
represent the size but, unlike doing it by hand, this is entirely hidden
from the API.
I switched a pile of fields over to this to confirm it mostly works,
though there are still a ton more that are array/length pairs. In
particular, we never came up with a good calling convention for all the
intermediate hash-sized values in the TLS 1.3 key schedule. I've left
that alone for now.
The immediate motivation here is that a lot of the transport-specific
epoch structs will need to store the original traffic secret, and
holding on to it everywhere without an abstraction is tedious.
Bug: 371998381
Change-Id: I17a987299dc9278b59d2b773e1f3fead69dff162
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71748
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
diff --git a/ssl/extensions.cc b/ssl/extensions.cc
index 30591a6..0e001a6 100644
--- a/ssl/extensions.cc
+++ b/ssl/extensions.cc
@@ -709,14 +709,14 @@
}
assert(ssl->s3->initial_handshake_complete ==
- (ssl->s3->previous_client_finished_len != 0));
+ !ssl->s3->previous_client_finished.empty());
CBB contents, prev_finished;
if (!CBB_add_u16(out, TLSEXT_TYPE_renegotiate) ||
!CBB_add_u16_length_prefixed(out, &contents) ||
!CBB_add_u8_length_prefixed(&contents, &prev_finished) ||
- !CBB_add_bytes(&prev_finished, ssl->s3->previous_client_finished,
- ssl->s3->previous_client_finished_len) ||
+ !CBB_add_bytes(&prev_finished, ssl->s3->previous_client_finished.data(),
+ ssl->s3->previous_client_finished.size()) ||
!CBB_flush(out)) {
return false;
}
@@ -752,16 +752,11 @@
return true;
}
- const size_t expected_len = ssl->s3->previous_client_finished_len +
- ssl->s3->previous_server_finished_len;
-
- // Check for logic errors
- assert(!expected_len || ssl->s3->previous_client_finished_len);
- assert(!expected_len || ssl->s3->previous_server_finished_len);
+ // Check for logic errors.
+ assert(ssl->s3->previous_client_finished.size() ==
+ ssl->s3->previous_server_finished.size());
assert(ssl->s3->initial_handshake_complete ==
- (ssl->s3->previous_client_finished_len != 0));
- assert(ssl->s3->initial_handshake_complete ==
- (ssl->s3->previous_server_finished_len != 0));
+ !ssl->s3->previous_client_finished.empty());
// Parse out the extension contents.
CBS renegotiated_connection;
@@ -773,15 +768,22 @@
}
// Check that the extension matches.
- if (CBS_len(&renegotiated_connection) != expected_len) {
+ CBS client_verify, server_verify;
+ if (!CBS_get_bytes(&renegotiated_connection, &client_verify,
+ ssl->s3->previous_client_finished.size()) ||
+ !CBS_get_bytes(&renegotiated_connection, &server_verify,
+ ssl->s3->previous_server_finished.size()) ||
+ CBS_len(&renegotiated_connection) != 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_RENEGOTIATION_MISMATCH);
*out_alert = SSL_AD_HANDSHAKE_FAILURE;
return false;
}
- const uint8_t *d = CBS_data(&renegotiated_connection);
- bool ok = CRYPTO_memcmp(d, ssl->s3->previous_client_finished,
- ssl->s3->previous_client_finished_len) == 0;
+ bool ok =
+ CBS_mem_equal(&client_verify, ssl->s3->previous_client_finished.data(),
+ ssl->s3->previous_client_finished.size()) &&
+ CBS_mem_equal(&server_verify, ssl->s3->previous_server_finished.data(),
+ ssl->s3->previous_server_finished.size());
#if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
ok = true;
#endif
@@ -790,20 +792,8 @@
*out_alert = SSL_AD_HANDSHAKE_FAILURE;
return false;
}
- d += ssl->s3->previous_client_finished_len;
- ok = CRYPTO_memcmp(d, ssl->s3->previous_server_finished,
- ssl->s3->previous_server_finished_len) == 0;
-#if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
- ok = true;
-#endif
- if (!ok) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_RENEGOTIATION_MISMATCH);
- *out_alert = SSL_AD_HANDSHAKE_FAILURE;
- return false;
- }
ssl->s3->send_connection_binding = true;
-
return true;
}
@@ -4079,9 +4069,8 @@
// Envoy's tests expect the session to have a session ID that matches the
// placeholder used by the client. It's unclear whether this is a good idea,
// but we maintain it for now.
- SHA256(ticket.data(), ticket.size(), session->session_id);
- // Other consumers may expect a non-empty session ID to indicate resumption.
- session->session_id_length = SHA256_DIGEST_LENGTH;
+ session->session_id.ResizeMaybeUninit(SHA256_DIGEST_LENGTH);
+ SHA256(ticket.data(), ticket.size(), session->session_id.data());
*out_session = std::move(session);
return ssl_ticket_aead_success;
@@ -4292,12 +4281,12 @@
if (ssl->session != NULL) {
static const char kResumptionMagic[] = "Resumption";
SHA256_Update(&ctx, kResumptionMagic, sizeof(kResumptionMagic));
- if (ssl->session->original_handshake_hash_len == 0) {
+ if (ssl->session->original_handshake_hash.empty()) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return false;
}
- SHA256_Update(&ctx, ssl->session->original_handshake_hash,
- ssl->session->original_handshake_hash_len);
+ SHA256_Update(&ctx, ssl->session->original_handshake_hash.data(),
+ ssl->session->original_handshake_hash.size());
}
uint8_t hs_hash[EVP_MAX_MD_SIZE];
@@ -4320,20 +4309,14 @@
return false;
}
- static_assert(
- sizeof(hs->new_session->original_handshake_hash) == EVP_MAX_MD_SIZE,
- "original_handshake_hash is too small");
-
size_t digest_len;
- if (!hs->transcript.GetHash(hs->new_session->original_handshake_hash,
+ hs->new_session->original_handshake_hash.ResizeMaybeUninit(
+ hs->transcript.DigestLen());
+ if (!hs->transcript.GetHash(hs->new_session->original_handshake_hash.data(),
&digest_len)) {
return false;
}
-
- static_assert(EVP_MAX_MD_SIZE <= 0xff,
- "EVP_MAX_MD_SIZE does not fit in uint8_t");
- hs->new_session->original_handshake_hash_len = (uint8_t)digest_len;
-
+ assert(digest_len == hs->new_session->original_handshake_hash.size());
return true;
}
diff --git a/ssl/handoff.cc b/ssl/handoff.cc
index ec950d0..e4e5d28 100644
--- a/ssl/handoff.cc
+++ b/ssl/handoff.cc
@@ -433,8 +433,8 @@
hs->server_handshake_secret().size()) ||
!CBB_add_asn1_octet_string(&seq, hs->secret().data(),
hs->secret().size()) ||
- !CBB_add_asn1_octet_string(&seq, s3->exporter_secret,
- s3->exporter_secret_len) ||
+ !CBB_add_asn1_octet_string(&seq, s3->exporter_secret.data(),
+ s3->exporter_secret.size()) ||
!CBB_add_asn1_bool(&seq, s3->used_hello_retry_request) ||
!CBB_add_asn1_bool(&seq, hs->accept_psk_mode) ||
!CBB_add_asn1_int64(&seq, s3->ticket_age_skew) ||
@@ -704,11 +704,9 @@
!CopyExact(hs->client_handshake_secret(), &client_handshake_secret) ||
!CopyExact(hs->server_handshake_secret(), &server_handshake_secret) ||
!CopyExact(hs->secret(), &secret) ||
- !CopyExact({s3->exporter_secret, hs->transcript.DigestLen()},
- &exporter_secret)) {
+ !s3->exporter_secret.TryCopyFrom(exporter_secret)) {
return false;
}
- s3->exporter_secret_len = CBS_len(&exporter_secret);
if (s3->early_data_accepted &&
!CopyExact(hs->early_traffic_secret(), &early_traffic_secret)) {
diff --git a/ssl/handshake.cc b/ssl/handshake.cc
index a9ec634..6fe37cc 100644
--- a/ssl/handshake.cc
+++ b/ssl/handshake.cc
@@ -495,18 +495,18 @@
}
// Copy the Finished so we can use it for renegotiation checks.
- if (finished_len > sizeof(ssl->s3->previous_client_finished) ||
- finished_len > sizeof(ssl->s3->previous_server_finished)) {
+ if (finished_len > ssl->s3->previous_client_finished.capacity() ||
+ finished_len > ssl->s3->previous_server_finished.capacity()) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return ssl_hs_error;
}
if (ssl->server) {
- OPENSSL_memcpy(ssl->s3->previous_client_finished, finished, finished_len);
- ssl->s3->previous_client_finished_len = finished_len;
+ ssl->s3->previous_client_finished.CopyFrom(
+ MakeConstSpan(finished, finished_len));
} else {
- OPENSSL_memcpy(ssl->s3->previous_server_finished, finished, finished_len);
- ssl->s3->previous_server_finished_len = finished_len;
+ ssl->s3->previous_server_finished.CopyFrom(
+ MakeConstSpan(finished, finished_len));
}
// The Finished message should be the end of a flight.
@@ -524,38 +524,32 @@
SSL *const ssl = hs->ssl;
const SSL_SESSION *session = ssl_handshake_session(hs);
- uint8_t finished[EVP_MAX_MD_SIZE];
+ uint8_t finished_buf[EVP_MAX_MD_SIZE];
size_t finished_len;
- if (!hs->transcript.GetFinishedMAC(finished, &finished_len, session,
+ if (!hs->transcript.GetFinishedMAC(finished_buf, &finished_len, session,
ssl->server)) {
return false;
}
+ auto finished = MakeConstSpan(finished_buf, finished_len);
// Log the master secret, if logging is enabled.
- if (!ssl_log_secret(ssl, "CLIENT_RANDOM",
- MakeConstSpan(session->secret, session->secret_length))) {
+ if (!ssl_log_secret(ssl, "CLIENT_RANDOM", session->secret)) {
return false;
}
// Copy the Finished so we can use it for renegotiation checks.
- if (finished_len > sizeof(ssl->s3->previous_client_finished) ||
- finished_len > sizeof(ssl->s3->previous_server_finished)) {
+ bool ok = ssl->server
+ ? ssl->s3->previous_server_finished.TryCopyFrom(finished)
+ : ssl->s3->previous_client_finished.TryCopyFrom(finished);
+ if (!ok) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
- return false;
- }
-
- if (ssl->server) {
- OPENSSL_memcpy(ssl->s3->previous_server_finished, finished, finished_len);
- ssl->s3->previous_server_finished_len = finished_len;
- } else {
- OPENSSL_memcpy(ssl->s3->previous_client_finished, finished, finished_len);
- ssl->s3->previous_client_finished_len = finished_len;
+ return ssl_hs_error;
}
ScopedCBB cbb;
CBB body;
if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_FINISHED) ||
- !CBB_add_bytes(&body, finished, finished_len) ||
+ !CBB_add_bytes(&body, finished.data(), finished.size()) ||
!ssl_add_message_cbb(ssl, cbb.get())) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return false;
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index 2f74ff5..3774787 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -328,7 +328,7 @@
// Do not send a session ID on renegotiation.
if (!ssl->s3->initial_handshake_complete &&
!empty_session_id &&
- !CBB_add_bytes(&child, hs->session_id, hs->session_id_len)) {
+ !CBB_add_bytes(&child, hs->session_id.data(), hs->session_id.size())) {
return false;
}
@@ -526,7 +526,7 @@
}
const bool has_id_session = ssl->session != nullptr &&
- ssl->session->session_id_length > 0 &&
+ !ssl->session->session_id.empty() &&
ssl->session->ticket.empty();
const bool has_ticket_session =
ssl->session != nullptr && !ssl->session->ticket.empty();
@@ -540,12 +540,10 @@
ssl->quic_method == nullptr &&
!SSL_is_dtls(hs->ssl);
if (has_id_session) {
- hs->session_id_len = ssl->session->session_id_length;
- OPENSSL_memcpy(hs->session_id, ssl->session->session_id,
- hs->session_id_len);
+ hs->session_id = ssl->session->session_id;
} else if (ticket_session_requires_random_id || enable_compatibility_mode) {
- hs->session_id_len = sizeof(hs->session_id);
- if (!RAND_bytes(hs->session_id, hs->session_id_len)) {
+ hs->session_id.ResizeMaybeUninit(SSL_MAX_SSL_SESSION_ID_LENGTH);
+ if (!RAND_bytes(hs->session_id.data(), hs->session_id.size())) {
return ssl_hs_error;
}
}
@@ -830,9 +828,8 @@
hs->new_cipher = cipher;
- if (hs->session_id_len != 0 &&
- CBS_mem_equal(&server_hello.session_id, hs->session_id,
- hs->session_id_len)) {
+ if (!hs->session_id.empty() &&
+ Span<const uint8_t>(server_hello.session_id) == hs->session_id) {
// Echoing the ClientHello session ID in TLS 1.2, whether from the session
// or a synthetic one, indicates resumption. If there was no session (or if
// the session was only offered in ECH ClientHelloInner), this was the
@@ -874,16 +871,9 @@
}
// Save the session ID from the server. This may be empty if the session
- // isn't resumable, or if we'll receive a session ticket later.
- assert(CBS_len(&server_hello.session_id) <= SSL3_SESSION_ID_SIZE);
- static_assert(SSL3_SESSION_ID_SIZE <= UINT8_MAX,
- "max session ID is too large");
- hs->new_session->session_id_length =
- static_cast<uint8_t>(CBS_len(&server_hello.session_id));
- OPENSSL_memcpy(hs->new_session->session_id,
- CBS_data(&server_hello.session_id),
- CBS_len(&server_hello.session_id));
-
+ // isn't resumable, or if we'll receive a session ticket later. The
+ // ServerHello parser ensures |server_hello.session_id| is within bounds.
+ hs->new_session->session_id.CopyFrom(server_hello.session_id);
hs->new_session->cipher = hs->new_cipher;
}
@@ -1619,13 +1609,13 @@
return ssl_hs_error;
}
- hs->new_session->secret_length =
- tls1_generate_master_secret(hs, hs->new_session->secret, pms);
- if (hs->new_session->secret_length == 0) {
+ hs->new_session->secret.ResizeMaybeUninit(SSL3_MASTER_SECRET_SIZE);
+ if (!tls1_generate_master_secret(hs, MakeSpan(hs->new_session->secret),
+ pms)) {
return ssl_hs_error;
}
- hs->new_session->extended_master_secret = hs->extended_master_secret;
+ hs->new_session->extended_master_secret = hs->extended_master_secret;
hs->state = state_send_client_certificate_verify;
return ssl_hs_ok;
}
@@ -1860,8 +1850,9 @@
// Historically, OpenSSL filled in fake session IDs for ticket-based sessions.
// TODO(davidben): Are external callers relying on this? Try removing this.
- SHA256(CBS_data(&ticket), CBS_len(&ticket), hs->new_session->session_id);
- hs->new_session->session_id_length = SHA256_DIGEST_LENGTH;
+ hs->new_session->session_id.ResizeMaybeUninit(SHA256_DIGEST_LENGTH);
+ SHA256(CBS_data(&ticket), CBS_len(&ticket),
+ hs->new_session->session_id.data());
ssl->method->next_message(ssl);
hs->state = state_process_change_cipher_spec;
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc
index 7821ce0..59531f7 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -894,10 +894,10 @@
hs->new_cipher = params.cipher;
hs->signature_algorithm = params.signature_algorithm;
- hs->session_id_len = client_hello.session_id_len;
- // This is checked in |ssl_client_hello_init|.
- assert(hs->session_id_len <= sizeof(hs->session_id));
- OPENSSL_memcpy(hs->session_id, client_hello.session_id, hs->session_id_len);
+ // |ssl_client_hello_init| checks that |client_hello.session_id| is not too
+ // large.
+ hs->session_id.CopyFrom(
+ MakeConstSpan(client_hello.session_id, client_hello.session_id_len));
// Determine whether we are doing session resumption.
UniquePtr<SSL_SESSION> session;
@@ -941,9 +941,9 @@
// Assign a session ID if not using session tickets.
if (!hs->ticket_expected &&
(ssl->ctx->session_cache_mode & SSL_SESS_CACHE_SERVER)) {
- hs->new_session->session_id_length = SSL3_SSL_SESSION_ID_LENGTH;
- RAND_bytes(hs->new_session->session_id,
- hs->new_session->session_id_length);
+ hs->new_session->session_id.ResizeMaybeUninit(SSL3_SSL_SESSION_ID_LENGTH);
+ RAND_bytes(hs->new_session->session_id.data(),
+ hs->new_session->session_id.size());
}
}
@@ -1027,8 +1027,8 @@
// If this is a resumption and the original handshake didn't support
// ChannelID then we didn't record the original handshake hashes in the
// session and so cannot resume with ChannelIDs.
- if (ssl->session != NULL &&
- ssl->session->original_handshake_hash_len == 0) {
+ if (ssl->session != nullptr &&
+ ssl->session->original_handshake_hash.empty()) {
hs->channel_id_negotiated = false;
}
@@ -1072,10 +1072,9 @@
Span<const uint8_t> session_id;
if (ssl->session != nullptr) {
// Echo the session ID from the ClientHello to indicate resumption.
- session_id = MakeConstSpan(hs->session_id, hs->session_id_len);
+ session_id = hs->session_id;
} else {
- session_id = MakeConstSpan(hs->new_session->session_id,
- hs->new_session->session_id_length);
+ session_id = hs->new_session->session_id;
}
ScopedCBB cbb;
@@ -1602,13 +1601,18 @@
}
// Compute the master secret.
- hs->new_session->secret_length = tls1_generate_master_secret(
- hs, hs->new_session->secret, premaster_secret);
- if (hs->new_session->secret_length == 0) {
+ hs->new_session->secret.ResizeMaybeUninit(SSL3_MASTER_SECRET_SIZE);
+ if (!tls1_generate_master_secret(hs, MakeSpan(hs->new_session->secret),
+ premaster_secret)) {
return ssl_hs_error;
}
hs->new_session->extended_master_secret = hs->extended_master_secret;
- CONSTTIME_DECLASSIFY(hs->new_session->secret, hs->new_session->secret_length);
+ // Declassify the secret to undo the RSA decryption validation above. We are
+ // not currently running most of the TLS library with constant-time
+ // validation.
+ // TODO(crbug.com/42290551): Remove this and cover the TLS library too.
+ CONSTTIME_DECLASSIFY(hs->new_session->secret.data(),
+ hs->new_session->secret.size());
hs->can_release_private_key = true;
ssl->method->next_message(ssl);
diff --git a/ssl/internal.h b/ssl/internal.h
index a0d4221..e6e69c7 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -204,6 +204,17 @@
return first;
}
+template <typename ForwardIt>
+ForwardIt cxx17_uninitialized_value_construct_n(ForwardIt first, size_t n) {
+ using T = typename std::iterator_traits<ForwardIt>::value_type;
+ while (n > 0) {
+ new (std::addressof(*first)) T();
+ first++;
+ n--;
+ }
+ return first;
+}
+
template <typename InputIt, typename OutputIt>
InputIt cxx17_uninitialized_move(InputIt first, InputIt last, OutputIt out) {
using OutputT = typename std::iterator_traits<OutputIt>::value_type;
@@ -498,6 +509,118 @@
static constexpr size_t kDefaultSize = 16;
};
+// A PackedSize is an integer that can store values from 0 to N, represented as
+// a minimal-width integer.
+template <size_t N>
+using PackedSize = std::conditional_t<
+ N <= 0xff, uint8_t,
+ std::conditional_t<N <= 0xffff, uint16_t,
+ std::conditional_t<N <= 0xffffffff, uint32_t, size_t>>>;
+
+// An InplaceVector is like a Vector, but stores up to N elements inline in the
+// object. It is inspired by std::inplace_vector in C++26.
+template <typename T, size_t N>
+class InplaceVector {
+ public:
+ InplaceVector() = default;
+ InplaceVector(const InplaceVector &other) { *this = other; }
+ InplaceVector(InplaceVector &&other) { *this = std::move(other); }
+ ~InplaceVector() { clear(); }
+ InplaceVector &operator=(const InplaceVector &other) {
+ if (this != &other) {
+ CopyFrom(other);
+ }
+ return *this;
+ }
+ InplaceVector &operator=(InplaceVector &&other) {
+ clear();
+ cxx17_uninitialized_move(other.begin(), other.end(), data());
+ size_ = other.size();
+ return *this;
+ }
+
+ const T *data() const { return reinterpret_cast<const T *>(storage_); }
+ T *data() { return reinterpret_cast<T *>(storage_); }
+ size_t size() const { return size_; }
+ static constexpr size_t capacity() { return N; }
+ bool empty() const { return size_ == 0; }
+
+ const T &operator[](size_t i) const {
+ BSSL_CHECK(i < size_);
+ return data()[i];
+ }
+ T &operator[](size_t i) {
+ BSSL_CHECK(i < size_);
+ return data()[i];
+ }
+
+ T *begin() { return data(); }
+ const T *begin() const { return data(); }
+ T *end() { return data() + size_; }
+ const T *end() const { return data() + size_; }
+
+ void clear() {
+ cxx17_destroy_n(data(), size_);
+ size_ = 0;
+ }
+
+ // TryResize resizes the vector to |new_size| and returns true, or returns
+ // false if |new_size| is too large. Any newly-added elements are
+ // value-initialized.
+ bool TryResize(size_t new_size) {
+ if (new_size > capacity()) {
+ return false;
+ }
+ if (new_size < size_) {
+ cxx17_destroy_n(data() + new_size, size_ - new_size);
+ } else {
+ cxx17_uninitialized_value_construct_n(data() + size_, new_size - size_);
+ }
+ size_ = static_cast<PackedSize<N>>(new_size);
+ return true;
+ }
+
+ // TryResizeMaybeUninit behaves like |TryResize|, but newly-added elements are
+ // default-initialized, so POD types may contain uninitialized values that the
+ // caller is responsible for filling in.
+ bool TryResizeMaybeUninit(size_t new_size) {
+ if (new_size > capacity()) {
+ return false;
+ }
+ if (new_size < size_) {
+ cxx17_destroy_n(data() + new_size, size_ - new_size);
+ } else {
+ cxx17_uninitialized_default_construct_n(data() + size_, new_size - size_);
+ }
+ size_ = static_cast<PackedSize<N>>(new_size);
+ return true;
+ }
+
+ // TryCopyFrom sets the vector to a copy of |in| and returns true, or returns
+ // false if |in| is too large.
+ bool TryCopyFrom(Span<const T> in) {
+ if (in.size() > capacity()) {
+ return false;
+ }
+ clear();
+ std::uninitialized_copy(in.begin(), in.end(), data());
+ size_ = in.size();
+ return true;
+ }
+
+ // The following methods behave like their |Try*| counterparts, but abort the
+ // program on failure.
+ void Resize(size_t size) { BSSL_CHECK(TryResize(size)); }
+ void ResizeMaybeUninit(size_t size) {
+ BSSL_CHECK(TryResizeMaybeUninit(size));
+ }
+ void CopyFrom(Span<const T> in) { BSSL_CHECK(TryCopyFrom(in)); }
+
+ private:
+ alignas(T) char storage_[sizeof(T[N])];
+ PackedSize<N> size_ = 0;
+};
+
// CBBFinishArray behaves like |CBB_finish| but stores the result in an Array.
OPENSSL_EXPORT bool CBBFinishArray(CBB *cbb, Array<uint8_t> *out);
@@ -1002,8 +1125,8 @@
ScopedEVP_AEAD_CTX ctx_;
// fixed_nonce_ contains any bytes of the nonce that are fixed for all
// records.
- uint8_t fixed_nonce_[12] = {0};
- uint8_t fixed_nonce_len_ = 0, variable_nonce_len_ = 0;
+ InplaceVector<uint8_t, 12> fixed_nonce_;
+ uint8_t variable_nonce_len_ = 0;
UniquePtr<RecordNumberEncrypter> rn_encrypter_;
// variable_nonce_included_in_record_ is true if the variable nonce
// for a record is included as a prefix before the ciphertext.
@@ -2292,8 +2415,7 @@
uint8_t ech_config_id = 0;
// session_id is the session ID in the ClientHello.
- uint8_t session_id[SSL_MAX_SSL_SESSION_ID_LENGTH] = {0};
- uint8_t session_id_len = 0;
+ InplaceVector<uint8_t, SSL_MAX_SSL_SESSION_ID_LENGTH> session_id;
// grease_seed is the entropy for GREASE values.
uint8_t grease_seed[ssl_grease_last_index + 1] = {0};
@@ -2639,8 +2761,7 @@
// sid_ctx partitions the session space within a shared session cache or
// ticket key. Only sessions with a matching value will be accepted.
- uint8_t sid_ctx_length = 0;
- uint8_t sid_ctx[SSL_MAX_SID_CTX_LENGTH] = {0};
+ InplaceVector<uint8_t, SSL_MAX_SID_CTX_LENGTH> sid_ctx;
};
// |SSL_PROTOCOL_METHOD| abstracts between TLS and DTLS.
@@ -3009,18 +3130,13 @@
// one.
UniquePtr<SSL_HANDSHAKE> hs;
- uint8_t write_traffic_secret[SSL_MAX_MD_SIZE] = {0};
- uint8_t read_traffic_secret[SSL_MAX_MD_SIZE] = {0};
- uint8_t exporter_secret[SSL_MAX_MD_SIZE] = {0};
- uint8_t write_traffic_secret_len = 0;
- uint8_t read_traffic_secret_len = 0;
- uint8_t exporter_secret_len = 0;
+ InplaceVector<uint8_t, SSL_MAX_MD_SIZE> write_traffic_secret;
+ InplaceVector<uint8_t, SSL_MAX_MD_SIZE> read_traffic_secret;
+ InplaceVector<uint8_t, SSL_MAX_MD_SIZE> exporter_secret;
// Connection binding to prevent renegotiation attacks
- uint8_t previous_client_finished[12] = {0};
- uint8_t previous_client_finished_len = 0;
- uint8_t previous_server_finished_len = 0;
- uint8_t previous_server_finished[12] = {0};
+ InplaceVector<uint8_t, 12> previous_client_finished;
+ InplaceVector<uint8_t, 12> previous_server_finished;
uint8_t send_alert[2] = {0};
@@ -3558,8 +3674,11 @@
bool tls1_change_cipher_state(SSL_HANDSHAKE *hs,
evp_aead_direction_t direction);
-int tls1_generate_master_secret(SSL_HANDSHAKE *hs, uint8_t *out,
- Span<const uint8_t> premaster);
+
+// tls1_generate_master_secret computes the master secret from |premaster| and
+// writes it to |out|. |out| must have size |SSL3_MASTER_SECRET_SIZE|.
+bool tls1_generate_master_secret(SSL_HANDSHAKE *hs, Span<uint8_t> out,
+ Span<const uint8_t> premaster);
// tls1_get_grouplist returns the locally-configured group preference list.
Span<const uint16_t> tls1_get_grouplist(const SSL_HANDSHAKE *ssl);
@@ -4087,17 +4206,14 @@
// session. In TLS 1.3 and up, it is the resumption PSK for sessions handed to
// the caller, but it stores the resumption secret when stored on |SSL|
// objects.
- uint8_t secret_length = 0;
- uint8_t secret[SSL_MAX_MASTER_KEY_LENGTH] = {0};
+ bssl::InplaceVector<uint8_t, SSL_MAX_MASTER_KEY_LENGTH> secret;
- // session_id - valid?
- uint8_t session_id_length = 0;
- uint8_t session_id[SSL_MAX_SSL_SESSION_ID_LENGTH] = {0};
+ bssl::InplaceVector<uint8_t, SSL_MAX_SSL_SESSION_ID_LENGTH> session_id;
+
// this is used to determine whether the session is being reused in
// the appropriate context. It is up to the application to set this,
// via SSL_new
- uint8_t sid_ctx_length = 0;
- uint8_t sid_ctx[SSL_MAX_SID_CTX_LENGTH] = {0};
+ bssl::InplaceVector<uint8_t, SSL_MAX_SID_CTX_LENGTH> sid_ctx;
bssl::UniquePtr<char> psk_identity;
@@ -4160,8 +4276,7 @@
// original_handshake_hash contains the handshake hash (either SHA-1+MD5 or
// SHA-2, depending on TLS version) for the original, full handshake that
// created a session. This is used by Channel IDs during resumption.
- uint8_t original_handshake_hash[EVP_MAX_MD_SIZE] = {0};
- uint8_t original_handshake_hash_len = 0;
+ bssl::InplaceVector<uint8_t, EVP_MAX_MD_SIZE> original_handshake_hash;
uint32_t ticket_lifetime_hint = 0; // Session lifetime hint in seconds
diff --git a/ssl/ssl_aead_ctx.cc b/ssl/ssl_aead_ctx.cc
index 4f25d43..8e7a387 100644
--- a/ssl/ssl_aead_ctx.cc
+++ b/ssl/ssl_aead_ctx.cc
@@ -80,9 +80,7 @@
aead_ctx->variable_nonce_len_ = (uint8_t)EVP_AEAD_nonce_length(aead);
if (mac_key.empty()) {
// This is an actual AEAD.
- assert(fixed_iv.size() <= sizeof(aead_ctx->fixed_nonce_));
- OPENSSL_memcpy(aead_ctx->fixed_nonce_, fixed_iv.data(), fixed_iv.size());
- aead_ctx->fixed_nonce_len_ = fixed_iv.size();
+ aead_ctx->fixed_nonce_.CopyFrom(fixed_iv);
if (protocol_version >= TLS1_3_VERSION ||
cipher->algorithm_enc & SSL_CHACHA20POLY1305) {
@@ -246,11 +244,11 @@
// Prepend the fixed nonce, or left-pad with zeros if XORing.
if (xor_fixed_nonce_) {
- nonce_len = fixed_nonce_len_ - variable_nonce_len_;
+ nonce_len = fixed_nonce_.size() - variable_nonce_len_;
OPENSSL_memset(nonce, 0, nonce_len);
} else {
- OPENSSL_memcpy(nonce, fixed_nonce_, fixed_nonce_len_);
- nonce_len += fixed_nonce_len_;
+ OPENSSL_memcpy(nonce, fixed_nonce_.data(), fixed_nonce_.size());
+ nonce_len += fixed_nonce_.size();
}
// Add the variable nonce.
@@ -270,8 +268,8 @@
// XOR the fixed nonce, if necessary.
if (xor_fixed_nonce_) {
- assert(nonce_len == fixed_nonce_len_);
- for (size_t i = 0; i < fixed_nonce_len_; i++) {
+ assert(nonce_len == fixed_nonce_.size());
+ for (size_t i = 0; i < fixed_nonce_.size(); i++) {
nonce[i] ^= fixed_nonce_[i];
}
}
@@ -323,11 +321,11 @@
// Prepend the fixed nonce, or left-pad with zeros if XORing.
if (xor_fixed_nonce_) {
- nonce_len = fixed_nonce_len_ - variable_nonce_len_;
+ nonce_len = fixed_nonce_.size() - variable_nonce_len_;
OPENSSL_memset(nonce, 0, nonce_len);
} else {
- OPENSSL_memcpy(nonce, fixed_nonce_, fixed_nonce_len_);
- nonce_len += fixed_nonce_len_;
+ OPENSSL_memcpy(nonce, fixed_nonce_.data(), fixed_nonce_.size());
+ nonce_len += fixed_nonce_.size();
}
// Select the variable nonce.
@@ -351,14 +349,14 @@
OPENSSL_PUT_ERROR(SSL, SSL_R_OUTPUT_ALIASES_INPUT);
return false;
}
- OPENSSL_memcpy(out_prefix, nonce + fixed_nonce_len_,
+ OPENSSL_memcpy(out_prefix, nonce + fixed_nonce_.size(),
variable_nonce_len_);
}
// XOR the fixed nonce, if necessary.
if (xor_fixed_nonce_) {
- assert(nonce_len == fixed_nonce_len_);
- for (size_t i = 0; i < fixed_nonce_len_; i++) {
+ assert(nonce_len == fixed_nonce_.size());
+ for (size_t i = 0; i < fixed_nonce_.size(); i++) {
nonce[i] ^= fixed_nonce_[i];
}
}
diff --git a/ssl/ssl_asn1.cc b/ssl/ssl_asn1.cc
index 3311246..3e1e9d6 100644
--- a/ssl/ssl_asn1.cc
+++ b/ssl/ssl_asn1.cc
@@ -216,9 +216,10 @@
!CBB_add_asn1(&session, &child, CBS_ASN1_OCTETSTRING) ||
!CBB_add_u16(&child, (uint16_t)(in->cipher->id & 0xffff)) ||
// The session ID is irrelevant for a session ticket.
- !CBB_add_asn1_octet_string(&session, in->session_id,
- for_ticket ? 0 : in->session_id_length) ||
- !CBB_add_asn1_octet_string(&session, in->secret, in->secret_length) ||
+ !CBB_add_asn1_octet_string(&session, in->session_id.data(),
+ for_ticket ? 0 : in->session_id.size()) ||
+ !CBB_add_asn1_octet_string(&session, in->secret.data(),
+ in->secret.size()) ||
!CBB_add_asn1(&session, &child, kTimeTag) ||
!CBB_add_asn1_uint64(&child, in->time) ||
!CBB_add_asn1(&session, &child, kTimeoutTag) ||
@@ -240,7 +241,8 @@
// Although it is OPTIONAL and usually empty, OpenSSL has
// historically always encoded the sid_ctx.
if (!CBB_add_asn1(&session, &child, kSessionIDContextTag) ||
- !CBB_add_asn1_octet_string(&child, in->sid_ctx, in->sid_ctx_length)) {
+ !CBB_add_asn1_octet_string(&child, in->sid_ctx.data(),
+ in->sid_ctx.size())) {
return 0;
}
@@ -283,10 +285,10 @@
}
}
- if (in->original_handshake_hash_len > 0) {
+ if (!in->original_handshake_hash.empty()) {
if (!CBB_add_asn1(&session, &child, kOriginalHandshakeHashTag) ||
- !CBB_add_asn1_octet_string(&child, in->original_handshake_hash,
- in->original_handshake_hash_len)) {
+ !CBB_add_asn1_octet_string(&child, in->original_handshake_hash.data(),
+ in->original_handshake_hash.size())) {
return 0;
}
}
@@ -473,23 +475,6 @@
return 1;
}
-// SSL_SESSION_parse_bounded_octet_string parses an optional ASN.1 OCTET STRING
-// explicitly tagged with |tag| of size at most |max_out|.
-static int SSL_SESSION_parse_bounded_octet_string(CBS *cbs, uint8_t *out,
- uint8_t *out_len,
- uint8_t max_out,
- CBS_ASN1_TAG tag) {
- CBS value;
- if (!CBS_get_optional_asn1_octet_string(cbs, &value, NULL, tag) ||
- CBS_len(&value) > max_out) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
- return 0;
- }
- OPENSSL_memcpy(out, CBS_data(&value), CBS_len(&value));
- *out_len = static_cast<uint8_t>(CBS_len(&value));
- return 1;
-}
-
static int SSL_SESSION_parse_long(CBS *cbs, long *out, CBS_ASN1_TAG tag,
long default_value) {
uint64_t value;
@@ -569,29 +554,16 @@
return nullptr;
}
- CBS session_id, secret;
- if (!CBS_get_asn1(&session, &session_id, CBS_ASN1_OCTETSTRING) ||
- CBS_len(&session_id) > SSL3_MAX_SSL_SESSION_ID_LENGTH ||
- !CBS_get_asn1(&session, &secret, CBS_ASN1_OCTETSTRING) ||
- CBS_len(&secret) > SSL_MAX_MASTER_KEY_LENGTH) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
- return nullptr;
- }
- OPENSSL_memcpy(ret->session_id, CBS_data(&session_id), CBS_len(&session_id));
- static_assert(SSL3_MAX_SSL_SESSION_ID_LENGTH <= UINT8_MAX,
- "max session ID is too large");
- ret->session_id_length = static_cast<uint8_t>(CBS_len(&session_id));
- OPENSSL_memcpy(ret->secret, CBS_data(&secret), CBS_len(&secret));
- static_assert(SSL_MAX_MASTER_KEY_LENGTH <= UINT8_MAX,
- "max secret is too large");
- ret->secret_length = static_cast<uint8_t>(CBS_len(&secret));
-
- CBS child;
+ CBS session_id, secret, child;
uint64_t timeout;
- if (!CBS_get_asn1(&session, &child, kTimeTag) ||
+ if (!CBS_get_asn1(&session, &session_id, CBS_ASN1_OCTETSTRING) ||
+ !ret->session_id.TryCopyFrom(session_id) ||
+ !CBS_get_asn1(&session, &secret, CBS_ASN1_OCTETSTRING) ||
+ !ret->secret.TryCopyFrom(secret) ||
+ !CBS_get_asn1(&session, &child, kTimeTag) ||
!CBS_get_asn1_uint64(&child, &ret->time) ||
!CBS_get_asn1(&session, &child, kTimeoutTag) ||
- !CBS_get_asn1_uint64(&child, &timeout) ||
+ !CBS_get_asn1_uint64(&child, &timeout) || //
timeout > UINT32_MAX) {
OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
return nullptr;
@@ -608,9 +580,10 @@
}
// |peer| is processed with the certificate chain.
- if (!SSL_SESSION_parse_bounded_octet_string(
- &session, ret->sid_ctx, &ret->sid_ctx_length, sizeof(ret->sid_ctx),
- kSessionIDContextTag) ||
+ CBS sid_ctx;
+ if (!CBS_get_optional_asn1_octet_string(
+ &session, &sid_ctx, /*out_present=*/nullptr, kSessionIDContextTag) ||
+ !ret->sid_ctx.TryCopyFrom(sid_ctx) ||
!SSL_SESSION_parse_long(&session, &ret->verify_result, kVerifyResultTag,
X509_V_OK)) {
return nullptr;
@@ -648,10 +621,11 @@
ret->peer_sha256_valid = false;
}
- if (!SSL_SESSION_parse_bounded_octet_string(
- &session, ret->original_handshake_hash,
- &ret->original_handshake_hash_len,
- sizeof(ret->original_handshake_hash), kOriginalHandshakeHashTag) ||
+ CBS original_handshake_hash;
+ if (!CBS_get_optional_asn1_octet_string(&session, &original_handshake_hash,
+ /*out_present=*/nullptr,
+ kOriginalHandshakeHashTag) ||
+ !ret->original_handshake_hash.TryCopyFrom(original_handshake_hash) ||
!SSL_SESSION_parse_crypto_buffer(&session,
&ret->signed_cert_timestamp_list,
kSignedCertTimestampListTag, pool) ||
diff --git a/ssl/ssl_cert.cc b/ssl/ssl_cert.cc
index e30e27d..bd0143e 100644
--- a/ssl/ssl_cert.cc
+++ b/ssl/ssl_cert.cc
@@ -165,9 +165,7 @@
ret->x509_method->cert_dup(ret.get(), cert);
- ret->sid_ctx_length = cert->sid_ctx_length;
- OPENSSL_memcpy(ret->sid_ctx, cert->sid_ctx, sizeof(ret->sid_ctx));
-
+ ret->sid_ctx = cert->sid_ctx;
return ret;
}
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 1458b45..c84c507 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -489,11 +489,8 @@
return false;
}
- *out_read_traffic_secret = Span<const uint8_t>(
- ssl->s3->read_traffic_secret, ssl->s3->read_traffic_secret_len);
- *out_write_traffic_secret = Span<const uint8_t>(
- ssl->s3->write_traffic_secret, ssl->s3->write_traffic_secret_len);
-
+ *out_read_traffic_secret = ssl->s3->read_traffic_secret;
+ *out_write_traffic_secret = ssl->s3->write_traffic_secret;
return true;
}
@@ -519,16 +516,11 @@
}
static uint32_t ssl_session_hash(const SSL_SESSION *sess) {
- return ssl_hash_session_id(
- MakeConstSpan(sess->session_id, sess->session_id_length));
+ return ssl_hash_session_id(sess->session_id);
}
static int ssl_session_cmp(const SSL_SESSION *a, const SSL_SESSION *b) {
- if (a->session_id_length != b->session_id_length) {
- return 1;
- }
-
- return OPENSSL_memcmp(a->session_id, b->session_id, a->session_id_length);
+ return MakeConstSpan(a->session_id) == b->session_id ? 0 : 1;
}
ssl_ctx_st::ssl_ctx_st(const SSL_METHOD *ssl_method)
@@ -1522,36 +1514,31 @@
// The tls-unique value is the first Finished message in the handshake, which
// is the client's in a full handshake and the server's for a resumption. See
// https://tools.ietf.org/html/rfc5929#section-3.1.
- const uint8_t *finished = ssl->s3->previous_client_finished;
- size_t finished_len = ssl->s3->previous_client_finished_len;
+ Span<const uint8_t> finished = ssl->s3->previous_client_finished;
if (ssl->session != NULL) {
// tls-unique is broken for resumed sessions unless EMS is used.
if (!ssl->session->extended_master_secret) {
return 0;
}
finished = ssl->s3->previous_server_finished;
- finished_len = ssl->s3->previous_server_finished_len;
}
- *out_len = finished_len;
- if (finished_len > max_out) {
+ *out_len = finished.size();
+ if (finished.size() > max_out) {
*out_len = max_out;
}
- OPENSSL_memcpy(out, finished, *out_len);
+ OPENSSL_memcpy(out, finished.data(), *out_len);
return 1;
}
static int set_session_id_context(CERT *cert, const uint8_t *sid_ctx,
- size_t sid_ctx_len) {
- if (sid_ctx_len > sizeof(cert->sid_ctx)) {
+ size_t sid_ctx_len) {
+ if (!cert->sid_ctx.TryCopyFrom(MakeConstSpan(sid_ctx, sid_ctx_len))) {
OPENSSL_PUT_ERROR(SSL, SSL_R_SSL_SESSION_ID_CONTEXT_TOO_LONG);
return 0;
}
- static_assert(sizeof(cert->sid_ctx) < 256, "sid_ctx too large");
- cert->sid_ctx_length = (uint8_t)sid_ctx_len;
- OPENSSL_memcpy(cert->sid_ctx, sid_ctx, sid_ctx_len);
return 1;
}
@@ -1574,8 +1561,8 @@
*out_len = 0;
return NULL;
}
- *out_len = ssl->config->cert->sid_ctx_length;
- return ssl->config->cert->sid_ctx;
+ *out_len = ssl->config->cert->sid_ctx.size();
+ return ssl->config->cert->sid_ctx.data();
}
int SSL_get_fd(const SSL *ssl) { return SSL_get_rfd(ssl); }
@@ -1650,13 +1637,12 @@
}
#endif // !OPENSSL_NO_SOCK
-static size_t copy_finished(void *out, size_t out_len, const uint8_t *in,
- size_t in_len) {
- if (out_len > in_len) {
- out_len = in_len;
+static size_t copy_finished(void *out, size_t out_len, Span<const uint8_t> in) {
+ if (out_len > in.size()) {
+ out_len = in.size();
}
- OPENSSL_memcpy(out, in, out_len);
- return in_len;
+ OPENSSL_memcpy(out, in.data(), out_len);
+ return in.size();
}
size_t SSL_get_finished(const SSL *ssl, void *buf, size_t count) {
@@ -1666,12 +1652,10 @@
}
if (ssl->server) {
- return copy_finished(buf, count, ssl->s3->previous_server_finished,
- ssl->s3->previous_server_finished_len);
+ return copy_finished(buf, count, ssl->s3->previous_server_finished);
}
- return copy_finished(buf, count, ssl->s3->previous_client_finished,
- ssl->s3->previous_client_finished_len);
+ return copy_finished(buf, count, ssl->s3->previous_client_finished);
}
size_t SSL_get_peer_finished(const SSL *ssl, void *buf, size_t count) {
@@ -1681,12 +1665,10 @@
}
if (ssl->server) {
- return copy_finished(buf, count, ssl->s3->previous_client_finished,
- ssl->s3->previous_client_finished_len);
+ return copy_finished(buf, count, ssl->s3->previous_client_finished);
}
- return copy_finished(buf, count, ssl->s3->previous_server_finished,
- ssl->s3->previous_server_finished_len);
+ return copy_finished(buf, count, ssl->s3->previous_server_finished);
}
int SSL_get_verify_mode(const SSL *ssl) {
diff --git a/ssl/ssl_session.cc b/ssl/ssl_session.cc
index 92bd822..f455b1b 100644
--- a/ssl/ssl_session.cc
+++ b/ssl/ssl_session.cc
@@ -197,12 +197,10 @@
new_session->is_server = session->is_server;
new_session->ssl_version = session->ssl_version;
new_session->is_quic = session->is_quic;
- new_session->sid_ctx_length = session->sid_ctx_length;
- OPENSSL_memcpy(new_session->sid_ctx, session->sid_ctx, session->sid_ctx_length);
+ new_session->sid_ctx = session->sid_ctx;
// Copy the key material.
- new_session->secret_length = session->secret_length;
- OPENSSL_memcpy(new_session->secret, session->secret, session->secret_length);
+ new_session->secret = session->secret;
new_session->cipher = session->cipher;
// Copy authentication state.
@@ -247,17 +245,9 @@
// Copy non-authentication connection properties.
if (dup_flags & SSL_SESSION_INCLUDE_NONAUTH) {
- new_session->session_id_length = session->session_id_length;
- OPENSSL_memcpy(new_session->session_id, session->session_id,
- session->session_id_length);
-
+ new_session->session_id = session->session_id;
new_session->group_id = session->group_id;
-
- OPENSSL_memcpy(new_session->original_handshake_hash,
- session->original_handshake_hash,
- session->original_handshake_hash_len);
- new_session->original_handshake_hash_len =
- session->original_handshake_hash_len;
+ new_session->original_handshake_hash = session->original_handshake_hash;
new_session->ticket_lifetime_hint = session->ticket_lifetime_hint;
new_session->ticket_age_add = session->ticket_age_add;
new_session->ticket_max_early_data = session->ticket_max_early_data;
@@ -383,13 +373,10 @@
session->auth_timeout = ssl->session_ctx->session_timeout;
}
- if (hs->config->cert->sid_ctx_length > sizeof(session->sid_ctx)) {
+ if (!session->sid_ctx.TryCopyFrom(hs->config->cert->sid_ctx)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return false;
}
- OPENSSL_memcpy(session->sid_ctx, hs->config->cert->sid_ctx,
- hs->config->cert->sid_ctx_length);
- session->sid_ctx_length = hs->config->cert->sid_ctx_length;
// The session is marked not resumable until it is completely filled in.
session->not_resumable = true;
@@ -580,13 +567,8 @@
bool ssl_session_is_context_valid(const SSL_HANDSHAKE *hs,
const SSL_SESSION *session) {
- if (session == NULL) {
- return false;
- }
-
- return session->sid_ctx_length == hs->config->cert->sid_ctx_length &&
- OPENSSL_memcmp(session->sid_ctx, hs->config->cert->sid_ctx,
- hs->config->cert->sid_ctx_length) == 0;
+ return session != nullptr &&
+ MakeConstSpan(session->sid_ctx) == hs->config->cert->sid_ctx;
}
bool ssl_session_is_time_valid(const SSL *ssl, const SSL_SESSION *session) {
@@ -655,9 +637,7 @@
auto cmp = [](const void *key, const SSL_SESSION *sess) -> int {
Span<const uint8_t> key_id =
*reinterpret_cast<const Span<const uint8_t> *>(key);
- Span<const uint8_t> sess_id =
- MakeConstSpan(sess->session_id, sess->session_id_length);
- return key_id == sess_id ? 0 : 1;
+ return key_id == sess->session_id ? 0 : 1;
};
MutexReadLock lock(&ssl->session_ctx->lock);
// |lh_SSL_SESSION_retrieve_key| returns a non-owning pointer.
@@ -752,7 +732,7 @@
}
static bool remove_session(SSL_CTX *ctx, SSL_SESSION *session, bool lock) {
- if (session == nullptr || session->session_id_length == 0) {
+ if (session == nullptr || session->session_id.empty()) {
return false;
}
@@ -971,21 +951,18 @@
const uint8_t *SSL_SESSION_get_id(const SSL_SESSION *session,
unsigned *out_len) {
if (out_len != NULL) {
- *out_len = session->session_id_length;
+ *out_len = session->session_id.size();
}
- return session->session_id;
+ return session->session_id.data();
}
int SSL_SESSION_set1_id(SSL_SESSION *session, const uint8_t *sid,
size_t sid_len) {
- if (sid_len > SSL_MAX_SSL_SESSION_ID_LENGTH) {
+ if (!session->session_id.TryCopyFrom(MakeConstSpan(sid, sid_len))) {
OPENSSL_PUT_ERROR(SSL, SSL_R_SSL_SESSION_ID_TOO_LONG);
return 0;
}
- // Use memmove in case someone passes in the output of |SSL_SESSION_get_id|.
- OPENSSL_memmove(session->session_id, sid, sid_len);
- session->session_id_length = sid_len;
return 1;
}
@@ -1035,14 +1012,13 @@
size_t SSL_SESSION_get_master_key(const SSL_SESSION *session, uint8_t *out,
size_t max_out) {
- // TODO(davidben): Fix secret_length's type and remove these casts.
if (max_out == 0) {
- return (size_t)session->secret_length;
+ return session->secret.size();
}
- if (max_out > (size_t)session->secret_length) {
- max_out = (size_t)session->secret_length;
+ if (max_out > session->secret.size()) {
+ max_out = session->secret.size();
}
- OPENSSL_memcpy(out, session->secret, max_out);
+ OPENSSL_memcpy(out, session->secret.data(), max_out);
return max_out;
}
@@ -1068,22 +1044,18 @@
const uint8_t *SSL_SESSION_get0_id_context(const SSL_SESSION *session,
unsigned *out_len) {
if (out_len != NULL) {
- *out_len = session->sid_ctx_length;
+ *out_len = session->sid_ctx.size();
}
- return session->sid_ctx;
+ return session->sid_ctx.data();
}
int SSL_SESSION_set1_id_context(SSL_SESSION *session, const uint8_t *sid_ctx,
size_t sid_ctx_len) {
- if (sid_ctx_len > sizeof(session->sid_ctx)) {
+ if (!session->sid_ctx.TryCopyFrom(MakeConstSpan(sid_ctx, sid_ctx_len))) {
OPENSSL_PUT_ERROR(SSL, SSL_R_SSL_SESSION_ID_CONTEXT_TOO_LONG);
return 0;
}
- static_assert(sizeof(session->sid_ctx) < 256, "sid_ctx_len does not fit");
- session->sid_ctx_length = (uint8_t)sid_ctx_len;
- OPENSSL_memcpy(session->sid_ctx, sid_ctx, sid_ctx_len);
-
return 1;
}
@@ -1093,7 +1065,7 @@
int SSL_SESSION_is_resumable(const SSL_SESSION *session) {
return !session->not_resumable &&
- (session->session_id_length != 0 || !session->ticket.empty());
+ (!session->session_id.empty() || !session->ticket.empty());
}
int SSL_SESSION_has_ticket(const SSL_SESSION *session) {
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index a51b594..46445bb 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -679,6 +679,140 @@
EXPECT_DEATH_IF_SUPPORTED(vec[10000], "");
}
+TEST(InplaceVector, Basic) {
+ InplaceVector<int, 4> vec;
+ EXPECT_TRUE(vec.empty());
+ EXPECT_EQ(0u, vec.size());
+ EXPECT_EQ(vec.begin(), vec.end());
+
+ int data3[] = {1, 2, 3};
+ ASSERT_TRUE(vec.TryCopyFrom(data3));
+ EXPECT_FALSE(vec.empty());
+ EXPECT_EQ(3u, vec.size());
+ auto iter = vec.begin();
+ EXPECT_EQ(1, vec[0]);
+ EXPECT_EQ(1, *iter);
+ iter++;
+ EXPECT_EQ(2, vec[1]);
+ EXPECT_EQ(2, *iter);
+ iter++;
+ EXPECT_EQ(3, vec[2]);
+ EXPECT_EQ(3, *iter);
+ iter++;
+ EXPECT_EQ(iter, vec.end());
+ EXPECT_EQ(MakeConstSpan(vec), MakeConstSpan(data3));
+
+ InplaceVector<int, 4> vec2 = vec;
+ EXPECT_EQ(MakeConstSpan(vec), MakeConstSpan(vec2));
+
+ InplaceVector<int, 4> vec3;
+ vec3 = vec;
+ EXPECT_EQ(MakeConstSpan(vec), MakeConstSpan(vec2));
+
+ int data4[] = {1, 2, 3, 4};
+ ASSERT_TRUE(vec.TryCopyFrom(data4));
+ EXPECT_EQ(MakeConstSpan(vec), MakeConstSpan(data4));
+
+ int data5[] = {1, 2, 3, 4, 5};
+ EXPECT_FALSE(vec.TryCopyFrom(data5));
+ EXPECT_FALSE(vec.TryResize(5));
+
+ // Shrink the vector.
+ ASSERT_TRUE(vec.TryResize(3));
+ EXPECT_EQ(MakeConstSpan(vec), MakeConstSpan(data3));
+
+ // Enlarge it again. The new value should have been value-initialized.
+ ASSERT_TRUE(vec.TryResize(4));
+ EXPECT_EQ(vec[3], 0);
+
+ // Self-assignment should not break the vector. Indirect through a pointer to
+ // avoid tripping a compiler warning.
+ vec.CopyFrom(data4);
+ const auto *ptr = &vec;
+ vec = *ptr;
+ EXPECT_EQ(MakeConstSpan(vec), MakeConstSpan(data4));
+}
+
+TEST(InplaceVectorTest, ComplexType) {
+ InplaceVector<std::vector<int>, 4> vec_of_vecs;
+ const std::vector<int> data[] = {{1, 2, 3}, {4, 5, 6}, {7, 8, 9}};
+ vec_of_vecs.CopyFrom(data);
+ EXPECT_EQ(MakeConstSpan(vec_of_vecs), MakeConstSpan(data));
+
+ vec_of_vecs.Resize(2);
+ EXPECT_EQ(MakeConstSpan(vec_of_vecs), MakeConstSpan(data, 2));
+
+ vec_of_vecs.Resize(4);
+ EXPECT_EQ(4u, vec_of_vecs.size());
+ EXPECT_EQ(vec_of_vecs[0], data[0]);
+ EXPECT_EQ(vec_of_vecs[1], data[1]);
+ EXPECT_TRUE(vec_of_vecs[2].empty());
+ EXPECT_TRUE(vec_of_vecs[3].empty());
+
+ // Copy-construction.
+ InplaceVector<std::vector<int>, 4> vec_of_vecs2 = vec_of_vecs;
+ EXPECT_EQ(4u, vec_of_vecs2.size());
+ EXPECT_EQ(vec_of_vecs2[0], data[0]);
+ EXPECT_EQ(vec_of_vecs2[1], data[1]);
+ EXPECT_TRUE(vec_of_vecs2[2].empty());
+ EXPECT_TRUE(vec_of_vecs2[3].empty());
+
+ // Copy-assignment.
+ InplaceVector<std::vector<int>, 4> vec_of_vecs3;
+ vec_of_vecs3 = vec_of_vecs;
+ EXPECT_EQ(4u, vec_of_vecs3.size());
+ EXPECT_EQ(vec_of_vecs3[0], data[0]);
+ EXPECT_EQ(vec_of_vecs3[1], data[1]);
+ EXPECT_TRUE(vec_of_vecs3[2].empty());
+ EXPECT_TRUE(vec_of_vecs3[3].empty());
+
+ // Move-construction.
+ InplaceVector<std::vector<int>, 4> vec_of_vecs4 = std::move(vec_of_vecs);
+ EXPECT_EQ(4u, vec_of_vecs4.size());
+ EXPECT_EQ(vec_of_vecs4[0], data[0]);
+ EXPECT_EQ(vec_of_vecs4[1], data[1]);
+ EXPECT_TRUE(vec_of_vecs4[2].empty());
+ EXPECT_TRUE(vec_of_vecs4[3].empty());
+
+ // The elements of the original vector should have been moved-from.
+ EXPECT_EQ(4u, vec_of_vecs.size());
+ for (const auto &vec : vec_of_vecs) {
+ EXPECT_TRUE(vec.empty());
+ }
+
+ // Move-assignment.
+ InplaceVector<std::vector<int>, 4> vec_of_vecs5;
+ vec_of_vecs5 = std::move(vec_of_vecs4);
+ EXPECT_EQ(4u, vec_of_vecs5.size());
+ EXPECT_EQ(vec_of_vecs5[0], data[0]);
+ EXPECT_EQ(vec_of_vecs5[1], data[1]);
+ EXPECT_TRUE(vec_of_vecs5[2].empty());
+ EXPECT_TRUE(vec_of_vecs5[3].empty());
+
+ // The elements of the original vector should have been moved-from.
+ EXPECT_EQ(4u, vec_of_vecs4.size());
+ for (const auto &vec : vec_of_vecs4) {
+ EXPECT_TRUE(vec.empty());
+ }
+}
+
+TEST(InplaceVectorDeathTest, BoundsChecks) {
+ InplaceVector<int, 4> vec;
+ // The vector is currently empty.
+ EXPECT_DEATH_IF_SUPPORTED(vec[0], "");
+ int data[] = {1, 2, 3};
+ vec.CopyFrom(data);
+ // Some more out-of-bounds elements.
+ EXPECT_DEATH_IF_SUPPORTED(vec[3], "");
+ EXPECT_DEATH_IF_SUPPORTED(vec[4], "");
+ EXPECT_DEATH_IF_SUPPORTED(vec[1000], "");
+ // The vector cannot be resized past the capacity.
+ EXPECT_DEATH_IF_SUPPORTED(vec.Resize(5), "");
+ EXPECT_DEATH_IF_SUPPORTED(vec.ResizeMaybeUninit(5), "");
+ int too_much_data[] = {1, 2, 3, 4, 5};
+ EXPECT_DEATH_IF_SUPPORTED(vec.CopyFrom(too_much_data), "");
+}
+
TEST(ReconstructSeqnumTest, Increment) {
// Test simple cases from the beginning of an epoch with both 8- and 16-bit
// wire sequence numbers.
diff --git a/ssl/ssl_transcript.cc b/ssl/ssl_transcript.cc
index 58fd21e..239363d 100644
--- a/ssl/ssl_transcript.cc
+++ b/ssl/ssl_transcript.cc
@@ -259,8 +259,7 @@
}
static const size_t kFinishedLen = 12;
- if (!tls1_prf(Digest(), MakeSpan(out, kFinishedLen),
- MakeConstSpan(session->secret, session->secret_length), label,
+ if (!tls1_prf(Digest(), MakeSpan(out, kFinishedLen), session->secret, label,
MakeConstSpan(digest, digest_len), {})) {
return false;
}
diff --git a/ssl/t1_enc.cc b/ssl/t1_enc.cc
index 1895bac..360c855 100644
--- a/ssl/t1_enc.cc
+++ b/ssl/t1_enc.cc
@@ -191,14 +191,13 @@
static bool generate_key_block(const SSL *ssl, Span<uint8_t> out,
const SSL_SESSION *session) {
- auto secret = MakeConstSpan(session->secret, session->secret_length);
static const char kLabel[] = "key expansion";
auto label = MakeConstSpan(kLabel, sizeof(kLabel) - 1);
const EVP_MD *digest = ssl_session_get_digest(session);
// Note this function assumes that |session|'s key material corresponds to
// |ssl->s3->client_random| and |ssl->s3->server_random|.
- return tls1_prf(digest, out, secret, label, ssl->s3->server_random,
+ return tls1_prf(digest, out, session->secret, label, ssl->s3->server_random,
ssl->s3->client_random);
}
@@ -266,33 +265,33 @@
ssl_handshake_session(hs), {});
}
-int tls1_generate_master_secret(SSL_HANDSHAKE *hs, uint8_t *out,
- Span<const uint8_t> premaster) {
+bool tls1_generate_master_secret(SSL_HANDSHAKE *hs, Span<uint8_t> out,
+ Span<const uint8_t> premaster) {
static const char kMasterSecretLabel[] = "master secret";
static const char kExtendedMasterSecretLabel[] = "extended master secret";
+ BSSL_CHECK(out.size() == SSL3_MASTER_SECRET_SIZE);
const SSL *ssl = hs->ssl;
- auto out_span = MakeSpan(out, SSL3_MASTER_SECRET_SIZE);
if (hs->extended_master_secret) {
auto label = MakeConstSpan(kExtendedMasterSecretLabel,
sizeof(kExtendedMasterSecretLabel) - 1);
uint8_t digests[EVP_MAX_MD_SIZE];
size_t digests_len;
if (!hs->transcript.GetHash(digests, &digests_len) ||
- !tls1_prf(hs->transcript.Digest(), out_span, premaster, label,
+ !tls1_prf(hs->transcript.Digest(), out, premaster, label,
MakeConstSpan(digests, digests_len), {})) {
- return 0;
+ return false;
}
} else {
auto label =
MakeConstSpan(kMasterSecretLabel, sizeof(kMasterSecretLabel) - 1);
- if (!tls1_prf(hs->transcript.Digest(), out_span, premaster, label,
+ if (!tls1_prf(hs->transcript.Digest(), out, premaster, label,
ssl->s3->client_random, ssl->s3->server_random)) {
- return 0;
+ return false;
}
}
- return SSL3_MASTER_SECRET_SIZE;
+ return true;
}
BSSL_NAMESPACE_END
@@ -334,7 +333,7 @@
int use_context) {
// In TLS 1.3, the exporter may be used whenever the secret has been derived.
if (ssl->s3->version != 0 && ssl_protocol_version(ssl) >= TLS1_3_VERSION) {
- if (ssl->s3->exporter_secret_len == 0) {
+ if (ssl->s3->exporter_secret.empty()) {
OPENSSL_PUT_ERROR(SSL, SSL_R_HANDSHAKE_NOT_COMPLETE);
return 0;
}
@@ -343,8 +342,7 @@
context_len = 0;
}
return tls13_export_keying_material(
- ssl, MakeSpan(out, out_len),
- MakeConstSpan(ssl->s3->exporter_secret, ssl->s3->exporter_secret_len),
+ ssl, MakeSpan(out, out_len), ssl->s3->exporter_secret,
MakeConstSpan(label, label_len), MakeConstSpan(context, context_len));
}
@@ -379,7 +377,6 @@
const SSL_SESSION *session = SSL_get_session(ssl);
const EVP_MD *digest = ssl_session_get_digest(session);
- return tls1_prf(digest, MakeSpan(out, out_len),
- MakeConstSpan(session->secret, session->secret_length),
+ return tls1_prf(digest, MakeSpan(out, out_len), session->secret,
MakeConstSpan(label, label_len), seed, {});
}
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc
index fd9ab0a..76f970c 100644
--- a/ssl/tls13_client.cc
+++ b/ssl/tls13_client.cc
@@ -109,24 +109,21 @@
if (!ssl_parse_server_hello(out, out_alert, msg)) {
return false;
}
- uint16_t server_hello_version = TLS1_2_VERSION;
- if (SSL_is_dtls(hs->ssl)) {
- server_hello_version = DTLS1_2_VERSION;
- }
+ uint16_t expected_version =
+ SSL_is_dtls(hs->ssl) ? DTLS1_2_VERSION : TLS1_2_VERSION;
// DTLS 1.3 disables "compatibility mode" (RFC 8446, appendix D.4). When
// disabled, servers MUST NOT echo the legacy_session_id (RFC 9147, section
// 5). The client could have sent a session ID indicating its willingness to
// resume a DTLS 1.2 session, so just checking that the session IDs match is
// incorrect.
- bool session_id_match =
- (SSL_is_dtls(hs->ssl) && CBS_len(&out->session_id) == 0) ||
- (!SSL_is_dtls(hs->ssl) &&
- CBS_mem_equal(&out->session_id, hs->session_id, hs->session_id_len));
+ Span<const uint8_t> expected_session_id = SSL_is_dtls(hs->ssl)
+ ? Span<const uint8_t>()
+ : MakeConstSpan(hs->session_id);
- // The RFC8446 version of the structure fixes some legacy values.
- // Additionally, the session ID must echo the original one.
- if (out->legacy_version != server_hello_version ||
- out->compression_method != 0 || !session_id_match ||
+ // RFC 8446 fixes some legacy values. Check them.
+ if (out->legacy_version != expected_version || //
+ out->compression_method != 0 ||
+ Span<const uint8_t>(out->session_id) != expected_session_id ||
CBS_len(&out->extensions) == 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
*out_alert = SSL_AD_DECODE_ERROR;
@@ -497,11 +494,9 @@
// Set up the key schedule and incorporate the PSK into the running secret.
size_t hash_len = EVP_MD_size(
ssl_get_handshake_digest(ssl_protocol_version(ssl), hs->new_cipher));
- if (!tls13_init_key_schedule(
- hs, ssl->s3->session_reused
- ? MakeConstSpan(hs->new_session->secret,
- hs->new_session->secret_length)
- : MakeConstSpan(kZeroes, hash_len))) {
+ if (!tls13_init_key_schedule(hs, ssl->s3->session_reused
+ ? MakeConstSpan(hs->new_session->secret)
+ : MakeConstSpan(kZeroes, hash_len))) {
return ssl_hs_error;
}
@@ -1166,8 +1161,8 @@
// Historically, OpenSSL filled in fake session IDs for ticket-based sessions.
// Envoy's tests depend on this, although perhaps they shouldn't.
- SHA256(CBS_data(&ticket), CBS_len(&ticket), session->session_id);
- session->session_id_length = SHA256_DIGEST_LENGTH;
+ session->session_id.ResizeMaybeUninit(SHA256_DIGEST_LENGTH);
+ SHA256(CBS_data(&ticket), CBS_len(&ticket), session->session_id.data());
session->ticket_age_add_valid = true;
session->not_resumable = false;
diff --git a/ssl/tls13_enc.cc b/ssl/tls13_enc.cc
index fad4117..f601b1a 100644
--- a/ssl/tls13_enc.cc
+++ b/ssl/tls13_enc.cc
@@ -83,9 +83,7 @@
return init_key_schedule(hs, transcript,
ssl_session_protocol_version(session),
session->cipher) &&
- hkdf_extract_to_secret(
- hs, *transcript,
- MakeConstSpan(session->secret, session->secret_length));
+ hkdf_extract_to_secret(hs, *transcript, session->secret);
}
static Span<const char> label_to_span(const char *label) {
@@ -249,17 +247,13 @@
secret_for_quic)) {
return false;
}
- OPENSSL_memmove(ssl->s3->read_traffic_secret, traffic_secret.data(),
- traffic_secret.size());
- ssl->s3->read_traffic_secret_len = traffic_secret.size();
+ ssl->s3->read_traffic_secret.CopyFrom(traffic_secret);
} else {
if (!ssl->method->set_write_state(ssl, level, std::move(traffic_aead),
secret_for_quic)) {
return false;
}
- OPENSSL_memmove(ssl->s3->write_traffic_secret, traffic_secret.data(),
- traffic_secret.size());
- ssl->s3->write_traffic_secret_len = traffic_secret.size();
+ ssl->s3->write_traffic_secret.CopyFrom(traffic_secret);
}
return true;
@@ -309,7 +303,6 @@
bool tls13_derive_application_secrets(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
- ssl->s3->exporter_secret_len = hs->transcript.DigestLen();
if (!derive_secret(hs, hs->client_traffic_secret_0(),
label_to_span(kTLS13LabelClientApplicationTraffic)) ||
!ssl_log_secret(ssl, "CLIENT_TRAFFIC_SECRET_0",
@@ -317,13 +310,13 @@
!derive_secret(hs, hs->server_traffic_secret_0(),
label_to_span(kTLS13LabelServerApplicationTraffic)) ||
!ssl_log_secret(ssl, "SERVER_TRAFFIC_SECRET_0",
- hs->server_traffic_secret_0()) ||
- !derive_secret(
- hs, MakeSpan(ssl->s3->exporter_secret, ssl->s3->exporter_secret_len),
- label_to_span(kTLS13LabelExporter)) ||
- !ssl_log_secret(ssl, "EXPORTER_SECRET",
- MakeConstSpan(ssl->s3->exporter_secret,
- ssl->s3->exporter_secret_len))) {
+ hs->server_traffic_secret_0())) {
+ return false;
+ }
+ ssl->s3->exporter_secret.ResizeMaybeUninit(hs->transcript.DigestLen());
+ if (!derive_secret(hs, MakeSpan(ssl->s3->exporter_secret),
+ label_to_span(kTLS13LabelExporter)) ||
+ !ssl_log_secret(ssl, "EXPORTER_SECRET", ssl->s3->exporter_secret)) {
return false;
}
@@ -333,14 +326,9 @@
static const char kTLS13LabelApplicationTraffic[] = "traffic upd";
bool tls13_rotate_traffic_key(SSL *ssl, enum evp_aead_direction_t direction) {
- Span<uint8_t> secret;
- if (direction == evp_aead_open) {
- secret = MakeSpan(ssl->s3->read_traffic_secret,
- ssl->s3->read_traffic_secret_len);
- } else {
- secret = MakeSpan(ssl->s3->write_traffic_secret,
- ssl->s3->write_traffic_secret_len);
- }
+ Span<uint8_t> secret = direction == evp_aead_open
+ ? MakeSpan(ssl->s3->read_traffic_secret)
+ : MakeSpan(ssl->s3->write_traffic_secret);
const SSL_SESSION *session = SSL_get_session(ssl);
const EVP_MD *digest = ssl_session_get_digest(session);
@@ -354,14 +342,9 @@
static const char kTLS13LabelResumption[] = "res master";
bool tls13_derive_resumption_secret(SSL_HANDSHAKE *hs) {
- if (hs->transcript.DigestLen() > SSL_MAX_MASTER_KEY_LENGTH) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
- return false;
- }
- hs->new_session->secret_length = hs->transcript.DigestLen();
- return derive_secret(
- hs, MakeSpan(hs->new_session->secret, hs->new_session->secret_length),
- label_to_span(kTLS13LabelResumption));
+ hs->new_session->secret.ResizeMaybeUninit(hs->transcript.DigestLen());
+ return derive_secret(hs, MakeSpan(hs->new_session->secret),
+ label_to_span(kTLS13LabelResumption));
}
static const char kTLS13LabelFinished[] = "finished";
@@ -410,8 +393,8 @@
const EVP_MD *digest = ssl_session_get_digest(session);
// The session initially stores the resumption_master_secret, which we
// override with the PSK.
- auto session_secret = MakeSpan(session->secret, session->secret_length);
- return hkdf_expand_label(session_secret, digest, session_secret,
+ assert(session->secret.size() == EVP_MD_size(digest));
+ return hkdf_expand_label(MakeSpan(session->secret), digest, session->secret,
label_to_span(kTLS13LabelResumptionPSK), nonce,
is_dtls);
}
@@ -473,8 +456,9 @@
auto binder_key = MakeSpan(binder_key_buf, EVP_MD_size(digest));
if (!EVP_Digest(nullptr, 0, binder_context, &binder_context_len, digest,
nullptr) ||
- !HKDF_extract(early_secret, &early_secret_len, digest, session->secret,
- session->secret_length, nullptr, 0) ||
+ !HKDF_extract(early_secret, &early_secret_len, digest,
+ session->secret.data(), session->secret.size(), nullptr,
+ 0) ||
!hkdf_expand_label(
binder_key, digest, MakeConstSpan(early_secret, early_secret_len),
label_to_span(kTLS13LabelPSKBinder),
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index e163d70..655141f 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -249,9 +249,8 @@
// 'legacy_session_id' value from the client" (RFC 9147, section 5) as it
// would in a TLS 1.3 handshake.
if (!SSL_is_dtls(ssl)) {
- OPENSSL_memcpy(hs->session_id, client_hello.session_id,
- client_hello.session_id_len);
- hs->session_id_len = client_hello.session_id_len;
+ hs->session_id.CopyFrom(
+ MakeConstSpan(client_hello.session_id, client_hello.session_id_len));
}
Array<SSL_CREDENTIAL *> creds;
@@ -561,11 +560,9 @@
ssl_get_handshake_digest(ssl_protocol_version(ssl), hs->new_cipher));
// Set up the key schedule and incorporate the PSK into the running secret.
- if (!tls13_init_key_schedule(
- hs, ssl->s3->session_reused
- ? MakeConstSpan(hs->new_session->secret,
- hs->new_session->secret_length)
- : MakeConstSpan(kZeroes, hash_len)) ||
+ if (!tls13_init_key_schedule(hs, ssl->s3->session_reused
+ ? MakeConstSpan(hs->new_session->secret)
+ : MakeConstSpan(kZeroes, hash_len)) ||
!ssl_hash_message(hs, msg)) {
return ssl_hs_error;
}
@@ -609,7 +606,8 @@
!CBB_add_u16(&body, TLS1_2_VERSION) ||
!CBB_add_bytes(&body, kHelloRetryRequest, SSL3_RANDOM_SIZE) ||
!CBB_add_u8_length_prefixed(&body, &session_id) ||
- !CBB_add_bytes(&session_id, hs->session_id, hs->session_id_len) ||
+ !CBB_add_bytes(&session_id, hs->session_id.data(),
+ hs->session_id.size()) ||
!CBB_add_u16(&body, SSL_CIPHER_get_protocol_id(hs->new_cipher)) ||
!CBB_add_u8(&body, 0 /* no compression */) ||
!CBB_add_u16_length_prefixed(&body, &extensions) ||
@@ -810,7 +808,8 @@
!CBB_add_bytes(&body, ssl->s3->server_random,
sizeof(ssl->s3->server_random)) ||
!CBB_add_u8_length_prefixed(&body, &session_id) ||
- !CBB_add_bytes(&session_id, hs->session_id, hs->session_id_len) ||
+ !CBB_add_bytes(&session_id, hs->session_id.data(),
+ hs->session_id.size()) ||
!CBB_add_u16(&body, SSL_CIPHER_get_protocol_id(hs->new_cipher)) ||
!CBB_add_u8(&body, 0) ||
!CBB_add_u16_length_prefixed(&body, &extensions) ||
