Merge pull request #219 from ebiggers/improve-errors

Improve error messages and suggestions

5 files changed, 211 insertions, 52 deletions

diff --git a/actions/config.go b/actions/config.go
index 2463b95..b848d92 100644
--- a/actions/config.go
+++ b/actions/config.go
@@ -22,12 +22,12 @@ package actions
 
 import (
 	"bytes"
+	"fmt"
 	"log"
 	"os"
 	"runtime"
 	"time"
 
-	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
 
 	"github.com/google/fscrypt/crypto"
@@ -40,6 +40,46 @@ import (
 // overridden by the user of this package.
 var ConfigFileLocation = "/etc/fscrypt.conf"
 
+// ErrBadConfig is an internal error that indicates that the config struct is invalid.
+type ErrBadConfig struct {
+	Config          *metadata.Config
+	UnderlyingError error
+}
+
+func (err *ErrBadConfig) Error() string {
+	return fmt.Sprintf(`internal error: config is invalid: %s
+
+	The invalid config is %s`, err.UnderlyingError, err.Config)
+}
+
+// ErrBadConfigFile indicates that the config file is invalid.
+type ErrBadConfigFile struct {
+	Path            string
+	UnderlyingError error
+}
+
+func (err *ErrBadConfigFile) Error() string {
+	return fmt.Sprintf("%q is invalid: %s", err.Path, err.UnderlyingError)
+}
+
+// ErrConfigFileExists indicates that the config file already exists.
+type ErrConfigFileExists struct {
+	Path string
+}
+
+func (err *ErrConfigFileExists) Error() string {
+	return fmt.Sprintf("%q already exists", err.Path)
+}
+
+// ErrNoConfigFile indicates that the config file doesn't exist.
+type ErrNoConfigFile struct {
+	Path string
+}
+
+func (err *ErrNoConfigFile) Error() string {
+	return fmt.Sprintf("%q doesn't exist", err.Path)
+}
+
 const (
 	// Permissions of the config file (global readable)
 	configPermissions = 0644
@@ -67,7 +107,7 @@ func CreateConfigFile(target time.Duration, policyVersion int64) error {
 		createFlags, configPermissions)
 	switch {
 	case os.IsExist(err):
-		return ErrConfigFileExists
+		return &ErrConfigFileExists{ConfigFileLocation}
 	case err != nil:
 		return err
 	}
@@ -98,7 +138,7 @@ func getConfig() (*metadata.Config, error) {
 	configFile, err := os.Open(ConfigFileLocation)
 	switch {
 	case os.IsNotExist(err):
-		return nil, ErrNoConfigFile
+		return nil, &ErrNoConfigFile{ConfigFileLocation}
 	case err != nil:
 		return nil, err
 	}
@@ -107,7 +147,7 @@ func getConfig() (*metadata.Config, error) {
 	log.Printf("Reading config from %q\n", ConfigFileLocation)
 	config, err := metadata.ReadConfig(configFile)
 	if err != nil {
-		return nil, errors.Wrap(ErrBadConfigFile, err.Error())
+		return nil, &ErrBadConfigFile{ConfigFileLocation, err}
 	}
 
 	// Use system defaults if not specified
@@ -133,7 +173,7 @@ func getConfig() (*metadata.Config, error) {
 	}
 
 	if err := config.CheckValidity(); err != nil {
-		return nil, errors.Wrap(ErrBadConfigFile, err.Error())
+		return nil, &ErrBadConfigFile{ConfigFileLocation, err}
 	}
 
 	return config, nil
diff --git a/actions/context.go b/actions/context.go
index 0db0671..26295ec 100644
--- a/actions/context.go
+++ b/actions/context.go
@@ -40,14 +40,8 @@ import (
 	"github.com/google/fscrypt/util"
 )
 
-// Errors relating to Config files or Config structures.
-var (
-	ErrNoConfigFile     = errors.New("global config file does not exist")
-	ErrBadConfigFile    = errors.New("global config file has invalid data")
-	ErrConfigFileExists = errors.New("global config file already exists")
-	ErrBadConfig        = errors.New("invalid Config structure provided")
-	ErrLocked           = errors.New("key needs to be unlocked first")
-)
+// ErrLocked indicates that the key hasn't been unwrapped yet.
+var ErrLocked = errors.New("key needs to be unlocked first")
 
 // Context contains the necessary global state to perform most of fscrypt's
 // actions.
@@ -126,7 +120,7 @@ func newContextFromUser(targetUser *user.User) (*Context, error) {
 // which is being used with fscrypt.
 func (ctx *Context) checkContext() error {
 	if err := ctx.Config.CheckValidity(); err != nil {
-		return errors.Wrap(ErrBadConfig, err.Error())
+		return &ErrBadConfig{ctx.Config, err}
 	}
 	return ctx.Mount.CheckSetup()
 }
diff --git a/actions/policy.go b/actions/policy.go
index 6c2aa51..6c48117 100644
--- a/actions/policy.go
+++ b/actions/policy.go
@@ -34,16 +34,109 @@ import (
 	"github.com/google/fscrypt/util"
 )
 
-// Errors relating to Policies
-var (
-	ErrMissingPolicyMetadata  = util.SystemError("missing policy metadata for encrypted directory")
-	ErrPolicyMetadataMismatch = util.SystemError("inconsistent metadata between filesystem and directory")
-	ErrDifferentFilesystem    = errors.New("policies may only protect files on the same filesystem")
-	ErrOnlyProtector          = errors.New("cannot remove the only protector for a policy")
-	ErrAlreadyProtected       = errors.New("policy already protected by protector")
-	ErrNotProtected           = errors.New("policy not protected by protector")
-	ErrAccessDeniedPossiblyV2 = errors.New("permission denied")
-)
+// ErrAccessDeniedPossiblyV2 indicates that a directory's encryption policy
+// couldn't be retrieved due to "permission denied", but it looks like it's due
+// to the directory using a v2 policy but the kernel not supporting it.
+type ErrAccessDeniedPossiblyV2 struct {
+	DirPath string
+}
+
+func (err *ErrAccessDeniedPossiblyV2) Error() string {
+	return fmt.Sprintf(`
+	failed to get encryption policy of %s: permission denied
+
+	This may be caused by the directory using a v2 encryption policy and the
+	current kernel not supporting it. If indeed the case, then this
+	directory can only be used on kernel v5.4 and later. You can create
+	directories accessible on older kernels by changing policy_version to 1
+	in %s.`,
+		err.DirPath, ConfigFileLocation)
+}
+
+// ErrAlreadyProtected indicates that a policy is already protected by the given
+// protector.
+type ErrAlreadyProtected struct {
+	Policy    *Policy
+	Protector *Protector
+}
+
+func (err *ErrAlreadyProtected) Error() string {
+	return fmt.Sprintf("policy %s is already protected by protector %s",
+		err.Policy.Descriptor(), err.Protector.Descriptor())
+}
+
+// ErrDifferentFilesystem indicates that a policy can't be applied to a
+// directory on a different filesystem.
+type ErrDifferentFilesystem struct {
+	PolicyMount *filesystem.Mount
+	PathMount   *filesystem.Mount
+}
+
+func (err *ErrDifferentFilesystem) Error() string {
+	return fmt.Sprintf(`cannot apply policy from filesystem %q to a
+	directory on filesystem %q. Policies may only protect files on the same
+	filesystem.`, err.PolicyMount.Path, err.PathMount.Path)
+}
+
+// ErrMissingPolicyMetadata indicates that a directory is encrypted but its
+// policy metadata cannot be found.
+type ErrMissingPolicyMetadata struct {
+	Mount      *filesystem.Mount
+	DirPath    string
+	Descriptor string
+}
+
+func (err *ErrMissingPolicyMetadata) Error() string {
+	return fmt.Sprintf(`filesystem %q does not contain the policy metadata
+	for %q. This directory has either been encrypted with another tool (such
+	as e4crypt), or the file %q has been deleted.`,
+		err.Mount.Path, err.DirPath,
+		err.Mount.PolicyPath(err.Descriptor))
+}
+
+// ErrNotProtected indicates that the given policy is not protected by the given
+// protector.
+type ErrNotProtected struct {
+	PolicyDescriptor    string
+	ProtectorDescriptor string
+}
+
+func (err *ErrNotProtected) Error() string {
+	return fmt.Sprintf(`policy %s is not protected by protector %s`,
+		err.PolicyDescriptor, err.ProtectorDescriptor)
+}
+
+// ErrOnlyProtector indicates that the last protector can't be removed from a
+// policy.
+type ErrOnlyProtector struct {
+	Policy *Policy
+}
+
+func (err *ErrOnlyProtector) Error() string {
+	return fmt.Sprintf(`cannot remove the only protector from policy %s. A
+	policy must have at least one protector.`, err.Policy.Descriptor())
+}
+
+// ErrPolicyMetadataMismatch indicates that the policy metadata for an encrypted
+// directory is inconsistent with that directory.
+type ErrPolicyMetadataMismatch struct {
+	DirPath   string
+	Mount     *filesystem.Mount
+	PathData  *metadata.PolicyData
+	MountData *metadata.PolicyData
+}
+
+func (err *ErrPolicyMetadataMismatch) Error() string {
+	return fmt.Sprintf(`inconsistent metadata between encrypted directory %q
+	and its corresponding metadata file %q.
+
+	Directory has descriptor:%s %s
+
+	Metadata file has descriptor:%s %s`,
+		err.DirPath, err.Mount.PolicyPath(err.PathData.KeyDescriptor),
+		err.PathData.KeyDescriptor, err.PathData.Options,
+		err.MountData.KeyDescriptor, err.MountData.Options)
+}
 
 // PurgeAllPolicies removes all policy keys on the filesystem from the kernel
 // keyring. In order for this to fully take effect, the filesystem may also need
@@ -153,6 +246,7 @@ func GetPolicyFromPath(ctx *Context, path string) (*Policy, error) {
 	// We double check that the options agree for both the data we get from
 	// the path, and the data we get from the mountpoint.
 	pathData, err := metadata.GetPolicy(path)
+	err = ctx.Mount.EncryptionSupportError(err)
 	if err != nil {
 		// On kernels that don't support v2 encryption policies, trying
 		// to open a directory with a v2 policy simply gave EACCES. This
@@ -161,7 +255,7 @@ func GetPolicyFromPath(ctx *Context, path string) (*Policy, error) {
 		if os.IsPermission(err) &&
 			filesystem.HaveReadAccessTo(path) &&
 			!keyring.IsFsKeyringSupported(ctx.Mount) {
-			return nil, errors.Wrapf(ErrAccessDeniedPossiblyV2, "open %s", path)
+			return nil, &ErrAccessDeniedPossiblyV2{path}
 		}
 		return nil, err
 	}
@@ -171,14 +265,16 @@ func GetPolicyFromPath(ctx *Context, path string) (*Policy, error) {
 	mountData, err := ctx.Mount.GetPolicy(descriptor)
 	if err != nil {
 		log.Printf("getting policy metadata: %v", err)
-		return nil, errors.Wrap(ErrMissingPolicyMetadata, path)
+		if _, ok := err.(*filesystem.ErrPolicyNotFound); ok {
+			return nil, &ErrMissingPolicyMetadata{ctx.Mount, path, descriptor}
+		}
+		return nil, err
 	}
 	log.Printf("found data for policy %s on %q", descriptor, ctx.Mount.Path)
 
-	if !proto.Equal(pathData.Options, mountData.Options) {
-		log.Printf("options from path: %+v", pathData.Options)
-		log.Printf("options from mount: %+v", mountData.Options)
-		return nil, errors.Wrapf(ErrPolicyMetadataMismatch, "policy %s", descriptor)
+	if !proto.Equal(pathData.Options, mountData.Options) ||
+		pathData.KeyDescriptor != mountData.KeyDescriptor {
+		return nil, &ErrPolicyMetadataMismatch{path, ctx.Mount, pathData, mountData}
 	}
 	log.Print("data from filesystem and path agree")
 
@@ -290,7 +386,7 @@ func (policy *Policy) UnlockWithProtector(protector *Protector) error {
 	}
 	idx, ok := policy.findWrappedKeyIndex(protector.Descriptor())
 	if !ok {
-		return ErrNotProtected
+		return &ErrNotProtected{policy.Descriptor(), protector.Descriptor()}
 	}
 
 	var err error
@@ -321,7 +417,7 @@ func (policy *Policy) UsesProtector(protector *Protector) bool {
 // protector must both be unlocked.
 func (policy *Policy) AddProtector(protector *Protector) error {
 	if policy.UsesProtector(protector) {
-		return ErrAlreadyProtected
+		return &ErrAlreadyProtected{policy, protector}
 	}
 	if policy.key == nil || protector.key == nil {
 		return ErrLocked
@@ -372,11 +468,11 @@ func (policy *Policy) AddProtector(protector *Protector) error {
 func (policy *Policy) RemoveProtector(protector *Protector) error {
 	idx, ok := policy.findWrappedKeyIndex(protector.Descriptor())
 	if !ok {
-		return ErrNotProtected
+		return &ErrNotProtected{policy.Descriptor(), protector.Descriptor()}
 	}
 
 	if len(policy.data.WrappedPolicyKeys) == 1 {
-		return ErrOnlyProtector
+		return &ErrOnlyProtector{policy}
 	}
 
 	// Remove the wrapped key from the data
@@ -397,10 +493,11 @@ func (policy *Policy) Apply(path string) error {
 	if pathMount, err := filesystem.FindMount(path); err != nil {
 		return err
 	} else if pathMount != policy.Context.Mount {
-		return ErrDifferentFilesystem
+		return &ErrDifferentFilesystem{policy.Context.Mount, pathMount}
 	}
 
-	return metadata.SetPolicy(path, policy.data)
+	err := metadata.SetPolicy(path, policy.data)
+	return policy.Context.Mount.EncryptionSupportError(err)
 }
 
 // GetProvisioningStatus returns the status of this policy's key in the keyring.
diff --git a/actions/protector.go b/actions/protector.go
index dab9c27..3278e63 100644
--- a/actions/protector.go
+++ b/actions/protector.go
@@ -22,8 +22,7 @@ package actions
 import (
 	"fmt"
 	"log"
-
-	"github.com/pkg/errors"
+	"os/user"
 
 	"github.com/google/fscrypt/crypto"
 	"github.com/google/fscrypt/metadata"
@@ -34,13 +33,44 @@ import (
 // This can be overridden by the user of this package.
 var LoginProtectorMountpoint = "/"
 
-// Errors relating to Protectors
-var (
-	ErrProtectorName        = errors.New("login protectors do not need a name")
-	ErrMissingProtectorName = errors.New("custom protectors must have a name")
-	ErrDuplicateName        = errors.New("protector with this name already exists")
-	ErrDuplicateUID         = errors.New("login protector for this user already exists")
-)
+// ErrLoginProtectorExists indicates that a user already has a login protector.
+type ErrLoginProtectorExists struct {
+	User *user.User
+}
+
+func (err *ErrLoginProtectorExists) Error() string {
+	return fmt.Sprintf("user %q already has a login protector", err.User.Username)
+}
+
+// ErrLoginProtectorName indicates that a name was given for a login protector.
+type ErrLoginProtectorName struct {
+	Name string
+	User *user.User
+}
+
+func (err *ErrLoginProtectorName) Error() string {
+	return fmt.Sprintf(`cannot assign name %q to new login protector for
+	user %q because login protectors are identified by user, not by name.`,
+		err.Name, err.User.Username)
+}
+
+// ErrMissingProtectorName indicates that a protector name is needed.
+type ErrMissingProtectorName struct {
+	Source metadata.SourceType
+}
+
+func (err *ErrMissingProtectorName) Error() string {
+	return fmt.Sprintf("%s protectors must be named", err.Source)
+}
+
+// ErrProtectorNameExists indicates that a protector name already exists.
+type ErrProtectorNameExists struct {
+	Name string
+}
+
+func (err *ErrProtectorNameExists) Error() string {
+	return fmt.Sprintf("there is already a protector named %q", err.Name)
+}
 
 // checkForProtectorWithName returns an error if there is already a protector
 // on the filesystem with a specific name (or if we cannot read the necessary
@@ -52,7 +82,7 @@ func checkForProtectorWithName(ctx *Context, name string) error {
 	}
 	for _, option := range options {
 		if option.Name() == name {
-			return errors.Wrapf(ErrDuplicateName, "name %q", name)
+			return &ErrProtectorNameExists{name}
 		}
 	}
 	return nil
@@ -68,7 +98,7 @@ func checkIfUserHasLoginProtector(ctx *Context, uid int64) error {
 	}
 	for _, option := range options {
 		if option.Source() == metadata.SourceType_pam_passphrase && option.UID() == uid {
-			return errors.Wrapf(ErrDuplicateUID, "user %q", ctx.TargetUser.Username)
+			return &ErrLoginProtectorExists{ctx.TargetUser}
 		}
 	}
 	return nil
@@ -97,12 +127,12 @@ func CreateProtector(ctx *Context, name string, keyFn KeyFunc) (*Protector, erro
 	if ctx.Config.Source == metadata.SourceType_pam_passphrase {
 		// login protectors don't need a name (we use the username instead)
 		if name != "" {
-			return nil, ErrProtectorName
+			return nil, &ErrLoginProtectorName{name, ctx.TargetUser}
 		}
 	} else {
 		// non-login protectors need a name (so we can distinguish between them)
 		if name == "" {
-			return nil, ErrMissingProtectorName
+			return nil, &ErrMissingProtectorName{ctx.Config.Source}
 		}
 		// we don't want to duplicate naming
 		if err := checkForProtectorWithName(ctx, name); err != nil {
diff --git a/actions/recovery.go b/actions/recovery.go
index 1c55ec5..458349b 100644
--- a/actions/recovery.go
+++ b/actions/recovery.go
@@ -23,8 +23,6 @@ import (
 	"os"
 	"strconv"
 
-	"github.com/pkg/errors"
-
 	"github.com/google/fscrypt/crypto"
 	"github.com/google/fscrypt/metadata"
 )
@@ -72,7 +70,7 @@ func AddRecoveryPassphrase(policy *Policy, dirname string) (*crypto.Key, *Protec
 		if err == nil {
 			break
 		}
-		if errors.Cause(err) != ErrDuplicateName {
+		if _, ok := err.(*ErrProtectorNameExists); !ok {
 			return nil, nil, err
 		}
 		seq++