crypto/pbkdf2: add keyLength limit

As specified by RFC 8018. Also prevent unexpected overflows on 32 bit
systems.

Change-Id: I50c4a177b7d1ebb15f9b3b96e515d93f19d3f68e
Reviewed-on: https://go-review.googlesource.com/c/go/+/644122
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>

diff --git a/src/crypto/internal/fips140/pbkdf2/pbkdf2.go b/src/crypto/internal/fips140/pbkdf2/pbkdf2.go
index 8f6d9915041d3f1ee701c21a7653378541b6089b..05923f68262ac674ed254a729b29cc9752b887b0 100644 (file)
--- a/src/crypto/internal/fips140/pbkdf2/pbkdf2.go
+++ b/src/crypto/internal/fips140/pbkdf2/pbkdf2.go
@@ -7,15 +7,33 @@ package pbkdf2
 import (
        "crypto/internal/fips140"
        "crypto/internal/fips140/hmac"
+       "errors"
 )
 
+// divRoundUp divides x+y-1 by y, rounding up if the result is not whole.
+// This function casts x and y to int64 in order to avoid cases where
+// x+y would overflow int on systems where int is an int32. The result
+// is an int, which is safe as (x+y-1)/y should always fit, regardless
+// of the integer size.
+func divRoundUp(x, y int) int {
+       return int((int64(x) + int64(y) - 1) / int64(y))
+}
+
 func Key[Hash fips140.Hash](h func() Hash, password string, salt []byte, iter, keyLength int) ([]byte, error) {
        setServiceIndicator(salt, keyLength)
 
+       if keyLength <= 0 {
+               return nil, errors.New("pkbdf2: keyLength must be larger than 0")
+       }
+
        prf := hmac.New(h, []byte(password))
        hmac.MarkAsUsedInKDF(prf)
        hashLen := prf.Size()
-       numBlocks := (keyLength + hashLen - 1) / hashLen
+       numBlocks := divRoundUp(keyLength, hashLen)
+       const maxBlocks = int64(1<<32 - 1)
+       if keyLength+hashLen < keyLength || int64(numBlocks) > maxBlocks {
+               return nil, errors.New("pbkdf2: keyLength too long")
+       }
 
        var buf [4]byte
        dk := make([]byte, 0, numBlocks*hashLen)

diff --git a/src/crypto/pbkdf2/pbkdf2.go b/src/crypto/pbkdf2/pbkdf2.go
index 271d2b03312ef05d621077764ad3b81a2728aac2..dd5fc33f2120c335328291e232721642ba83d779 100644 (file)
--- a/src/crypto/pbkdf2/pbkdf2.go
+++ b/src/crypto/pbkdf2/pbkdf2.go
@@ -34,6 +34,9 @@ import (
 //
 // Using a higher iteration count will increase the cost of an exhaustive
 // search but will also make derivation proportionally slower.
+//
+// keyLength must be a positive integer between 1 and (2^32 - 1) * h.Size().
+// Setting keyLength to a value outside of this range will result in an error.
 func Key[Hash hash.Hash](h func() Hash, password string, salt []byte, iter, keyLength int) ([]byte, error) {
        fh := fips140hash.UnwrapNew(h)
        if fips140only.Enabled {

diff --git a/src/crypto/pbkdf2/pbkdf2_test.go b/src/crypto/pbkdf2/pbkdf2_test.go
index 03980c7e54d3be59a85c9569ae358f6a32069056..eb0ed14e243c6ba22e4de47cadc139f0c0ced177 100644 (file)
--- a/src/crypto/pbkdf2/pbkdf2_test.go
+++ b/src/crypto/pbkdf2/pbkdf2_test.go
@@ -221,3 +221,33 @@ func TestPBKDF2ServiceIndicator(t *testing.T) {
                t.Error("FIPS service indicator should not be set")
        }
 }
+
+func TestMaxKeyLength(t *testing.T) {
+       // This error cannot be triggered on platforms where int is 31 bits (i.e.
+       // 32-bit platforms), since the max value for keyLength is 1<<31-1 and
+       // 1<<31-1 * hLen will always be less than 1<<32-1 * hLen.
+       keySize := int64(1<<63 - 1)
+       if int64(int(keySize)) != keySize {
+               t.Skip("cannot be replicated on platforms where int is 31 bits")
+       }
+       _, err := pbkdf2.Key(sha256.New, "password", []byte("salt"), 1, int(keySize))
+       if err == nil {
+               t.Fatal("expected pbkdf2.Key to fail with extremely large keyLength")
+       }
+       keySize = int64(1<<32-1) * (sha256.Size + 1)
+       _, err = pbkdf2.Key(sha256.New, "password", []byte("salt"), 1, int(keySize))
+       if err == nil {
+               t.Fatal("expected pbkdf2.Key to fail with extremely large keyLength")
+       }
+}
+
+func TestZeroKeyLength(t *testing.T) {
+       _, err := pbkdf2.Key(sha256.New, "password", []byte("salt"), 1, 0)
+       if err == nil {
+               t.Fatal("expected pbkdf2.Key to fail with zero keyLength")
+       }
+       _, err = pbkdf2.Key(sha256.New, "password", []byte("salt"), 1, -1)
+       if err == nil {
+               t.Fatal("expected pbkdf2.Key to fail with negative keyLength")
+       }
+}