From 8a936b143e7cfa2e756d260ea56d1868d9a59e15 Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Tue, 21 Nov 2023 20:48:13 +0000 Subject: [PATCH] luks2: don't time tests to verify KDF settings There are a few tests where we time the KDF operation because there isn't another way to determine if it was configured correctly by cryptsetup. These tests have always been a little bit flaky, and they have been significantly more so recently. Stop timing them, and instead: - in internal/luks2, just rely on the fact that we verify the expected commandline arguments were passed to cryptsetup. We have to assume that these options are correct, which is what the existing tests attempted to confirm. - in crypt_test.go, ensure that we pass the correct target time to internal/luks2. --- crypt_test.go | 75 +++++++++++++++++-------------- internal/luks2/cryptsetup_test.go | 26 +++-------- 2 files changed, 48 insertions(+), 53 deletions(-) diff --git a/crypt_test.go b/crypt_test.go index dac38f7d..979430b7 100644 --- a/crypt_test.go +++ b/crypt_test.go @@ -3539,6 +3539,16 @@ var _ = Suite(&cryptSuiteUnmocked{}) var _ = Suite(&cryptSuiteUnmockedExpensive{}) func (s *cryptSuiteUnmockedBase) testInitializeLUKS2Container(c *C, options *InitializeLUKS2ContainerOptions) { + restore := MockLUKS2Format(func(devicePath, label string, key []byte, opts *luks2.FormatOptions) error { + var expectedTargetDuration time.Duration + if options != nil && options.KDFOptions != nil { + expectedTargetDuration = options.KDFOptions.TargetDuration + } + c.Check(opts.KDFOptions.TargetDuration, Equals, expectedTargetDuration) + return luks2.Format(devicePath, label, key, opts) + }) + defer restore() + key := s.newPrimaryKey() path := luks2test.CreateEmptyDiskImage(c, 20) @@ -3595,20 +3605,13 @@ func (s *cryptSuiteUnmockedBase) testInitializeLUKS2Container(c *C, options *Ini c.Check(keyslot.KDF.Memory, Equals, expectedMemoryKiB) luks2test.CheckLUKS2Passphrase(c, path, key) } else { - expectedKDFTime := 2 * time.Second - if options.KDFOptions.TargetDuration > 0 { - expectedKDFTime = options.KDFOptions.TargetDuration - } - c.Check(keyslot.KDF.Memory, snapd_testutil.IntLessEqual, expectedMemoryKiB) - start := time.Now() + // We used to time this to make sure we are supplying the correct parameters to + // cryptsetup, but that was unreliable. For now, we rely on verifying that we + // pass the correct TargetDuration to internal/luks2 and trust that it does + // the right thing with it. luks2test.CheckLUKS2Passphrase(c, path, key) - elapsed := time.Now().Sub(start) - - // Check KDF time here with +/-20% tolerance and additional 500ms for cryptsetup exec and other activities - c.Check(int(elapsed/time.Millisecond), snapd_testutil.IntGreaterThan, int(float64(expectedKDFTime/time.Millisecond)*0.8)) - c.Check(int(elapsed/time.Millisecond), snapd_testutil.IntLessThan, int(float64(expectedKDFTime/time.Millisecond)*1.2)+500) } } @@ -3655,6 +3658,16 @@ type testAddLUKS2ContainerUnlockKeyUnmockedData struct { } func (s *cryptSuiteUnmockedBase) testAddLUKS2ContainerUnlockKey(c *C, data *testAddLUKS2ContainerUnlockKeyUnmockedData) { + restore := MockLUKS2AddKey(func(devicePath string, existingKey, key []byte, opts *luks2.AddKeyOptions) error { + var expectedTargetDuration time.Duration + if data.options != nil { + expectedTargetDuration = data.options.TargetDuration + } + c.Check(opts.KDFOptions.TargetDuration, Equals, expectedTargetDuration) + return luks2.AddKey(devicePath, existingKey, key, opts) + }) + defer restore() + key := s.newPrimaryKey() path := luks2test.CreateEmptyDiskImage(c, 20) @@ -3698,20 +3711,13 @@ func (s *cryptSuiteUnmockedBase) testAddLUKS2ContainerUnlockKey(c *C, data *test c.Check(keyslot.KDF.Memory, Equals, expectedMemoryKiB) luks2test.CheckLUKS2Passphrase(c, path, newKey) } else { - expectedKDFTime := 2 * time.Second - if options.TargetDuration > 0 { - expectedKDFTime = options.TargetDuration - } - c.Check(keyslot.KDF.Memory, snapd_testutil.IntLessEqual, expectedMemoryKiB) - start := time.Now() + // We used to time this to make sure we are supplying the correct parameters to + // cryptsetup, but that was unreliable. For now, we rely on verifying that we + // pass the correct TargetDuration to internal/luks2 and trust that it does + // the right thing with it. luks2test.CheckLUKS2Passphrase(c, path, newKey) - elapsed := time.Now().Sub(start) - - // Check KDF time here with +/-20% tolerance and additional 500ms for cryptsetup exec and other activities - c.Check(int(elapsed/time.Millisecond), snapd_testutil.IntGreaterThan, int(float64(expectedKDFTime/time.Millisecond)*0.8)) - c.Check(int(elapsed/time.Millisecond), snapd_testutil.IntLessThan, int(float64(expectedKDFTime/time.Millisecond)*1.2)+500) } luks2test.CheckLUKS2Passphrase(c, path, key) @@ -3756,6 +3762,16 @@ type testAddLUKS2ContainerRecoveryKeyUnmockedData struct { } func (s *cryptSuiteUnmockedBase) testAddLUKS2ContainerRecoveryKey(c *C, data *testAddLUKS2ContainerRecoveryKeyUnmockedData) { + restore := MockLUKS2AddKey(func(devicePath string, existingKey, key []byte, opts *luks2.AddKeyOptions) error { + var expectedTargetDuration time.Duration + if data.options != nil { + expectedTargetDuration = data.options.TargetDuration + } + c.Check(opts.KDFOptions.TargetDuration, Equals, expectedTargetDuration) + return luks2.AddKey(devicePath, existingKey, key, opts) + }) + restore() + key := s.newPrimaryKey() path := luks2test.CreateEmptyDiskImage(c, 20) @@ -3799,20 +3815,13 @@ func (s *cryptSuiteUnmockedBase) testAddLUKS2ContainerRecoveryKey(c *C, data *te c.Check(keyslot.KDF.Memory, Equals, expectedMemoryKiB) luks2test.CheckLUKS2Passphrase(c, path, recoveryKey[:]) } else { - expectedKDFTime := 2 * time.Second - if options.TargetDuration > 0 { - expectedKDFTime = options.TargetDuration - } - c.Check(keyslot.KDF.Memory, snapd_testutil.IntLessEqual, expectedMemoryKiB) - start := time.Now() + // We used to time this to make sure we are supplying the correct parameters to + // cryptsetup, but that was unreliable. For now, we rely on verifying that we + // pass the correct TargetDuration to internal/luks2 and trust that it does + // the right thing with it. luks2test.CheckLUKS2Passphrase(c, path, recoveryKey[:]) - elapsed := time.Now().Sub(start) - - // Check KDF time here with +/-20% tolerance and additional 500ms for cryptsetup exec and other activities - c.Check(int(elapsed/time.Millisecond), snapd_testutil.IntGreaterThan, int(float64(expectedKDFTime/time.Millisecond)*0.8)) - c.Check(int(elapsed/time.Millisecond), snapd_testutil.IntLessThan, int(float64(expectedKDFTime/time.Millisecond)*1.2)+500) } luks2test.CheckLUKS2Passphrase(c, path, key) diff --git a/internal/luks2/cryptsetup_test.go b/internal/luks2/cryptsetup_test.go index 30e3503d..28022d39 100644 --- a/internal/luks2/cryptsetup_test.go +++ b/internal/luks2/cryptsetup_test.go @@ -274,19 +274,12 @@ func (s *cryptsetupSuiteBase) testFormat(c *C, data *testFormatData) { c.Check(keyslot.KDF.Time, Equals, options.KDFOptions.ForceIterations) c.Check(keyslot.KDF.Memory, Equals, expectedMemoryKiB) } else { - expectedKDFTime := 2000 * time.Millisecond - if options.KDFOptions.TargetDuration > 0 { - expectedKDFTime = options.KDFOptions.TargetDuration - } - c.Check(keyslot.KDF.Memory, snapd_testutil.IntLessEqual, expectedMemoryKiB) - start := time.Now() + // We used to time this to make sure we are supplying the correct parameters to + // cryptsetup, but that was unreliable. For now, we rely on the command line + // parameters that were tested earlier, and trust that those are correct. luks2test.CheckLUKS2Passphrase(c, devicePath, data.key) - elapsed := time.Now().Sub(start) - // Check KDF time here with +/-20% tolerance and additional 500ms for cryptsetup exec and other activities - c.Check(int(elapsed/time.Millisecond), snapd_testutil.IntGreaterThan, int(float64(expectedKDFTime/time.Millisecond)*0.8)) - c.Check(int(elapsed/time.Millisecond), snapd_testutil.IntLessThan, int(float64(expectedKDFTime/time.Millisecond)*1.2)+500) } } @@ -497,19 +490,12 @@ func (s *cryptsetupSuiteBase) testAddKey(c *C, data *testAddKeyData) { c.Check(keyslot.KDF.Time, Equals, options.KDFOptions.ForceIterations) c.Check(keyslot.KDF.Memory, Equals, expectedMemoryKiB) } else { - expectedKDFTime := 2000 * time.Millisecond - if options.KDFOptions.TargetDuration > 0 { - expectedKDFTime = options.KDFOptions.TargetDuration - } - c.Check(keyslot.KDF.Memory, snapd_testutil.IntLessEqual, expectedMemoryKiB) - start := time.Now() + // We used to time this to make sure we are supplying the correct parameters to + // cryptsetup, but that was unreliable. For now, we rely on the command line + // parameters that were tested earlier, and trust that those are correct. luks2test.CheckLUKS2Passphrase(c, devicePath, data.key) - elapsed := time.Now().Sub(start) - // Check KDF time here with +/-20% tolerance and additional 500ms for cryptsetup exec and other activities - c.Check(int(elapsed/time.Millisecond), snapd_testutil.IntGreaterThan, int(float64(expectedKDFTime/time.Millisecond)*0.8)) - c.Check(int(elapsed/time.Millisecond), snapd_testutil.IntLessThan, int(float64(expectedKDFTime/time.Millisecond)*1.2)+500) } }