Add Span::first() and Span::last().
absl::Span, base::span, and std::span have first() and last() methods
which give prefixes and suffixes. first() just saves 5 characters, but
last() is nicer to write than subspan() for suffixes.
Unlike subspan(), they also do not have clipping behavior, so we're
guaranteed the length is correct. The clipping behavior comes from
absl::Span::subspan() and is not present in std::span or base::span.
I've left it in, in case we switch to absl::Span in the future, but I
imagine absl::Span will need to migrate this at some point.
Change-Id: I042dd6c566b6d753ec6de9d84e8c09ac7c270267
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48905
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/span.h b/include/openssl/span.h
index 7410bf9..6b1b232 100644
--- a/include/openssl/span.h
+++ b/include/openssl/span.h
@@ -158,11 +158,30 @@
Span subspan(size_t pos = 0, size_t len = npos) const {
if (pos > size_) {
- abort(); // absl::Span throws an exception here.
+ // absl::Span throws an exception here. Note std::span and Chromium
+ // base::span additionally forbid pos + len being out of range, with a
+ // special case at npos/dynamic_extent, while absl::Span::subspan clips
+ // the span. For now, we align with absl::Span in case we switch to it in
+ // the future.
+ abort();
}
return Span(data_ + pos, std::min(size_ - pos, len));
}
+ Span first(size_t len) {
+ if (len > size_) {
+ abort();
+ }
+ return Span(data_, len);
+ }
+
+ Span last(size_t len) {
+ if (len > size_) {
+ abort();
+ }
+ return Span(data_ + size_ - len, len);
+ }
+
private:
T *data_;
size_t size_;
diff --git a/ssl/encrypted_client_hello.cc b/ssl/encrypted_client_hello.cc
index b70f66c..f5d02fd 100644
--- a/ssl/encrypted_client_hello.cc
+++ b/ssl/encrypted_client_hello.cc
@@ -857,22 +857,24 @@
// Construct ClientHelloInner and EncodedClientHelloInner. See
// draft-ietf-tls-esni-10, sections 5.1 and 6.1.
- bssl::ScopedCBB cbb, encoded;
+ ScopedCBB cbb, encoded_cbb;
CBB body;
bool needs_psk_binder;
- bssl::Array<uint8_t> hello_inner;
+ Array<uint8_t> hello_inner, encoded;
if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_CLIENT_HELLO) ||
- !CBB_init(encoded.get(), 256) ||
+ !CBB_init(encoded_cbb.get(), 256) ||
!ssl_write_client_hello_without_extensions(hs, &body,
ssl_client_hello_inner,
/*empty_session_id=*/false) ||
- !ssl_write_client_hello_without_extensions(hs, encoded.get(),
+ !ssl_write_client_hello_without_extensions(hs, encoded_cbb.get(),
ssl_client_hello_inner,
/*empty_session_id=*/true) ||
- !ssl_add_clienthello_tlsext(hs, &body, encoded.get(), &needs_psk_binder,
- ssl_client_hello_inner, CBB_len(&body),
+ !ssl_add_clienthello_tlsext(hs, &body, encoded_cbb.get(),
+ &needs_psk_binder, ssl_client_hello_inner,
+ CBB_len(&body),
/*omit_ech_len=*/0) ||
- !ssl->method->finish_message(ssl, cbb.get(), &hello_inner)) {
+ !ssl->method->finish_message(ssl, cbb.get(), &hello_inner) ||
+ !CBBFinishArray(encoded_cbb.get(), &encoded)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return false;
}
@@ -884,13 +886,9 @@
return false;
}
// Also update the EncodedClientHelloInner.
- if (CBB_len(encoded.get()) < binder_len) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
- return false;
- }
- OPENSSL_memcpy(const_cast<uint8_t *>(CBB_data(encoded.get())) +
- CBB_len(encoded.get()) - binder_len,
- hello_inner.data() + hello_inner.size() - binder_len,
+ auto encoded_binder = MakeSpan(encoded).last(binder_len);
+ auto hello_inner_binder = MakeConstSpan(hello_inner).last(binder_len);
+ OPENSSL_memcpy(encoded_binder.data(), hello_inner_binder.data(),
binder_len);
}
@@ -905,7 +903,7 @@
const EVP_HPKE_KDF *kdf = EVP_HPKE_CTX_kdf(hs->ech_hpke_ctx.get());
const EVP_HPKE_AEAD *aead = EVP_HPKE_CTX_aead(hs->ech_hpke_ctx.get());
const size_t extension_len =
- compute_extension_length(aead, enc.size(), CBB_len(encoded.get()));
+ compute_extension_length(aead, enc.size(), encoded.size());
bssl::ScopedCBB aad;
CBB outer_hello;
CBB enc_cbb;
@@ -943,19 +941,17 @@
}
#if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
// In fuzzer mode, the server expects a cleartext payload.
- if (!CBB_add_bytes(&payload_cbb, CBB_data(encoded.get()),
- CBB_len(encoded.get()))) {
+ if (!CBB_add_bytes(&payload_cbb, encoded.data(), encoded.size())) {
return false;
}
#else
uint8_t *payload;
size_t payload_len =
- CBB_len(encoded.get()) + EVP_AEAD_max_overhead(EVP_HPKE_AEAD_aead(aead));
+ encoded.size() + EVP_AEAD_max_overhead(EVP_HPKE_AEAD_aead(aead));
if (!CBB_reserve(&payload_cbb, &payload, payload_len) ||
!EVP_HPKE_CTX_seal(hs->ech_hpke_ctx.get(), payload, &payload_len,
- payload_len, CBB_data(encoded.get()),
- CBB_len(encoded.get()), CBB_data(aad.get()),
- CBB_len(aad.get())) ||
+ payload_len, encoded.data(), encoded.size(),
+ CBB_data(aad.get()), CBB_len(aad.get())) ||
!CBB_did_write(&payload_cbb, payload_len)) {
return false;
}
diff --git a/ssl/extensions.cc b/ssl/extensions.cc
index 4950d26..48fc5c5 100644
--- a/ssl/extensions.cc
+++ b/ssl/extensions.cc
@@ -3880,8 +3880,8 @@
return ssl_ticket_aead_ignore_ticket;
}
// Split the ticket into the ticket and the MAC.
- auto ticket_mac = ticket.subspan(ticket.size() - mac_len);
- ticket = ticket.subspan(0, ticket.size() - mac_len);
+ auto ticket_mac = ticket.last(mac_len);
+ ticket = ticket.first(ticket.size() - mac_len);
HMAC_Update(hmac_ctx, ticket.data(), ticket.size());
HMAC_Final(hmac_ctx, mac, NULL);
assert(mac_len == ticket_mac.size());
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc
index c8a23a1..29fc3a4 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -973,8 +973,7 @@
}
static void copy_suffix(Span<uint8_t> out, Span<const uint8_t> in) {
- out = out.subspan(out.size() - in.size());
- assert(out.size() == in.size());
+ out = out.last(in.size());
OPENSSL_memcpy(out.data(), in.data(), in.size());
}
diff --git a/ssl/tls13_enc.cc b/ssl/tls13_enc.cc
index 174f5f1..9c54a4d 100644
--- a/ssl/tls13_enc.cc
+++ b/ssl/tls13_enc.cc
@@ -489,8 +489,8 @@
return false;
}
- OPENSSL_memcpy(msg.data() + msg.size() - verify_data_len, verify_data,
- verify_data_len);
+ auto msg_binder = msg.last(verify_data_len);
+ OPENSSL_memcpy(msg_binder.data(), verify_data, verify_data_len);
if (out_binder_len != nullptr) {
*out_binder_len = verify_data_len;
}
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index 501d01f..72ec4a1 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -764,8 +764,7 @@
assert(ssl->s3->ech_status != ssl_ech_accepted || hs->ech_is_inner_present);
if (hs->ech_is_inner_present) {
// Fill in the ECH confirmation signal.
- Span<uint8_t> random_suffix =
- random.subspan(SSL3_RANDOM_SIZE - ECH_CONFIRMATION_SIGNAL_LEN);
+ Span<uint8_t> random_suffix = random.last(ECH_CONFIRMATION_SIGNAL_LEN);
if (!ssl_ech_accept_confirmation(hs, random_suffix, hs->transcript,
server_hello)) {
return ssl_hs_error;
