Switch another low-level function to spans. Get this out of the way for the various TLS 1.3 secrets to use spans. Change-Id: Ia6c3fa4b35ecfad721af665f54bde5ab16baf7ca Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/37126 Commit-Queue: Steven Valdez <svaldez@google.com> Reviewed-by: Steven Valdez <svaldez@google.com>
diff --git a/ssl/handshake.cc b/ssl/handshake.cc index b8e0070..3df004c 100644 --- a/ssl/handshake.cc +++ b/ssl/handshake.cc
@@ -479,8 +479,9 @@ } // Log the master secret, if logging is enabled. - if (!ssl_log_secret(ssl, "CLIENT_RANDOM", session->master_key, - session->master_key_length)) { + if (!ssl_log_secret( + ssl, "CLIENT_RANDOM", + MakeConstSpan(session->master_key, session->master_key_length))) { return 0; }
diff --git a/ssl/internal.h b/ssl/internal.h index 621052f..e9b8201 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -1799,9 +1799,9 @@ // SSLKEYLOGFILE functions. // ssl_log_secret logs |secret| with label |label|, if logging is enabled for -// |ssl|. It returns one on success and zero on failure. -int ssl_log_secret(const SSL *ssl, const char *label, const uint8_t *secret, - size_t secret_len); +// |ssl|. It returns true on success and false on failure. +bool ssl_log_secret(const SSL *ssl, const char *label, + Span<const uint8_t> secret); // ClientHello functions.
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index 00ee7da..80a33bc 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc
@@ -323,46 +323,45 @@ } } -static int cbb_add_hex(CBB *cbb, const uint8_t *in, size_t in_len) { +static bool cbb_add_hex(CBB *cbb, Span<const uint8_t> in) { static const char hextable[] = "0123456789abcdef"; uint8_t *out; - if (!CBB_add_space(cbb, &out, in_len * 2)) { - return 0; + if (!CBB_add_space(cbb, &out, in.size() * 2)) { + return false; } - for (size_t i = 0; i < in_len; i++) { - *(out++) = (uint8_t)hextable[in[i] >> 4]; - *(out++) = (uint8_t)hextable[in[i] & 0xf]; + for (uint8_t b : in) { + *(out++) = (uint8_t)hextable[b >> 4]; + *(out++) = (uint8_t)hextable[b & 0xf]; } - return 1; + return true; } -int ssl_log_secret(const SSL *ssl, const char *label, const uint8_t *secret, - size_t secret_len) { +bool ssl_log_secret(const SSL *ssl, const char *label, + Span<const uint8_t> secret) { if (ssl->ctx->keylog_callback == NULL) { - return 1; + return true; } ScopedCBB cbb; - uint8_t *out; - size_t out_len; + Array<uint8_t> line; if (!CBB_init(cbb.get(), strlen(label) + 1 + SSL3_RANDOM_SIZE * 2 + 1 + - secret_len * 2 + 1) || - !CBB_add_bytes(cbb.get(), (const uint8_t *)label, strlen(label)) || - !CBB_add_bytes(cbb.get(), (const uint8_t *)" ", 1) || - !cbb_add_hex(cbb.get(), ssl->s3->client_random, SSL3_RANDOM_SIZE) || - !CBB_add_bytes(cbb.get(), (const uint8_t *)" ", 1) || - !cbb_add_hex(cbb.get(), secret, secret_len) || + secret.size() * 2 + 1) || + !CBB_add_bytes(cbb.get(), reinterpret_cast<const uint8_t *>(label), + strlen(label)) || + !CBB_add_u8(cbb.get(), ' ') || + !cbb_add_hex(cbb.get(), ssl->s3->client_random) || + !CBB_add_u8(cbb.get(), ' ') || + !cbb_add_hex(cbb.get(), secret) || !CBB_add_u8(cbb.get(), 0 /* NUL */) || - !CBB_finish(cbb.get(), &out, &out_len)) { - return 0; + !CBBFinishArray(cbb.get(), &line)) { + return false; } - ssl->ctx->keylog_callback(ssl, (const char *)out); - OPENSSL_free(out); - return 1; + ssl->ctx->keylog_callback(ssl, reinterpret_cast<const char *>(line.data())); + return true; } void ssl_do_info_callback(const SSL *ssl, int type, int value) {
diff --git a/ssl/tls13_enc.cc b/ssl/tls13_enc.cc index 19ae052..c07afd1 100644 --- a/ssl/tls13_enc.cc +++ b/ssl/tls13_enc.cc
@@ -233,7 +233,7 @@ if (!derive_secret(hs, MakeSpan(hs->early_traffic_secret, hs->hash_len), label_to_span(kTLS13LabelClientEarlyTraffic)) || !ssl_log_secret(ssl, "CLIENT_EARLY_TRAFFIC_SECRET", - hs->early_traffic_secret, hs->hash_len)) { + MakeConstSpan(hs->early_traffic_secret, hs->hash_len))) { return false; } @@ -262,12 +262,14 @@ SSL *const ssl = hs->ssl; if (!derive_secret(hs, MakeSpan(hs->client_handshake_secret, hs->hash_len), label_to_span(kTLS13LabelClientHandshakeTraffic)) || - !ssl_log_secret(ssl, "CLIENT_HANDSHAKE_TRAFFIC_SECRET", - hs->client_handshake_secret, hs->hash_len) || + !ssl_log_secret( + ssl, "CLIENT_HANDSHAKE_TRAFFIC_SECRET", + MakeConstSpan(hs->client_handshake_secret, hs->hash_len)) || !derive_secret(hs, MakeSpan(hs->server_handshake_secret, hs->hash_len), label_to_span(kTLS13LabelServerHandshakeTraffic)) || - !ssl_log_secret(ssl, "SERVER_HANDSHAKE_TRAFFIC_SECRET", - hs->server_handshake_secret, hs->hash_len)) { + !ssl_log_secret( + ssl, "SERVER_HANDSHAKE_TRAFFIC_SECRET", + MakeConstSpan(hs->server_handshake_secret, hs->hash_len))) { return false; } @@ -297,16 +299,20 @@ ssl->s3->exporter_secret_len = hs->hash_len; if (!derive_secret(hs, MakeSpan(hs->client_traffic_secret_0, hs->hash_len), label_to_span(kTLS13LabelClientApplicationTraffic)) || - !ssl_log_secret(ssl, "CLIENT_TRAFFIC_SECRET_0", - hs->client_traffic_secret_0, hs->hash_len) || + !ssl_log_secret( + ssl, "CLIENT_TRAFFIC_SECRET_0", + MakeConstSpan(hs->client_traffic_secret_0, hs->hash_len)) || !derive_secret(hs, MakeSpan(hs->server_traffic_secret_0, hs->hash_len), label_to_span(kTLS13LabelServerApplicationTraffic)) || - !ssl_log_secret(ssl, "SERVER_TRAFFIC_SECRET_0", - hs->server_traffic_secret_0, hs->hash_len) || - !derive_secret(hs, MakeSpan(ssl->s3->exporter_secret, hs->hash_len), - label_to_span(kTLS13LabelExporter)) || - !ssl_log_secret(ssl, "EXPORTER_SECRET", ssl->s3->exporter_secret, - hs->hash_len)) { + !ssl_log_secret( + ssl, "SERVER_TRAFFIC_SECRET_0", + MakeConstSpan(hs->server_traffic_secret_0, hs->hash_len)) || + !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))) { return false; }