From 2a53b948e84b3598dc816e8c038d78cd1d7142f9 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Fri, 8 Sep 2023 13:25:48 +0200 Subject: [PATCH 1/3] tls1prf: require callers to pass in the result buffer --- cng/tls1prf.go | 21 +++++++++++++-------- cng/tls1prf_test.go | 7 ++++--- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/cng/tls1prf.go b/cng/tls1prf.go index c011af9..ae2835f 100644 --- a/cng/tls1prf.go +++ b/cng/tls1prf.go @@ -24,7 +24,10 @@ func loadTLS1PRF(id string) (bcrypt.ALG_HANDLE, error) { return h.(bcrypt.ALG_HANDLE), nil } -func TLS1PRF(secret, label, seed []byte, keyLen int, h func() hash.Hash) ([]byte, error) { +// TLS1PRF implements the TLS 1.0/1.1 pseudo-random function if h is nil or crypto.MD5SHA1, +// else it implements the TLS 1.2 pseudo-random function. +// The pseudo-random number will be written to result and will be of length len(result). +func TLS1PRF(result, secret, label, seed []byte, h func() hash.Hash) error { // TLS 1.0/1.1 PRF uses MD5SHA1. algID := bcrypt.TLS1_1_KDF_ALGORITHM var hashID string @@ -32,18 +35,18 @@ func TLS1PRF(secret, label, seed []byte, keyLen int, h func() hash.Hash) ([]byte // If h is specified, assume the caller wants to use TLS 1.2 PRF. // TLS 1.0/1.1 PRF doesn't allow specifying the hash function. if hashID = hashToID(h()); hashID == "" { - return nil, errors.New("cng: unsupported hash function") + return errors.New("cng: unsupported hash function") } algID = bcrypt.TLS1_2_KDF_ALGORITHM } alg, err := loadTLS1PRF(algID) if err != nil { - return nil, err + return err } var kh bcrypt.KEY_HANDLE if err := bcrypt.GenerateSymmetricKey(alg, &kh, nil, secret, 0); err != nil { - return nil, err + return err } buffers := make([]bcrypt.Buffer, 0, 3) @@ -73,11 +76,13 @@ func TLS1PRF(secret, label, seed []byte, keyLen int, h func() hash.Hash) ([]byte Count: uint32(len(buffers)), Buffers: &buffers[0], } - out := make([]byte, keyLen) var size uint32 - err = bcrypt.KeyDerivation(kh, params, out, &size, 0) + err = bcrypt.KeyDerivation(kh, params, result, &size, 0) if err != nil { - return nil, err + return err } - return out[:size], nil + if size != uint32(len(result)) { + return errors.New("tls1-prf: entropy limit reached") + } + return nil } diff --git a/cng/tls1prf_test.go b/cng/tls1prf_test.go index dbd56cf..2bf6801 100644 --- a/cng/tls1prf_test.go +++ b/cng/tls1prf_test.go @@ -153,12 +153,13 @@ var tls1prfTests = []tls1prfTest{ func TestTLS1PRF(t *testing.T) { for i, tt := range tls1prfTests { - out, err := cng.TLS1PRF(tt.secret, tt.label, tt.seed, len(tt.out), tt.hash) + result := make([]byte, len(tt.out)) + err := cng.TLS1PRF(result, tt.secret, tt.label, tt.seed, tt.hash) if err != nil { t.Errorf("test %d: error deriving TLS 1.2 PRF: %v.", i, err) } - if !bytes.Equal(out, tt.out) { - t.Errorf("test %d: incorrect key output: have %v, need %v.", i, out, tt.out) + if !bytes.Equal(result, tt.out) { + t.Errorf("test %d: incorrect key output: have %v, need %v.", i, result, tt.out) } } } From a58667e3fd3e6d1704a96dc0d5e7e2d33ea35902 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Fri, 8 Sep 2023 14:56:47 +0200 Subject: [PATCH 2/3] improve TLS1PRF comments --- cng/tls1prf.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cng/tls1prf.go b/cng/tls1prf.go index ae2835f..a527bda 100644 --- a/cng/tls1prf.go +++ b/cng/tls1prf.go @@ -24,7 +24,7 @@ func loadTLS1PRF(id string) (bcrypt.ALG_HANDLE, error) { return h.(bcrypt.ALG_HANDLE), nil } -// TLS1PRF implements the TLS 1.0/1.1 pseudo-random function if h is nil or crypto.MD5SHA1, +// TLS1PRF implements the TLS 1.0/1.1 pseudo-random function if h is nil, // else it implements the TLS 1.2 pseudo-random function. // The pseudo-random number will be written to result and will be of length len(result). func TLS1PRF(result, secret, label, seed []byte, h func() hash.Hash) error { @@ -81,8 +81,10 @@ func TLS1PRF(result, secret, label, seed []byte, h func() hash.Hash) error { if err != nil { return err } + // The Go standard library expects TLS1PRF to return the requested number of bytes, + // fail if it doesn't. if size != uint32(len(result)) { - return errors.New("tls1-prf: entropy limit reached") + return errors.New("tls1-prf: derived less bytes than requested") } return nil } From 9ac6cb8d21358ab2bf56246402cdba8f000ab16f Mon Sep 17 00:00:00 2001 From: Quim Muntal Date: Thu, 14 Sep 2023 10:55:14 +0200 Subject: [PATCH 3/3] Update cng/tls1prf.go Co-authored-by: Davis Goodin --- cng/tls1prf.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cng/tls1prf.go b/cng/tls1prf.go index a527bda..30ef224 100644 --- a/cng/tls1prf.go +++ b/cng/tls1prf.go @@ -82,7 +82,9 @@ func TLS1PRF(result, secret, label, seed []byte, h func() hash.Hash) error { return err } // The Go standard library expects TLS1PRF to return the requested number of bytes, - // fail if it doesn't. + // fail if it doesn't. While there is no known situation where this will happen, + // BCryptKeyDerivation handles multiple algorithms and there could be a subtle mismatch + // after more code changes in the future. if size != uint32(len(result)) { return errors.New("tls1-prf: derived less bytes than requested") }