Merge pull request #290 from chrisccoulson/use-fixed-keylen-for-argon…

…2-benchmark argon2: use a fixed key length for benchmarking. The key length has a negligible effect on the execution time, so use a fixed key length of 32 bytes for benchmarking. Note that this keeps the keyLen argument to KDFOptions.deriveCostParams as this is going to become an interface in a future PR, where the key length will be used to select the default PBKDF2 hash.
canonical · Mar 14, 2024 · 75a8293 · 75a8293
2 parents 3690477 + 258cbde
commit 75a8293
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 59 deletions.
diff --git a/argon2.go b/argon2.go
@@ -69,8 +69,6 @@ func (o *KDFOptions) deriveCostParams(keyLen int, kdf KDF) (*KDFCostParams, erro
 		return nil, errors.New("Parallel too large")
 	case o.ForceIterations < 0:
 		return nil, errors.New("ForceIterations can't be negative")
-	case int64(keyLen) > math.MaxUint32:
-		return nil, errors.New("keyLen too large")
 	case o.ForceIterations > 0:
 		threads := runtimeNumCPU()
 		if threads > 4 {
@@ -114,7 +112,7 @@ func (o *KDFOptions) deriveCostParams(keyLen int, kdf KDF) (*KDFCostParams, erro
 			return kdf.Time(&KDFCostParams{
 				Time:      params.Time,
 				MemoryKiB: params.MemoryKiB,
-				Threads:   params.Threads}, uint32(keyLen))
+				Threads:   params.Threads})
 		})
 		if err != nil {
 			return nil, xerrors.Errorf("cannot benchmark KDF: %w", err)
@@ -157,8 +155,8 @@ type KDF interface {
 	Derive(passphrase string, salt []byte, params *KDFCostParams, keyLen uint32) ([]byte, error)
 
 	// Time measures the amount of time the KDF takes to execute with the
-	// specified cost parameters and key length in bytes.
-	Time(params *KDFCostParams, keyLen uint32) (time.Duration, error)
+	// specified cost parameters.
+	Time(params *KDFCostParams) (time.Duration, error)
 }
 
 type argon2iKDFImpl struct{}
@@ -167,8 +165,8 @@ func (_ argon2iKDFImpl) Derive(passphrase string, salt []byte, params *KDFCostPa
 	return argon2.Key(passphrase, salt, params.internalParams(), keyLen), nil
 }
 
-func (_ argon2iKDFImpl) Time(params *KDFCostParams, keyLen uint32) (time.Duration, error) {
-	return argon2.KeyDuration(params.internalParams(), keyLen), nil
+func (_ argon2iKDFImpl) Time(params *KDFCostParams) (time.Duration, error) {
+	return argon2.KeyDuration(params.internalParams()), nil
 }
 
 var argon2iKDF = argon2iKDFImpl{}

diff --git a/argon2_test.go b/argon2_test.go
@@ -68,7 +68,7 @@ func (s *argon2Suite) checkParams(c *C, opts *KDFOptions, ncpus int, params *KDF
 			targetDuration = 2 * time.Second
 		}
 		var kdf testutil.MockKDF
-		duration, _ := kdf.Time(params, 0)
+		duration, _ := kdf.Time(params)
 		c.Check(duration, Equals, targetDuration)
 
 		maxMem := uint64(opts.MemoryKiB)
@@ -97,9 +97,8 @@ func (s *argon2Suite) TestDeriveCostParamsDefault(c *C) {
 	var kdf testutil.MockKDF
 
 	var opts KDFOptions
-	params, err := opts.DeriveCostParams(48, &kdf)
+	params, err := opts.DeriveCostParams(0, &kdf)
 	c.Assert(err, IsNil)
-	c.Check(kdf.BenchmarkKeyLen, Equals, uint32(48))
 
 	s.checkParams(c, &opts, s.cpus, params)
 }
@@ -109,9 +108,8 @@ func (s *argon2Suite) TestDeriveCostParamsMemoryLimit(c *C) {
 
 	var opts KDFOptions
 	opts.MemoryKiB = 32 * 1024
-	params, err := opts.DeriveCostParams(48, &kdf)
+	params, err := opts.DeriveCostParams(0, &kdf)
 	c.Assert(err, IsNil)
-	c.Check(kdf.BenchmarkKeyLen, Equals, uint32(48))
 
 	s.checkParams(c, &opts, s.cpus, params)
 }
@@ -121,20 +119,8 @@ func (s *argon2Suite) TestDeriveCostParamsForceBenchmarkedThreads(c *C) {
 
 	var opts KDFOptions
 	opts.Parallel = 1
-	params, err := opts.DeriveCostParams(48, &kdf)
+	params, err := opts.DeriveCostParams(0, &kdf)
 	c.Assert(err, IsNil)
-	c.Check(kdf.BenchmarkKeyLen, Equals, uint32(48))
-
-	s.checkParams(c, &opts, s.cpus, params)
-}
-
-func (s *argon2Suite) TestDeriveCostParamsDifferentKeyLen(c *C) {
-	var kdf testutil.MockKDF
-
-	var opts KDFOptions
-	params, err := opts.DeriveCostParams(32, &kdf)
-	c.Assert(err, IsNil)
-	c.Check(kdf.BenchmarkKeyLen, Equals, uint32(32))
 
 	s.checkParams(c, &opts, s.cpus, params)
 }
@@ -147,9 +133,8 @@ func (s *argon2Suite) TestDeriveCostParamsForceIterations(c *C) {
 
 	var opts KDFOptions
 	opts.ForceIterations = 3
-	params, err := opts.DeriveCostParams(48, &kdf)
+	params, err := opts.DeriveCostParams(0, &kdf)
 	c.Assert(err, IsNil)
-	c.Check(kdf.BenchmarkKeyLen, Equals, uint32(0))
 
 	s.checkParams(c, &opts, 2, params)
 }
@@ -163,9 +148,8 @@ func (s *argon2Suite) TestDeriveCostParamsForceMemory(c *C) {
 	var opts KDFOptions
 	opts.ForceIterations = 3
 	opts.MemoryKiB = 32 * 1024
-	params, err := opts.DeriveCostParams(48, &kdf)
+	params, err := opts.DeriveCostParams(0, &kdf)
 	c.Assert(err, IsNil)
-	c.Check(kdf.BenchmarkKeyLen, Equals, uint32(0))
 
 	s.checkParams(c, &opts, 2, params)
 }
@@ -178,9 +162,8 @@ func (s *argon2Suite) TestDeriveCostParamsForceIterationsDifferentCPUNum(c *C) {
 
 	var opts KDFOptions
 	opts.ForceIterations = 3
-	params, err := opts.DeriveCostParams(48, &kdf)
+	params, err := opts.DeriveCostParams(0, &kdf)
 	c.Assert(err, IsNil)
-	c.Check(kdf.BenchmarkKeyLen, Equals, uint32(0))
 
 	s.checkParams(c, &opts, 4, params)
 }
@@ -194,9 +177,8 @@ func (s *argon2Suite) TestDeriveCostParamsForceThreads(c *C) {
 	var opts KDFOptions
 	opts.ForceIterations = 3
 	opts.Parallel = 1
-	params, err := opts.DeriveCostParams(48, &kdf)
+	params, err := opts.DeriveCostParams(0, &kdf)
 	c.Assert(err, IsNil)
-	c.Check(kdf.BenchmarkKeyLen, Equals, uint32(0))
 
 	s.checkParams(c, &opts, 1, params)
 }
@@ -304,25 +286,25 @@ func (s *argon2SuiteExpensive) TestArgon2iKDFTime(c *C) {
 	kdf := Argon2iKDF()
 	c.Assert(kdf, NotNil)
 
-	time1, err := kdf.Time(&KDFCostParams{Time: 4, MemoryKiB: 32 * 1024, Threads: 4}, 32)
+	time1, err := kdf.Time(&KDFCostParams{Time: 4, MemoryKiB: 32 * 1024, Threads: 4})
 	runtime.GC()
 	c.Check(err, IsNil)
 
-	time2, err := kdf.Time(&KDFCostParams{Time: 16, MemoryKiB: 32 * 1024, Threads: 4}, 32)
+	time2, err := kdf.Time(&KDFCostParams{Time: 16, MemoryKiB: 32 * 1024, Threads: 4})
 	runtime.GC()
 	c.Check(err, IsNil)
 	// XXX: this needs a checker like go-tpm2/testutil's IntGreater, which copes with
 	// types of int64 kind
 	c.Check(time2 > time1, testutil.IsTrue)
 
-	time2, err = kdf.Time(&KDFCostParams{Time: 4, MemoryKiB: 128 * 1024, Threads: 4}, 32)
+	time2, err = kdf.Time(&KDFCostParams{Time: 4, MemoryKiB: 128 * 1024, Threads: 4})
 	runtime.GC()
 	c.Check(err, IsNil)
 	// XXX: this needs a checker like go-tpm2/testutil's IntGreater, which copes with
 	// types of int64 kind
 	c.Check(time2 > time1, testutil.IsTrue)
 
-	time2, err = kdf.Time(&KDFCostParams{Time: 4, MemoryKiB: 32 * 1024, Threads: 1}, 32)
+	time2, err = kdf.Time(&KDFCostParams{Time: 4, MemoryKiB: 32 * 1024, Threads: 1})
 	runtime.GC()
 	c.Check(err, IsNil)
 	// XXX: this needs a checker like go-tpm2/testutil's IntGreater, which copes with

diff --git a/internal/argon2/argon2.go b/internal/argon2/argon2.go
@@ -32,6 +32,7 @@ import (
 const (
 	// Dummy password for benchmarking (same value used by cryptsetup)
 	benchmarkPassword = "foo"
+	benchmarkKeyLen   = 32
 
 	initialTargetDuration = 250 * time.Millisecond
 
@@ -79,7 +80,6 @@ type CostParams struct {
 }
 
 type benchmarkContext struct {
-	keyLen           uint32          // desired key length
 	keyFn            KeyDurationFunc // callback for running an individual measurement
 	maxMemoryCostKiB uint32          // maximum memory cost
 	cost             CostParams      // current computed cost parameters
@@ -272,15 +272,14 @@ func (c *benchmarkContext) run(params *BenchmarkParams, keyFn KeyDurationFunc, s
 	return &c.cost, nil
 }
 
-// KeyDuration runs a key derivation with the built-in benchmarking values and
-// the specified cost parameters and length, and then returns the amount of time
-// taken to execute.
+// KeyDuration runs the key derivation with the built-in benchmarking values for the
+// supplied set of cost parameters, and then returns the amount of time taken to execute.
 //
-// By design, this function consumes a lot of memory depending on the supplied parameters.
-// It may be desirable to execute it in a short-lived utility process.
-func KeyDuration(params *CostParams, keyLen uint32) time.Duration {
+// By design, this function consumes a lot of memory depending on the supplied
+// parameters. It may be desirable to execute it in a short-lived utility process.
+func KeyDuration(params *CostParams) time.Duration {
 	start := time.Now()
-	Key(benchmarkPassword, benchmarkSalt, params, keyLen)
+	Key(benchmarkPassword, benchmarkSalt, params, benchmarkKeyLen)
 	return time.Now().Sub(start)
 }
 

diff --git a/internal/argon2/argon2_test.go b/internal/argon2/argon2_test.go
@@ -431,22 +431,22 @@ func (s *argon2SuiteExpensive) TestKey6(c *C) {
 }
 
 func (s *argon2SuiteExpensive) TestKeyDuration(c *C) {
-	time1 := KeyDuration(&CostParams{Time: 4, MemoryKiB: 32 * 1024, Threads: 4}, 32)
+	time1 := KeyDuration(&CostParams{Time: 4, MemoryKiB: 32 * 1024, Threads: 4})
 	runtime.GC()
 
-	time2 := KeyDuration(&CostParams{Time: 16, MemoryKiB: 32 * 1024, Threads: 4}, 32)
+	time2 := KeyDuration(&CostParams{Time: 16, MemoryKiB: 32 * 1024, Threads: 4})
 	runtime.GC()
 	// XXX: this needs a checker like go-tpm2/testutil's IntGreater, which copes with
 	// types of int64 kind
 	c.Check(time2 > time1, testutil.IsTrue)
 
-	time2 = KeyDuration(&CostParams{Time: 4, MemoryKiB: 128 * 1024, Threads: 4}, 32)
+	time2 = KeyDuration(&CostParams{Time: 4, MemoryKiB: 128 * 1024, Threads: 4})
 	runtime.GC()
 	// XXX: this needs a checker like go-tpm2/testutil's IntGreater, which copes with
 	// types of int64 kind
 	c.Check(time2 > time1, testutil.IsTrue)
 
-	time2 = KeyDuration(&CostParams{Time: 4, MemoryKiB: 32 * 1024, Threads: 1}, 32)
+	time2 = KeyDuration(&CostParams{Time: 4, MemoryKiB: 32 * 1024, Threads: 1})
 	runtime.GC()
 	// XXX: this needs a checker like go-tpm2/testutil's IntGreater, which copes with
 	// types of int64 kind

diff --git a/internal/testutil/kdf.go b/internal/testutil/kdf.go
@@ -23,7 +23,6 @@ import (
 	"crypto"
 	_ "crypto/sha256"
 	"encoding/binary"
-	"errors"
 	"time"
 
 	kdf "github.com/canonical/go-sp800.108-kdf"
@@ -34,9 +33,6 @@ import (
 // MockKDF provides a mock implementation of secboot.KDF that isn't
 // memory intensive.
 type MockKDF struct {
-	// BenchmarkKeyLen is the key length that Time was called with. Set this
-	// to zero before running a mock benchmark.
-	BenchmarkKeyLen uint32
 }
 
 // Derive implements secboot.KDF.Derive and derives a key from the supplied
@@ -54,12 +50,7 @@ func (_ *MockKDF) Derive(passphrase string, salt []byte, params *secboot.KDFCost
 
 // Time implements secboot.KDF.Time and returns a time that is linearly
 // related to the specified cost parameters, suitable for mocking benchmarking.
-func (k *MockKDF) Time(params *secboot.KDFCostParams, keyLen uint32) (time.Duration, error) {
-	if k.BenchmarkKeyLen != 0 && k.BenchmarkKeyLen != keyLen {
-		return 0, errors.New("unexpected key length")
-	}
-	k.BenchmarkKeyLen = keyLen
-
+func (_ *MockKDF) Time(params *secboot.KDFCostParams) (time.Duration, error) {
 	const memBandwidthKiBPerMs = 2048
 	duration := (time.Duration(float64(params.MemoryKiB)/float64(memBandwidthKiBPerMs)) * time.Duration(params.Time)) * time.Millisecond
 	return duration, nil