From 01e2af045dac1a037b36b1605dea687b0680f049 Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Wed, 28 Jan 2026 22:45:52 +0000 Subject: [PATCH 1/3] AuthRequestor: Add NotifyUserAuthResult API. This adds a new API to AuthRequestor so that ActivcateContext can notify the user about auth results. It notifies about success, failure and when the user supplied credential is badly formatted for the indicated type. It also provides a way to indicate when there are no more tries left for a type. The Plymouth implementation of this sends messages to Plymouth to be displayed. The systemd implementation of this logs errors to stderr. Fixes: FR-12404 --- activate.go | 79 +++++----- activate_test.go | 120 +++++++++++++-- auth_requestor.go | 65 +++++++- auth_requestor_plymouth.go | 35 ++++- auth_requestor_plymouth_test.go | 255 +++++++++++++++++++++++++++++++- auth_requestor_systemd.go | 28 +++- auth_requestor_systemd_test.go | 129 +++++++++++++++- auth_requestor_test.go | 62 ++++++++ crypt_test.go | 16 ++ export_test.go | 31 ++++ 10 files changed, 756 insertions(+), 64 deletions(-) create mode 100644 auth_requestor_test.go diff --git a/activate.go b/activate.go index 5451d796..3109761a 100644 --- a/activate.go +++ b/activate.go @@ -633,17 +633,7 @@ func (m *activateOneContainerStateMachine) tryWithUserAuthKeyslots(ctx context.C authType |= UserAuthTypeRecoveryKey } - // XXX: When PIN support lands, this will loop on available PIN tries as well. - for passphraseTries > 0 || pinTries > 0 || recoveryKeyTries > 0 { - // Don't try a method where there are no more usable keyslots. - if !passphraseSlotRecords.hasUsable(m.flags) { - passphraseTries = 0 - } - if !pinSlotRecords.hasUsable(m.flags) { - pinTries = 0 - } - - // Update authType flags + updateAvailableAuthType := func() { if passphraseTries == 0 { // No more passphrase key tries are left. authType &^= UserAuthTypePassphrase @@ -656,11 +646,16 @@ func (m *activateOneContainerStateMachine) tryWithUserAuthKeyslots(ctx context.C // No more recovery key tries are left. authType &^= UserAuthTypeRecoveryKey } + } + updateAvailableAuthType() - if authType == UserAuthType(0) { - break - } + var ( + triedAuthType UserAuthType // The types of credentials that were tried. + invalidAuthType UserAuthType // The types of credentials that weren't tried because the format is invalid. + successfulAuthType UserAuthType // The type of credential that was used successfully. + ) + for authType != UserAuthType(0) { cred, credAuthType, err := authRequestor.RequestUserCredential(ctx, name, m.container.Path(), authType) if err != nil { return fmt.Errorf("cannot request user credential: %w", err) @@ -679,8 +674,10 @@ func (m *activateOneContainerStateMachine) tryWithUserAuthKeyslots(ctx context.C ) if credAuthType&UserAuthTypePassphrase > 0 { + triedAuthType |= UserAuthTypePassphrase passphraseTries -= 1 if uk, pk, success := m.tryPassphraseKeyslotsHelper(ctx, passphraseSlotRecords, cred); success { + successfulAuthType = UserAuthTypePassphrase unlockKey = uk primaryKey = pk } @@ -689,20 +686,15 @@ func (m *activateOneContainerStateMachine) tryWithUserAuthKeyslots(ctx context.C if m.status == activationIncomplete && credAuthType&UserAuthTypePIN > 0 { pin, err := ParsePIN(cred) switch { - case err != nil && authType == UserAuthTypePIN: - // We are only expecting a PIN and the user supplied a badly formatted - // one. We can log this to stderr and allow them another attempt. - // XXX: Maybe display a notice in Plymouth for this case in the - // future. - fmt.Fprintf(m.stderr, "Cannot parse PIN: %v\n", err) case err != nil: - // The user supplied credential isn't a valid PIN, but it could be - // a valid passphrase or recovery key, so ignore the error in this - // case. + // The user supplied credential isn't a valid PIN. + invalidAuthType |= UserAuthTypePIN default: // This is a valid PIN + triedAuthType |= UserAuthTypePIN pinTries -= 1 if uk, pk, success := m.tryPINKeyslotsHelper(ctx, pinSlotRecords, pin); success { + successfulAuthType = UserAuthTypePIN unlockKey = uk primaryKey = pk } @@ -712,21 +704,15 @@ func (m *activateOneContainerStateMachine) tryWithUserAuthKeyslots(ctx context.C if m.status == activationIncomplete && credAuthType&UserAuthTypeRecoveryKey > 0 { recoveryKey, err := ParseRecoveryKey(cred) switch { - case err != nil && authType == UserAuthTypeRecoveryKey: - // We are only expecting a recovery key and the user supplied a badly - // formatted one. We can log this to stderr and allow them another - // attempt. - // XXX: Maybe display a notice in Plymouth for this case in the - // future. - fmt.Fprintf(m.stderr, "Cannot parse recovery key: %v\n", err) case err != nil: - // The user supplied credential isn't a valid recovery key, but it - // could be a valid PIN or passphrase, so ignore the error in this - // case. + // The user supplied credential isn't a valid recovery key. + invalidAuthType |= UserAuthTypeRecoveryKey default: // This is a valid recovery key + triedAuthType |= UserAuthTypeRecoveryKey recoveryKeyTries -= 1 if m.tryRecoveryKeyslotsHelper(ctx, recoverySlotRecords, recoveryKey) { + successfulAuthType = UserAuthTypeRecoveryKey unlockKey = DiskUnlockKey(recoveryKey[:]) } } @@ -734,10 +720,37 @@ func (m *activateOneContainerStateMachine) tryWithUserAuthKeyslots(ctx context.C if m.status == activationIncomplete { // We haven't unlocked yet, so try again. + // Firstly, don't retry a method where there are no more usable keyslots. + if !passphraseSlotRecords.hasUsable(m.flags) { + passphraseTries = 0 + } + if !pinSlotRecords.hasUsable(m.flags) { + pinTries = 0 + } + + // Update authType flags + prevAuthType := authType + updateAvailableAuthType() + exhaustedAuthType := prevAuthType &^ authType + + // Notify the UI + result := UserAuthResultFailed + failedAuthType := triedAuthType + if triedAuthType == UserAuthType(0) && invalidAuthType != UserAuthType(0) { + result = UserAuthResultInvalidFormat + failedAuthType = invalidAuthType + } + if err := authRequestor.NotifyUserAuthResult(ctx, result, failedAuthType, exhaustedAuthType); err != nil { + fmt.Fprintf(m.stderr, "Cannot notify user of auth failure: %v\n", err) + } + continue } // We have unlocked successfully. + if err := authRequestor.NotifyUserAuthResult(ctx, UserAuthResultSuccess, successfulAuthType, 0); err != nil { + fmt.Fprintf(m.stderr, "Cannot notify user of auth success: %v\n", err) + } m.next = activateOneContainerStateMachineTask{ name: "add-keyring-keys", fn: func(ctx context.Context) error { diff --git a/activate_test.go b/activate_test.go index 66d8e2ef..4a74a7b7 100644 --- a/activate_test.go +++ b/activate_test.go @@ -412,16 +412,17 @@ type testActivateContextActivateContainerParams struct { legacyV1KeyUnlock bool - expectedStderr string - expectedTryKeys [][]byte - expectedAuthRequestName string - expectedAuthRequestPath string - expectedAuthRequestTypes []UserAuthType - expectedActivateConfig map[any]any - expectedKeyringKeyPrefix string - expectedPrimaryKey PrimaryKey - expectedUnlockKey DiskUnlockKey - expectedState *ContainerActivateState + expectedStderr string + expectedTryKeys [][]byte + expectedAuthRequestName string + expectedAuthRequestPath string + expectedAuthRequestTypes []UserAuthType + expectedAuthRequestResults []mockAuthRequestorResult + expectedActivateConfig map[any]any + expectedKeyringKeyPrefix string + expectedPrimaryKey PrimaryKey + expectedUnlockKey DiskUnlockKey + expectedState *ContainerActivateState } func (s *activateSuite) testActivateContextActivateContainer(c *C, params *testActivateContextActivateContainerParams) error { @@ -491,6 +492,39 @@ func (s *activateSuite) testActivateContextActivateContainer(c *C, params *testA c.Check(req.path, Equals, params.expectedAuthRequestPath) c.Check(req.authTypes, Equals, params.expectedAuthRequestTypes[i]) } + expectedResults := params.expectedAuthRequestResults + if expectedResults == nil { + var authType UserAuthType + state := expectedState.Activations[params.container.CredentialName()] + switch state.Status { + case ActivationSucceededWithRecoveryKey: + authType = UserAuthTypeRecoveryKey + case ActivationSucceededWithPlatformKey: + data := params.container.slots[state.Keyslot].data + kd, err := ReadKeyData(newMockKeyDataReader("", data)) + c.Assert(err, IsNil) + switch kd.AuthMode() { + case AuthModePassphrase: + authType = UserAuthTypePassphrase + case AuthModePIN: + authType = UserAuthTypePIN + } + } + if authType != UserAuthType(0) { + for i := 0; i < len(params.authRequestor.requests)-1; i++ { + expectedResults = append(expectedResults, mockAuthRequestorResult{ + result: UserAuthResultFailed, + authTypes: params.expectedAuthRequestTypes[i], + }) + } + + expectedResults = append(expectedResults, mockAuthRequestorResult{ + result: UserAuthResultSuccess, + authTypes: authType, + }) + } + } + c.Check(params.authRequestor.results, DeepEquals, expectedResults) } expectedCfg := params.expectedActivateConfig @@ -1418,14 +1452,16 @@ func (s *activateSuite) TestActivateContainerRecoveryKeyRetryAfterInvalidRecover opts: []ActivateOption{ WithAuthRequestorUserVisibleName("data"), }, - expectedStderr: `Cannot parse recovery key: incorrectly formatted: insufficient characters -`, expectedAuthRequestName: "data", expectedAuthRequestPath: "/dev/sda1", expectedAuthRequestTypes: []UserAuthType{ UserAuthTypeRecoveryKey, UserAuthTypeRecoveryKey, }, + expectedAuthRequestResults: []mockAuthRequestorResult{ + {result: UserAuthResultInvalidFormat, authTypes: UserAuthTypeRecoveryKey}, + {result: UserAuthResultSuccess, authTypes: UserAuthTypeRecoveryKey}, + }, expectedActivateConfig: map[any]any{ AuthRequestorKey: authRequestor, AuthRequestorUserVisibleNameKey: "data", @@ -1515,8 +1551,6 @@ func (s *activateSuite) TestActivateContainerRecoveryKeyWithRecoveryKeyTries(c * opts: []ActivateOption{ WithAuthRequestorUserVisibleName("data"), }, - expectedStderr: `Cannot parse recovery key: incorrectly formatted: insufficient characters -`, expectedTryKeys: [][]byte{incorrectRecoveryKey, incorrectRecoveryKey, incorrectRecoveryKey}, expectedAuthRequestName: "data", expectedAuthRequestPath: "/dev/sda1", @@ -4443,6 +4477,10 @@ func (s *activateSuite) TestActivateContainerAuthModePassphraseWithRecoveryKeyFa UserAuthTypePassphrase | UserAuthTypeRecoveryKey, UserAuthTypePassphrase | UserAuthTypeRecoveryKey, }, + expectedAuthRequestResults: []mockAuthRequestorResult{ + {result: UserAuthResultFailed, authTypes: UserAuthTypePassphrase}, + {result: UserAuthResultSuccess, authTypes: UserAuthTypeRecoveryKey}, + }, expectedActivateConfig: map[any]any{ AuthRequestorKey: authRequestor, PassphraseTriesKey: uint(3), @@ -4517,6 +4555,11 @@ Error with keyslot "default-fallback": cannot recover keys from keyslot: incompa UserAuthTypePassphrase | UserAuthTypeRecoveryKey, UserAuthTypeRecoveryKey, }, + expectedAuthRequestResults: []mockAuthRequestorResult{ + {result: UserAuthResultFailed, authTypes: UserAuthTypePassphrase}, + {result: UserAuthResultFailed, authTypes: UserAuthTypePassphrase, exhaustedAuthTypes: UserAuthTypePassphrase}, + {result: UserAuthResultSuccess, authTypes: UserAuthTypeRecoveryKey}, + }, expectedActivateConfig: map[any]any{ AuthRequestorKey: authRequestor, PassphraseTriesKey: uint(3), @@ -4581,6 +4624,12 @@ func (s *activateSuite) TestActivateContainerAuthModePassphraseWithRecoveryKeyFa UserAuthTypePassphrase | UserAuthTypeRecoveryKey, UserAuthTypeRecoveryKey, }, + expectedAuthRequestResults: []mockAuthRequestorResult{ + {result: UserAuthResultFailed, authTypes: UserAuthTypePassphrase}, + {result: UserAuthResultFailed, authTypes: UserAuthTypePassphrase}, + {result: UserAuthResultFailed, authTypes: UserAuthTypePassphrase, exhaustedAuthTypes: UserAuthTypePassphrase}, + {result: UserAuthResultSuccess, authTypes: UserAuthTypeRecoveryKey}, + }, expectedActivateConfig: map[any]any{ AuthRequestorKey: authRequestor, PassphraseTriesKey: uint(3), @@ -4652,6 +4701,12 @@ func (s *activateSuite) TestActivateContainerAuthModePassphraseAfterRecoveryKeyF UserAuthTypePassphrase | UserAuthTypeRecoveryKey, UserAuthTypePassphrase, }, + expectedAuthRequestResults: []mockAuthRequestorResult{ + {result: UserAuthResultFailed, authTypes: UserAuthTypePassphrase | UserAuthTypeRecoveryKey}, + {result: UserAuthResultFailed, authTypes: UserAuthTypePassphrase | UserAuthTypeRecoveryKey}, + {result: UserAuthResultFailed, authTypes: UserAuthTypePassphrase | UserAuthTypeRecoveryKey, exhaustedAuthTypes: UserAuthTypeRecoveryKey}, + {result: UserAuthResultSuccess, authTypes: UserAuthTypePassphrase}, + }, expectedActivateConfig: map[any]any{ AuthRequestorKey: authRequestor, PassphraseTriesKey: uint(5), @@ -4884,6 +4939,10 @@ func (s *activateSuite) TestActivateContainerAuthModePassphraseAuthRequestorOnly UserAuthTypePassphrase | UserAuthTypeRecoveryKey, UserAuthTypePassphrase | UserAuthTypeRecoveryKey, }, + expectedAuthRequestResults: []mockAuthRequestorResult{ + {result: UserAuthResultInvalidFormat, authTypes: UserAuthTypeRecoveryKey}, + {result: UserAuthResultSuccess, authTypes: UserAuthTypeRecoveryKey}, + }, expectedActivateConfig: map[any]any{ AuthRequestorKey: authRequestor, PassphraseTriesKey: uint(3), @@ -5074,8 +5133,6 @@ func (s *activateSuite) TestActivateContainerAuthModePINWithPINTries(c *C) { opts: []ActivateOption{ WithAuthRequestorUserVisibleName("data"), }, - expectedStderr: `Cannot parse PIN: invalid PIN: unexpected character -`, expectedAuthRequestName: "data", expectedAuthRequestPath: "/dev/sda1", expectedAuthRequestTypes: []UserAuthType{ @@ -5083,6 +5140,12 @@ func (s *activateSuite) TestActivateContainerAuthModePINWithPINTries(c *C) { UserAuthTypePIN, UserAuthTypePIN, }, + expectedAuthRequestResults: []mockAuthRequestorResult{ + {result: UserAuthResultFailed, authTypes: UserAuthTypePIN}, + {result: UserAuthResultFailed, authTypes: UserAuthTypePIN}, + {result: UserAuthResultInvalidFormat, authTypes: UserAuthTypePIN}, + {result: UserAuthResultFailed, authTypes: UserAuthTypePIN, exhaustedAuthTypes: UserAuthTypePIN}, + }, expectedActivateConfig: map[any]any{ AuthRequestorKey: authRequestor, PinTriesKey: uint(3), @@ -5191,6 +5254,10 @@ func (s *activateSuite) TestActivateContainerAuthModePINWithRecoveryKeyFallback( UserAuthTypePIN | UserAuthTypeRecoveryKey, UserAuthTypePIN | UserAuthTypeRecoveryKey, }, + expectedAuthRequestResults: []mockAuthRequestorResult{ + {result: UserAuthResultFailed, authTypes: UserAuthTypePIN}, + {result: UserAuthResultSuccess, authTypes: UserAuthTypeRecoveryKey}, + }, expectedActivateConfig: map[any]any{ AuthRequestorKey: authRequestor, PinTriesKey: uint(3), @@ -5264,6 +5331,11 @@ Error with keyslot "default-fallback": cannot recover keys from keyslot: incompa UserAuthTypePIN | UserAuthTypeRecoveryKey, UserAuthTypeRecoveryKey, }, + expectedAuthRequestResults: []mockAuthRequestorResult{ + {result: UserAuthResultFailed, authTypes: UserAuthTypePIN}, + {result: UserAuthResultFailed, authTypes: UserAuthTypePIN, exhaustedAuthTypes: UserAuthTypePIN}, + {result: UserAuthResultSuccess, authTypes: UserAuthTypeRecoveryKey}, + }, expectedActivateConfig: map[any]any{ AuthRequestorKey: authRequestor, PinTriesKey: uint(3), @@ -5328,6 +5400,12 @@ func (s *activateSuite) TestActivateContainerAuthModePINWithRecoveryKeyFallbackA UserAuthTypePIN | UserAuthTypeRecoveryKey, UserAuthTypeRecoveryKey, }, + expectedAuthRequestResults: []mockAuthRequestorResult{ + {result: UserAuthResultFailed, authTypes: UserAuthTypePIN}, + {result: UserAuthResultFailed, authTypes: UserAuthTypePIN}, + {result: UserAuthResultFailed, authTypes: UserAuthTypePIN, exhaustedAuthTypes: UserAuthTypePIN}, + {result: UserAuthResultSuccess, authTypes: UserAuthTypeRecoveryKey}, + }, expectedActivateConfig: map[any]any{ AuthRequestorKey: authRequestor, PinTriesKey: uint(3), @@ -5399,6 +5477,12 @@ func (s *activateSuite) TestActivateContainerAuthModePINAfterRecoveryKeyFallback UserAuthTypePIN | UserAuthTypeRecoveryKey, UserAuthTypePIN, }, + expectedAuthRequestResults: []mockAuthRequestorResult{ + {result: UserAuthResultFailed, authTypes: UserAuthTypeRecoveryKey}, + {result: UserAuthResultFailed, authTypes: UserAuthTypeRecoveryKey}, + {result: UserAuthResultFailed, authTypes: UserAuthTypeRecoveryKey, exhaustedAuthTypes: UserAuthTypeRecoveryKey}, + {result: UserAuthResultSuccess, authTypes: UserAuthTypePIN}, + }, expectedActivateConfig: map[any]any{ AuthRequestorKey: authRequestor, PinTriesKey: uint(5), @@ -5633,6 +5717,10 @@ func (s *activateSuite) TestActivateContainerAuthModePINAuthRequestorOnlyReturns UserAuthTypePIN | UserAuthTypeRecoveryKey, UserAuthTypePIN | UserAuthTypeRecoveryKey, }, + expectedAuthRequestResults: []mockAuthRequestorResult{ + {result: UserAuthResultInvalidFormat, authTypes: UserAuthTypeRecoveryKey}, + {result: UserAuthResultSuccess, authTypes: UserAuthTypeRecoveryKey}, + }, expectedActivateConfig: map[any]any{ AuthRequestorKey: authRequestor, PinTriesKey: uint(3), diff --git a/auth_requestor.go b/auth_requestor.go index b7d242c7..e275c08e 100644 --- a/auth_requestor.go +++ b/auth_requestor.go @@ -19,7 +19,11 @@ package secboot -import "context" +import ( + "context" + "fmt" + "strings" +) // UserAuthType describes a user authentication type that can be // requested via [AuthRequestor]. @@ -38,6 +42,45 @@ const ( UserAuthTypeRecoveryKey ) +func formatUserAuthTypeString(authTypes UserAuthType) string { + var s []string + if authTypes&UserAuthTypePassphrase > 0 { + s = append(s, "passphrase") + } + if authTypes&UserAuthTypePIN > 0 { + s = append(s, "PIN") + } + if authTypes&UserAuthTypeRecoveryKey > 0 { + s = append(s, "recovery key") + } + + switch len(s) { + case 0: + return "" + case 1: + return s[0] + default: + return fmt.Sprintf("%s or %s", strings.Join(s[0:len(s)-1], ", "), s[len(s)-1]) + } +} + +// UserAuthResult indicates the result of a user auth attempt. +type UserAuthResult int + +const ( + // UserAuthResultSuccess indicates that an authentication attempt + // was successful. + UserAuthResultSuccess UserAuthResult = iota + + // UserAuthResultFailed indicates that an authentication attempt failed. + UserAuthResultFailed + + // UserAuthResultInvalidFormat indicates that authentication + // could not be attempted because the supplied credential was formatted + // incorrectly for the type. + UserAuthResultInvalidFormat +) + // AuthRequestor is an interface for requesting credentials. type AuthRequestor interface { // RequestUserCredential is used to request a user credential that is @@ -46,7 +89,27 @@ type AuthRequestor interface { // and can be supplied via the ActivateContext API using the // WithAuthRequestorUserVisibleName option. The authTypes argument is used // to indicate what types of credential are being requested. + // // The implementation returns the requested credential and its type, which // may be a subset of the requested credential types. RequestUserCredential(ctx context.Context, name, path string, authTypes UserAuthType) (string, UserAuthType, error) + + // NotifyUserAuthResult is used to inform the user about the result of an + // authentication attempt. + // + // If the result is UserAuthResultSuccess, the supplied authTypes argument + // indicates the credential type that was successfully used. The + // exhaustedAuthTypes argument is unused. + // + // If the result is UserAuthResultFailed, the supplied authTypes argument + // indicates the credential types that were attempted but failed. The + // exhaustedAuthTypes argument indicates the credential types that + // will no longer be available following the last attempt because there are + // no more tries permitted. + // + // If the result is UserAuthResultInvalidFormat, the supplied + // authTypes argument indicates the credential types that the user supplied + // credential was badly formatted for. The exhaustedAuthTypes argument + // is unused. + NotifyUserAuthResult(ctx context.Context, result UserAuthResult, authTypes, exhaustedAuthTypes UserAuthType) error } diff --git a/auth_requestor_plymouth.go b/auth_requestor_plymouth.go index 8a4af157..33f3134d 100644 --- a/auth_requestor_plymouth.go +++ b/auth_requestor_plymouth.go @@ -25,7 +25,6 @@ import ( "errors" "fmt" "io" - "os" "os/exec" ) @@ -36,10 +35,20 @@ type PlymouthAuthRequestorStringer interface { // name is a string supplied via the WithAuthRequestorUserVisibleName option, and the // path is the storage container path. RequestUserCredentialString(name, path string, authTypes UserAuthType) (string, error) + + // NotifyUserAuthResultString returns messages used by NotifyUserAuthResult. + NotifyUserAuthResultString(name, path string, result UserAuthResult, authTypes, exhaustedAuthTypes UserAuthType) (string, error) +} + +type plymouthRequestUserCredentialContext struct { + Name string + Path string } type plymouthAuthRequestor struct { stringer PlymouthAuthRequestorStringer + + lastRequestUserCredentialCtx plymouthRequestUserCredentialContext } func (r *plymouthAuthRequestor) RequestUserCredential(ctx context.Context, name, path string, authTypes UserAuthType) (string, UserAuthType, error) { @@ -53,7 +62,6 @@ func (r *plymouthAuthRequestor) RequestUserCredential(ctx context.Context, name, "--prompt", msg) out := new(bytes.Buffer) cmd.Stdout = out - cmd.Stdin = os.Stdin if err := cmd.Run(); err != nil { return "", 0, fmt.Errorf("cannot execute plymouth ask-for-password: %w", err) } @@ -63,9 +71,32 @@ func (r *plymouthAuthRequestor) RequestUserCredential(ctx context.Context, name, // which io.ReadAll filters out. return "", 0, fmt.Errorf("unexpected error: %w", err) } + + r.lastRequestUserCredentialCtx = plymouthRequestUserCredentialContext{ + Name: name, + Path: path, + } + return string(result), authTypes, nil } +func (r *plymouthAuthRequestor) NotifyUserAuthResult(ctx context.Context, result UserAuthResult, authTypes, exhaustedAuthTypes UserAuthType) error { + msg, err := r.stringer.NotifyUserAuthResultString(r.lastRequestUserCredentialCtx.Name, r.lastRequestUserCredentialCtx.Path, result, authTypes, exhaustedAuthTypes) + if err != nil { + return fmt.Errorf("cannot request message string: %w", err) + } + + cmd := exec.CommandContext( + ctx, "plymouth", "display-message", + "--text", msg) + if err := cmd.Run(); err != nil { + return fmt.Errorf("cannot execute plymouth display-message: %w", err) + } + + r.lastRequestUserCredentialCtx = plymouthRequestUserCredentialContext{} + return nil +} + // NewPlymouthAuthRequestor creates an implementation of AuthRequestor that // communicates directly with Plymouth. func NewPlymouthAuthRequestor(stringer PlymouthAuthRequestorStringer) (AuthRequestor, error) { diff --git a/auth_requestor_plymouth_test.go b/auth_requestor_plymouth_test.go index e1308a5e..d4ada857 100644 --- a/auth_requestor_plymouth_test.go +++ b/auth_requestor_plymouth_test.go @@ -23,8 +23,10 @@ import ( "context" "errors" "fmt" + "io" "io/ioutil" "path/filepath" + "strings" "github.com/snapcore/secboot/internal/testutil" snapd_testutil "github.com/snapcore/snapd/testutil" @@ -45,7 +47,9 @@ func (s *authRequestorPlymouthSuite) SetUpTest(c *C) { dir := c.MkDir() s.passwordFile = filepath.Join(dir, "password") // password to be returned by the mock plymouth - plymouthBottom := `cat %[1]s` + plymouthBottom := `if [ "$1" = "ask-for-password" ]; then +cat %[1]s +fi` s.mockPlymouth = snapd_testutil.MockCommand(c, "plymouth", fmt.Sprintf(plymouthBottom, s.passwordFile)) s.AddCleanup(s.mockPlymouth.Restore) } @@ -57,12 +61,12 @@ func (s *authRequestorPlymouthSuite) setPassphrase(c *C, passphrase string) { var _ = Suite(&authRequestorPlymouthSuite{}) type mockPlymouthAuthRequestorStringer struct { - rucErr error + err error } func (s *mockPlymouthAuthRequestorStringer) RequestUserCredentialString(name, path string, authType UserAuthType) (string, error) { - if s.rucErr != nil { - return "", s.rucErr + if s.err != nil { + return "", s.err } var fmtString string @@ -87,6 +91,84 @@ func (s *mockPlymouthAuthRequestorStringer) RequestUserCredentialString(name, pa return fmt.Sprintf(fmtString, name, path), nil } +func (s *mockPlymouthAuthRequestorStringer) NotifyUserAuthResultString(name, path string, result UserAuthResult, authTypes, unavailableAuthTypes UserAuthType) (string, error) { + if s.err != nil { + return "", s.err + } + + switch result { + case UserAuthResultSuccess: + var fmtString string + switch authTypes { + case UserAuthTypePassphrase: + fmtString = "Unlocked %s (%s) successfully with passphrase" + case UserAuthTypePIN: + fmtString = "Unlocked %s (%s) successfully with PIN" + case UserAuthTypeRecoveryKey: + fmtString = "Unlocked %s (%s) successfully with recovery key" + default: + return "", errors.New("unexpected UserAuthType") + } + return fmt.Sprintf(fmtString, name, path), nil + case UserAuthResultFailed: + var b strings.Builder + + switch authTypes { + case UserAuthTypePassphrase: + io.WriteString(&b, "Incorrect passphrase") + case UserAuthTypePIN: + io.WriteString(&b, "Incorrect PIN") + case UserAuthTypeRecoveryKey: + io.WriteString(&b, "Incorrect recovery key") + case UserAuthTypePassphrase | UserAuthTypePIN: + io.WriteString(&b, "Incorrect passphrase or PIN") + case UserAuthTypePassphrase | UserAuthTypeRecoveryKey: + io.WriteString(&b, "Incorrect passphrase or recovery key") + case UserAuthTypePIN | UserAuthTypeRecoveryKey: + io.WriteString(&b, "Incorrect PIN or recovery key") + case UserAuthTypePassphrase | UserAuthTypePIN | UserAuthTypeRecoveryKey: + io.WriteString(&b, "Incorrect passphrase, PIN or recovery key") + default: + return "", errors.New("unexpected UserAuthType") + } + + switch unavailableAuthTypes { + case UserAuthType(0): + case UserAuthTypePassphrase: + io.WriteString(&b, ". No more passphrase tries remaining") + case UserAuthTypePIN: + io.WriteString(&b, ". No more PIN tries remaining") + case UserAuthTypeRecoveryKey: + io.WriteString(&b, ". No more recovery key tries remaining") + case UserAuthTypePassphrase | UserAuthTypePIN: + io.WriteString(&b, ". No more passphrase or PIN tries remaining") + case UserAuthTypePassphrase | UserAuthTypeRecoveryKey: + io.WriteString(&b, ". No more passphrase or recovery key tries remaining") + case UserAuthTypePIN | UserAuthTypeRecoveryKey: + io.WriteString(&b, ". No more PIN or recovery key tries remaining") + case UserAuthTypePassphrase | UserAuthTypePIN | UserAuthTypeRecoveryKey: + io.WriteString(&b, ". No more passphrase, PIN or recovery key tries remaining") + default: + return "", errors.New("unexpected UserAuthType") + } + + return b.String(), nil + case UserAuthResultInvalidFormat: + switch authTypes { + case UserAuthTypePIN: + return "Invalid PIN", nil + case UserAuthTypeRecoveryKey: + return "Invalid recovery key", nil + case UserAuthTypePIN | UserAuthTypeRecoveryKey: + return "Invalid PIN or recovery key", nil + default: + return "", errors.New("unexpected UserAuthType") + } + default: + return "", errors.New("unexpected UserAuthResult") + } +} + type testPlymouthRequestUserCredentialsParams struct { passphrase string @@ -111,6 +193,12 @@ func (s *authRequestorPlymouthSuite) testRequestUserCredential(c *C, params *tes c.Check(s.mockPlymouth.Calls(), HasLen, 1) c.Check(s.mockPlymouth.Calls()[0], DeepEquals, []string{"plymouth", "ask-for-password", "--prompt", params.expectedMsg}) + + c.Assert(requestor, testutil.ConvertibleTo, &PlymouthAuthRequestor{}) + c.Check(requestor.(*PlymouthAuthRequestor).LastRequestUserCredentialCtx(), DeepEquals, PlymouthRequestUserCredentialContext{ + Name: params.name, + Path: params.path, + }) } func (s *authRequestorPlymouthSuite) TestRequestUserCredentialPassphrase(c *C) { @@ -230,12 +318,14 @@ func (s *authRequestorPlymouthSuite) TestNewRequestorNoStringer(c *C) { func (s *authRequestorPlymouthSuite) TestRequestUserCredentialObtainMessageError(c *C) { requestor, err := NewPlymouthAuthRequestor(&mockPlymouthAuthRequestorStringer{ - rucErr: errors.New("some error"), + err: errors.New("some error"), }) c.Assert(err, IsNil) _, _, err = requestor.RequestUserCredential(context.Background(), "data", "/dev/sda1", UserAuthTypePassphrase) c.Check(err, ErrorMatches, `cannot request message string: some error`) + c.Assert(requestor, testutil.ConvertibleTo, &PlymouthAuthRequestor{}) + c.Check(requestor.(*PlymouthAuthRequestor).LastRequestUserCredentialCtx(), DeepEquals, PlymouthRequestUserCredentialContext{}) } func (s *authRequestorPlymouthSuite) TestRequestUserCredentialFailure(c *C) { @@ -244,6 +334,8 @@ func (s *authRequestorPlymouthSuite) TestRequestUserCredentialFailure(c *C) { _, _, err = requestor.RequestUserCredential(context.Background(), "data", "/dev/sda1", UserAuthTypePassphrase) c.Check(err, ErrorMatches, "cannot execute plymouth ask-for-password: exit status 1") + c.Assert(requestor, testutil.ConvertibleTo, &PlymouthAuthRequestor{}) + c.Check(requestor.(*PlymouthAuthRequestor).LastRequestUserCredentialCtx(), DeepEquals, PlymouthRequestUserCredentialContext{}) } func (s *authRequestorPlymouthSuite) TestRequestUserCredentialCanceledContext(c *C) { @@ -258,4 +350,157 @@ func (s *authRequestorPlymouthSuite) TestRequestUserCredentialCanceledContext(c _, _, err = requestor.RequestUserCredential(ctx, "data", "/dev/sda1", UserAuthTypePassphrase) c.Check(err, ErrorMatches, "cannot execute plymouth ask-for-password: context canceled") c.Check(errors.Is(err, context.Canceled), testutil.IsTrue) + c.Assert(requestor, testutil.ConvertibleTo, &PlymouthAuthRequestor{}) + c.Check(requestor.(*PlymouthAuthRequestor).LastRequestUserCredentialCtx(), DeepEquals, PlymouthRequestUserCredentialContext{}) +} + +type testPlymouthNotifyUserAuthResultParams struct { + name string + path string + result UserAuthResult + authTypes UserAuthType + unavailableAuthTypes UserAuthType + + expectedMsg string +} + +func (s *authRequestorPlymouthSuite) testNotifyUserAuthResult(c *C, params *testPlymouthNotifyUserAuthResultParams) { + requestor := NewPlymouthAuthRequestorForTesting(new(mockPlymouthAuthRequestorStringer), &PlymouthRequestUserCredentialContext{Name: params.name, Path: params.path}) + + c.Check(requestor.NotifyUserAuthResult(context.Background(), params.result, params.authTypes, params.unavailableAuthTypes), IsNil) + + c.Check(s.mockPlymouth.Calls(), HasLen, 1) + c.Check(s.mockPlymouth.Calls()[0], DeepEquals, []string{"plymouth", "display-message", "--text", params.expectedMsg}) +} + +func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultSuccessPassphrase(c *C) { + s.testNotifyUserAuthResult(c, &testPlymouthNotifyUserAuthResultParams{ + name: "data", + path: "/dev/sda1", + result: UserAuthResultSuccess, + authTypes: UserAuthTypePassphrase, + expectedMsg: "Unlocked data (/dev/sda1) successfully with passphrase", + }) +} + +func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultSuccessDifferentName(c *C) { + s.testNotifyUserAuthResult(c, &testPlymouthNotifyUserAuthResultParams{ + name: "foo", + path: "/dev/sda1", + result: UserAuthResultSuccess, + authTypes: UserAuthTypePassphrase, + expectedMsg: "Unlocked foo (/dev/sda1) successfully with passphrase", + }) +} + +func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultSuccessDifferentPath(c *C) { + s.testNotifyUserAuthResult(c, &testPlymouthNotifyUserAuthResultParams{ + name: "data", + path: "/dev/nvme0n1p3", + result: UserAuthResultSuccess, + authTypes: UserAuthTypePassphrase, + expectedMsg: "Unlocked data (/dev/nvme0n1p3) successfully with passphrase", + }) +} + +func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultSuccessPIN(c *C) { + s.testNotifyUserAuthResult(c, &testPlymouthNotifyUserAuthResultParams{ + name: "data", + path: "/dev/sda1", + result: UserAuthResultSuccess, + authTypes: UserAuthTypePIN, + expectedMsg: "Unlocked data (/dev/sda1) successfully with PIN", + }) +} + +func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultSuccessRecoveryKey(c *C) { + s.testNotifyUserAuthResult(c, &testPlymouthNotifyUserAuthResultParams{ + name: "data", + path: "/dev/sda1", + result: UserAuthResultSuccess, + authTypes: UserAuthTypeRecoveryKey, + expectedMsg: "Unlocked data (/dev/sda1) successfully with recovery key", + }) +} + +func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultFailurePassphrase(c *C) { + s.testNotifyUserAuthResult(c, &testPlymouthNotifyUserAuthResultParams{ + name: "data", + path: "/dev/sda1", + result: UserAuthResultFailed, + authTypes: UserAuthTypePassphrase, + expectedMsg: "Incorrect passphrase", + }) +} + +func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultFailurePIN(c *C) { + s.testNotifyUserAuthResult(c, &testPlymouthNotifyUserAuthResultParams{ + name: "data", + path: "/dev/sda1", + result: UserAuthResultFailed, + authTypes: UserAuthTypePIN, + expectedMsg: "Incorrect PIN", + }) +} + +func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultFailurePassphraseOrRecoveryKey(c *C) { + s.testNotifyUserAuthResult(c, &testPlymouthNotifyUserAuthResultParams{ + name: "data", + path: "/dev/sda1", + result: UserAuthResultFailed, + authTypes: UserAuthTypePassphrase | UserAuthTypeRecoveryKey, + expectedMsg: "Incorrect passphrase or recovery key", + }) +} + +func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultFailurePassphraseNoMoreTriesLeft(c *C) { + s.testNotifyUserAuthResult(c, &testPlymouthNotifyUserAuthResultParams{ + name: "data", + path: "/dev/sda1", + result: UserAuthResultFailed, + authTypes: UserAuthTypePassphrase, + unavailableAuthTypes: UserAuthTypePassphrase, + expectedMsg: "Incorrect passphrase. No more passphrase tries remaining", + }) +} + +func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultFailureRecoveryKeyNoMoreTriesLeft(c *C) { + s.testNotifyUserAuthResult(c, &testPlymouthNotifyUserAuthResultParams{ + name: "data", + path: "/dev/sda1", + result: UserAuthResultFailed, + authTypes: UserAuthTypeRecoveryKey, + unavailableAuthTypes: UserAuthTypeRecoveryKey, + expectedMsg: "Incorrect recovery key. No more recovery key tries remaining", + }) +} + +func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultInvalidPIN(c *C) { + s.testNotifyUserAuthResult(c, &testPlymouthNotifyUserAuthResultParams{ + name: "data", + path: "/dev/sda1", + result: UserAuthResultInvalidFormat, + authTypes: UserAuthTypePIN, + expectedMsg: "Invalid PIN", + }) +} + +func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultInvalidRecoveryKey(c *C) { + s.testNotifyUserAuthResult(c, &testPlymouthNotifyUserAuthResultParams{ + name: "data", + path: "/dev/sda1", + result: UserAuthResultInvalidFormat, + authTypes: UserAuthTypeRecoveryKey, + expectedMsg: "Invalid recovery key", + }) +} + +func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultInvalidPINOrRecoveryKey(c *C) { + s.testNotifyUserAuthResult(c, &testPlymouthNotifyUserAuthResultParams{ + name: "data", + path: "/dev/sda1", + result: UserAuthResultInvalidFormat, + authTypes: UserAuthTypePIN | UserAuthTypeRecoveryKey, + expectedMsg: "Invalid PIN or recovery key", + }) } diff --git a/auth_requestor_systemd.go b/auth_requestor_systemd.go index dfc48672..383c4208 100644 --- a/auth_requestor_systemd.go +++ b/auth_requestor_systemd.go @@ -24,6 +24,7 @@ import ( "context" "errors" "fmt" + "io" "os" "os/exec" "path/filepath" @@ -37,7 +38,10 @@ import ( type SystemdAuthRequestorStringFn func(name, path string, authTypes UserAuthType) (string, error) type systemdAuthRequestor struct { + stderr io.Writer stringFn SystemdAuthRequestorStringFn + + lastRequestUserCredentialPath string } func (r *systemdAuthRequestor) RequestUserCredential(ctx context.Context, name, path string, authTypes UserAuthType) (string, UserAuthType, error) { @@ -62,17 +66,39 @@ func (r *systemdAuthRequestor) RequestUserCredential(ctx context.Context, name, // The only error returned from bytes.Buffer.ReadString is io.EOF. return "", 0, errors.New("systemd-ask-password output is missing terminating newline") } + + r.lastRequestUserCredentialPath = path + return strings.TrimRight(result, "\n"), authTypes, nil } +func (r *systemdAuthRequestor) NotifyUserAuthResult(ctx context.Context, result UserAuthResult, authTypes, exhaustedAuthTypes UserAuthType) error { + switch result { + case UserAuthResultFailed: + fmt.Fprintf(r.stderr, "Incorrect %s for %s\n", formatUserAuthTypeString(authTypes), r.lastRequestUserCredentialPath) + if exhaustedAuthTypes != UserAuthType(0) { + fmt.Fprintf(r.stderr, "No more %s tries remaining\n", formatUserAuthTypeString(exhaustedAuthTypes)) + } + case UserAuthResultInvalidFormat: + fmt.Fprintf(r.stderr, "Incorrectly formatted %s\n", formatUserAuthTypeString(authTypes)) + } + + r.lastRequestUserCredentialPath = "" + return nil +} + // NewSystemdAuthRequestor creates an implementation of AuthRequestor that // delegates to the systemd-ask-password binary. The caller supplies a callback // to supply messages for user auth requests. -func NewSystemdAuthRequestor(stringFn SystemdAuthRequestorStringFn) (AuthRequestor, error) { +func NewSystemdAuthRequestor(stderr io.Writer, stringFn SystemdAuthRequestorStringFn) (AuthRequestor, error) { + if stderr == nil { + stderr = os.Stderr + } if stringFn == nil { return nil, errors.New("must supply a SystemdAuthRequestorStringFn") } return &systemdAuthRequestor{ + stderr: stderr, stringFn: stringFn, }, nil } diff --git a/auth_requestor_systemd_test.go b/auth_requestor_systemd_test.go index 6504a3c2..8cb3d25f 100644 --- a/auth_requestor_systemd_test.go +++ b/auth_requestor_systemd_test.go @@ -20,6 +20,7 @@ package secboot_test import ( + "bytes" "context" "errors" "fmt" @@ -71,7 +72,7 @@ type testSystemdRequestUserCredentialsParams struct { func (s *authRequestorSystemdSuite) testRequestUserCredential(c *C, params *testSystemdRequestUserCredentialsParams) { s.setPassphrase(c, params.passphrase) - requestor, err := NewSystemdAuthRequestor(func(name, path string, authType UserAuthType) (string, error) { + requestor, err := NewSystemdAuthRequestor(nil, func(name, path string, authType UserAuthType) (string, error) { var fmtString string switch authType { case UserAuthTypePassphrase: @@ -103,6 +104,9 @@ func (s *authRequestorSystemdSuite) testRequestUserCredential(c *C, params *test c.Check(s.mockSdAskPassword.Calls(), HasLen, 1) c.Check(s.mockSdAskPassword.Calls()[0], DeepEquals, []string{"systemd-ask-password", "--icon", "drive-harddisk", "--id", filepath.Base(os.Args[0]) + ":" + params.path, params.expectedMsg}) + + c.Assert(requestor, testutil.ConvertibleTo, &SystemdAuthRequestor{}) + c.Check(requestor.(*SystemdAuthRequestor).LastRequestUserCredentialPath(), Equals, params.path) } func (s *authRequestorSystemdSuite) TestRequestUserCredentialPassphrase(c *C) { @@ -216,46 +220,52 @@ func (s *authRequestorSystemdSuite) TestRequestUserCredentialPassphraseOrPINOrRe } func (s *authRequestorSystemdSuite) TestNewRequestorNoFormatStringCallback(c *C) { - _, err := NewSystemdAuthRequestor(nil) + _, err := NewSystemdAuthRequestor(nil, nil) c.Check(err, ErrorMatches, `must supply a SystemdAuthRequestorStringFn`) } func (s *authRequestorSystemdSuite) TestRequestUserCredentialObtainMessageError(c *C) { - requestor, err := NewSystemdAuthRequestor(func(string, string, UserAuthType) (string, error) { + requestor, err := NewSystemdAuthRequestor(nil, func(string, string, UserAuthType) (string, error) { return "", errors.New("some error") }) c.Assert(err, IsNil) _, _, err = requestor.RequestUserCredential(context.Background(), "data", "/dev/sda1", UserAuthTypePassphrase) c.Check(err, ErrorMatches, `cannot request message string: some error`) + c.Assert(requestor, testutil.ConvertibleTo, &SystemdAuthRequestor{}) + c.Check(requestor.(*SystemdAuthRequestor).LastRequestUserCredentialPath(), Equals, "") } func (s *authRequestorSystemdSuite) TestRequestUserCredentialInvalidResponse(c *C) { c.Assert(ioutil.WriteFile(s.passwordFile, []byte("foo"), 0600), IsNil) - requestor, err := NewSystemdAuthRequestor(func(string, string, UserAuthType) (string, error) { + requestor, err := NewSystemdAuthRequestor(nil, func(string, string, UserAuthType) (string, error) { return "", nil }) c.Assert(err, IsNil) _, _, err = requestor.RequestUserCredential(context.Background(), "data", "/dev/sda1", UserAuthTypePassphrase) c.Check(err, ErrorMatches, "systemd-ask-password output is missing terminating newline") + c.Assert(requestor, testutil.ConvertibleTo, &SystemdAuthRequestor{}) + c.Check(requestor.(*SystemdAuthRequestor).LastRequestUserCredentialPath(), Equals, "") } func (s *authRequestorSystemdSuite) TestRequestUserCredentialFailure(c *C) { - requestor, err := NewSystemdAuthRequestor(func(string, string, UserAuthType) (string, error) { + requestor, err := NewSystemdAuthRequestor(nil, func(string, string, UserAuthType) (string, error) { return "", nil }) c.Assert(err, IsNil) _, _, err = requestor.RequestUserCredential(context.Background(), "data", "/dev/sda1", UserAuthTypePassphrase) c.Check(err, ErrorMatches, "cannot execute systemd-ask-password: exit status 1") + c.Assert(requestor, testutil.ConvertibleTo, &SystemdAuthRequestor{}) + c.Check(requestor.(*SystemdAuthRequestor).LastRequestUserCredentialPath(), Equals, "") } func (s *authRequestorSystemdSuite) TestRequestUserCredentialCanceledContext(c *C) { c.Assert(ioutil.WriteFile(s.passwordFile, []byte("foo"), 0600), IsNil) - requestor, err := NewSystemdAuthRequestor(func(string, string, UserAuthType) (string, error) { + requestor, err := NewSystemdAuthRequestor(nil, func(string, string, UserAuthType) (string, error) { return "", nil }) c.Assert(err, IsNil) @@ -266,4 +276,111 @@ func (s *authRequestorSystemdSuite) TestRequestUserCredentialCanceledContext(c * _, _, err = requestor.RequestUserCredential(ctx, "data", "/dev/sda1", UserAuthTypePassphrase) c.Check(err, ErrorMatches, "cannot execute systemd-ask-password: context canceled") c.Check(errors.Is(err, context.Canceled), testutil.IsTrue) + c.Assert(requestor, testutil.ConvertibleTo, &SystemdAuthRequestor{}) + c.Check(requestor.(*SystemdAuthRequestor).LastRequestUserCredentialPath(), Equals, "") +} + +type testSystemdNotifyUserAuthResultParams struct { + path string + result UserAuthResult + authTypes UserAuthType + unavailableAuthTypes UserAuthType + + expectedMsg string +} + +func (s *authRequestorSystemdSuite) testNotifyUserAuthResult(c *C, params *testSystemdNotifyUserAuthResultParams) { + stderr := new(bytes.Buffer) + requestor := NewSystemdAuthRequestorForTesting(stderr, nil, params.path) + + c.Check(requestor.NotifyUserAuthResult(nil, params.result, params.authTypes, params.unavailableAuthTypes), IsNil) + c.Check(stderr.String(), Equals, params.expectedMsg) +} + +func (s *authRequestorSystemdSuite) TestNotifyUserAuthResultSuccess(c *C) { + s.testNotifyUserAuthResult(c, &testSystemdNotifyUserAuthResultParams{ + result: UserAuthResultSuccess, + }) +} + +func (s *authRequestorSystemdSuite) TestNotifyUserAuthResultFailurePassphrase(c *C) { + s.testNotifyUserAuthResult(c, &testSystemdNotifyUserAuthResultParams{ + path: "/dev/sda1", + result: UserAuthResultFailed, + authTypes: UserAuthTypePassphrase, + expectedMsg: `Incorrect passphrase for /dev/sda1 +`, + }) +} + +func (s *authRequestorSystemdSuite) TestNotifyUserAuthResultFailurePIN(c *C) { + s.testNotifyUserAuthResult(c, &testSystemdNotifyUserAuthResultParams{ + path: "/dev/sda1", + result: UserAuthResultFailed, + authTypes: UserAuthTypePIN, + expectedMsg: `Incorrect PIN for /dev/sda1 +`, + }) +} + +func (s *authRequestorSystemdSuite) TestNotifyUserAuthResultFailurePassphraseOrRecoveryKey(c *C) { + s.testNotifyUserAuthResult(c, &testSystemdNotifyUserAuthResultParams{ + path: "/dev/sda1", + result: UserAuthResultFailed, + authTypes: UserAuthTypePassphrase | UserAuthTypeRecoveryKey, + expectedMsg: `Incorrect passphrase or recovery key for /dev/sda1 +`, + }) +} + +func (s *authRequestorSystemdSuite) TestNotifyUserAuthResultFailureDifferentPath(c *C) { + s.testNotifyUserAuthResult(c, &testSystemdNotifyUserAuthResultParams{ + path: "/dev/nvme0n1p3", + result: UserAuthResultFailed, + authTypes: UserAuthTypePassphrase, + expectedMsg: `Incorrect passphrase for /dev/nvme0n1p3 +`, + }) +} + +func (s *authRequestorSystemdSuite) TestNotifyUserAuthResultFailurePassphraseNoMoreTriesLeft(c *C) { + s.testNotifyUserAuthResult(c, &testSystemdNotifyUserAuthResultParams{ + path: "/dev/sda1", + result: UserAuthResultFailed, + authTypes: UserAuthTypePassphrase, + unavailableAuthTypes: UserAuthTypePassphrase, + expectedMsg: `Incorrect passphrase for /dev/sda1 +No more passphrase tries remaining +`, + }) +} + +func (s *authRequestorSystemdSuite) TestNotifyUserAuthResultFailureRecoveryKeyNoMoreTriesLeft(c *C) { + s.testNotifyUserAuthResult(c, &testSystemdNotifyUserAuthResultParams{ + path: "/dev/sda1", + result: UserAuthResultFailed, + authTypes: UserAuthTypeRecoveryKey, + unavailableAuthTypes: UserAuthTypeRecoveryKey, + expectedMsg: `Incorrect recovery key for /dev/sda1 +No more recovery key tries remaining +`, + }) +} + +func (s *authRequestorSystemdSuite) TestNotifyUserAuthResultInvalidPIN(c *C) { + s.testNotifyUserAuthResult(c, &testSystemdNotifyUserAuthResultParams{ + result: UserAuthResultInvalidFormat, + authTypes: UserAuthTypePIN, + expectedMsg: `Incorrectly formatted PIN +`, + }) +} + +func (s *authRequestorSystemdSuite) TestNotifyUserAuthResultInvalidPINOrRecoveryKey(c *C) { + s.testNotifyUserAuthResult(c, &testSystemdNotifyUserAuthResultParams{ + result: UserAuthResultInvalidFormat, + authTypes: UserAuthTypePIN | UserAuthTypeRecoveryKey, + expectedMsg: `Incorrectly formatted PIN or recovery key +`, + }) } diff --git a/auth_requestor_test.go b/auth_requestor_test.go new file mode 100644 index 00000000..4ae7dc83 --- /dev/null +++ b/auth_requestor_test.go @@ -0,0 +1,62 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2021 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package secboot_test + +import ( + . "gopkg.in/check.v1" + + . "github.com/snapcore/secboot" +) + +type authRequestorSuite struct{} + +var _ = Suite(&authRequestorSuite{}) + +func (*authRequestorSuite) TestFormatUserAuthTypeStringNone(c *C) { + c.Check(FormatUserAuthTypeString(0), Equals, "") +} + +func (*authRequestorSuite) TestFormatUserAuthTypeStringPassphrase(c *C) { + c.Check(FormatUserAuthTypeString(UserAuthTypePassphrase), Equals, "passphrase") +} + +func (*authRequestorSuite) TestFormatUserAuthTypeStringPIN(c *C) { + c.Check(FormatUserAuthTypeString(UserAuthTypePIN), Equals, "PIN") +} + +func (*authRequestorSuite) TestFormatUserAuthTypeStringRecoveryKey(c *C) { + c.Check(FormatUserAuthTypeString(UserAuthTypeRecoveryKey), Equals, "recovery key") +} + +func (*authRequestorSuite) TestFormatUserAuthTypeStringPassphraseOrPIN(c *C) { + c.Check(FormatUserAuthTypeString(UserAuthTypePassphrase|UserAuthTypePIN), Equals, "passphrase or PIN") +} + +func (*authRequestorSuite) TestFormatUserAuthTypeStringPassphraseOrRecoveryKey(c *C) { + c.Check(FormatUserAuthTypeString(UserAuthTypePassphrase|UserAuthTypeRecoveryKey), Equals, "passphrase or recovery key") +} + +func (*authRequestorSuite) TestFormatUserAuthTypeStringPINOrRecoveryKey(c *C) { + c.Check(FormatUserAuthTypeString(UserAuthTypePIN|UserAuthTypeRecoveryKey), Equals, "PIN or recovery key") +} + +func (*authRequestorSuite) TestFormatUserAuthTypeStringPassphraseOrPINOrRecoveryKey(c *C) { + c.Check(FormatUserAuthTypeString(UserAuthTypePassphrase|UserAuthTypePIN|UserAuthTypeRecoveryKey), Equals, "passphrase, PIN or recovery key") +} diff --git a/crypt_test.go b/crypt_test.go index ad3f08e1..9fd9085c 100644 --- a/crypt_test.go +++ b/crypt_test.go @@ -69,6 +69,12 @@ type mockAuthRequestorResponse struct { authTypes UserAuthType } +type mockAuthRequestorResult struct { + result UserAuthResult + authTypes UserAuthType + exhaustedAuthTypes UserAuthType +} + type mockAuthRequestor struct { responses []any requests []struct { @@ -76,6 +82,7 @@ type mockAuthRequestor struct { path string authTypes UserAuthType } + results []mockAuthRequestorResult } func (r *mockAuthRequestor) RequestUserCredential(ctx context.Context, name, path string, authTypes UserAuthType) (string, UserAuthType, error) { @@ -113,6 +120,15 @@ func (r *mockAuthRequestor) RequestUserCredential(ctx context.Context, name, pat } } +func (r *mockAuthRequestor) NotifyUserAuthResult(ctx context.Context, result UserAuthResult, authTypes, exhaustedAuthTypes UserAuthType) error { + r.results = append(r.results, mockAuthRequestorResult{ + result: result, + authTypes: authTypes, + exhaustedAuthTypes: exhaustedAuthTypes, + }) + return nil +} + // mockLUKS2Container represents a LUKS2 container and its associated state type mockLUKS2Container struct { keyslots map[int][]byte diff --git a/export_test.go b/export_test.go index 3d4480a2..892cd6c9 100644 --- a/export_test.go +++ b/export_test.go @@ -61,6 +61,7 @@ var ( ErrInvalidRecoveryKey = errInvalidRecoveryKey ErrorToKeyslotError = errorToKeyslotError FormatKeyringKeyDesc = formatKeyringKeyDesc + FormatUserAuthTypeString = formatUserAuthTypeString NewActivateOneContainerStateMachine = newActivateOneContainerStateMachine ParseKeyringKeyDesc = parseKeyringKeyDesc StorageContainerHandlers = storageContainerHandlers @@ -76,7 +77,10 @@ type ( ExternalKeyData = externalKeyData ExternalUnlockKey = externalUnlockKey KdfParams = kdfParams + PlymouthAuthRequestor = plymouthAuthRequestor + PlymouthRequestUserCredentialContext = plymouthRequestUserCredentialContext ProtectedKeys = protectedKeys + SystemdAuthRequestor = systemdAuthRequestor ) func KDFOptionsKdfParams(opts KDFOptions, defaultTargetDuration time.Duration, keyLen uint32) (*KdfParams, error) { @@ -307,6 +311,33 @@ func (d *KeyData) DerivePassphraseKeys(passphrase string) (key, iv, auth []byte, return d.derivePassphraseKeys(passphrase) } +func (r *plymouthAuthRequestor) LastRequestUserCredentialCtx() plymouthRequestUserCredentialContext { + return r.lastRequestUserCredentialCtx +} + +func NewPlymouthAuthRequestorForTesting(stringer PlymouthAuthRequestorStringer, lastRequestUserCredentialCtx *plymouthRequestUserCredentialContext) *plymouthAuthRequestor { + if lastRequestUserCredentialCtx == nil { + var zeroCtx plymouthRequestUserCredentialContext + lastRequestUserCredentialCtx = &zeroCtx + } + return &plymouthAuthRequestor{ + stringer: stringer, + lastRequestUserCredentialCtx: *lastRequestUserCredentialCtx, + } +} + +func (r *systemdAuthRequestor) LastRequestUserCredentialPath() string { + return r.lastRequestUserCredentialPath +} + +func NewSystemdAuthRequestorForTesting(stderr io.Writer, stringFn SystemdAuthRequestorStringFn, lastRequestUserCredentialPath string) *systemdAuthRequestor { + return &systemdAuthRequestor{ + stderr: stderr, + stringFn: stringFn, + lastRequestUserCredentialPath: lastRequestUserCredentialPath, + } +} + func MockUnixStat(f func(devicePath string, st *unix.Stat_t) error) (restore func()) { old := unixStat unixStat = f From b7853d4c040abceed192f6cd605b2e74604d3e81 Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Tue, 3 Feb 2026 16:26:18 +0000 Subject: [PATCH 2/3] Add an extra couple of test cases --- auth_requestor_plymouth_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/auth_requestor_plymouth_test.go b/auth_requestor_plymouth_test.go index d4ada857..bd6f1b4c 100644 --- a/auth_requestor_plymouth_test.go +++ b/auth_requestor_plymouth_test.go @@ -504,3 +504,24 @@ func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultInvalidPINOrRecover expectedMsg: "Invalid PIN or recovery key", }) } + +func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultObtainMessageError(c *C) { + requestor, err := NewPlymouthAuthRequestor(&mockPlymouthAuthRequestorStringer{ + err: errors.New("some error"), + }) + c.Assert(err, IsNil) + + err = requestor.NotifyUserAuthResult(context.Background(), UserAuthResultSuccess, UserAuthTypePassphrase, 0) + c.Check(err, ErrorMatches, `cannot request message string: some error`) +} + +func (s *authRequestorPlymouthSuite) TestNotifyUserAuthResultCanceledContext(c *C) { + requestor := NewPlymouthAuthRequestorForTesting(new(mockPlymouthAuthRequestorStringer), &PlymouthRequestUserCredentialContext{Name: "data", Path: "/dev/sda1"}) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := requestor.NotifyUserAuthResult(ctx, UserAuthResultSuccess, UserAuthTypePassphrase, 0) + c.Check(err, ErrorMatches, "cannot execute plymouth display-message: context canceled") + c.Check(errors.Is(err, context.Canceled), testutil.IsTrue) +} From fd218c4422c75eef7f5c28c0e900a5c2b2b48afd Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Mon, 9 Feb 2026 23:49:12 +0000 Subject: [PATCH 3/3] rename stderr to console in NewSystemdAuthRequestor --- auth_requestor_systemd.go | 20 +++++++++++--------- auth_requestor_systemd_test.go | 6 +++--- export_test.go | 4 ++-- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/auth_requestor_systemd.go b/auth_requestor_systemd.go index 383c4208..1e4a9548 100644 --- a/auth_requestor_systemd.go +++ b/auth_requestor_systemd.go @@ -38,7 +38,7 @@ import ( type SystemdAuthRequestorStringFn func(name, path string, authTypes UserAuthType) (string, error) type systemdAuthRequestor struct { - stderr io.Writer + console io.Writer stringFn SystemdAuthRequestorStringFn lastRequestUserCredentialPath string @@ -75,12 +75,12 @@ func (r *systemdAuthRequestor) RequestUserCredential(ctx context.Context, name, func (r *systemdAuthRequestor) NotifyUserAuthResult(ctx context.Context, result UserAuthResult, authTypes, exhaustedAuthTypes UserAuthType) error { switch result { case UserAuthResultFailed: - fmt.Fprintf(r.stderr, "Incorrect %s for %s\n", formatUserAuthTypeString(authTypes), r.lastRequestUserCredentialPath) + fmt.Fprintf(r.console, "Incorrect %s for %s\n", formatUserAuthTypeString(authTypes), r.lastRequestUserCredentialPath) if exhaustedAuthTypes != UserAuthType(0) { - fmt.Fprintf(r.stderr, "No more %s tries remaining\n", formatUserAuthTypeString(exhaustedAuthTypes)) + fmt.Fprintf(r.console, "No more %s tries remaining\n", formatUserAuthTypeString(exhaustedAuthTypes)) } case UserAuthResultInvalidFormat: - fmt.Fprintf(r.stderr, "Incorrectly formatted %s\n", formatUserAuthTypeString(authTypes)) + fmt.Fprintf(r.console, "Incorrectly formatted %s\n", formatUserAuthTypeString(authTypes)) } r.lastRequestUserCredentialPath = "" @@ -89,16 +89,18 @@ func (r *systemdAuthRequestor) NotifyUserAuthResult(ctx context.Context, result // NewSystemdAuthRequestor creates an implementation of AuthRequestor that // delegates to the systemd-ask-password binary. The caller supplies a callback -// to supply messages for user auth requests. -func NewSystemdAuthRequestor(stderr io.Writer, stringFn SystemdAuthRequestorStringFn) (AuthRequestor, error) { - if stderr == nil { - stderr = os.Stderr +// to supply messages for user auth requests. The console argument is used by +// the implementation of [AuthRequestor.NotifyUserAuthResult] where result is +// not [UserAuthResultSuccess]. If not provided, it defaults to [os.Stderr]. +func NewSystemdAuthRequestor(console io.Writer, stringFn SystemdAuthRequestorStringFn) (AuthRequestor, error) { + if console == nil { + console = os.Stderr } if stringFn == nil { return nil, errors.New("must supply a SystemdAuthRequestorStringFn") } return &systemdAuthRequestor{ - stderr: stderr, + console: console, stringFn: stringFn, }, nil } diff --git a/auth_requestor_systemd_test.go b/auth_requestor_systemd_test.go index 8cb3d25f..16671094 100644 --- a/auth_requestor_systemd_test.go +++ b/auth_requestor_systemd_test.go @@ -290,11 +290,11 @@ type testSystemdNotifyUserAuthResultParams struct { } func (s *authRequestorSystemdSuite) testNotifyUserAuthResult(c *C, params *testSystemdNotifyUserAuthResultParams) { - stderr := new(bytes.Buffer) - requestor := NewSystemdAuthRequestorForTesting(stderr, nil, params.path) + console := new(bytes.Buffer) + requestor := NewSystemdAuthRequestorForTesting(console, nil, params.path) c.Check(requestor.NotifyUserAuthResult(nil, params.result, params.authTypes, params.unavailableAuthTypes), IsNil) - c.Check(stderr.String(), Equals, params.expectedMsg) + c.Check(console.String(), Equals, params.expectedMsg) } func (s *authRequestorSystemdSuite) TestNotifyUserAuthResultSuccess(c *C) { diff --git a/export_test.go b/export_test.go index 892cd6c9..0d5c17d9 100644 --- a/export_test.go +++ b/export_test.go @@ -330,9 +330,9 @@ func (r *systemdAuthRequestor) LastRequestUserCredentialPath() string { return r.lastRequestUserCredentialPath } -func NewSystemdAuthRequestorForTesting(stderr io.Writer, stringFn SystemdAuthRequestorStringFn, lastRequestUserCredentialPath string) *systemdAuthRequestor { +func NewSystemdAuthRequestorForTesting(console io.Writer, stringFn SystemdAuthRequestorStringFn, lastRequestUserCredentialPath string) *systemdAuthRequestor { return &systemdAuthRequestor{ - stderr: stderr, + console: console, stringFn: stringFn, lastRequestUserCredentialPath: lastRequestUserCredentialPath, }