diff --git a/actions/policy.go b/actions/policy.go index 3baad726..6c2aa510 100644 --- a/actions/policy.go +++ b/actions/policy.go @@ -417,12 +417,6 @@ func (policy *Policy) IsProvisionedByTargetUser() bool { return policy.GetProvisioningStatus() == keyring.KeyPresent } -// IsFullyDeprovisioned returns true if the policy has been fully deprovisioned, -// including by all users and with all files protected by it having been closed. -func (policy *Policy) IsFullyDeprovisioned() bool { - return policy.GetProvisioningStatus() == keyring.KeyAbsent -} - // Provision inserts the Policy key into the kernel keyring. This allows reading // and writing of files encrypted with this directory. Requires unlocked Policy. func (policy *Policy) Provision() error { @@ -435,14 +429,15 @@ func (policy *Policy) Provision() error { // Deprovision removes the Policy key from the kernel keyring. This prevents // reading and writing to the directory --- unless the target keyring is a user -// keyring, in which case caches must be dropped too. +// keyring, in which case caches must be dropped too. If the Policy key was +// already removed, returns keyring.ErrKeyNotPresent. func (policy *Policy) Deprovision(allUsers bool) error { return keyring.RemoveEncryptionKey(policy.Descriptor(), policy.Context.getKeyringOptions(), allUsers) } // NeedsUserKeyring returns true if Provision and Deprovision for this policy -// will use a user keyring, not a filesystem keyring. +// will use a user keyring (deprecated), not a filesystem keyring. func (policy *Policy) NeedsUserKeyring() bool { return policy.Version() == 1 && !policy.Context.Config.GetUseFsKeyringForV1Policies() } diff --git a/cli-tests/t_v1_policy.out b/cli-tests/t_v1_policy.out index 747cf81d..0ff52196 100644 --- a/cli-tests/t_v1_policy.out +++ b/cli-tests/t_v1_policy.out @@ -96,3 +96,42 @@ Protected with 1 protector: PROTECTOR LINKED DESCRIPTION desc2 No custom protector "prot" cat: MNT/dir/file: No such file or directory + +# Testing incompletely locking v1-encrypted directory +Enter custom passphrase for protector "prot": "MNT/dir" is now unlocked and ready for use. +Encrypted data removed from filesystem cache. +[ERROR] fscrypt lock: some files using the key are still open + +Directory was incompletely locked because some files are still open. These files +remain accessible. Try killing any processes using files in the directory, then +re-running 'fscrypt lock'. +"MNT/dir" is encrypted with fscrypt. + +Policy: desc1 +Options: padding:32 contents:AES_256_XTS filenames:AES_256_CTS policy_version:1 +Unlocked: Partially (incompletely locked) + +Protected with 1 protector: +PROTECTOR LINKED DESCRIPTION +desc2 No custom protector "prot" +ext4 filesystem "MNT" has 1 protector and 1 policy + +PROTECTOR LINKED DESCRIPTION +desc2 No custom protector "prot" + +POLICY UNLOCKED PROTECTORS +desc1 No desc2 + +# Finishing locking v1-encrypted directory +Encrypted data removed from filesystem cache. +"MNT/dir" is now locked. +"MNT/dir" is encrypted with fscrypt. + +Policy: desc1 +Options: padding:32 contents:AES_256_XTS filenames:AES_256_CTS policy_version:1 +Unlocked: No + +Protected with 1 protector: +PROTECTOR LINKED DESCRIPTION +desc2 No custom protector "prot" +cat: MNT/dir/file: No such file or directory diff --git a/cli-tests/t_v1_policy.sh b/cli-tests/t_v1_policy.sh index 1ebfae55..e9f3acf8 100755 --- a/cli-tests/t_v1_policy.sh +++ b/cli-tests/t_v1_policy.sh @@ -54,3 +54,18 @@ _print_header "Lock v1-encrypted directory" fscrypt lock "$dir" --user="$TEST_USER" _user_do "fscrypt status '$dir'" _expect_failure "cat '$dir/file'" + +# 'fscrypt lock' and 'fscrypt status' implement a heuristic that should detect +# the "files busy" case with v1. +_print_header "Testing incompletely locking v1-encrypted directory" +_user_do "echo hunter2 | fscrypt unlock '$dir'" +exec 3<"$dir/file" +_expect_failure "fscrypt lock '$dir' --user='$TEST_USER'" +_user_do "fscrypt status '$dir'" +# ... except in this case, because we can't detect it without a directory path. +_user_do "fscrypt status '$MNT'" +exec 3<&- +_print_header "Finishing locking v1-encrypted directory" +fscrypt lock "$dir" --user="$TEST_USER" +_user_do "fscrypt status '$dir'" +_expect_failure "cat '$dir/file'" diff --git a/cmd/fscrypt/commands.go b/cmd/fscrypt/commands.go index ec755846..51cf1362 100644 --- a/cmd/fscrypt/commands.go +++ b/cmd/fscrypt/commands.go @@ -496,30 +496,55 @@ func lockAction(c *cli.Context) error { 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()) - return newExitError(c, errors.Wrapf(ErrPolicyLocked, path)) - } - // Check for permission to drop caches, if it will be needed. + // Check for permission to drop caches, if it may be needed. if policy.NeedsUserKeyring() && dropCachesFlag.Value && !util.IsUserRoot() { return newExitError(c, ErrDropCachesPerm) } if err = policy.Deprovision(allUsersFlag.Value); err != nil { - return newExitError(c, err) + if err != keyring.ErrKeyNotPresent { + return newExitError(c, err) + } + // Key is no longer present. Normally that means the directory + // is already locked; in that case we exit with an error. But + // if the policy uses the user keyring (v1 policies only), then + // the directory might have been incompletely locked earlier, + // due to open files. Try to detect that case and finish + // locking the directory by dropping caches again. + if !policy.NeedsUserKeyring() || !isDirUnlockedHeuristic(path) { + log.Printf("policy %s is already fully deprovisioned", policy.Descriptor()) + return newExitError(c, errors.Wrapf(ErrPolicyLocked, path)) + } } if policy.NeedsUserKeyring() { if err = dropCachesIfRequested(c, ctx); err != nil { return newExitError(c, err) } + if isDirUnlockedHeuristic(path) { + return newExitError(c, keyring.ErrKeyFilesOpen) + } } fmt.Fprintf(c.App.Writer, "%q is now locked.\n", path) return nil } +// isDirUnlockedHeuristic returns true if we can create a subdirectory of the +// given directory and therefore it is definitely still unlocked. It returns +// false if the directory is probably locked (though it could also be unlocked). +// +// This is only useful if the directory's policy uses the user keyring, since +// otherwise the status can be easily found via the filesystem keyring. +func isDirUnlockedHeuristic(dirPath string) bool { + subdirPath := filepath.Join(dirPath, "fscrypt-is-dir-unlocked") + if err := os.Mkdir(subdirPath, 0700); err == nil { + os.Remove(subdirPath) + return true + } + return false +} + // Purge removes all the policy keys from the keyring (also need unmount). var Purge = cli.Command{ Name: "purge", diff --git a/cmd/fscrypt/status.go b/cmd/fscrypt/status.go index bf11495c..40bb49e8 100644 --- a/cmd/fscrypt/status.go +++ b/cmd/fscrypt/status.go @@ -66,8 +66,20 @@ func yesNoString(b bool) string { return "No" } -func policyUnlockedStatus(policy *actions.Policy) string { - switch policy.GetProvisioningStatus() { +func policyUnlockedStatus(policy *actions.Policy, path string) string { + status := policy.GetProvisioningStatus() + + // Due to a limitation in the old kernel API for fscrypt, for v1 + // policies using the user keyring that are incompletely locked we'll + // get KeyAbsent, not KeyAbsentButFilesBusy as expected. If we have a + // directory path, use a heuristic to try to detect whether it is still + // usable and thus the policy is actually incompletely locked. + if status == keyring.KeyAbsent && policy.NeedsUserKeyring() && + path != "" && isDirUnlockedHeuristic(path) { + status = keyring.KeyAbsentButFilesBusy + } + + switch status { case keyring.KeyPresent, keyring.KeyPresentButOnlyOtherUsers: return "Yes" case keyring.KeyAbsent: @@ -174,7 +186,8 @@ func writeFilesystemStatus(w io.Writer, ctx *actions.Context) error { continue } - fmt.Fprintf(t, "%s\t%s\t%s\n", descriptor, policyUnlockedStatus(policy), + fmt.Fprintf(t, "%s\t%s\t%s\n", descriptor, + policyUnlockedStatus(policy, ""), strings.Join(policy.ProtectorDescriptors(), ", ")) } return t.Flush() @@ -194,7 +207,7 @@ func writePathStatus(w io.Writer, path string) error { fmt.Fprintln(w) fmt.Fprintf(w, "Policy: %s\n", policy.Descriptor()) fmt.Fprintf(w, "Options: %s\n", policy.Options()) - fmt.Fprintf(w, "Unlocked: %s\n", policyUnlockedStatus(policy)) + fmt.Fprintf(w, "Unlocked: %s\n", policyUnlockedStatus(policy, path)) fmt.Fprintln(w) options := policy.ProtectorOptions() diff --git a/keyring/keyring.go b/keyring/keyring.go index 66239431..fb9cc0e1 100644 --- a/keyring/keyring.go +++ b/keyring/keyring.go @@ -173,7 +173,7 @@ func GetEncryptionKeyStatus(descriptor string, options *Options) (KeyStatus, err if useFsKeyring { return fsGetEncryptionKeyStatus(descriptor, options.Mount, options.User) } - _, err = userFindKey(buildKeyDescription(options, descriptor), options.User) + _, _, err = userFindKey(buildKeyDescription(options, descriptor), options.User) if err != nil { return KeyAbsent, nil } diff --git a/keyring/user_keyring.go b/keyring/user_keyring.go index 71f519d7..beeb36d4 100644 --- a/keyring/user_keyring.go +++ b/keyring/user_keyring.go @@ -78,39 +78,37 @@ func userRemoveKey(description string, targetUser *user.User) error { runtime.LockOSThread() // ensure target user keyring remains possessed in thread keyring defer runtime.UnlockOSThread() - keyID, err := userFindKey(description, targetUser) + keyID, keyringID, err := userFindKey(description, targetUser) if err != nil { return ErrKeyNotPresent } - // We use KEYCTL_INVALIDATE instead of KEYCTL_REVOKE because - // invalidating a key immediately removes it. - _, err = unix.KeyctlInt(unix.KEYCTL_INVALIDATE, keyID, 0, 0, 0) - log.Printf("KeyctlInvalidate(%d) = %v", keyID, err) + _, err = unix.KeyctlInt(unix.KEYCTL_UNLINK, keyID, keyringID, 0, 0) + log.Printf("KeyctlUnlink(%d, %d) = %v", keyID, keyringID, err) if err != nil { return errors.Wrap(ErrKeyRemove, err.Error()) } return nil } -// userFindKey tries to locate a key in the user keyring with the provided -// description. The key ID is returned if we can find the key. An error is -// returned if the key does not exist. -func userFindKey(description string, targetUser *user.User) (int, error) { +// userFindKey tries to locate a key with the provided description in the user +// keyring for the target user. The key ID and keyring ID are returned if we can +// find the key. An error is returned if the key does not exist. +func userFindKey(description string, targetUser *user.User) (int, int, error) { runtime.LockOSThread() // ensure target user keyring remains possessed in thread keyring defer runtime.UnlockOSThread() keyringID, err := UserKeyringID(targetUser, false) if err != nil { - return 0, err + return 0, 0, err } keyID, err := unix.KeyctlSearch(keyringID, KeyType, description, 0) log.Printf("KeyctlSearch(%d, %s, %s) = %d, %v", keyringID, KeyType, description, keyID, err) if err != nil { - return 0, errors.Wrap(ErrKeySearch, err.Error()) + return 0, 0, errors.Wrap(ErrKeySearch, err.Error()) } - return keyID, err + return keyID, keyringID, err } // UserKeyringID returns the key id of the target user's user keyring. We also