From fbc161a77962fe64e3caad80efb535d28d8c1f74 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sat, 9 May 2020 14:52:07 -0700 Subject: [PATCH] metadata: improve errors ErrBadOwners: Rename to ErrDirectoryNotOwned for clarity, move it from cmd/fscrypt/ to metadata/ where it better belongs, and improve the message. ErrEncrypted: Rename to ErrAlreadyEncrypted for clarity, and include the path. ErrNotEncrypted: Include the path. ErrBadEncryptionOptions: Include the path and bad options. ErrEncryptionNotSupported: ErrEncryptionNotEnabled: Don't wrap with "get encryption policy %s", in preparation for wrapping these with filesystem-level context instead. Also avoid mixing together the error handling for the "get policy" and "set policy" ioctls. Make it very clear how we're handling the errors from each ioctl. --- cli-tests/t_encrypt.out | 24 ++--- cli-tests/t_encrypt_custom.out | 4 +- cli-tests/t_encrypt_login.out | 8 +- cli-tests/t_encrypt_raw_key.out | 4 +- cli-tests/t_not_enabled.out | 9 +- cli-tests/t_not_supported.out | 3 +- cli-tests/t_status.out | 8 +- cli-tests/t_v1_policy_fs_keyring.out | 4 +- cmd/fscrypt/commands.go | 17 ++-- cmd/fscrypt/errors.go | 4 - metadata/policy.go | 131 ++++++++++++++++++++------- 11 files changed, 133 insertions(+), 83 deletions(-) diff --git a/cli-tests/t_encrypt.out b/cli-tests/t_encrypt.out index af38299..e3bace0 100644 --- a/cli-tests/t_encrypt.out +++ b/cli-tests/t_encrypt.out @@ -3,8 +3,8 @@ [ERROR] fscrypt encrypt: no such file or directory ext4 filesystem "MNT" has 0 protectors and 0 policies -[ERROR] fscrypt status: get encryption policy MNT/dir: file - or directory not encrypted +[ERROR] fscrypt status: file or directory "MNT/dir" is not + encrypted # Try to encrypt a nonempty directory [ERROR] fscrypt encrypt: MNT/dir: not an empty directory @@ -14,8 +14,8 @@ in-place. Instead, encrypt an empty directory, copy the files into that encrypted directory, and securely delete the originals with "shred". ext4 filesystem "MNT" has 0 protectors and 0 policies -[ERROR] fscrypt status: get encryption policy MNT/dir: file - or directory not encrypted +[ERROR] fscrypt status: file or directory "MNT/dir" is not + encrypted # Encrypt a directory as non-root user ext4 filesystem "MNT" has 1 protector and 1 policy @@ -52,16 +52,16 @@ PROTECTOR LINKED DESCRIPTION desc1 No custom protector "prot" # Try to encrypt an already-encrypted directory -[ERROR] fscrypt encrypt: MNT/dir: file or directory already - encrypted +[ERROR] fscrypt encrypt: file or directory "MNT/dir" is + already encrypted # Try to encrypt another user's directory as a non-root user -[ERROR] fscrypt encrypt: MNT/dir: you do not own this - directory +[ERROR] fscrypt encrypt: cannot encrypt "MNT/dir" because + it's owned by another user (root). -Encryption can only be setup on directories you own, even if you have write -permission for the directory. + Encryption can only be enabled on a directory you own, + even if you have write access to the directory. ext4 filesystem "MNT" has 0 protectors and 0 policies -[ERROR] fscrypt status: get encryption policy MNT/dir: file - or directory not encrypted +[ERROR] fscrypt status: file or directory "MNT/dir" is not + encrypted diff --git a/cli-tests/t_encrypt_custom.out b/cli-tests/t_encrypt_custom.out index e7b8656..8dd15e3 100644 --- a/cli-tests/t_encrypt_custom.out +++ b/cli-tests/t_encrypt_custom.out @@ -51,5 +51,5 @@ desc6 No custom protector "prot" Use --name=PROTECTOR_NAME to specify a protector name. ext4 filesystem "MNT" has 0 protectors and 0 policies -[ERROR] fscrypt status: get encryption policy MNT/dir: file - or directory not encrypted +[ERROR] fscrypt status: file or directory "MNT/dir" is not + encrypted diff --git a/cli-tests/t_encrypt_login.out b/cli-tests/t_encrypt_login.out index 7ee66a2..e8e0e41 100644 --- a/cli-tests/t_encrypt_login.out +++ b/cli-tests/t_encrypt_login.out @@ -139,8 +139,8 @@ ext4 filesystem "MNT" has 0 protectors and 0 policies ext4 filesystem "MNT_ROOT" has 0 protectors and 0 policies -[ERROR] fscrypt status: get encryption policy MNT/dir: file - or directory not encrypted +[ERROR] fscrypt status: file or directory "MNT/dir" is not + encrypted # Try to use the wrong login passphrase [ERROR] fscrypt encrypt: incorrect login passphrase @@ -148,5 +148,5 @@ ext4 filesystem "MNT" has 0 protectors and 0 policies ext4 filesystem "MNT_ROOT" has 0 protectors and 0 policies -[ERROR] fscrypt status: get encryption policy MNT/dir: file - or directory not encrypted +[ERROR] fscrypt status: file or directory "MNT/dir" is not + encrypted diff --git a/cli-tests/t_encrypt_raw_key.out b/cli-tests/t_encrypt_raw_key.out index c7c46eb..8765ba2 100644 --- a/cli-tests/t_encrypt_raw_key.out +++ b/cli-tests/t_encrypt_raw_key.out @@ -21,5 +21,5 @@ desc1 No raw key protector "prot" [ERROR] fscrypt encrypt: TMPDIR/raw_key: key file must be 32 bytes ext4 filesystem "MNT" has 0 protectors and 0 policies -[ERROR] fscrypt status: get encryption policy MNT/dir: file - or directory not encrypted +[ERROR] fscrypt status: file or directory "MNT/dir" is not + encrypted diff --git a/cli-tests/t_not_enabled.out b/cli-tests/t_not_enabled.out index 7d74bcf..760f9dd 100644 --- a/cli-tests/t_not_enabled.out +++ b/cli-tests/t_not_enabled.out @@ -2,24 +2,21 @@ # Disable encryption on DEV # Try to encrypt a directory when encryption is disabled -[ERROR] fscrypt encrypt: get encryption policy MNT/dir: - encryption not enabled +[ERROR] fscrypt encrypt: encryption not enabled Encryption is either disabled in the kernel config, or needs to be enabled for this filesystem. See the documentation on how to enable encryption on ext4 systems (and the risks of doing so). # Try to unlock a directory when encryption is disabled -[ERROR] fscrypt unlock: get encryption policy MNT/dir: - encryption not enabled +[ERROR] fscrypt unlock: encryption not enabled Encryption is either disabled in the kernel config, or needs to be enabled for this filesystem. See the documentation on how to enable encryption on ext4 systems (and the risks of doing so). # Try to lock a directory when encryption is disabled -[ERROR] fscrypt lock: get encryption policy MNT/dir: - encryption not enabled +[ERROR] fscrypt lock: encryption not enabled Encryption is either disabled in the kernel config, or needs to be enabled for this filesystem. See the documentation on how to enable encryption on ext4 diff --git a/cli-tests/t_not_supported.out b/cli-tests/t_not_supported.out index 8af840c..dd71599 100644 --- a/cli-tests/t_not_supported.out +++ b/cli-tests/t_not_supported.out @@ -5,7 +5,6 @@ Metadata directories created at "MNT/.fscrypt". # Try to encrypt a directory on tmpfs -[ERROR] fscrypt encrypt: get encryption policy MNT/dir: - encryption not supported +[ERROR] fscrypt encrypt: encryption not supported Encryption for this type of filesystem is not supported on this kernel version. diff --git a/cli-tests/t_status.out b/cli-tests/t_status.out index b036712..08ce3b2 100644 --- a/cli-tests/t_status.out +++ b/cli-tests/t_status.out @@ -10,10 +10,10 @@ ext4 filesystem "MNT" has 0 protectors and 0 policies # Get status of unencrypted directory on setup mountpoint -[ERROR] fscrypt status: get encryption policy MNT/dir: file - or directory not encrypted -[ERROR] fscrypt status: get encryption policy MNT/dir: file - or directory not encrypted +[ERROR] fscrypt status: file or directory "MNT/dir" is not + encrypted +[ERROR] fscrypt status: file or directory "MNT/dir" is not + encrypted # Remove fscrypt metadata from MNT diff --git a/cli-tests/t_v1_policy_fs_keyring.out b/cli-tests/t_v1_policy_fs_keyring.out index ca32ec1..cfc8f7c 100644 --- a/cli-tests/t_v1_policy_fs_keyring.out +++ b/cli-tests/t_v1_policy_fs_keyring.out @@ -10,8 +10,8 @@ Either this command should be run as root, or you should set re-create your encrypted directories using v2 encryption policies rather than v1 (this requires setting '"policy_version": "2"' in the "options" section of /etc/fscrypt.conf). -[ERROR] fscrypt status: get encryption policy MNT/dir: file - or directory not encrypted +[ERROR] fscrypt status: file or directory "MNT/dir" is not + encrypted # Encrypt directory as user with --skip-unlock "MNT/dir" is encrypted with fscrypt. diff --git a/cmd/fscrypt/commands.go b/cmd/fscrypt/commands.go index 51cf136..86816ba 100644 --- a/cmd/fscrypt/commands.go +++ b/cmd/fscrypt/commands.go @@ -282,11 +282,7 @@ func encryptPath(path string) (err error) { } }() } - if err = policy.Apply(path); os.IsPermission(errors.Cause(err)) { - // EACCES at this point indicates ownership issues. - err = errors.Wrap(ErrBadOwners, path) - } - if err != nil { + if err = policy.Apply(path); err != nil { return } if recoveryPassphrase != nil { @@ -320,14 +316,15 @@ func checkEncryptable(ctx *actions.Context, path string) error { log.Printf("ensuring %s supports encryption and filesystem is using fscrypt", path) switch _, err := actions.GetPolicyFromPath(ctx, path); errors.Cause(err) { - case metadata.ErrNotEncrypted: - // We are not encrypted. Finally, we check that the filesystem - // supports encryption - return ctx.Mount.CheckSupport() case nil: // We are encrypted - return errors.Wrap(metadata.ErrEncrypted, path) + return &metadata.ErrAlreadyEncrypted{path} default: + if _, ok := err.(*metadata.ErrNotEncrypted); ok { + // We are not encrypted. Finally, we check that the filesystem + // supports encryption + return ctx.Mount.CheckSupport() + } return err } } diff --git a/cmd/fscrypt/errors.go b/cmd/fscrypt/errors.go index 3f7150b..6119862 100644 --- a/cmd/fscrypt/errors.go +++ b/cmd/fscrypt/errors.go @@ -57,7 +57,6 @@ var ( ErrMustBeRoot = errors.New("this command must be run as root") ErrPolicyUnlocked = errors.New("this file or directory is already unlocked") ErrPolicyLocked = errors.New("this file or directory is already locked") - ErrBadOwners = errors.New("you do not own this directory") ErrNotEmptyDir = errors.New("not an empty directory") ErrNotPassphrase = errors.New("protector does not use a passphrase") ErrUnknownUser = errors.New("unknown user") @@ -133,9 +132,6 @@ func getErrorSuggestions(err error) string { return fmt.Sprintf("Use %s to specify a protector.", shortDisplay(protectorFlag)) case ErrSpecifyKeyFile: return fmt.Sprintf("Use %s to specify a key file.", shortDisplay(keyFileFlag)) - case ErrBadOwners: - return `Encryption can only be setup on directories you own, - even if you have write permission for the directory.` case ErrNotEmptyDir: return `Encryption can only be setup on empty directories; files cannot be encrypted in-place. Instead, encrypt an empty diff --git a/metadata/policy.go b/metadata/policy.go index b95bf42..76c2e6f 100644 --- a/metadata/policy.go +++ b/metadata/policy.go @@ -22,9 +22,12 @@ package metadata import ( "encoding/hex" + "fmt" "log" "math" "os" + "os/user" + "strconv" "unsafe" "github.com/pkg/errors" @@ -33,38 +36,70 @@ import ( "github.com/google/fscrypt/util" ) -// Encryption specific errors var ( + // ErrEncryptionNotSupported indicates that encryption is not supported + // on the given filesystem, and there is no way to enable it. ErrEncryptionNotSupported = errors.New("encryption not supported") - ErrEncryptionNotEnabled = errors.New("encryption not enabled") - ErrNotEncrypted = errors.New("file or directory not encrypted") - ErrEncrypted = errors.New("file or directory already encrypted") - ErrBadEncryptionOptions = util.SystemError("invalid encryption options provided") + + // ErrEncryptionNotEnabled indicates that encryption is not supported on + // the given filesystem, but there is a way to enable it. + ErrEncryptionNotEnabled = errors.New("encryption not enabled") ) -// policyIoctl is a wrapper around the ioctls that get and set encryption -// policies: FS_IOC_GET_ENCRYPTION_POLICY, FS_IOC_GET_ENCRYPTION_POLICY_EX, and -// FS_IOC_SET_ENCRYPTION_POLICY. It translates the raw errno values into more -// descriptive errors. +// ErrAlreadyEncrypted indicates that the path is already encrypted. +type ErrAlreadyEncrypted struct { + Path string +} + +func (err *ErrAlreadyEncrypted) Error() string { + return fmt.Sprintf("file or directory %q is already encrypted", err.Path) +} + +// ErrBadEncryptionOptions indicates that unsupported encryption options were given. +type ErrBadEncryptionOptions struct { + Path string + Options *EncryptionOptions +} + +func (err *ErrBadEncryptionOptions) Error() string { + return fmt.Sprintf(`cannot encrypt %q because the kernel doesn't support the requested encryption options. + + The options are %s`, err.Path, err.Options) +} + +// ErrDirectoryNotOwned indicates a directory can't be encrypted because it's +// owned by another user. +type ErrDirectoryNotOwned struct { + Path string + Owner uint32 +} + +func (err *ErrDirectoryNotOwned) Error() string { + owner := strconv.Itoa(int(err.Owner)) + if u, e := user.LookupId(owner); e == nil && u.Username != "" { + owner = u.Username + } + return fmt.Sprintf(`cannot encrypt %q because it's owned by another user (%s). + + Encryption can only be enabled on a directory you own, even if you have + write access to the directory.`, err.Path, owner) +} + +// ErrNotEncrypted indicates that the path is not encrypted. +type ErrNotEncrypted struct { + Path string +} + +func (err *ErrNotEncrypted) Error() string { + return fmt.Sprintf("file or directory %q is not encrypted", err.Path) +} + func policyIoctl(file *os.File, request uintptr, arg unsafe.Pointer) error { - // The returned errno value can sometimes give strange errors, so we - // return encryption specific errors. _, _, errno := unix.Syscall(unix.SYS_IOCTL, file.Fd(), request, uintptr(arg)) - switch errno { - case 0: + if errno == 0 { return nil - case unix.ENOTTY: - return ErrEncryptionNotSupported - case unix.EOPNOTSUPP: - return ErrEncryptionNotEnabled - case unix.ENODATA, unix.ENOENT: - // ENOENT was returned instead of ENODATA on some filesystems before v4.11. - return ErrNotEncrypted - case unix.EEXIST: - return ErrEncrypted - default: - return errno } + return errno } // Maps EncryptionOptions.Padding <-> FSCRYPT_POLICY_FLAGS @@ -125,13 +160,23 @@ func GetPolicy(path string) (*PolicyData, error) { arg.Size = uint64(unsafe.Sizeof(arg.Policy)) policyPtr := util.Ptr(arg.Policy[:]) err = policyIoctl(file, unix.FS_IOC_GET_ENCRYPTION_POLICY_EX, unsafe.Pointer(&arg)) - if err == ErrEncryptionNotSupported { + if err == unix.ENOTTY { // Fall back to the old version of the ioctl. This works for v1 policies only. err = policyIoctl(file, unix.FS_IOC_GET_ENCRYPTION_POLICY, policyPtr) arg.Size = uint64(unsafe.Sizeof(unix.FscryptPolicyV1{})) } - if err != nil { - return nil, errors.Wrapf(err, "get encryption policy %s", path) + switch err { + case nil: + break + case unix.ENOTTY: + return nil, ErrEncryptionNotSupported + case unix.EOPNOTSUPP: + return nil, ErrEncryptionNotEnabled + case unix.ENODATA, unix.ENOENT: + // ENOENT was returned instead of ENODATA on some filesystems before v4.11. + return nil, &ErrNotEncrypted{path} + default: + return nil, errors.Wrapf(err, "failed to get encryption policy of %q", path) } switch arg.Policy[0] { // arg.policy.version case unix.FSCRYPT_POLICY_V1: @@ -237,7 +282,6 @@ func SetPolicy(path string, data *PolicyData) error { default: err = errors.Errorf("policy version of %d is invalid", data.Options.PolicyVersion) } - if err == unix.EINVAL { // Before kernel v4.11, many different errors all caused unix.EINVAL to be returned. // We try to disambiguate this error here. This disambiguation will not always give @@ -247,14 +291,27 @@ func SetPolicy(path string, data *PolicyData) error { err = unix.ENOTDIR } else if _, policyErr := GetPolicy(path); policyErr == nil { // Checking if a policy is already set on this directory - err = ErrEncrypted - } else { - // Default to generic "bad options". - err = ErrBadEncryptionOptions + err = unix.EEXIST } } - - return errors.Wrapf(err, "set encryption policy %s", path) + switch err { + case nil: + return nil + case unix.EACCES: + var stat unix.Stat_t + if statErr := unix.Stat(path, &stat); statErr == nil && stat.Uid != uint32(os.Geteuid()) { + return &ErrDirectoryNotOwned{path, stat.Uid} + } + case unix.EEXIST: + return &ErrAlreadyEncrypted{path} + case unix.EINVAL: + return &ErrBadEncryptionOptions{path, data.Options} + case unix.ENOTTY: + return ErrEncryptionNotSupported + case unix.EOPNOTSUPP: + return ErrEncryptionNotEnabled + } + return errors.Wrapf(err, "failed to set encryption policy on %q", path) } // CheckSupport returns an error if the filesystem containing path does not @@ -282,6 +339,10 @@ func CheckSupport(path string) error { Please open an issue, filesystem %q may be corrupted.`, path) case unix.EINVAL, unix.EACCES: return nil + case unix.ENOTTY: + return ErrEncryptionNotSupported + case unix.EOPNOTSUPP: + return ErrEncryptionNotEnabled } - return err + return errors.Wrapf(err, "unexpected error checking for encryption support on filesystem %q", path) } -- 2.39.5