Change error handling to new package

This commit changes the error handing for the crypto, filesystem,
metadata, pam, and util packages to use the error handling library
github.com/pkg/errors. This means elimination of the FSError type, an
increased use of wrapping errors (as opposed to logging), switching
on the Cause() of an error (as opposed to its value), and improving our
integration tests involving TEST_FILESYSTEM_ROOT.

This commit also fixes a few bugs with the keyring code to ensure that
our {Find|Remove|Insert}PolicyKey functions are always operating on the
same keyring. The check for filesystem support has been moved from the
filesystem package to the metadata package. Finally, the API for the
filesystem package has been slightly modified:
	* filesystem.AllFilesystems() now returns all the filesystems in
	  sorted order
	* certain path methods are now public

O_SYNC is also removed for writing the metadata. We don't get that much
from syncing the metadata, as the actual file data could also be
corrupted by and IO error. The sync operation is also occasionally very
slow (~3 seconds) and can be unfriendly to battery life.

Change-Id: I392c2655141714b16dfdbc84ac09780072be2cf0

3 files changed, 173 insertions, 144 deletions

diff --git a/metadata/checks.go b/metadata/checks.go
index 5d0ce59..074d79e 100644
--- a/metadata/checks.go
+++ b/metadata/checks.go
@@ -20,178 +20,177 @@
 package metadata
 
 import (
-	"log"
-
 	"github.com/golang/protobuf/proto"
+	"github.com/pkg/errors"
 
 	"fscrypt/util"
 )
 
+var errNotInitialized = errors.New("not initialized")
+
 // Metadata is the interface to all of the protobuf structures that can be
-// checked with the IsValid method.
+// checked for validity.
 type Metadata interface {
-	IsValid() bool
+	CheckValidity() error
 	proto.Message
 }
 
