Skip to content

Commit

Permalink
Avoid an allocation when deriving the DTLS record number key
Browse files Browse the repository at this point in the history
We really should make like a bounded array helper type. Maybe call it
bssl::InplaceVector to match C++26's upcoming name.

Bug: 42290594
Change-Id: I5d03a867fd1bae5e3ecca04d91a3d0900fdeac1b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71648
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
  • Loading branch information
davidben authored and Boringssl LUCI CQ committed Sep 27, 2024
1 parent 58e5330 commit 37571ee
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 21 deletions.
5 changes: 5 additions & 0 deletions include/openssl/span.h
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,11 @@ constexpr auto MakeSpan(C &c) -> decltype(MakeSpan(c.data(), c.size())) {
return MakeSpan(c.data(), c.size());
}

template <typename T, size_t N>
constexpr Span<T> MakeSpan(T (&array)[N]) {
return Span<T>(array, N);
}

template <typename T>
constexpr Span<const T> MakeConstSpan(T *ptr, size_t size) {
return Span<const T>(ptr, size);
Expand Down
1 change: 1 addition & 0 deletions ssl/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,7 @@ class RecordNumberEncrypter {
public:
virtual ~RecordNumberEncrypter() = default;
static constexpr bool kAllowUniquePtr = true;
static constexpr size_t kMaxKeySize = 32;

virtual size_t KeySize() = 0;
virtual bool SetKey(Span<const uint8_t> key) = 0;
Expand Down
34 changes: 13 additions & 21 deletions ssl/tls13_enc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -203,27 +203,19 @@ bool tls13_set_traffic_key(SSL *ssl, enum ssl_encryption_level_t level,
return false;
}

// Derive the key.
size_t key_len = EVP_AEAD_key_length(aead);
uint8_t key_buf[EVP_AEAD_MAX_KEY_LENGTH];
auto key = MakeSpan(key_buf, key_len);
// Derive the key and IV.
uint8_t key_buf[EVP_AEAD_MAX_KEY_LENGTH], iv_buf[EVP_AEAD_MAX_NONCE_LENGTH];
auto key = MakeSpan(key_buf).first(EVP_AEAD_key_length(aead));
auto iv = MakeSpan(iv_buf).first(EVP_AEAD_nonce_length(aead));
if (!hkdf_expand_label(key, digest, traffic_secret, label_to_span("key"),
{}, is_dtls)) {
return false;
}

// Derive the IV.
size_t iv_len = EVP_AEAD_nonce_length(aead);
uint8_t iv_buf[EVP_AEAD_MAX_NONCE_LENGTH];
auto iv = MakeSpan(iv_buf, iv_len);
if (!hkdf_expand_label(iv, digest, traffic_secret, label_to_span("iv"), {},
{}, is_dtls) ||
!hkdf_expand_label(iv, digest, traffic_secret, label_to_span("iv"), {},
is_dtls)) {
return false;
}

traffic_aead =
SSLAEADContext::Create(direction, session->ssl_version, is_dtls,
session->cipher, key, Span<const uint8_t>(), iv);
traffic_aead = SSLAEADContext::Create(
direction, session->ssl_version, is_dtls, session->cipher, key, {}, iv);
}

if (!traffic_aead) {
Expand All @@ -236,11 +228,11 @@ bool tls13_set_traffic_key(SSL *ssl, enum ssl_encryption_level_t level,
if (!rn_encrypter) {
return false;
}
Array<uint8_t> rne_key;
if (!rne_key.Init(rn_encrypter->KeySize()) ||
!hkdf_expand_label(MakeSpan(rne_key), digest, traffic_secret,
label_to_span("sn"), {}, is_dtls) ||
!rn_encrypter->SetKey(MakeSpan(rne_key))) {
uint8_t rne_key_buf[RecordNumberEncrypter::kMaxKeySize];
auto rne_key = MakeSpan(rne_key_buf).first(rn_encrypter->KeySize());
if (!hkdf_expand_label(rne_key, digest, traffic_secret, label_to_span("sn"),
{}, is_dtls) ||
!rn_encrypter->SetKey(rne_key)) {
return false;
}
}
Expand Down

0 comments on commit 37571ee

Please sign in to comment.