crypto/ecies: improve concatKDF (#20836)

This removes a bunch of weird code around the counter overflow check in
concatKDF and makes it actually work for different hash output sizes.

The overflow check worked as follows: concatKDF applies the hash function N
times, where N is roundup(kdLen, hashsize) / hashsize. N should not
overflow 32 bits because that would lead to a repetition in the KDF output.

A couple issues with the overflow check:

- It used the hash.BlockSize, which is wrong because the
  block size is about the input of the hash function. Luckily, all standard
  hash functions have a block size that's greater than the output size, so
  concatKDF didn't crash, it just generated too much key material.
- The check used big.Int to compare against 2^32-1.
- The calculation could still overflow before reaching the check.

The new code in concatKDF doesn't check for overflow. Instead, there is a
new check on ECIESParams which ensures that params.KeyLen is < 512. This
removes any possibility of overflow.

There are a couple of miscellaneous improvements bundled in with this
change:

- The key buffer is pre-allocated instead of appending the hash output
  to an initially empty slice.
- The code that uses concatKDF to derive keys is now shared between Encrypt
  and Decrypt.
- There was a redundant invocation of IsOnCurve in Decrypt. This is now removed
  because elliptic.Unmarshal already checks whether the input is a valid curve
  point since Go 1.5.

Co-authored-by: Felix Lange <fjl@twurst.com>
This commit is contained in:
Luke Champine
2020-04-03 05:57:24 -04:00
committed by GitHub
parent f7b29ec942
commit 462ddce5b2
3 changed files with 94 additions and 110 deletions

View File

@ -42,17 +42,23 @@ import (
"github.com/ethereum/go-ethereum/crypto"
)
// Ensure the KDF generates appropriately sized keys.
func TestKDF(t *testing.T) {
msg := []byte("Hello, world")
h := sha256.New()
k, err := concatKDF(h, msg, nil, 64)
if err != nil {
t.Fatal(err)
tests := []struct {
length int
output []byte
}{
{6, decode("858b192fa2ed")},
{32, decode("858b192fa2ed4395e2bf88dd8d5770d67dc284ee539f12da8bceaa45d06ebae0")},
{48, decode("858b192fa2ed4395e2bf88dd8d5770d67dc284ee539f12da8bceaa45d06ebae0700f1ab918a5f0413b8140f9940d6955")},
{64, decode("858b192fa2ed4395e2bf88dd8d5770d67dc284ee539f12da8bceaa45d06ebae0700f1ab918a5f0413b8140f9940d6955f3467fd6672cce1024c5b1effccc0f61")},
}
if len(k) != 64 {
t.Fatalf("KDF: generated key is the wrong size (%d instead of 64\n", len(k))
for _, test := range tests {
h := sha256.New()
k := concatKDF(h, []byte("input"), nil, test.length)
if !bytes.Equal(k, test.output) {
t.Fatalf("KDF: generated key %x does not match expected output %x", k, test.output)
}
}
}
@ -293,8 +299,8 @@ func TestParamSelection(t *testing.T) {
func testParamSelection(t *testing.T, c testCase) {
params := ParamsFromCurve(c.Curve)
if params == nil && c.Expected != nil {
t.Fatalf("%s (%s)\n", ErrInvalidParams.Error(), c.Name)
if params == nil {
t.Fatal("ParamsFromCurve returned nil")
} else if params != nil && !cmpParams(params, c.Expected) {
t.Fatalf("ecies: parameters should be invalid (%s)\n", c.Name)
}
@ -401,7 +407,7 @@ func TestSharedKeyStatic(t *testing.T) {
t.Fatal(ErrBadSharedKeys)
}
sk, _ := hex.DecodeString("167ccc13ac5e8a26b131c3446030c60fbfac6aa8e31149d0869f93626a4cdf62")
sk := decode("167ccc13ac5e8a26b131c3446030c60fbfac6aa8e31149d0869f93626a4cdf62")
if !bytes.Equal(sk1, sk) {
t.Fatalf("shared secret mismatch: want: %x have: %x", sk, sk1)
}
@ -414,3 +420,11 @@ func hexKey(prv string) *PrivateKey {
}
return ImportECDSA(key)
}
func decode(s string) []byte {
bytes, err := hex.DecodeString(s)
if err != nil {
panic(err)
}
return bytes
}