From fbc161a77962fe64e3caad80efb535d28d8c1f74 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sat, 9 May 2020 14:52:07 -0700 Subject: 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. --- cmd/fscrypt/commands.go | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) (limited to 'cmd/fscrypt/commands.go') 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 } } -- cgit v1.2.3 From 66fb4c557644ba2c37951a7568c06c47a6c718a7 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sat, 9 May 2020 14:52:07 -0700 Subject: filesystem: improve errors Introduce filesystem.ErrEncryptionNotEnabled and filesystem.ErrEncryptionNotSupported which include the Mount as context, and translate the corresponding metadata/ errors into them. Then make these errors show much better suggestions. Also replace lots of other filesystem/ errors with either custom types or with unnamed one-off errors that include more context. Fix backwards wrapping in lots of cases. Finally, don't include the mountpoint in places where it's not useful, like OS-level errors that already include the path. --- cmd/fscrypt/commands.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'cmd/fscrypt/commands.go') diff --git a/cmd/fscrypt/commands.go b/cmd/fscrypt/commands.go index 86816ba..ea393bb 100644 --- a/cmd/fscrypt/commands.go +++ b/cmd/fscrypt/commands.go @@ -74,7 +74,7 @@ func setupAction(c *cli.Context) error { return newExitError(c, err) } if err := setupFilesystem(c.App.Writer, actions.LoginProtectorMountpoint); err != nil { - if errors.Cause(err) != filesystem.ErrAlreadySetup { + if _, ok := err.(*filesystem.ErrAlreadySetup); !ok { return newExitError(c, err) } fmt.Fprintf(c.App.Writer, @@ -660,17 +660,15 @@ func statusAction(c *cli.Context) error { err = writeGlobalStatus(c.App.Writer) case 1: path := c.Args().Get(0) - ctx, mntErr := actions.NewContextFromMountpoint(path, nil) - switch errors.Cause(mntErr) { - case nil: + var ctx *actions.Context + ctx, err = actions.NewContextFromMountpoint(path, nil) + if err == nil { // Case (2) - mountpoint status err = writeFilesystemStatus(c.App.Writer, ctx) - case filesystem.ErrNotAMountpoint: + } else if _, ok := err.(*filesystem.ErrNotAMountpoint); ok { // Case (3) - file or directory status err = writePathStatus(c.App.Writer, path) - default: - err = mntErr } default: return expectedArgsErr(c, 1, true) -- cgit v1.2.3 From 181600d6327ed34a3f62eda0dd03a6d2ae49e5f9 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sat, 9 May 2020 14:52:07 -0700 Subject: 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. --- cmd/fscrypt/commands.go | 51 +++++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 23 deletions(-) (limited to 'cmd/fscrypt/commands.go') 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}) } } -- cgit v1.2.3