commit 19fe7943ce593068bf447f05c2dbed4a324146ef
author David Benjamin <davidben@google.com> Thu Sep 02 13:07:24 2021 -0400
committer Adam Langley <agl@google.com> Thu Sep 02 22:11:49 2021 +0000
tree bc4655e3f3044323c7a93c378140220309fdca86
parent 18b6836b2f6340187a7981c82be9be9d092d36d6

Fix calculation of draft-13 ECH confirmation signal.

Apparently both we and Go flipped the parameter order for HKDF-Extract
relative to the HKDF spec. (The spec orders the salt before the key.)
Not sure how that happened.

Found doing interop testing with Stephen Farrell's implementation.

https://pkg.go.dev/golang.org/x/crypto/hkdf#Extract
https://datatracker.ietf.org/doc/html/rfc5869#section-2.2
https://datatracker.ietf.org/doc/html/draft-ietf-tls-esni-13#section-7.2

Bug: 275
Change-Id: I40a7d53b45cb548e93e6a7ae235e98e55dec4a7a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49185
Reviewed-by: Adam Langley <agl@google.com>

include/openssl/hkdf.h

diff --git a/include/openssl/hkdf.h b/include/openssl/hkdf.h
index 59aaa49..5b27acc 100644
--- a/include/openssl/hkdf.h
+++ b/include/openssl/hkdf.h
@@ -41,6 +41,10 @@
 // keying material |secret| and salt |salt| using |digest|, and outputs
 // |out_len| bytes to |out_key|. The maximum output size is |EVP_MAX_MD_SIZE|.
 // It returns one on success and zero on error.
+//
+// WARNING: This function orders the inputs differently from RFC 5869
+// specification. Double-check which parameter is the secret/IKM and which is
+// the salt when using.
 OPENSSL_EXPORT int HKDF_extract(uint8_t *out_key, size_t *out_len,
                                 const EVP_MD *digest, const uint8_t *secret,
                                 size_t secret_len, const uint8_t *salt,

ssl/test/runner/prf.go

diff --git a/ssl/test/runner/prf.go b/ssl/test/runner/prf.go
index f5290c3..5731be0 100644
--- a/ssl/test/runner/prf.go
+++ b/ssl/test/runner/prf.go
@@ -410,7 +410,7 @@
 // sections 7.2 and 7.2.1 of draft-ietf-tls-esni-13. The transcript hash is
 // computed by concatenating |h| with |extraMessages|.
 func (h *finishedHash) echAcceptConfirmation(clientRandom, label, extraMessages []byte) []byte {
-	secret := hkdf.Extract(h.suite.hash().New, h.zeroSecret(), clientRandom)
+	secret := hkdf.Extract(h.suite.hash().New, clientRandom, h.zeroSecret())
 	hashCopy := copyHash(h.hash, h.suite.hash())
 	hashCopy.Write(extraMessages)
 	return hkdfExpandLabel(h.suite.hash(), secret, label, hashCopy.Sum(nil), echAcceptConfirmationLength)

ssl/tls13_enc.cc

diff --git a/ssl/tls13_enc.cc b/ssl/tls13_enc.cc
index 6942887..c7b75a6 100644
--- a/ssl/tls13_enc.cc
+++ b/ssl/tls13_enc.cc
@@ -565,9 +565,9 @@
 
   uint8_t secret[EVP_MAX_MD_SIZE];
   size_t secret_len;
-  if (!HKDF_extract(secret, &secret_len, transcript.Digest(), kZeros,
-                    transcript.DigestLen(), client_random.data(),
-                    client_random.size())) {
+  if (!HKDF_extract(secret, &secret_len, transcript.Digest(),
+                    client_random.data(), client_random.size(), kZeros,
+                    transcript.DigestLen())) {
     return false;
   }