Skip to content

Commit

Permalink
Use spans for the various TLS 1.3 secrets.
Browse files Browse the repository at this point in the history 
This undoes a lot of the MakeConstSpans and MakeSpans that were just
added, though it does require a bit of helper machinery. This should
make us much more consistent about which buffer is sized with which size
(even though they are secretly all the same size).

Change-Id: I772ffd2e69141ff20511bcd3add865afa82cf3a0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/37127
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
  • Loading branch information
@davidben
davidben authored and CQ bot account: commit-bot@chromium.org committed Aug 20, 2019
1 parent b244e3a commit e530ea3
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 152 deletions.
7 changes: 7 additions & 0 deletions ssl/handshake.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,13 @@ SSL_HANDSHAKE::~SSL_HANDSHAKE() {
ssl->ctx->x509_method->hs_flush_cached_ca_names(this);
}

void SSL_HANDSHAKE::ResizeSecrets(size_t hash_len) {
if (hash_len > SSL_MAX_MD_SIZE) {
abort();
}
hash_len_ = hash_len;
}

UniquePtr<SSL_HANDSHAKE> ssl_handshake_new(SSL *ssl) {
UniquePtr<SSL_HANDSHAKE> hs = MakeUnique<SSL_HANDSHAKE>(ssl);
if (!hs || !hs->transcript.Init()) {
Expand Down
5 changes: 2 additions & 3 deletions ssl/handshake_client.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -460,9 +460,8 @@ static enum ssl_hs_wait_t do_enter_early_data(SSL_HANDSHAKE *hs) {
hs, MakeConstSpan(ssl->session->master_key,
ssl->session->master_key_length)) ||
!tls13_derive_early_secrets(hs) ||
!tls13_set_traffic_key(
ssl, ssl_encryption_early_data, evp_aead_seal,
MakeConstSpan(hs->early_traffic_secret, hs->hash_len))) {
!tls13_set_traffic_key(ssl, ssl_encryption_early_data, evp_aead_seal,
hs->early_traffic_secret())) {
return ssl_hs_error;
}

Expand Down
40 changes: 32 additions & 8 deletions ssl/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1452,14 +1452,38 @@ struct SSL_HANDSHAKE {
// |SSL_OP_NO_*| and |SSL_CTX_set_max_proto_version| APIs.
uint16_t max_version = 0;

size_t hash_len = 0;
uint8_t secret[SSL_MAX_MD_SIZE] = {0};
uint8_t early_traffic_secret[SSL_MAX_MD_SIZE] = {0};
uint8_t client_handshake_secret[SSL_MAX_MD_SIZE] = {0};
uint8_t server_handshake_secret[SSL_MAX_MD_SIZE] = {0};
uint8_t client_traffic_secret_0[SSL_MAX_MD_SIZE] = {0};
uint8_t server_traffic_secret_0[SSL_MAX_MD_SIZE] = {0};
uint8_t expected_client_finished[SSL_MAX_MD_SIZE] = {0};
private:
size_t hash_len_ = 0;
uint8_t secret_[SSL_MAX_MD_SIZE] = {0};
uint8_t early_traffic_secret_[SSL_MAX_MD_SIZE] = {0};
uint8_t client_handshake_secret_[SSL_MAX_MD_SIZE] = {0};
uint8_t server_handshake_secret_[SSL_MAX_MD_SIZE] = {0};
uint8_t client_traffic_secret_0_[SSL_MAX_MD_SIZE] = {0};
uint8_t server_traffic_secret_0_[SSL_MAX_MD_SIZE] = {0};
uint8_t expected_client_finished_[SSL_MAX_MD_SIZE] = {0};

public:
void ResizeSecrets(size_t hash_len);

Span<uint8_t> secret() { return MakeSpan(secret_, hash_len_); }
Span<uint8_t> early_traffic_secret() {
return MakeSpan(early_traffic_secret_, hash_len_);
}
Span<uint8_t> client_handshake_secret() {
return MakeSpan(client_handshake_secret_, hash_len_);
}
Span<uint8_t> server_handshake_secret() {
return MakeSpan(server_handshake_secret_, hash_len_);
}
Span<uint8_t> client_traffic_secret_0() {
return MakeSpan(client_traffic_secret_0_, hash_len_);
}
Span<uint8_t> server_traffic_secret_0() {
return MakeSpan(server_traffic_secret_0_, hash_len_);
}
Span<uint8_t> expected_client_finished() {
return MakeSpan(expected_client_finished_, hash_len_);
}

union {
// sent is a bitset where the bits correspond to elements of kExtensions
Expand Down
15 changes: 7 additions & 8 deletions ssl/tls13_both.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,21 +384,20 @@ bool tls13_process_finished(SSL_HANDSHAKE *hs, const SSLMessage &msg,
bool use_saved_value) {
SSL *const ssl = hs->ssl;
uint8_t verify_data_buf[EVP_MAX_MD_SIZE];
const uint8_t *verify_data;
size_t verify_data_len;
Span<const uint8_t> verify_data;
if (use_saved_value) {
assert(ssl->server);
verify_data = hs->expected_client_finished;
verify_data_len = hs->hash_len;
verify_data = hs->expected_client_finished();
} else {
if (!tls13_finished_mac(hs, verify_data_buf, &verify_data_len,
!ssl->server)) {
size_t len;
if (!tls13_finished_mac(hs, verify_data_buf, &len, !ssl->server)) {
return false;
}
verify_data = verify_data_buf;
verify_data = MakeConstSpan(verify_data_buf, len);
}

bool finished_ok = CBS_mem_equal(&msg.body, verify_data, verify_data_len);
bool finished_ok =
CBS_mem_equal(&msg.body, verify_data.data(), verify_data.size());
#if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
finished_ok = true;
#endif
Expand Down
28 changes: 12 additions & 16 deletions ssl/tls13_client.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,18 +393,16 @@ static enum ssl_hs_wait_t do_read_server_hello(SSL_HANDSHAKE *hs) {
if (!tls13_advance_key_schedule(hs, dhe_secret) ||
!ssl_hash_message(hs, msg) ||
!tls13_derive_handshake_secrets(hs) ||
!tls13_set_traffic_key(
ssl, ssl_encryption_handshake, evp_aead_open,
MakeConstSpan(hs->server_handshake_secret, hs->hash_len))) {
!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_open,
hs->server_handshake_secret())) {
return ssl_hs_error;
}

if (!hs->early_data_offered) {
// If not sending early data, set client traffic keys now so that alerts are
// encrypted.
if (!tls13_set_traffic_key(
ssl, ssl_encryption_handshake, evp_aead_seal,
MakeConstSpan(hs->client_handshake_secret, hs->hash_len))) {
if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_seal,
hs->client_handshake_secret())) {
return ssl_hs_error;
}
}
Expand Down Expand Up @@ -619,7 +617,8 @@ static enum ssl_hs_wait_t do_read_server_finished(SSL_HANDSHAKE *hs) {
!tls13_process_finished(hs, msg, false /* don't use saved value */) ||
!ssl_hash_message(hs, msg) ||
// Update the secret to the master secret and derive traffic keys.
!tls13_advance_key_schedule(hs, MakeConstSpan(kZeroes, hs->hash_len)) ||
!tls13_advance_key_schedule(
hs, MakeConstSpan(kZeroes, hs->transcript.DigestLen())) ||
!tls13_derive_application_secrets(hs)) {
return ssl_hs_error;
}
Expand All @@ -644,9 +643,8 @@ static enum ssl_hs_wait_t do_send_end_of_early_data(SSL_HANDSHAKE *hs) {
}

if (hs->early_data_offered) {
if (!tls13_set_traffic_key(
ssl, ssl_encryption_handshake, evp_aead_seal,
MakeConstSpan(hs->client_handshake_secret, hs->hash_len))) {
if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_seal,
hs->client_handshake_secret())) {
return ssl_hs_error;
}
}
Expand Down Expand Up @@ -740,12 +738,10 @@ static enum ssl_hs_wait_t do_complete_second_flight(SSL_HANDSHAKE *hs) {
}

// Derive the final keys and enable them.
if (!tls13_set_traffic_key(
ssl, ssl_encryption_application, evp_aead_open,
MakeConstSpan(hs->server_traffic_secret_0, hs->hash_len)) ||
!tls13_set_traffic_key(
ssl, ssl_encryption_application, evp_aead_seal,
MakeConstSpan(hs->client_traffic_secret_0, hs->hash_len)) ||
if (!tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_open,
hs->server_traffic_secret_0()) ||
!tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_seal,
hs->client_traffic_secret_0()) ||
!tls13_derive_resumption_secret(hs)) {
return ssl_hs_error;
}
Expand Down
Loading

0 comments on commit e530ea3

Please sign in to comment.