-// checkValidLength returns true if expected == actual, otherwise it logs an
-// InvalidLengthError.
-func checkValidLength(name string, expected int, actual int) bool {
-	if expected != actual {
-		log.Print(util.InvalidLengthError(name, expected, actual))
-		return false
+// CheckValidity ensures the mode has a name and isn't empty.
+func (m EncryptionOptions_Mode) CheckValidity() error {
+	if m == EncryptionOptions_default {
+		return errNotInitialized
 	}
-	return true
-}
-
-// IsValid ensures the mode has a name and isn't empty.
-func (m EncryptionOptions_Mode) IsValid() bool {
 	if m.String() == "" {
-		log.Print("Encryption mode cannot be the empty string")
-		return false
-	}
-	if m == EncryptionOptions_default {
-		log.Print("Encryption mode must be set to a non-default value")
-		return false
+		return errors.Errorf("unknown %d", m)
 	}
-	return true
+	return nil
 }
 
-// IsValid ensures the source has a name and isn't empty.
-func (s SourceType) IsValid() bool {
-	if s.String() == "" {
-		log.Print("SourceType cannot be the empty string")
-		return false
-	}
+// CheckValidity ensures the source has a name and isn't empty.
+func (s SourceType) CheckValidity() error {
 	if s == SourceType_default {
-		log.Print("SourceType must be set to a non-default value")
-		return false
+		return errNotInitialized
+	}
+	if s.String() == "" {
+		return errors.Errorf("unknown %d", s)
 	}
-	return true
+	return nil
 }
 
-// IsValid ensures the hash costs will be accepted by Argon2.
-func (h *HashingCosts) IsValid() bool {
+// CheckValidity ensures the hash costs will be accepted by Argon2.
+func (h *HashingCosts) CheckValidity() error {
 	if h == nil {
-		log.Print("HashingCosts not initialized")
-		return false
+		return errNotInitialized
 	}
-	if h.Time == 0 {
-		log.Print("Hashing time cost not initialized")
-		return false
+	if h.Time <= 0 {
+		return errors.Errorf("time=%d is not positive", h.Time)
 	}
-	if h.Parallelism == 0 {
-		log.Print("Hashing parallelism cost not initialized")
-		return false
+	if h.Parallelism <= 0 {
+		return errors.Errorf("parallelism=%d is not positive", h.Parallelism)
 	}
 	minMemory := 8 * h.Parallelism
 	if h.Memory < minMemory {
-		log.Printf("Hashing memory cost must be at least %d", minMemory)
-		return false
+		return errors.Errorf("memory=%d is less than minimum (%d)", h.Memory, minMemory)
 	}
-	return true
+	return nil
 }
 
-// IsValid ensures our buffers are the correct length (or just exist).
-func (w *WrappedKeyData) IsValid() bool {
+// CheckValidity ensures our buffers are the correct length.
+func (w *WrappedKeyData) CheckValidity() error {
 	if w == nil {
-		log.Print("WrappedKeyData not initialized")
-		return false
+		return errNotInitialized
 	}
 	if len(w.EncryptedKey) == 0 {
-		log.Print("EncryptedKey not initialized")
-		return false
+		return errors.Wrap(errNotInitialized, "encrypted key")
+	}
+	if err := util.CheckValidLength(IVLen, len(w.IV)); err != nil {
+		return errors.Wrap(err, "IV")
 	}
-	return checkValidLength("IV", IVLen, len(w.IV)) &&
-		checkValidLength("HMAC", HMACLen, len(w.Hmac))
+	return errors.Wrap(util.CheckValidLength(HMACLen, len(w.Hmac)), "HMAC")
 }
 
-// IsValid ensures our ProtectorData has the correct fields for its source.
-func (p *ProtectorData) IsValid() bool {
+// CheckValidity ensures our ProtectorData has the correct fields for its source.
+func (p *ProtectorData) CheckValidity() error {
 	if p == nil {
-		log.Print("ProtectorData not initialized")
-		return false
+		return errNotInitialized
+	}
+
+	if err := p.Source.CheckValidity(); err != nil {
+		return errors.Wrap(err, "protector source")
 	}
 
 	// Source specific checks
 	switch p.Source {
 	case SourceType_pam_passphrase:
 		if p.Uid < 0 {
-			log.Print("The UID should never be negative")
-			return false
+			return errors.Errorf("UID=%d is negative", p.Uid)
 		}
 		fallthrough
 	case SourceType_custom_passphrase:
-		if !p.Costs.IsValid() || !checkValidLength("Salt", SaltLen, len(p.Salt)) {
-			return false
+		if err := p.Costs.CheckValidity(); err != nil {
+			return errors.Wrap(err, "passphrase hashing costs")
+		}
+		if err := util.CheckValidLength(SaltLen, len(p.Salt)); err != nil {
+			return errors.Wrap(err, "passphrase hashing salt")
 		}
 	}
 
 	// Generic checks
-	return p.Source.IsValid() &&
-		p.WrappedKey.IsValid() &&
-		checkValidLength("EncryptedKey", InternalKeyLen, len(p.WrappedKey.EncryptedKey)) &&
-		checkValidLength("ProtectorDescriptor", DescriptorLen, len(p.ProtectorDescriptor))
+	if err := p.WrappedKey.CheckValidity(); err != nil {
+		return errors.Wrap(err, "wrapped protector key")
+	}
+	if err := util.CheckValidLength(DescriptorLen, len(p.ProtectorDescriptor)); err != nil {
+		return errors.Wrap(err, "protector descriptor")
 
+	}
+	err := util.CheckValidLength(InternalKeyLen, len(p.WrappedKey.EncryptedKey))
+	return errors.Wrap(err, "encrypted protector key")
 }
 
-// IsValid ensures each of the options is valid.
-func (e *EncryptionOptions) IsValid() bool {
+// CheckValidity ensures each of the options is valid.
+func (e *EncryptionOptions) CheckValidity() error {
 	if e == nil {
-		log.Print("EncryptionOptions not initialized")
-		return false
+		return errNotInitialized
 	}
 	if _, ok := util.Index(e.Padding, paddingArray); !ok {
-		log.Printf("Padding of %d is invalid", e.Padding)
-		return false
+		return errors.Errorf("padding of %d is invalid", e.Padding)
 	}
-
-	return e.Contents.IsValid() && e.Filenames.IsValid()
+	if err := e.Contents.CheckValidity(); err != nil {
+		return errors.Wrap(err, "contents encryption mode")
+	}
+	return errors.Wrap(e.Filenames.CheckValidity(), "filenames encryption mode")
 }
 
-// IsValid ensures the fields are valid and have the correct lengths.
-func (w *WrappedPolicyKey) IsValid() bool {
+// CheckValidity ensures the fields are valid and have the correct lengths.
+func (w *WrappedPolicyKey) CheckValidity() error {
 	if w == nil {
-		log.Print("WrappedPolicyKey not initialized")
-		return false
+		return errNotInitialized
 	}
-	return w.WrappedKey.IsValid() &&
-		checkValidLength("EncryptedKey", PolicyKeyLen, len(w.WrappedKey.EncryptedKey)) &&
-		checkValidLength("ProtectorDescriptor", DescriptorLen, len(w.ProtectorDescriptor))
+	if err := w.WrappedKey.CheckValidity(); err != nil {
+		return errors.Wrap(err, "wrapped key")
+	}
+	if err := util.CheckValidLength(PolicyKeyLen, len(w.WrappedKey.EncryptedKey)); err != nil {
+		return errors.Wrap(err, "encrypted key")
+	}
+	err := util.CheckValidLength(DescriptorLen, len(w.ProtectorDescriptor))
+	return errors.Wrap(err, "wrapping protector descriptor")
 }
 
-// IsValid ensures the fields and each wrapped key are valid.
-func (p *PolicyData) IsValid() bool {
+// CheckValidity ensures the fields and each wrapped key are valid.
+func (p *PolicyData) CheckValidity() error {
 	if p == nil {
-		log.Print("PolicyData not initialized")
-		return false
+		return errNotInitialized
 	}
 	// Check each wrapped key
-	for _, w := range p.WrappedPolicyKeys {
-		if !w.IsValid() {
-			return false
+	for i, w := range p.WrappedPolicyKeys {
+		if err := w.CheckValidity(); err != nil {
+			return errors.Wrapf(err, "policy key slot %d", i)
 		}
 	}
-	return p.Options.IsValid() &&
-		checkValidLength("KeyDescriptor", DescriptorLen, len(p.KeyDescriptor))
+	if err := util.CheckValidLength(DescriptorLen, len(p.KeyDescriptor)); err != nil {
+		return errors.Wrap(err, "policy key descriptor")
+	}
+
+	return errors.Wrap(p.Options.CheckValidity(), "policy options")
 }
 
-// IsValid ensures the Config has all the necessary info for its Source.
-func (c *Config) IsValid() bool {
+// CheckValidity ensures the Config has all the necessary info for its Source.
+func (c *Config) CheckValidity() error {
 	// General checks
 	if c == nil {
-		log.Print("Config not initialized")
-		return false
+		return errNotInitialized
 	}
-	if !c.Source.IsValid() || !c.Options.IsValid() {
-		return false
+	if err := c.Source.CheckValidity(); err != nil {
+		return errors.Wrap(err, "default config source")
 	}
 
 	// Source specific checks
 	switch c.Source {
 	case SourceType_pam_passphrase, SourceType_custom_passphrase:
-		return c.HashCosts.IsValid()
-	default:
-		return true
+		if err := c.HashCosts.CheckValidity(); err != nil {
+			return errors.Wrap(err, "config hashing costs")
+		}
 	}
+
+	return errors.Wrap(c.Options.CheckValidity(), "config options")
 }
diff --git a/metadata/policy.go b/metadata/policy.go
index ac2fde7..259fe04 100644
--- a/metadata/policy.go
+++ b/metadata/policy.go
@@ -22,12 +22,12 @@ package metadata
 
 import (
 	"encoding/hex"
-	"errors"
-	"fmt"
 	"log"
+	"math"
 	"os"
 	"unsafe"
 
+	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
 
 	"fscrypt/util"
@@ -35,25 +35,18 @@ import (
 
 // Encryption specific errors
 var (
-	ErrEncryptionNotSupported = errors.New("filesystem encryption not supported")
-	ErrEncryptionDisabled     = errors.New("filesystem encryption disabled in the kernel config")
+	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")
 )
 
-// policyIoctl is a wrapper for the ioctl syscall. If opens the file at the path
-// and passes the correct pointers and file descriptors to the IOCTL syscall.
-// This function also takes some of the unclear errors returned by the syscall
-// and translates then into more specific error strings.
-func policyIoctl(path string, request uintptr, policy *unix.FscryptPolicy) error {
-	file, err := os.Open(path)
-	if err != nil {
-		// For PathErrors, we just want the underlying error
-		return util.UnderlyingError(err)
-	}
-	defer file.Close()
-
+// policyIoctl is a wrapper for the ioctl syscall. It passes the correct
+// pointers and file descriptors to the IOCTL syscall. This function also takes
+// some of the unclear errors returned by the syscall and translates then into
+// more specific error strings.
+func policyIoctl(file *os.File, request uintptr, policy *unix.FscryptPolicy) 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(unsafe.Pointer(policy)))
@@ -63,7 +56,7 @@ func policyIoctl(path string, request uintptr, policy *unix.FscryptPolicy) error
 	case unix.ENOTTY:
 		return ErrEncryptionNotSupported
 	case unix.EOPNOTSUPP:
-		return ErrEncryptionDisabled
+		return ErrEncryptionNotEnabled
 	case unix.ENODATA, unix.ENOENT:
 		// ENOENT was returned instead of ENODATA on some filesystems before v4.11.
 		return ErrNotEncrypted
@@ -86,10 +79,16 @@ var (
 // the KeyDescriptor and the encryption options). Returns an error if the
 // path is not encrypted or the policy couldn't be retrieved.
 func GetPolicy(path string) (*PolicyData, error) {
-	var policy unix.FscryptPolicy
-	if err := policyIoctl(path, unix.FS_IOC_GET_ENCRYPTION_POLICY, &policy); err != nil {
+	file, err := os.Open(path)
+	if err != nil {
 		return nil, err
 	}
+	defer file.Close()
+
+	var policy unix.FscryptPolicy
+	if err := policyIoctl(file, unix.FS_IOC_GET_ENCRYPTION_POLICY, &policy); err != nil {
+		return nil, errors.Wrapf(err, "get encryption policy %s", path)
+	}
 
 	// Convert the padding flag into an amount of padding
 	paddingFlag := int64(policy.Flags & unix.FS_POLICY_FLAGS_PAD_MASK)
@@ -97,8 +96,7 @@ func GetPolicy(path string) (*PolicyData, error) {
 	// This lookup should always succeed
 	padding, ok := util.Lookup(paddingFlag, flagsArray, paddingArray)
 	if !ok {
-		log.Printf("padding flag of %x not found", paddingFlag)
-		util.NeverError(util.SystemError("invalid padding flag"))
+		log.Panicf("padding flag of %x not found", paddingFlag)
 	}
 
 	return &PolicyData{
@@ -115,22 +113,25 @@ func GetPolicy(path string) (*PolicyData, error) {
 // policy. Returns an error if we cannot set the policy for any reason (not a
 // directory, invalid options or KeyDescriptor, etc).
 func SetPolicy(path string, data *PolicyData) error {
-	// Convert the padding value to a flag
-	paddingFlag, ok := util.Lookup(data.Options.Padding, paddingArray, flagsArray)
-	if !ok {
-		return util.InvalidInput(fmt.Sprintf("padding of %d", data.Options.Padding))
+	file, err := os.Open(path)
+	if err != nil {
+		return err
+	}
+	defer file.Close()
+
+	if err := data.CheckValidity(); err != nil {
+		return errors.Wrap(err, "invalid policy")
 	}
 
-	// Convert the policyDescriptor to a byte array
-	if len(data.KeyDescriptor) != DescriptorLen {
-		return util.InvalidLengthError(
-			"policy descriptor", DescriptorLen, len(data.KeyDescriptor))
+	// This lookup should always succeed (as policy is valid)
+	paddingFlag, ok := util.Lookup(data.Options.Padding, paddingArray, flagsArray)
+	if !ok {
+		log.Panicf("padding of %d was not found", data.Options.Padding)
 	}
 
 	descriptorBytes, err := hex.DecodeString(data.KeyDescriptor)
 	if err != nil {
-		return util.InvalidInput(
-			fmt.Sprintf("policy descriptor of %s: %v", data.KeyDescriptor, err))
+		return errors.New("invalid descriptor: " + data.KeyDescriptor)
 	}
 
 	policy := unix.FscryptPolicy{
@@ -141,24 +142,50 @@ func SetPolicy(path string, data *PolicyData) error {
 	}
 	copy(policy.Master_key_descriptor[:], descriptorBytes)
 
-	if err = policyIoctl(path, unix.FS_IOC_SET_ENCRYPTION_POLICY, &policy); err != nil {
+	if err = policyIoctl(file, unix.FS_IOC_SET_ENCRYPTION_POLICY, &policy); 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
 		// the correct error due to a potential race condition on path.
-		if err == unix.EINVAL {
+		if info, statErr := os.Stat(path); statErr != nil || !info.IsDir() {
 			// Checking if the path is not a directory
-			if info, err := os.Stat(path); err != nil || !info.IsDir() {
-				return unix.ENOTDIR
-			}
+			err = unix.ENOTDIR
+		} else if _, policyErr := GetPolicy(path); policyErr == nil {
 			// Checking if a policy is already set on this directory
-			if _, err := GetPolicy(path); err == nil {
-				return ErrEncrypted
-			}
-			// Could not get a more detailed error, return generic "bad options".
-			return ErrBadEncryptionOptions
+			err = ErrEncrypted
+		} else {
+			// Default to generic "bad options".
+			err = ErrBadEncryptionOptions
 		}
+	}
+
+	return errors.Wrapf(err, "set encryption policy %s", path)
+}
+
+// CheckSupport returns an error if the filesystem containing path does not
+// support filesystem encryption. This can be for many reasons including an
+// incompatible kernel or filesystem or not enabling the right feature flags.
+func CheckSupport(path string) error {
+	file, err := os.Open(path)
+	if err != nil {
 		return err
 	}
+	defer file.Close()
+
+	// On supported directories, giving a bad policy will return EINVAL
+	badPolicy := unix.FscryptPolicy{
+		Version:                   math.MaxUint8,
+		Contents_encryption_mode:  math.MaxUint8,
+		Filenames_encryption_mode: math.MaxUint8,
+		Flags: math.MaxUint8,
+	}
 
-	return nil
+	err = policyIoctl(file, unix.FS_IOC_SET_ENCRYPTION_POLICY, &badPolicy)
+	switch err {
+	case nil:
+		log.Panicf(`FS_IOC_SET_ENCRYPTION_POLICY succeeded when it should have failed.
+			Please open an issue, filesystem %q may be corrupted.`, path)
+	case unix.EINVAL, unix.EACCES:
+		return nil
+	}
+	return err
 }
diff --git a/metadata/policy_test.go b/metadata/policy_test.go
index 6dc2567..58e19d7 100644
--- a/metadata/policy_test.go
+++ b/metadata/policy_test.go
@@ -25,6 +25,8 @@ import (
 	"path/filepath"
 	"reflect"
 	"testing"
+
+	. "fscrypt/util"
 )
 
 const goodDescriptor = "0123456789abcdef"
@@ -34,13 +36,14 @@ var goodPolicy = &PolicyData{
 	Options:       DefaultOptions,
 }
 
-// Creates a temporary directory in TEST_FILESYSTEM_ROOT for testing. Fails if
-// the root directory is not specified.
+// Creates a temporary directory for testing.
 func createTestDirectory() (directory string, err error) {
-	baseDirectory := os.Getenv("TEST_FILESYSTEM_ROOT")
+	baseDirectory, err := TestPath()
+	if err != nil {
+		return
+	}
 	if s, err := os.Stat(baseDirectory); err != nil || !s.IsDir() {
-		return "", fmt.Errorf("invalid directory %q: "+
-			"set TEST_FILESYSTEM_ROOT to be a valid directory", baseDirectory)
+		return "", fmt.Errorf("%s: %q is not a valid directory", TestEnvVarName, baseDirectory)
 	}
 
 	directoryPath := filepath.Join(baseDirectory, "test")