From 6c586e17d90a7d08bbbc4069984180dce3b04117 Mon Sep 17 00:00:00 2001 From: Andreas Auernhammer Date: Sat, 20 May 2017 22:24:58 +0200 Subject: [PATCH] bcrypt: fix C compatibility code The bcrypt implementation must append a zero byte to the user provided key to be compatible to C implementations. This will change the user provided key if the slice has enough capacity to hold the extra zero byte. This change always allocates a new slice for the C-compatible key. Fixes golang/go#20425 Change-Id: I8dc4e840c29711daabdabe58d83643cc0103cedd Reviewed-on: https://go-review.googlesource.com/43715 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- bcrypt/bcrypt.go | 7 ++++--- bcrypt/bcrypt_test.go | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/bcrypt/bcrypt.go b/bcrypt/bcrypt.go index f8b807f..202fa8a 100644 --- a/bcrypt/bcrypt.go +++ b/bcrypt/bcrypt.go @@ -12,9 +12,10 @@ import ( "crypto/subtle" "errors" "fmt" - "golang.org/x/crypto/blowfish" "io" "strconv" + + "golang.org/x/crypto/blowfish" ) const ( @@ -205,7 +206,6 @@ func bcrypt(password []byte, cost int, salt []byte) ([]byte, error) { } func expensiveBlowfishSetup(key []byte, cost uint32, salt []byte) (*blowfish.Cipher, error) { - csalt, err := base64Decode(salt) if err != nil { return nil, err @@ -213,7 +213,8 @@ func expensiveBlowfishSetup(key []byte, cost uint32, salt []byte) (*blowfish.Cip // Bug compatibility with C bcrypt implementations. They use the trailing // NULL in the key string during expansion. - ckey := append(key, 0) + // We copy the key to prevent changing the underlying array. + ckey := append(key[:len(key):len(key)], 0) c, err := blowfish.NewSaltedCipher(ckey, csalt) if err != nil { diff --git a/bcrypt/bcrypt_test.go b/bcrypt/bcrypt_test.go index f08a6f5..aecf759 100644 --- a/bcrypt/bcrypt_test.go +++ b/bcrypt/bcrypt_test.go @@ -224,3 +224,20 @@ func BenchmarkGeneration(b *testing.B) { GenerateFromPassword(passwd, 10) } } + +// See Issue https://github.com/golang/go/issues/20425. +func TestNoSideEffectsFromCompare(t *testing.T) { + source := []byte("passw0rd123456") + password := source[:len(source)-6] + token := source[len(source)-6:] + want := make([]byte, len(source)) + copy(want, source) + + wantHash := []byte("$2a$10$LK9XRuhNxHHCvjX3tdkRKei1QiCDUKrJRhZv7WWZPuQGRUM92rOUa") + _ = CompareHashAndPassword(wantHash, password) + + got := bytes.Join([][]byte{password, token}, []byte("")) + if !bytes.Equal(got, want) { + t.Errorf("got=%q want=%q", got, want) + } +}