cmd/fscrypt: adjust user and keyring validation and preparation

Don't force the user to provide a --user argument when running fscrypt
as root if they're doing something where the TargetUser isn't actually
needed, such as provisioning/deprovisioning a v1 encryption policy
to/from the filesystem keyring, or creating a non-login protector.

Also don't set up the user keyring (or check for it being set up) if it
won't actually be used.

Finally, if we'll be provisioning/deprovisioning a v1 encryption policy
to/from the filesystem keyring, make sure the command is running as
root, since the kernel requires this.

4 files changed, 74 insertions, 29 deletions

diff --git a/cmd/fscrypt/commands.go b/cmd/fscrypt/commands.go
index 8f2d21b..621651e 100644
--- a/cmd/fscrypt/commands.go
+++ b/cmd/fscrypt/commands.go
@@ -30,6 +30,7 @@ import (
 
 	"github.com/google/fscrypt/actions"
 	"github.com/google/fscrypt/filesystem"
+	"github.com/google/fscrypt/keyring"
 	"github.com/google/fscrypt/metadata"
 	"github.com/google/fscrypt/security"
 	"github.com/google/fscrypt/util"
@@ -134,11 +135,35 @@ func encryptAction(c *cli.Context) error {
 	return nil
 }
 
+// validateKeyringPrereqs ensures we're ready to add, remove, or get the status
+// of the key for the given encryption policy (if policy != nil) or for the
+// current default encryption policy (if policy == nil).
+func validateKeyringPrereqs(ctx *actions.Context, policy *actions.Policy) error {
+	if ctx.Config.GetUseFsKeyringForV1Policies() {
+		// We'll be using the filesystem keyring, but it's a v1
+		// encryption policy so root is required.
+		if !util.IsUserRoot() {
+			return ErrFsKeyringPerm
+		}
+		return nil
+	}
+	// We'll be using the target user's user keyring, so make sure a user
+	// was explicitly specified if the command is being run as root, and
+	// make sure that user's keyring is accessible.
+	if userFlag.Value == "" && util.IsUserRoot() {
+		return ErrSpecifyUser
+	}
+	if _, err := keyring.UserKeyringID(ctx.TargetUser, true); err != nil {
+		return err
+	}
+	return nil
+}
+
 // encryptPath sets up encryption on path and provisions the policy to the
 // keyring unless --skip-unlock is used. On failure, an error is returned, any
 // metadata creation is reverted, and the directory is unmodified.
 func encryptPath(path string) (err error) {
-	targetUser, err := parseUserFlag(!skipUnlockFlag.Value)
+	targetUser, err := parseUserFlag()
 	if err != nil {
 		return
 	}
@@ -154,10 +179,24 @@ func encryptPath(path string) (err error) {
 	if policyFlag.Value != "" {
 		log.Printf("getting policy for %q", path)
 
-		policy, err = getPolicyFromFlag(policyFlag.Value, ctx.TargetUser)
+		if policy, err = getPolicyFromFlag(policyFlag.Value, ctx.TargetUser); err != nil {
+			return
+		}
+
+		if !skipUnlockFlag.Value {
+			if err = validateKeyringPrereqs(ctx, policy); err != nil {
+				return
+			}
+		}
 	} else {
 		log.Printf("creating policy for %q", path)
 
+		if !skipUnlockFlag.Value {
+			if err = validateKeyringPrereqs(ctx, nil); err != nil {
+				return
+			}
+		}
+
 		protector, created, protErr := selectOrCreateProtector(ctx)
 		// Successfully created protector should be reverted on failure.
 		if protErr != nil {
@@ -173,12 +212,11 @@ func encryptPath(path string) (err error) {
 		if err = protector.Unlock(existingKeyFn); err != nil {
 			return
 		}
-		policy, err = actions.CreatePolicy(ctx, protector)
+		if policy, err = actions.CreatePolicy(ctx, protector); err != nil {
+			return
+		}
 	}
 	// Successfully created policy should be reverted on failure.
-	if err != nil {
-		return
-	}
 	defer func() {
 		policy.Lock()
 		if err != nil {
@@ -293,7 +331,7 @@ func unlockAction(c *cli.Context) error {
 		return expectedArgsErr(c, 1, false)
 	}
 
-	targetUser, err := parseUserFlag(true)
+	targetUser, err := parseUserFlag()
 	if err != nil {
 		return newExitError(c, err)
 	}
@@ -309,6 +347,10 @@ func unlockAction(c *cli.Context) error {
 	if err != nil {
 		return newExitError(c, err)
 	}
+	// Ensure the keyring is ready.
+	if err = validateKeyringPrereqs(ctx, policy); err != nil {
+		return newExitError(c, err)
+	}
 	// Check if directory is already unlocked
 	if policy.IsProvisioned() {
 		log.Printf("policy %s is already provisioned", policy.Descriptor())
@@ -370,7 +412,7 @@ func lockAction(c *cli.Context) error {
 		return expectedArgsErr(c, 1, false)
 	}
 
-	targetUser, err := parseUserFlag(true)
+	targetUser, err := parseUserFlag()
 	if err != nil {
 		return newExitError(c, err)
 	}
@@ -386,6 +428,10 @@ func lockAction(c *cli.Context) error {
 	if err != nil {
 		return newExitError(c, err)
 	}
+	// Ensure the keyring is ready.
+	if err = validateKeyringPrereqs(ctx, policy); err != nil {
+		return newExitError(c, err)
+	}
 	// Check if directory is already locked
 	if policy.IsFullyDeprovisioned() {
 		log.Printf("policy %s is already fully deprovisioned", policy.Descriptor())
@@ -459,7 +505,7 @@ func purgeAction(c *cli.Context) error {
 		}
 	}
 
-	targetUser, err := parseUserFlag(true)
+	targetUser, err := parseUserFlag()
 	if err != nil {
 		return newExitError(c, err)
 	}
@@ -468,6 +514,9 @@ func purgeAction(c *cli.Context) error {
 	if err != nil {
 		return newExitError(c, err)
 	}
+	if err = validateKeyringPrereqs(ctx, nil); err != nil {
+		return newExitError(c, err)
+	}
 
 	question := fmt.Sprintf("Purge all policy keys from %q", ctx.Mount.Path)
 	if dropCachesFlag.Value {
@@ -604,7 +653,7 @@ func createProtectorAction(c *cli.Context) error {
 		return expectedArgsErr(c, 1, false)
 	}
 
-	targetUser, err := parseUserFlag(false)
+	targetUser, err := parseUserFlag()
 	if err != nil {
 		return newExitError(c, err)
 	}
diff --git a/cmd/fscrypt/errors.go b/cmd/fscrypt/errors.go
index 68135fe..ac969f6 100644
--- a/cmd/fscrypt/errors.go
+++ b/cmd/fscrypt/errors.go
@@ -63,6 +63,7 @@ var (
 	ErrUnknownUser        = errors.New("unknown user")
 	ErrDropCachesPerm     = errors.New("inode cache can only be dropped as root")
 	ErrSpecifyUser        = errors.New("user must be specified when run as root")
+	ErrFsKeyringPerm      = errors.New("root is required to add/remove v1 encryption policy keys to/from filesystem")
 )
 
 var loadHelpText = fmt.Sprintf("You may need to mount a linked filesystem. Run with %s for more information.", shortDisplay(verboseFlag))
@@ -141,6 +142,10 @@ func getErrorSuggestions(err error) string {
 			properly clear the inode cache, or it should be run with
 			%s=false (this may leave encrypted files and directories
 			in an accessible state).`, shortDisplay(dropCachesFlag))
+	case ErrFsKeyringPerm:
+		return `Either this command should be run as root, or you should
+			set '"use_fs_keyring_for_v1_policies": false' in
+			/etc/fscrypt.conf.`
 	case ErrSpecifyUser:
 		return fmt.Sprintf(`When running this command as root, you
 			usually still want to provision/remove keys for a normal
diff --git a/cmd/fscrypt/flags.go b/cmd/fscrypt/flags.go
index 2eea8de..361732c 100644
--- a/cmd/fscrypt/flags.go
+++ b/cmd/fscrypt/flags.go
@@ -33,7 +33,6 @@ import (
 	"github.com/urfave/cli"
 
 	"github.com/google/fscrypt/actions"
-	"github.com/google/fscrypt/keyring"
 	"github.com/google/fscrypt/util"
 )
 
@@ -283,24 +282,10 @@ func getPolicyFromFlag(flagValue string, targetUser *user.User) (*actions.Policy
 }
 
 // parseUserFlag returns the user specified by userFlag or the current effective
-// user if the flag value is missing. If the effective user is root, however, a
-// user must specified in the flag. If checkKeyring is true, we also make sure
-// there are no problems accessing the user keyring.
-func parseUserFlag(checkKeyring bool) (targetUser *user.User, err error) {
+// user if the flag value is missing.
+func parseUserFlag() (targetUser *user.User, err error) {
 	if userFlag.Value != "" {
-		targetUser, err = user.Lookup(userFlag.Value)
-	} else {
-		if util.IsUserRoot() {
-			return nil, ErrSpecifyUser
-		}
-		targetUser, err = util.EffectiveUser()
+		return user.Lookup(userFlag.Value)
 	}
-	if err != nil {
-		return nil, err
-	}
-
-	if checkKeyring {
-		_, err = keyring.UserKeyringID(targetUser, true)
-	}
-	return targetUser, err
+	return util.EffectiveUser()
 }
diff --git a/cmd/fscrypt/protector.go b/cmd/fscrypt/protector.go
index 8cbcf03..25f1984 100644
--- a/cmd/fscrypt/protector.go
+++ b/cmd/fscrypt/protector.go
@@ -26,6 +26,7 @@ import (
 	"github.com/google/fscrypt/actions"
 	"github.com/google/fscrypt/filesystem"
 	"github.com/google/fscrypt/metadata"
+	"github.com/google/fscrypt/util"
 )
 
 // createProtector makes a new protector on either ctx.Mount or if the requested
@@ -37,6 +38,11 @@ func createProtectorFromContext(ctx *actions.Context) (*actions.Protector, error
 	}
 	log.Printf("using source: %s", ctx.Config.Source.String())
 
+	if ctx.Config.Source == metadata.SourceType_pam_passphrase &&
+		userFlag.Value == "" && util.IsUserRoot() {
+		return nil, ErrSpecifyUser
+	}
+
 	name, err := promptForName(ctx)
 	if err != nil {
 		return nil, err