From 181600d6327ed34a3f62eda0dd03a6d2ae49e5f9 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sat, 9 May 2020 14:52:07 -0700 Subject: [PATCH] cmd/fscrypt: improve errors In checkEncryptable(), check whether the directory is already encrypted before checking whether it's empty. Also improve the error message for when a directory is nonempty. Finally, translate keyring.ErrKeyAddedByOtherUsers and keyring.ErrKeyFilesOpen into errors which include the directory. --- cli-tests/t_encrypt.out | 21 +++++++--- cli-tests/t_lock.out | 22 ++++++---- cli-tests/t_setup.out | 2 +- cli-tests/t_v1_policy.out | 13 ++++-- cmd/fscrypt/commands.go | 51 ++++++++++++----------- cmd/fscrypt/errors.go | 85 ++++++++++++++++++++++++++++++--------- 6 files changed, 135 insertions(+), 59 deletions(-) diff --git a/cli-tests/t_encrypt.out b/cli-tests/t_encrypt.out index e3bace0..26cb451 100644 --- a/cli-tests/t_encrypt.out +++ b/cli-tests/t_encrypt.out @@ -7,11 +7,22 @@ ext4 filesystem "MNT" has 0 protectors and 0 policies encrypted # Try to encrypt a nonempty directory -[ERROR] fscrypt encrypt: MNT/dir: not an empty directory - -Encryption can only be setup on empty directories; files cannot be encrypted -in-place. Instead, encrypt an empty directory, copy the files into that -encrypted directory, and securely delete the originals with "shred". +[ERROR] fscrypt encrypt: Directory "MNT/dir" cannot be + encrypted because it is non-empty. + +Files cannot be encrypted in-place. Instead, encrypt a new directory, copy the +files into it, and securely delete the original directory. For example: + + mkdir MNT/dir.new + fscrypt encrypt MNT/dir.new + cp -a -T MNT/dir MNT/dir.new + find MNT/dir -type f -print0 | xargs -0 shred -n1 --remove=unlink + rm -rf MNT/dir + mv MNT/dir.new MNT/dir + +Caution: due to the nature of modern storage devices and filesystems, the +original data may still be recoverable from disk. It's much better to encrypt +your files from the start. ext4 filesystem "MNT" has 0 protectors and 0 policies [ERROR] fscrypt status: file or directory "MNT/dir" is not diff --git a/cli-tests/t_lock.out b/cli-tests/t_lock.out index c0f9279..b8c8dcb 100644 --- a/cli-tests/t_lock.out +++ b/cli-tests/t_lock.out @@ -33,11 +33,16 @@ desc2 No custom protector "prot" contents # Try to lock directory while files busy -[ERROR] fscrypt lock: some files using the key are still open +[ERROR] fscrypt lock: Directory was incompletely locked because some files are + still open. These files remain accessible. -Directory was incompletely locked because some files are still open. These files -remain accessible. Try killing any processes using files in the directory, then -re-running 'fscrypt lock'. +Try killing any processes using files in the directory, for example using: + + find "MNT/dir" -print0 | xargs -0 fuser -k + +Then re-run: + + fscrypt lock "MNT/dir" # => status should be incompletely locked "MNT/dir" is encrypted with fscrypt. @@ -72,11 +77,12 @@ mkdir: cannot create directory 'MNT/dir/subdir': Required key not available # Try to lock directory while other user has unlocked Enter custom passphrase for protector "prot": "MNT/dir" is now unlocked and ready for use. -[ERROR] fscrypt lock: other users have added the key too +[ERROR] fscrypt lock: Directory "MNT/dir" couldn't be fully + locked because other user(s) have unlocked it. + +If you want to force the directory to be locked, use: -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'. + sudo fscrypt lock --all-users "MNT/dir" contents "MNT/dir" is now locked. cat: MNT/dir/file: No such file or directory diff --git a/cli-tests/t_setup.out b/cli-tests/t_setup.out index ef0d133..943a781 100644 --- a/cli-tests/t_setup.out +++ b/cli-tests/t_setup.out @@ -26,7 +26,7 @@ Skipping creating MNT_ROOT/.fscrypt because it already exists. # fscrypt setup --quiet when fscrypt.conf already exists [ERROR] fscrypt setup: operation would be destructive -Use --force to automatically run destructive operations. +If desired, use --force to automatically run destructive operations. # fscrypt setup --quiet --force when fscrypt.conf already exists diff --git a/cli-tests/t_v1_policy.out b/cli-tests/t_v1_policy.out index e693bf5..b47bcca 100644 --- a/cli-tests/t_v1_policy.out +++ b/cli-tests/t_v1_policy.out @@ -101,11 +101,16 @@ cat: MNT/dir/file: No such file or directory # Testing incompletely locking v1-encrypted directory Enter custom passphrase for protector "prot": "MNT/dir" is now unlocked and ready for use. Encrypted data removed from filesystem cache. -[ERROR] fscrypt lock: some files using the key are still open +[ERROR] fscrypt lock: Directory was incompletely locked because some files are + still open. These files remain accessible. -Directory was incompletely locked because some files are still open. These files -remain accessible. Try killing any processes using files in the directory, then -re-running 'fscrypt lock'. +Try killing any processes using files in the directory, for example using: + + find "MNT/dir" -print0 | xargs -0 fuser -k + +Then re-run: + + fscrypt lock "MNT/dir" "MNT/dir" is encrypted with fscrypt. Policy: desc1 diff --git a/cmd/fscrypt/commands.go b/cmd/fscrypt/commands.go index ea393bb..8058cb3 100644 --- a/cmd/fscrypt/commands.go +++ b/cmd/fscrypt/commands.go @@ -297,7 +297,18 @@ func encryptPath(path string) (err error) { // checkEncryptable returns an error if the path cannot be encrypted. func checkEncryptable(ctx *actions.Context, path string) error { - log.Printf("ensuring %s is an empty and readable directory", path) + + log.Printf("checking whether %q is already encrypted", path) + if _, err := metadata.GetPolicy(path); err == nil { + return &metadata.ErrAlreadyEncrypted{Path: path} + } + + log.Printf("checking whether filesystem %s supports encryption", ctx.Mount.Path) + if err := ctx.Mount.CheckSupport(); err != nil { + return err + } + + log.Printf("checking whether %q is an empty and readable directory", path) f, err := os.Open(path) if err != nil { return err @@ -307,26 +318,13 @@ func checkEncryptable(ctx *actions.Context, path string) error { switch names, err := f.Readdirnames(-1); { case err != nil: // Could not read directory (might not be a directory) - log.Print(errors.Wrap(err, path)) - return errors.Wrap(ErrNotEmptyDir, path) - case len(names) > 0: - log.Printf("directory %s is not empty", path) - return errors.Wrap(ErrNotEmptyDir, path) - } - - log.Printf("ensuring %s supports encryption and filesystem is using fscrypt", path) - switch _, err := actions.GetPolicyFromPath(ctx, path); errors.Cause(err) { - case nil: - // We are encrypted - 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() - } + err = errors.Wrap(err, path) + log.Print(err) return err + case len(names) > 0: + return &ErrDirNotEmpty{path} } + return err } // selectOrCreateProtector uses user input (or flags) to either create a new @@ -410,7 +408,7 @@ func unlockAction(c *cli.Context) error { if policy.IsProvisionedByTargetUser() { log.Printf("policy %s is already provisioned by %v", policy.Descriptor(), ctx.TargetUser.Username) - return newExitError(c, errors.Wrapf(ErrPolicyUnlocked, path)) + return newExitError(c, errors.Wrapf(ErrDirAlreadyUnlocked, path)) } if err := policy.Unlock(optionFn, existingKeyFn); err != nil { @@ -499,7 +497,14 @@ func lockAction(c *cli.Context) error { } if err = policy.Deprovision(allUsersFlag.Value); err != nil { - if err != keyring.ErrKeyNotPresent { + switch err { + case keyring.ErrKeyNotPresent: + break + case keyring.ErrKeyAddedByOtherUsers: + return newExitError(c, &ErrDirUnlockedByOtherUsers{path}) + case keyring.ErrKeyFilesOpen: + return newExitError(c, &ErrDirFilesOpen{path}) + default: return newExitError(c, err) } // Key is no longer present. Normally that means the directory @@ -510,7 +515,7 @@ func lockAction(c *cli.Context) error { // locking the directory by dropping caches again. if !policy.NeedsUserKeyring() || !isDirUnlockedHeuristic(path) { log.Printf("policy %s is already fully deprovisioned", policy.Descriptor()) - return newExitError(c, errors.Wrapf(ErrPolicyLocked, path)) + return newExitError(c, errors.Wrapf(ErrDirAlreadyLocked, path)) } } @@ -519,7 +524,7 @@ func lockAction(c *cli.Context) error { return newExitError(c, err) } if isDirUnlockedHeuristic(path) { - return newExitError(c, keyring.ErrKeyFilesOpen) + return newExitError(c, &ErrDirFilesOpen{path}) } } diff --git a/cmd/fscrypt/errors.go b/cmd/fscrypt/errors.go index 4ce4504..63ddaf4 100644 --- a/cmd/fscrypt/errors.go +++ b/cmd/fscrypt/errors.go @@ -55,9 +55,8 @@ var ( ErrKeyFileLength = errors.Errorf("key file must be %d bytes", metadata.InternalKeyLen) ErrAllLoadsFailed = errors.New("could not load any protectors") 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") - ErrNotEmptyDir = errors.New("not an empty directory") + ErrDirAlreadyUnlocked = errors.New("this file or directory is already unlocked") + ErrDirAlreadyLocked = errors.New("this file or directory is already locked") ErrNotPassphrase = errors.New("protector does not use a passphrase") ErrUnknownUser = errors.New("unknown user") ErrDropCachesPerm = errors.New("inode cache can only be dropped as root") @@ -65,6 +64,38 @@ var ( ErrFsKeyringPerm = errors.New("root is required to add/remove v1 encryption policy keys to/from filesystem") ) +// ErrDirFilesOpen indicates that a directory can't be fully locked because +// files protected by the directory's policy are still open. +type ErrDirFilesOpen struct { + DirPath string +} + +func (err *ErrDirFilesOpen) Error() string { + return fmt.Sprintf(`Directory was incompletely locked because some files + are still open. These files remain accessible.`) +} + +// ErrDirUnlockedByOtherUsers indicates that a directory can't be locked because +// the directory's policy is still provisioned by other users. +type ErrDirUnlockedByOtherUsers struct { + DirPath string +} + +func (err *ErrDirUnlockedByOtherUsers) Error() string { + return fmt.Sprintf(`Directory %q couldn't be fully locked because other + user(s) have unlocked it.`, err.DirPath) +} + +// ErrDirNotEmpty indicates that a directory can't be encrypted because it's not +// empty. +type ErrDirNotEmpty struct { + DirPath string +} + +func (err *ErrDirNotEmpty) Error() string { + return fmt.Sprintf("Directory %q cannot be encrypted because it is non-empty.", err.DirPath) +} + var loadHelpText = fmt.Sprintf("You may need to mount a linked filesystem. Run with %s for more information.", shortDisplay(verboseFlag)) // getFullName returns the full name of the application or command being used. @@ -138,6 +169,37 @@ func suggestEnablingEncryption(mnt *filesystem.Mount) string { // an error. If no suggestion is necessary or available, return empty string. func getErrorSuggestions(err error) string { switch e := err.(type) { + case *ErrDirFilesOpen: + return fmt.Sprintf(`Try killing any processes using files in the + directory, for example using: + + > find %q -print0 | xargs -0 fuser -k + + Then re-run: + + > fscrypt lock %q`, e.DirPath, e.DirPath) + case *ErrDirNotEmpty: + dir := e.DirPath + newDir := dir + ".new" + return fmt.Sprintf(`Files cannot be encrypted in-place. Instead, + encrypt a new directory, copy the files into it, and securely + delete the original directory. For example: + + > mkdir %s + > fscrypt encrypt %s + > cp -a -T %s %s + > find %s -type f -print0 | xargs -0 shred -n1 --remove=unlink + > rm -rf %s + > mv %s %s + + Caution: due to the nature of modern storage devices and filesystems, + the original data may still be recoverable from disk. It's much better + to encrypt your files from the start.`, newDir, newDir, dir, newDir, dir, dir, newDir, dir) + case *ErrDirUnlockedByOtherUsers: + return fmt.Sprintf(`If you want to force the directory to be + locked, use: + + > sudo fscrypt lock --all-users %q`, e.DirPath) case *actions.ErrBadConfigFile: return `Either fix this file manually, or run "sudo fscrypt setup" to recreate it.` case *actions.ErrLoginProtectorName: @@ -183,30 +245,17 @@ func getErrorSuggestions(err error) string { -l". The limit can be modified by either changing the "memlock" item in /etc/security/limits.conf or by changing the "LimitMEMLOCK" value in systemd.` - case keyring.ErrKeyFilesOpen: - return `Directory was incompletely locked because some files are - still open. These files remain accessible. Try killing - any processes using files in the directory, then - re-running 'fscrypt lock'.` - case keyring.ErrKeyAddedByOtherUsers: - 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.ErrV2PoliciesUnsupported: return fmt.Sprintf(`v2 encryption policies are only supported by kernel version 5.4 and later. Either use a newer kernel, or change policy_version to 1 in %s.`, actions.ConfigFileLocation) case ErrNoDestructiveOps: - return fmt.Sprintf("Use %s to automatically run destructive operations.", shortDisplay(forceFlag)) + return fmt.Sprintf("If desired, use %s to automatically run destructive operations.", + shortDisplay(forceFlag)) case ErrSpecifyProtector: 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 ErrNotEmptyDir: - return `Encryption can only be setup on empty directories; files - cannot be encrypted in-place. Instead, encrypt an empty - directory, copy the files into that encrypted directory, - and securely delete the originals with "shred".` case ErrDropCachesPerm: return fmt.Sprintf(`Either this command should be run as root to properly clear the inode cache, or it should be run with -- 2.39.5