[go] crypto/pbkdf2: add keyLength limit

0 views

Skip to first unread message

Gopher Robot (Gerrit)

unread,

Jan 28, 2025, 9:20:22 PMJan 28

to Roland Shoemaker, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go LUCI, Robert Griesemer, golang-co...@googlegroups.com

Gopher Robot submitted the change with unreviewed changes

Unreviewed changes

7 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: src/crypto/internal/fips140/pbkdf2/pbkdf2.go
Insertions: 21, Deletions: 15.

@@ -10,29 +10,35 @@
 	"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)
 
-	prf := hmac.New(h, []byte(password))
-	hmac.MarkAsUsedInKDF(prf)
-	hashLen := int64(prf.Size())
-	// If we're on a 32 bit system where int is 31 bits, the calculations below
-	// may overflow if keyLength is near 1<<31 - 1, so in order to avoid this,
-	// we just use int64s for everything internally.
-	keySize := int64(keyLength)
-	// Maximum derived key size per RFC 8018
-	maxKeySize := int64(1<<32-1) * hashLen
-	if keySize > maxKeySize {
-		return nil, errors.New("pbkdf2: keyLength too long")
-	} else if keySize <= 0 {
+	if keyLength <= 0 {
 		return nil, errors.New("pkbdf2: keyLength must be larger than 0")
 	}
-	numBlocks := (keySize + hashLen - 1) / hashLen
+
+	prf := hmac.New(h, []byte(password))
+	hmac.MarkAsUsedInKDF(prf)
+	hashLen := prf.Size()
+	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)
 	U := make([]byte, hashLen)
-	for block := int64(1); block <= numBlocks; block++ {
+	for block := 1; block <= numBlocks; block++ {
 		// N.B.: || means concatenation, ^ means XOR
 		// for each block T_i = U_1 ^ U_2 ^ ... ^ U_iter
 		// U_1 = PRF(password, salt || uint(i))
@@ -44,7 +50,7 @@
 		buf[3] = byte(block)
 		prf.Write(buf[:4])
 		dk = prf.Sum(dk)
-		T := dk[len(dk)-int(hashLen):]
+		T := dk[len(dk)-hashLen:]
 		copy(U, T)
 
 		// U_n = PRF(password, U_(n-1))
```
```
The name of the file: src/crypto/pbkdf2/pbkdf2_test.go
Insertions: 5, Deletions: 0.

@@ -234,6 +234,11 @@
 	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) {
```

Change information

Commit message:

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...@luci-project-accounts.iam.gserviceaccount.com>

Auto-Submit: Roland Shoemaker <rol...@golang.org>

Reviewed-by: Filippo Valsorda <fil...@golang.org>

Reviewed-by: Robert Griesemer <g...@google.com>

Files:

M src/crypto/internal/fips140/pbkdf2/pbkdf2.go
M src/crypto/pbkdf2/pbkdf2.go
M src/crypto/pbkdf2/pbkdf2_test.go

Change size: M

Delta: 3 files changed, 52 insertions(+), 1 deletion(-)

Branch: refs/heads/master

Submit Requirements:

Code-Review: +1 by Robert Griesemer, +2 by Filippo Valsorda
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

0 new messages