Tidy up the PSK binder logic.
Computing the binders on ClientHelloInner is a little interesting. While
I'm in the area, tidy this up a bit. The exploded parameters may as well
be an SSL_SESSION, and hash_transcript_and_truncated_client_hello can
just get folded in.
Change-Id: I9d3a7e0ae9f391d6b9a23b51b5d7198e15569b11
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47997
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/internal.h b/ssl/internal.h
index 72e1fba..6a1a650 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -700,9 +700,9 @@
// the transcript. It returns true on success and false on failure. If the
// handshake buffer is still present, |digest| may be any supported digest.
// Otherwise, |digest| must match the transcript hash.
- bool CopyToHashContext(EVP_MD_CTX *ctx, const EVP_MD *digest);
+ bool CopyToHashContext(EVP_MD_CTX *ctx, const EVP_MD *digest) const;
- Span<const uint8_t> buffer() {
+ Span<const uint8_t> buffer() const {
return MakeConstSpan(reinterpret_cast<const uint8_t *>(buffer_->data),
buffer_->length);
}
@@ -725,14 +725,14 @@
// GetHash writes the handshake hash to |out| which must have room for at
// least |DigestLen| bytes. On success, it returns true and sets |*out_len| to
// the number of bytes written. Otherwise, it returns false.
- bool GetHash(uint8_t *out, size_t *out_len);
+ bool GetHash(uint8_t *out, size_t *out_len) const;
// GetFinishedMAC computes the MAC for the Finished message into the bytes
// pointed by |out| and writes the number of bytes to |*out_len|. |out| must
// have room for |EVP_MAX_MD_SIZE| bytes. It returns true on success and false
// on failure.
bool GetFinishedMAC(uint8_t *out, size_t *out_len, const SSL_SESSION *session,
- bool from_server);
+ bool from_server) const;
private:
// buffer_, if non-null, contains the handshake transcript.
@@ -1418,13 +1418,14 @@
// tls13_write_psk_binder calculates the PSK binder value and replaces the last
// bytes of |msg| with the resulting value. It returns true on success, and
// false on failure.
-bool tls13_write_psk_binder(SSL_HANDSHAKE *hs, Span<uint8_t> msg);
+bool tls13_write_psk_binder(const SSL_HANDSHAKE *hs, Span<uint8_t> msg);
// tls13_verify_psk_binder verifies that the handshake transcript, truncated up
// to the binders has a valid signature using the value of |session|'s
// resumption secret. It returns true on success, and false on failure.
-bool tls13_verify_psk_binder(SSL_HANDSHAKE *hs, SSL_SESSION *session,
- const SSLMessage &msg, CBS *binders);
+bool tls13_verify_psk_binder(const SSL_HANDSHAKE *hs,
+ const SSL_SESSION *session, const SSLMessage &msg,
+ CBS *binders);
// Encrypted ClientHello.
diff --git a/ssl/ssl_transcript.cc b/ssl/ssl_transcript.cc
index 0bc13b9..1599c80 100644
--- a/ssl/ssl_transcript.cc
+++ b/ssl/ssl_transcript.cc
@@ -206,7 +206,8 @@
return true;
}
-bool SSLTranscript::CopyToHashContext(EVP_MD_CTX *ctx, const EVP_MD *digest) {
+bool SSLTranscript::CopyToHashContext(EVP_MD_CTX *ctx,
+ const EVP_MD *digest) const {
const EVP_MD *transcript_digest = Digest();
if (transcript_digest != nullptr &&
EVP_MD_type(transcript_digest) == EVP_MD_type(digest)) {
@@ -237,7 +238,7 @@
return true;
}
-bool SSLTranscript::GetHash(uint8_t *out, size_t *out_len) {
+bool SSLTranscript::GetHash(uint8_t *out, size_t *out_len) const {
ScopedEVP_MD_CTX ctx;
unsigned len;
if (!EVP_MD_CTX_copy_ex(ctx.get(), hash_.get()) ||
@@ -250,7 +251,7 @@
bool SSLTranscript::GetFinishedMAC(uint8_t *out, size_t *out_len,
const SSL_SESSION *session,
- bool from_server) {
+ bool from_server) const {
static const char kClientLabel[] = "client finished";
static const char kServerLabel[] = "server finished";
auto label = from_server
diff --git a/ssl/tls13_enc.cc b/ssl/tls13_enc.cc
index 9c3063f..2ac9985 100644
--- a/ssl/tls13_enc.cc
+++ b/ssl/tls13_enc.cc
@@ -395,29 +395,52 @@
static const char kTLS13LabelPSKBinder[] = "res binder";
-static bool tls13_psk_binder(uint8_t *out, size_t *out_len, uint16_t version,
- const EVP_MD *digest, Span<const uint8_t> psk,
- Span<const uint8_t> context) {
+static bool tls13_psk_binder(uint8_t *out, size_t *out_len,
+ const SSL_SESSION *session,
+ const SSLTranscript &transcript,
+ Span<const uint8_t> client_hello,
+ size_t binders_len) {
+ const EVP_MD *digest = ssl_session_get_digest(session);
+
+ // Compute the binder key.
+ //
+ // TODO(davidben): Ideally we wouldn't recompute early secret and the binder
+ // key each time.
uint8_t binder_context[EVP_MAX_MD_SIZE];
unsigned binder_context_len;
- if (!EVP_Digest(NULL, 0, binder_context, &binder_context_len, digest, NULL)) {
- return false;
- }
-
uint8_t early_secret[EVP_MAX_MD_SIZE] = {0};
size_t early_secret_len;
- if (!HKDF_extract(early_secret, &early_secret_len, digest, psk.data(),
- psk.size(), NULL, 0)) {
+ uint8_t binder_key_buf[EVP_MAX_MD_SIZE] = {0};
+ 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_expand_label(binder_key, digest,
+ MakeConstSpan(early_secret, early_secret_len),
+ label_to_span(kTLS13LabelPSKBinder),
+ MakeConstSpan(binder_context, binder_context_len))) {
return false;
}
- uint8_t binder_key_buf[EVP_MAX_MD_SIZE] = {0};
- auto binder_key = MakeSpan(binder_key_buf, EVP_MD_size(digest));
- if (!hkdf_expand_label(binder_key, digest,
- MakeConstSpan(early_secret, early_secret_len),
- label_to_span(kTLS13LabelPSKBinder),
- MakeConstSpan(binder_context, binder_context_len)) ||
- !tls13_verify_data(out, out_len, digest, version, binder_key, context)) {
+ // Hash the transcript and truncated ClientHello.
+ if (client_hello.size() < binders_len) {
+ OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+ return false;
+ }
+ auto truncated = client_hello.subspan(0, client_hello.size() - binders_len);
+ uint8_t context[EVP_MAX_MD_SIZE];
+ unsigned context_len;
+ ScopedEVP_MD_CTX ctx;
+ if (!transcript.CopyToHashContext(ctx.get(), digest) ||
+ !EVP_DigestUpdate(ctx.get(), truncated.data(),
+ truncated.size()) ||
+ !EVP_DigestFinal_ex(ctx.get(), context, &context_len)) {
+ return false;
+ }
+
+ if (!tls13_verify_data(out, out_len, digest, session->ssl_version, binder_key,
+ MakeConstSpan(context, context_len))) {
return false;
}
@@ -425,44 +448,19 @@
return true;
}
-static bool hash_transcript_and_truncated_client_hello(
- SSL_HANDSHAKE *hs, uint8_t *out, size_t *out_len, const EVP_MD *digest,
- Span<const uint8_t> client_hello, size_t binders_len) {
- // Truncate the ClientHello.
- if (binders_len + 2 < binders_len || client_hello.size() < binders_len + 2) {
- return false;
- }
- client_hello = client_hello.subspan(0, client_hello.size() - binders_len - 2);
-
- ScopedEVP_MD_CTX ctx;
- unsigned len;
- if (!hs->transcript.CopyToHashContext(ctx.get(), digest) ||
- !EVP_DigestUpdate(ctx.get(), client_hello.data(), client_hello.size()) ||
- !EVP_DigestFinal_ex(ctx.get(), out, &len)) {
- return false;
- }
-
- *out_len = len;
- return true;
-}
-
-bool tls13_write_psk_binder(SSL_HANDSHAKE *hs, Span<uint8_t> msg) {
- SSL *const ssl = hs->ssl;
+bool tls13_write_psk_binder(const SSL_HANDSHAKE *hs,
+ Span<uint8_t> msg) {
+ const SSL *const ssl = hs->ssl;
const EVP_MD *digest = ssl_session_get_digest(ssl->session.get());
- size_t hash_len = EVP_MD_size(digest);
-
- ScopedEVP_MD_CTX ctx;
- uint8_t context[EVP_MAX_MD_SIZE];
- size_t context_len;
+ const size_t hash_len = EVP_MD_size(digest);
+ // We only offer one PSK, so the binders are a u16 and u8 length
+ // prefix, followed by the binder. The caller is assumed to have constructed
+ // |msg| with placeholder binders.
+ const size_t binders_len = 3 + hash_len;
uint8_t verify_data[EVP_MAX_MD_SIZE];
size_t verify_data_len;
- if (!hash_transcript_and_truncated_client_hello(
- hs, context, &context_len, digest, msg,
- 1 /* length prefix */ + hash_len) ||
- !tls13_psk_binder(
- verify_data, &verify_data_len, ssl->session->ssl_version, digest,
- MakeConstSpan(ssl->session->secret, ssl->session->secret_length),
- MakeConstSpan(context, context_len)) ||
+ if (!tls13_psk_binder(verify_data, &verify_data_len, ssl->session.get(),
+ hs->transcript, msg, binders_len) ||
verify_data_len != hash_len) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return false;
@@ -473,20 +471,17 @@
return true;
}
-bool tls13_verify_psk_binder(SSL_HANDSHAKE *hs, SSL_SESSION *session,
- const SSLMessage &msg, CBS *binders) {
- uint8_t context[EVP_MAX_MD_SIZE];
- size_t context_len;
+bool tls13_verify_psk_binder(const SSL_HANDSHAKE *hs,
+ const SSL_SESSION *session, const SSLMessage &msg,
+ CBS *binders) {
uint8_t verify_data[EVP_MAX_MD_SIZE];
size_t verify_data_len;
CBS binder;
- if (!hash_transcript_and_truncated_client_hello(hs, context, &context_len,
- hs->transcript.Digest(),
- msg.raw, CBS_len(binders)) ||
- !tls13_psk_binder(verify_data, &verify_data_len, hs->ssl->version,
- hs->transcript.Digest(),
- MakeConstSpan(session->secret, session->secret_length),
- MakeConstSpan(context, context_len)) ||
+ // The binders are computed over |msg| with |binders| and its u16 length
+ // prefix removed. The caller is assumed to have parsed |msg|, extracted
+ // |binders|, and verified the PSK extension is last.
+ if (!tls13_psk_binder(verify_data, &verify_data_len, session, hs->transcript,
+ msg.raw, 2 + CBS_len(binders)) ||
// We only consider the first PSK, so compare against the first binder.
!CBS_get_u8_length_prefixed(binders, &binder)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
