Skip to content

Commit

Permalink
Have a single ActivateVolumeOptions struct for the ActivateVolumeWith…
Browse files Browse the repository at this point in the history 
…* family

Merge pull request #116 from snapcore/activate-volume-options

Also rename PIN in its definition to Passphrase to move toward
better and also more abstract naming.

Same in the corresponding RecoveryKeyUsageReason code.

See also issue #113
  • Loading branch information
@pedronis
pedronis authored Sep 25, 2020
2 parents 9fd6c3e + 8995bba commit 70fb38d
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 72 deletions.
116 changes: 60 additions & 56 deletions crypt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,10 @@ const (
// is also not correctly provisioned.
RecoveryKeyUsageReasonInvalidKeyFile

// RecoveryKeyUsageReasonPINFail indicates that a volume had to be activated with the fallback recovery key because the correct PIN
// was not provided.
RecoveryKeyUsageReasonPINFail
// RecoveryKeyUsageReasonPassphraseFail indicates that a
// volume had to be activated with the fallback recovery key
// because the correct user passphrase/PIN was not provided.
RecoveryKeyUsageReasonPassphraseFail
)

func activateWithRecoveryKey(volumeName, sourceDevicePath string, keyReader io.Reader, tries int, reason RecoveryKeyUsageReason, activateOptions []string, keyringPrefix string) error {
Expand Down Expand Up @@ -367,7 +368,7 @@ func isLockAccessError(err error) bool {
return xerrors.As(err, &e)
}

func activateWithTPMKey(tpm *TPMConnection, volumeName, sourceDevicePath, keyPath string, pinReader io.Reader, pinTries int, lock bool, activateOptions []string, keyringPrefix string) error {
func activateWithTPMKey(tpm *TPMConnection, volumeName, sourceDevicePath, keyPath string, passphraseReader io.Reader, passphraseTries int, lock bool, activateOptions []string, keyringPrefix string) error {
var lockErr error
sealedKey, authPrivateKey, err := func() ([]byte, TPMPolicyAuthKey, error) {
defer func() {
Expand All @@ -383,20 +384,20 @@ func activateWithTPMKey(tpm *TPMConnection, volumeName, sourceDevicePath, keyPat
}

switch {
case pinTries == 0 && k.AuthMode2F() == AuthModePIN:
case passphraseTries == 0 && k.AuthMode2F() == AuthModePIN:
return nil, nil, requiresPinErr
case pinTries == 0:
pinTries = 1
case passphraseTries == 0:
passphraseTries = 1
}

var sealedKey []byte
var authPrivateKey []byte

for ; pinTries > 0; pinTries-- {
for ; passphraseTries > 0; passphraseTries-- {
var pin string
if k.AuthMode2F() == AuthModePIN {
r := pinReader
pinReader = nil
r := passphraseReader
passphraseReader = nil
pin, err = getPassword(sourceDevicePath, "PIN", r)
if err != nil {
return nil, nil, xerrors.Errorf("cannot obtain PIN: %w", err)
Expand Down Expand Up @@ -454,40 +455,56 @@ func makeActivateOptions(in []string) ([]string, error) {
return append(out, "tries=1"), nil
}

// ActivateWithTPMSealedKeyOptions provides options to ActivateVolumeWtthTPMSealedKey.
type ActivateWithTPMSealedKeyOptions struct {
// PINTries specifies the maximum number of times that unsealing with a PIN should be attempted before failing with an error and
// falling back to activating with the recovery key if RecoveryKeyTries is greater than zero. Setting this to zero disables unsealing
// with a PIN - in this case, an error will be returned if the sealed key object indicates that a PIN has been set. Attempts to
// unseal with a PIN will stop if the TPM enters dictionary attack lockout mode before this limit is reached.
PINTries int

// RecoveryKeyTries specifies the maximum number of times that activation with the fallback recovery key should be attempted
// if activation with the TPM sealed key fails, before failing with an error. Setting this to zero will disable attempts to activate
// with the fallback recovery key.
// ActivateVolumeOptions provides options to the ActivateVolumeWith*
// family of functions.
type ActivateVolumeOptions struct {
// PassphraseTries specifies the maximum number of times
// that unsealing with a user passphrase should be attempted
// before failing with an error and falling back to activating
// with the recovery key (see RecoveryKeyTries).
// Setting this to zero disables unsealing with a user
// passphrase - in this case, an error will be returned if the
// sealed key object indicates that a user passphrase has been
// set.
// With a TPM, attempts to unseal will stop if the TPM enters
// dictionary attack lockout mode before this limit is
// reached.
// It is ignored by ActivateWithRecoveryKey.
PassphraseTries int

// RecoveryKeyTries specifies the maximum number of times that
// activation with the fallback recovery key should be
// attempted.
// It is used directly by ActivateWithRecoveryKey and
// indirectly with other methods upon failure, for example
// failed TPM unsealing. Setting this to zero will disable
// attempts to activate with the fallback recovery key.
RecoveryKeyTries int

// ActivateOptions provides a mechanism to pass additional options to systemd-cryptsetup.
// ActivateOptions provides a mechanism to pass additional
// options to systemd-cryptsetup.
ActivateOptions []string

// LockSealedKeyAccess controls whether LockAccessToSealedKeys should be called after unsealing the TPM sealed key. It is called if
// this is set to true, and not called if this is set to false.
LockSealedKeyAccess bool
// LockSealedKeys controls whether access to the keys is
// locked after unsealing by calling LockAccessToSealedKeys in
// TPM case for example.
LockSealedKeys bool

// KeyringPrefix is the prefix used for the description of any kernel keys created during activation.
// KeyringPrefix is the prefix used for the description of any
// kernel keys created during activation.
KeyringPrefix string
}

// ActivateVolumeWithTPMSealedKey attempts to activate the LUKS encrypted volume at sourceDevicePath and create a mapping with the
// name volumeName, using the TPM sealed key object at the specified keyPath. This makes use of systemd-cryptsetup.
//
// If the TPM sealed key object has a PIN defined, then this function will use systemd-ask-password to request it. If pinReader is not
// nil, then an attempt to read the PIN from this will be made instead by reading all characters until the first newline. The PINTries
// field of options defines how many attempts should be made to obtain the correct PIN before failing.
// If the TPM sealed key object has a user passphrase/PIN defined, then this function will use systemd-ask-password to request it. If passphraseReader is not
// nil, then an attempt to read the user passphrase/PIN from this will be made instead by reading all characters until the first newline. The PassphraseTries
// field of options defines how many attempts should be made to obtain the correct passphrase before failing.
//
// The ActivateOptions field of options can be used to specify additional options to pass to systemd-cryptsetup.
//
// If the LockSealedKeyAccess field of options is true, then this function will call LockAccessToSealedKeys after unsealing the key
// If the LockSealedKeys field of options is true, then this function will call LockAccessToSealedKeys after unsealing the key
// and before activating the LUKS volume.
//
// If activation with the TPM sealed key object fails, this function will attempt to activate it with the fallback recovery key
Expand All @@ -497,7 +514,7 @@ type ActivateWithTPMSealedKeyOptions struct {
// calling GetActivationDataFromKernel will return a *RecoveryActivationData containing the recovery key and the reason that the
// recovery key was requested.
//
// If either the PINTries or RecoveryKeyTries fields of options are less than zero, an error will be returned. If the ActivateOptions
// If either the PassphraseTries or RecoveryKeyTries fields of options are less than zero, an error will be returned. If the ActivateOptions
// field of options contains the "tries=" option, then an error will be returned. This option cannot be used with this function.
//
// If the LockSealedKeyAccess field of options is true and the call to LockAccessToSealedKeys fails, a LockAccessToSealedKeysError
Expand All @@ -514,9 +531,9 @@ type ActivateWithTPMSealedKeyOptions struct {
//
// If the volume is successfully activated, either with the TPM sealed key or the fallback recovery key, this function returns true.
// If it is not successfully activated, then this function returns false.
func ActivateVolumeWithTPMSealedKey(tpm *TPMConnection, volumeName, sourceDevicePath, keyPath string, pinReader io.Reader, options *ActivateWithTPMSealedKeyOptions) (bool, error) {
if options.PINTries < 0 {
return false, errors.New("invalid PINTries")
func ActivateVolumeWithTPMSealedKey(tpm *TPMConnection, volumeName, sourceDevicePath, keyPath string, passphraseReader io.Reader, options *ActivateVolumeOptions) (bool, error) {
if options.PassphraseTries < 0 {
return false, errors.New("invalid PassphraseTries")
}
if options.RecoveryKeyTries < 0 {
return false, errors.New("invalid RecoveryKeyTries")
Expand All @@ -527,7 +544,7 @@ func ActivateVolumeWithTPMSealedKey(tpm *TPMConnection, volumeName, sourceDevice
return false, err
}

if err := activateWithTPMKey(tpm, volumeName, sourceDevicePath, keyPath, pinReader, options.PINTries, options.LockSealedKeyAccess, activateOptions, options.KeyringPrefix); err != nil {
if err := activateWithTPMKey(tpm, volumeName, sourceDevicePath, keyPath, passphraseReader, options.PassphraseTries, options.LockSealedKeys, activateOptions, options.KeyringPrefix); err != nil {
reason := RecoveryKeyUsageReasonUnexpectedError
switch {
case isLockAccessError(err):
Expand All @@ -539,9 +556,9 @@ func ActivateVolumeWithTPMSealedKey(tpm *TPMConnection, volumeName, sourceDevice
case isInvalidKeyFileError(err):
reason = RecoveryKeyUsageReasonInvalidKeyFile
case xerrors.Is(err, requiresPinErr):
reason = RecoveryKeyUsageReasonPINFail
reason = RecoveryKeyUsageReasonPassphraseFail
case xerrors.Is(err, ErrPINFail):
reason = RecoveryKeyUsageReasonPINFail
reason = RecoveryKeyUsageReasonPassphraseFail
case isExecError(err, systemdCryptsetupPath):
// systemd-cryptsetup only provides 2 exit codes - success or fail - so we don't know the reason it failed yet. If activation
// with the recovery key is successful, then it's safe to assume that it failed because the key unsealed from the TPM is incorrect.
Expand All @@ -554,44 +571,31 @@ func ActivateVolumeWithTPMSealedKey(tpm *TPMConnection, volumeName, sourceDevice
return true, nil
}

// ActivateWithRecoveryKeyOptions provides options to ActivateVolumeWithRecoveryKey.
type ActivateWithRecoveryKeyOptions struct {
// Tries specifies the maximum number of times that activation with the fallback recovery key should be attempted before failing
// with an error.
Tries int

// ActivateOptions provides a mechanism to pass additional options to systemd-cryptsetup.
ActivateOptions []string

// KeyringPrefix is the prefix used for the description of any kernel keys created during activation.
KeyringPrefix string
}

// ActivateVolumeWithRecoveryKey attempts to activate the LUKS encrypted volume at sourceDevicePath and create a mapping with the
// name volumeName, using the fallback recovery key. This makes use of systemd-cryptsetup.
//
// This function will use systemd-ask-password to request the recovery key. If keyReader is not nil, then an attempt to read the key
// from this will be made instead by reading all characters until the first newline. The Tries field of options defines how many
// from this will be made instead by reading all characters until the first newline. The RecoveryKeyTries field of options defines how many
// attempts should be made to activate the volume with the recovery key before failing.
//
// The ActivateOptions field of options can be used to specify additional options to pass to systemd-cryptsetup.
//
// If activation with the recovery key is successful, calling GetActivationDataFromKernel will return a *RecoveryActivationData
// containing the recovery key and RecoveryKeyUsageReasonRequested as the recovery reason.
//
// If the Tries field of options is less than zero, an error will be returned. If the ActivateOptions field of options contains the
// If the RecoveryKeyTries field of options is less than zero, an error will be returned. If the ActivateOptions field of options contains the
// "tries=" option, then an error will be returned. This option cannot be used with this function.
func ActivateVolumeWithRecoveryKey(volumeName, sourceDevicePath string, keyReader io.Reader, options *ActivateWithRecoveryKeyOptions) error {
if options.Tries < 0 {
return errors.New("invalid Tries")
func ActivateVolumeWithRecoveryKey(volumeName, sourceDevicePath string, keyReader io.Reader, options *ActivateVolumeOptions) error {
if options.RecoveryKeyTries < 0 {
return errors.New("invalid RecoveryKeyTries")
}

activateOptions, err := makeActivateOptions(options.ActivateOptions)
if err != nil {
return err
}

return activateWithRecoveryKey(volumeName, sourceDevicePath, keyReader, options.Tries, RecoveryKeyUsageReasonRequested, activateOptions, options.KeyringPrefix)
return activateWithRecoveryKey(volumeName, sourceDevicePath, keyReader, options.RecoveryKeyTries, RecoveryKeyUsageReasonRequested, activateOptions, options.KeyringPrefix)
}

// ActivationData corresponds to some data added to the user keyring by one of the ActivateVolume functions.
Expand Down
32 changes: 16 additions & 16 deletions crypt_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,8 @@ type testActivateVolumeWithTPMSealedKeyNo2FAData struct {
}

func (s *cryptTPMSimulatorSuite) testActivateVolumeWithTPMSealedKeyNo2FA(c *C, data *testActivateVolumeWithTPMSealedKeyNo2FAData) {
options := ActivateWithTPMSealedKeyOptions{
PINTries: data.pinTries,
options := ActivateVolumeOptions{
PassphraseTries: data.pinTries,
RecoveryKeyTries: data.recoveryKeyTries,
ActivateOptions: data.activateOptions,
KeyringPrefix: data.keyringPrefix}
Expand All @@ -336,7 +336,7 @@ func (s *cryptTPMSimulatorSuite) TestActivateVolumeWithTPMSealedKeyNo2FA1(c *C)
}

func (s *cryptTPMSimulatorSuite) TestActivateVolumeWithTPMSealedKeyNo2FA2(c *C) {
// Test with a non-zero PINTries when a PIN isn't set.
// Test with a non-zero PassphraseTries when a PIN isn't set.
s.testActivateVolumeWithTPMSealedKeyNo2FA(c, &testActivateVolumeWithTPMSealedKeyNo2FAData{
volumeName: "data",
sourceDevicePath: "/dev/sda1",
Expand Down Expand Up @@ -399,7 +399,7 @@ type testActivateVolumeWithTPMSealedKeyAndPINData struct {
func (s *cryptTPMSimulatorSuite) testActivateVolumeWithTPMSealedKeyAndPIN(c *C, data *testActivateVolumeWithTPMSealedKeyAndPINData) {
c.Assert(ioutil.WriteFile(s.passwordFile, []byte(strings.Join(data.pins, "\n")+"\n"), 0644), IsNil)

options := ActivateWithTPMSealedKeyOptions{PINTries: data.pinTries}
options := ActivateVolumeOptions{PassphraseTries: data.pinTries}
success, err := ActivateVolumeWithTPMSealedKey(s.TPM, "data", "/dev/sda1", s.keyFile, nil, &options)
c.Check(success, Equals, true)
c.Check(err, IsNil)
Expand Down Expand Up @@ -454,7 +454,7 @@ func (s *cryptTPMSimulatorSuite) testActivateVolumeWithTPMSealedKeyAndPINUsingPI
c.Assert(err, IsNil)
defer r.Close()

options := ActivateWithTPMSealedKeyOptions{PINTries: data.pinTries}
options := ActivateVolumeOptions{PassphraseTries: data.pinTries}
success, err := ActivateVolumeWithTPMSealedKey(s.TPM, "data", "/dev/sda1", s.keyFile, r, &options)
c.Check(success, Equals, true)
c.Check(err, IsNil)
Expand Down Expand Up @@ -536,8 +536,8 @@ type testActivateVolumeWithTPMSealedKeyErrorHandlingData struct {
func (s *cryptTPMSimulatorSuite) testActivateVolumeWithTPMSealedKeyErrorHandling(c *C, data *testActivateVolumeWithTPMSealedKeyErrorHandlingData) {
c.Assert(ioutil.WriteFile(s.passwordFile, []byte(strings.Join(data.passphrases, "\n")+"\n"), 0644), IsNil)

options := ActivateWithTPMSealedKeyOptions{
PINTries: data.pinTries,
options := ActivateVolumeOptions{
PassphraseTries: data.pinTries,
RecoveryKeyTries: data.recoveryKeyTries,
ActivateOptions: data.activateOptions,
KeyringPrefix: data.keyringPrefix}
Expand Down Expand Up @@ -571,11 +571,11 @@ func (s *cryptTPMSimulatorSuite) testActivateVolumeWithTPMSealedKeyErrorHandling
}

func (s *cryptTPMSimulatorSuite) TestActivateVolumeWithTPMSealedKeyErrorHandling1(c *C) {
// Test with an invalid value for PINTries.
// Test with an invalid value for PassphraseTries.
s.testActivateVolumeWithTPMSealedKeyErrorHandling(c, &testActivateVolumeWithTPMSealedKeyErrorHandlingData{
pinTries: -1,
errChecker: ErrorMatches,
errCheckerArgs: []interface{}{"invalid PINTries"},
errCheckerArgs: []interface{}{"invalid PassphraseTries"},
})
}

Expand Down Expand Up @@ -705,7 +705,7 @@ func (s *cryptTPMSimulatorSuite) TestActivateVolumeWithTPMSealedKeyErrorHandling
},
sdCryptsetupCalls: 1,
success: true,
recoveryReason: RecoveryKeyUsageReasonPINFail,
recoveryReason: RecoveryKeyUsageReasonPassphraseFail,
errChecker: ErrorMatches,
errCheckerArgs: []interface{}{"cannot activate with TPM sealed key \\(cannot unseal key: the provided PIN is incorrect\\) but " +
"activation with recovery key was successful"},
Expand All @@ -720,7 +720,7 @@ func (s *cryptTPMSimulatorSuite) TestActivateVolumeWithTPMSealedKeyErrorHandling
passphrases: []string{strings.Join(s.recoveryKeyAscii, "-")},
sdCryptsetupCalls: 1,
success: true,
recoveryReason: RecoveryKeyUsageReasonPINFail,
recoveryReason: RecoveryKeyUsageReasonPassphraseFail,
errChecker: ErrorMatches,
errCheckerArgs: []interface{}{"cannot activate with TPM sealed key \\(no PIN tries permitted when a PIN is required\\) but " +
"activation with recovery key was successful"},
Expand Down Expand Up @@ -793,7 +793,7 @@ type testActivateVolumeWithRecoveryKeyData struct {
func (s *cryptSuite) testActivateVolumeWithRecoveryKey(c *C, data *testActivateVolumeWithRecoveryKeyData) {
c.Assert(ioutil.WriteFile(s.passwordFile, []byte(strings.Join(data.recoveryPassphrases, "\n")+"\n"), 0644), IsNil)

options := ActivateWithRecoveryKeyOptions{Tries: data.tries, ActivateOptions: data.activateOptions, KeyringPrefix: data.keyringPrefix}
options := ActivateVolumeOptions{RecoveryKeyTries: data.tries, ActivateOptions: data.activateOptions, KeyringPrefix: data.keyringPrefix}
c.Assert(ActivateVolumeWithRecoveryKey(data.volumeName, data.sourceDevicePath, nil, &options), IsNil)

c.Check(len(s.mockSdAskPassword.Calls()), Equals, len(data.recoveryPassphrases))
Expand Down Expand Up @@ -915,7 +915,7 @@ func (s *cryptSuite) testActivateVolumeWithRecoveryKeyUsingKeyReader(c *C, data
c.Assert(err, IsNil)
defer r.Close()

options := ActivateWithRecoveryKeyOptions{Tries: data.tries}
options := ActivateVolumeOptions{RecoveryKeyTries: data.tries}
c.Assert(ActivateVolumeWithRecoveryKey("data", "/dev/sda1", r, &options), IsNil)

c.Check(len(s.mockSdAskPassword.Calls()), Equals, len(data.recoveryPassphrases))
Expand Down Expand Up @@ -1112,7 +1112,7 @@ type testActivateVolumeWithRecoveryKeyErrorHandlingData struct {
func (s *cryptSuite) testActivateVolumeWithRecoveryKeyErrorHandling(c *C, data *testActivateVolumeWithRecoveryKeyErrorHandlingData) {
c.Assert(ioutil.WriteFile(s.passwordFile, []byte(strings.Join(data.recoveryPassphrases, "\n")+"\n"), 0644), IsNil)

options := ActivateWithRecoveryKeyOptions{Tries: data.tries, ActivateOptions: data.activateOptions}
options := ActivateVolumeOptions{RecoveryKeyTries: data.tries, ActivateOptions: data.activateOptions}
c.Check(ActivateVolumeWithRecoveryKey("data", "/dev/sda1", nil, &options), data.errChecker, data.errCheckerArgs...)

c.Check(len(s.mockSdAskPassword.Calls()), Equals, len(data.recoveryPassphrases))
Expand All @@ -1132,11 +1132,11 @@ func (s *cryptSuite) testActivateVolumeWithRecoveryKeyErrorHandling(c *C, data *
}

func (s *cryptSuite) TestActivateVolumeWithRecoveryKeyErrorHandling1(c *C) {
// Test with an invalid Tries value.
// Test with an invalid RecoveryKeyTries value.
s.testActivateVolumeWithRecoveryKeyErrorHandling(c, &testActivateVolumeWithRecoveryKeyErrorHandlingData{
tries: -1,
errChecker: ErrorMatches,
errCheckerArgs: []interface{}{"invalid Tries"},
errCheckerArgs: []interface{}{"invalid RecoveryKeyTries"},
})
}

Expand Down

0 comments on commit 70fb38d

Please sign in to comment.