From 651f6dd48b96c6ca1827429537b85a079add68a9 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 | 72 +++++++++++++++++-------------- internal/luks2/cryptsetup_test.go | 26 +++-------- 2 files changed, 45 insertions(+), 53 deletions(-) diff --git a/crypt_test.go b/crypt_test.go index dac38f7d..f1882f2a 100644 --- a/crypt_test.go +++ b/crypt_test.go @@ -3539,6 +3539,15 @@ 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.KDFOptions != nil { + expectedTargetDuration = options.KDFOptions.TargetDuration + } + c.Check(opts.KDFOptions.TargetDuration, Equals, expectedTargetDuration) + return luks2.Format(devicePath, label, key, opts) + }) + key := s.newPrimaryKey() path := luks2test.CreateEmptyDiskImage(c, 20) @@ -3595,20 +3604,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 +3657,15 @@ 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) + }) + key := s.newPrimaryKey() path := luks2test.CreateEmptyDiskImage(c, 20) @@ -3698,20 +3709,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 +3760,15 @@ 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) + }) + key := s.newPrimaryKey() path := luks2test.CreateEmptyDiskImage(c, 20) @@ -3799,20 +3812,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) } }