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.

diff --git a/cmd/fscrypt/commands.go b/cmd/fscrypt/commands.go
index 8f2d21b4693dda82895d92f35849f3a5f11cd91b..621651efcc901e908d9dcd0b3399925cbcbb29fb 100644 (file)
--- 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 68135fe1dc901878465343cd760926b91c008b99..ac969f665ad93f01a9314d5186272308711be909 100644 (file)
--- 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 2eea8de5cc931ba903f323b167da4f01677df2b5..361732c4117700ac8491a204d0b3d2e8f4272c58 100644 (file)
--- 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 8cbcf03b298238a28716b1f0617c716770c119aa..25f198499259f54fc8bbd0215bf373273e5d5545 100644 (file)
--- 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