From fb88d74f0335cdf8218bb8dfbaa03f23773318cf Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sat, 9 May 2020 14:52:07 -0700 Subject: keyring: improve errors ErrAccessUserKeyring: Include the user, and fix the backwards wrapping. ErrSessionUserKeyring: Include the user. ErrKeyAdd: ErrKeyRemove: ErrKeySearch: ErrLinkUserKeyring: Replace these with one-off unnamed errors because they are never checked for, and this makes it easier for the callers to provide better messages, e.g. fixing the backwards wrapping. --- cli-tests/t_v1_policy.out | 7 ++++--- cmd/fscrypt/errors.go | 16 ++++++++-------- keyring/fs_keyring.go | 16 ++++++++++++---- keyring/keyring.go | 10 ++-------- keyring/user_keyring.go | 45 ++++++++++++++++++++++++++++++++++++++------- 5 files changed, 64 insertions(+), 30 deletions(-) diff --git a/cli-tests/t_v1_policy.out b/cli-tests/t_v1_policy.out index 0ff5219..e693bf5 100644 --- a/cli-tests/t_v1_policy.out +++ b/cli-tests/t_v1_policy.out @@ -11,14 +11,15 @@ can be done with --user=USERNAME. To use the root user's keyring or passphrase, use --user=root. # Try to use --user=root as user -[ERROR] fscrypt encrypt: setting uids: operation not permitted: could not access - user keyring +[ERROR] fscrypt encrypt: could not access user keyring for "root": setting uids: + operation not permitted You can only use --user=USERNAME to access the user keyring of another user if you are running as root. # Try to encrypt without user keyring in session keyring -[ERROR] fscrypt encrypt: user keyring not linked into session keyring +[ERROR] fscrypt encrypt: user keyring for "fscrypt-test-user" is not linked into + the session keyring This is usually the result of a bad PAM configuration. Either correct the problem in your PAM stack, enable pam_keyinit.so, or run "keyctl link @u @s". diff --git a/cmd/fscrypt/errors.go b/cmd/fscrypt/errors.go index f4f3ddb..3f7150b 100644 --- a/cmd/fscrypt/errors.go +++ b/cmd/fscrypt/errors.go @@ -88,6 +88,14 @@ func getErrorSuggestions(err error) string { return fmt.Sprintf("Use %s to specify a protector name.", shortDisplay(nameFlag)) case *actions.ErrNoConfigFile: return `Run "sudo fscrypt setup" to create this file.` + case *keyring.ErrAccessUserKeyring: + return fmt.Sprintf(`You can only use %s to access the user + keyring of another user if you are running as root.`, + shortDisplay(userFlag)) + case *keyring.ErrSessionUserKeyring: + return `This is usually the result of a bad PAM configuration. + Either correct the problem in your PAM stack, enable + pam_keyinit.so, or run "keyctl link @u @s".` } switch errors.Cause(err) { case filesystem.ErrNotSetup: @@ -115,14 +123,6 @@ func getErrorSuggestions(err error) string { return `Directory couldn't be fully locked because other user(s) have unlocked it. If you want to force the directory to be locked, use 'sudo fscrypt lock --all-users DIR'.` - case keyring.ErrSessionUserKeying: - return `This is usually the result of a bad PAM configuration. - Either correct the problem in your PAM stack, enable - pam_keyinit.so, or run "keyctl link @u @s".` - case keyring.ErrAccessUserKeyring: - return fmt.Sprintf(`You can only use %s to access the user - keyring of another user if you are running as root.`, - shortDisplay(userFlag)) case keyring.ErrV2PoliciesUnsupported: return fmt.Sprintf(`v2 encryption policies are only supported by kernel version 5.4 and later. Either use a newer kernel, or change diff --git a/keyring/fs_keyring.go b/keyring/fs_keyring.go index 262e0e5..9b949b9 100644 --- a/keyring/fs_keyring.go +++ b/keyring/fs_keyring.go @@ -203,7 +203,9 @@ func fsAddEncryptionKey(key *crypto.Key, descriptor string, log.Printf("FS_IOC_ADD_ENCRYPTION_KEY(%q, %s, ) = %v", mount.Path, descriptor, errno) if errno != 0 { - return errors.Wrap(ErrKeyAdd, errno.Error()) + return errors.Wrapf(errno, + "error adding key with descriptor %s to filesystem %s", + descriptor, mount.Path) } if descriptor, err = validateKeyDescriptor(&arg.Key_spec, descriptor); err != nil { fsRemoveEncryptionKey(descriptor, mount, user) @@ -266,7 +268,9 @@ func fsRemoveEncryptionKey(descriptor string, mount *filesystem.Mount, } return ErrKeyNotPresent default: - return errors.Wrap(ErrKeyRemove, errno.Error()) + return errors.Wrapf(errno, + "error removing key with descriptor %s from filesystem %s", + descriptor, mount.Path) } } @@ -298,7 +302,10 @@ func fsGetEncryptionKeyStatus(descriptor string, mount *filesystem.Mount, log.Printf("FS_IOC_GET_ENCRYPTION_KEY_STATUS(%q, %s) = %v, status=%d, status_flags=0x%x", mount.Path, descriptor, errno, arg.Status, arg.Status_flags) if errno != 0 { - return KeyStatusUnknown, errors.Wrap(ErrKeySearch, errno.Error()) + return KeyStatusUnknown, + errors.Wrapf(errno, + "error getting status of key with descriptor %s on filesystem %s", + descriptor, mount.Path) } switch arg.Status { case unix.FSCRYPT_KEY_STATUS_ABSENT: @@ -313,6 +320,7 @@ func fsGetEncryptionKeyStatus(descriptor string, mount *filesystem.Mount, return KeyAbsentButFilesBusy, nil default: return KeyStatusUnknown, - errors.Wrapf(ErrKeySearch, "unknown key status (%d)", arg.Status) + errors.Errorf("unknown key status (%d) for key with descriptor %s on filesystem %s", + arg.Status, descriptor, mount.Path) } } diff --git a/keyring/keyring.go b/keyring/keyring.go index fb9cc0e..5ddceaf 100644 --- a/keyring/keyring.go +++ b/keyring/keyring.go @@ -43,15 +43,9 @@ import ( // Keyring error values var ( - ErrKeyAdd = util.SystemError("could not add key to the keyring") - ErrKeyRemove = util.SystemError("could not remove key from the keyring") - ErrKeyNotPresent = errors.New("key not present or already removed") - ErrKeyFilesOpen = errors.New("some files using the key are still open") ErrKeyAddedByOtherUsers = errors.New("other users have added the key too") - ErrKeySearch = errors.New("could not find key with descriptor") - ErrSessionUserKeying = errors.New("user keyring not linked into session keyring") - ErrAccessUserKeyring = errors.New("could not access user keyring") - ErrLinkUserKeyring = util.SystemError("could not link user keyring into root keyring") + ErrKeyFilesOpen = errors.New("some files using the key are still open") + ErrKeyNotPresent = errors.New("key not present or already removed") ErrV2PoliciesUnsupported = errors.New("kernel is too old to support v2 encryption policies") ) diff --git a/keyring/user_keyring.go b/keyring/user_keyring.go index beeb36d..0ea4689 100644 --- a/keyring/user_keyring.go +++ b/keyring/user_keyring.go @@ -36,6 +36,29 @@ import ( "github.com/google/fscrypt/util" ) +// ErrAccessUserKeyring indicates that a user's keyring cannot be +// accessed. +type ErrAccessUserKeyring struct { + TargetUser *user.User + UnderlyingError error +} + +func (err *ErrAccessUserKeyring) Error() string { + return fmt.Sprintf("could not access user keyring for %q: %s", + err.TargetUser.Username, err.UnderlyingError) +} + +// ErrSessionUserKeyring indicates that a user's keyring is not linked +// into the session keyring. +type ErrSessionUserKeyring struct { + TargetUser *user.User +} + +func (err *ErrSessionUserKeyring) Error() string { + return fmt.Sprintf("user keyring for %q is not linked into the session keyring", + err.TargetUser.Username) +} + // KeyType is always logon as required by filesystem encryption. const KeyType = "logon" @@ -67,7 +90,9 @@ func userAddKey(key *crypto.Key, description string, targetUser *user.User) erro log.Printf("KeyctlAddKey(%s, %s, , %d) = %d, %v", KeyType, description, keyringID, keyID, err) if err != nil { - return errors.Wrap(ErrKeyAdd, err.Error()) + return errors.Wrapf(err, + "error adding key with description %s to user keyring for %q", + description, targetUser.Username) } return nil } @@ -86,7 +111,9 @@ func userRemoveKey(description string, targetUser *user.User) error { _, err = unix.KeyctlInt(unix.KEYCTL_UNLINK, keyID, keyringID, 0, 0) log.Printf("KeyctlUnlink(%d, %d) = %v", keyID, keyringID, err) if err != nil { - return errors.Wrap(ErrKeyRemove, err.Error()) + return errors.Wrapf(err, + "error removing key with description %s from user keyring for %q", + description, targetUser.Username) } return nil } @@ -106,7 +133,9 @@ func userFindKey(description string, targetUser *user.User) (int, int, error) { keyID, err := unix.KeyctlSearch(keyringID, KeyType, description, 0) log.Printf("KeyctlSearch(%d, %s, %s) = %d, %v", keyringID, KeyType, description, keyID, err) if err != nil { - return 0, 0, errors.Wrap(ErrKeySearch, err.Error()) + return 0, 0, errors.Wrapf(err, + "error searching for key %s in user keyring for %q", + description, targetUser.Username) } return keyID, keyringID, err } @@ -123,14 +152,14 @@ func UserKeyringID(targetUser *user.User, checkSession bool) (int, error) { uid := util.AtoiOrPanic(targetUser.Uid) targetKeyring, err := userKeyringIDLookup(uid) if err != nil { - return 0, errors.Wrap(ErrAccessUserKeyring, err.Error()) + return 0, &ErrAccessUserKeyring{targetUser, err} } if !util.IsUserRoot() { // Make sure the returned keyring will be accessible by checking // that it is in the session keyring. if checkSession && !isUserKeyringInSession(uid) { - return 0, ErrSessionUserKeying + return 0, &ErrSessionUserKeyring{targetUser} } return targetKeyring, nil } @@ -139,12 +168,14 @@ func UserKeyringID(targetUser *user.User, checkSession bool) (int, error) { // the root user's user keyring (which will not be garbage collected). rootKeyring, err := userKeyringIDLookup(0) if err != nil { - return 0, errors.Wrap(ErrLinkUserKeyring, err.Error()) + return 0, errors.Wrapf(err, "error looking up root's user keyring") } if rootKeyring != targetKeyring { if err = keyringLink(targetKeyring, rootKeyring); err != nil { - return 0, errors.Wrap(ErrLinkUserKeyring, err.Error()) + return 0, errors.Wrapf(err, + "error linking user keyring for %q into root's user keyring", + targetUser.Username) } } return targetKeyring, nil -- cgit v1.2.3