Merge pull request #205 from ebiggers/autoselect-v2

Automatically enable policy_version 2 when kernel support is detected

5 files changed, 44 insertions, 34 deletions

diff --git a/actions/config.go b/actions/config.go
index 6b019df..2463b95 100644
--- a/actions/config.go
+++ b/actions/config.go
@@ -36,10 +36,6 @@ import (
 	"github.com/google/fscrypt/util"
 )
 
-// LegacyConfig indicates that keys should be inserted into the keyring with the
-// legacy service prefixes. Needed for kernels before v4.8.
-const LegacyConfig = "legacy"
-
 // ConfigFileLocation is the location of fscrypt's global settings. This can be
 // overridden by the user of this package.
 var ConfigFileLocation = "/etc/fscrypt.conf"
@@ -61,12 +57,10 @@ var (
 )
 
 // CreateConfigFile creates a new config file at the appropriate location with
-// the appropriate hashing costs and encryption parameters. This creation is
-// configurable in two ways. First, a time target must be specified. This target
-// will determine the hashing costs, by picking parameters that make the hashing
-// take as long as the specified target. Second, the config can include the
-// legacy option, which is needed for systems with kernels older than v4.8.
-func CreateConfigFile(target time.Duration, useLegacy bool) error {
+// the appropriate hashing costs and encryption parameters. The hashing will be
+// configured to take as long as the specified time target. In addition, the
+// version of encryption policy to use may be overridden from the default of v1.
+func CreateConfigFile(target time.Duration, policyVersion int64) error {
 	// Create the config file before computing the hashing costs, so we fail
 	// immediately if the program has insufficient permissions.
 	configFile, err := filesystem.OpenFileOverridingUmask(ConfigFileLocation,
@@ -83,9 +77,9 @@ func CreateConfigFile(target time.Duration, useLegacy bool) error {
 		Source:  metadata.DefaultSource,
 		Options: metadata.DefaultOptions,
 	}
-	if useLegacy {
-		config.Compatibility = LegacyConfig
-		log.Printf("Using %q compatibility option\n", LegacyConfig)
+
+	if policyVersion != 0 {
+		config.Options.PolicyVersion = policyVersion
 	}
 
 	if config.HashCosts, err = getHashingCosts(target); err != nil {
diff --git a/actions/config_test.go b/actions/config_test.go
index 037e433..3599667 100644
--- a/actions/config_test.go
+++ b/actions/config_test.go
@@ -26,6 +26,8 @@ import (
 	"time"
 
 	"golang.org/x/sys/unix"
+
+	"github.com/google/fscrypt/metadata"
 )
 
 // Test that the global config file is created with mode 0644, regardless of the
@@ -42,7 +44,7 @@ func TestConfigFileIsCreatedWithCorrectMode(t *testing.T) {
 	defer os.RemoveAll(tempDir)
 	ConfigFileLocation = filepath.Join(tempDir, "test.conf")
 
-	if err = CreateConfigFile(time.Millisecond, false); err != nil {
+	if err = CreateConfigFile(time.Millisecond, 0); err != nil {
 		t.Fatal(err)
 	}
 	fileInfo, err := os.Stat(ConfigFileLocation)
@@ -53,3 +55,25 @@ func TestConfigFileIsCreatedWithCorrectMode(t *testing.T) {
 		t.Error("Expected newly created config file to have mode 0644")
 	}
 }
+
+func TestCreateConfigFileV2Policy(t *testing.T) {
+	tempDir, err := ioutil.TempDir("", "fscrypt")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tempDir)
+	ConfigFileLocation = filepath.Join(tempDir, "test.conf")
+
+	if err = CreateConfigFile(time.Millisecond, 2); err != nil {
+		t.Fatal(err)
+	}
+
+	var config *metadata.Config
+	config, err = getConfig()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if config.Options.PolicyVersion != 2 {
+		t.Error("Expected PolicyVersion 2")
+	}
+}
diff --git a/actions/context.go b/actions/context.go
index f07f225..0db0671 100644
--- a/actions/context.go
+++ b/actions/context.go
@@ -32,8 +32,6 @@ import (
 	"log"
 	"os/user"
 
-	"golang.org/x/sys/unix"
-
 	"github.com/pkg/errors"
 
 	"github.com/google/fscrypt/filesystem"
@@ -133,27 +131,10 @@ func (ctx *Context) checkContext() error {
 	return ctx.Mount.CheckSetup()
 }
 
-// getService returns the keyring service for this context. We use the presence
-// of the LegacyConfig flag to determine if we should use the legacy services.
-// For ext4 systems before v4.8 and f2fs systems before v4.6, filesystem
-// specific services must be used (these legacy services will still work with
-// later kernels).
-func (ctx *Context) getService() string {
-	// For legacy configurations, we may need non-standard services
-	if ctx.Config.HasCompatibilityOption(LegacyConfig) {
-		switch ctx.Mount.FilesystemType {
-		case "ext4", "f2fs":
-			return ctx.Mount.FilesystemType + ":"
-		}
-	}
-	return unix.FSCRYPT_KEY_DESC_PREFIX
-}
-
 func (ctx *Context) getKeyringOptions() *keyring.Options {
 	return &keyring.Options{
 		Mount:                     ctx.Mount,
 		User:                      ctx.TargetUser,
-		Service:                   ctx.getService(),
 		UseFsKeyringForV1Policies: ctx.Config.GetUseFsKeyringForV1Policies(),
 	}
 }
diff --git a/actions/context_test.go b/actions/context_test.go
index e8aefd7..4488a6b 100644
--- a/actions/context_test.go
+++ b/actions/context_test.go
@@ -52,7 +52,7 @@ func setupContext() (ctx *Context, err error) {
 		return nil, fmt.Errorf("created context at %q without config file", badCtx.Mount.Path)
 	}
 
-	if err = CreateConfigFile(testTime, true); err != nil {
+	if err = CreateConfigFile(testTime, 0); err != nil {
 		return nil, err
 	}
 	defer func() {
diff --git a/actions/policy.go b/actions/policy.go
index b7fe5a6..3baad72 100644
--- a/actions/policy.go
+++ b/actions/policy.go
@@ -22,6 +22,7 @@ package actions
 import (
 	"fmt"
 	"log"
+	"os"
 
 	"github.com/golang/protobuf/proto"
 	"github.com/pkg/errors"
@@ -41,6 +42,7 @@ var (
 	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")
 )
 
 // PurgeAllPolicies removes all policy keys on the filesystem from the kernel
@@ -152,6 +154,15 @@ func GetPolicyFromPath(ctx *Context, path string) (*Policy, error) {
 	// the path, and the data we get from the mountpoint.
 	pathData, err := metadata.GetPolicy(path)
 	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
+		// is ambiguous with other errors, but try to detect this case
+		// and show a better error message.
+		if os.IsPermission(err) &&
+			filesystem.HaveReadAccessTo(path) &&
+			!keyring.IsFsKeyringSupported(ctx.Mount) {
+			return nil, errors.Wrapf(ErrAccessDeniedPossiblyV2, "open %s", path)
+		}
 		return nil, err
 	}
 	descriptor := pathData.KeyDescriptor