From c39fc85f8045bb24f773a3eb5dee7738cdc4339f Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sat, 13 Jun 2020 10:06:15 -0700 Subject: [PATCH 1/2] cli-tests/t_v1_policy: clean up user keyrings at end of test The test user's user keyring is still linked into root's user keyring at the end of the test. This is making the test flaky, as there is a failure that only occurs the first time it is run. Fix the test to restore the initial state. This makes it consistently fail (to be fixed by the next commit). --- cli-tests/common.sh | 12 ++++++++++++ cli-tests/t_v1_policy.sh | 1 + 2 files changed, 13 insertions(+) diff --git a/cli-tests/common.sh b/cli-tests/common.sh index fcebfd65..79b42ae7 100644 --- a/cli-tests/common.sh +++ b/cli-tests/common.sh @@ -128,6 +128,18 @@ _user_do_and_expect_failure() _expect_failure "_user_do '$1'" } +# Clear the test user's user keyring and unlink it from root's user keyring, if +# it is linked into it. +_cleanup_user_keyrings() +{ + local ringid + + ringid=$(_user_do "keyctl show @u" | awk '/keyring: _uid/{print $1}') + + _user_do "keyctl clear $ringid" + keyctl unlink "$ringid" @u &> /dev/null || true +} + # Gives the test a new session keyring which contains the test user's keyring # but not root's keyring. Also clears the test user's keyring. This must be # called at the beginning of the test script as it may re-execute the script. diff --git a/cli-tests/t_v1_policy.sh b/cli-tests/t_v1_policy.sh index e9f3acf8..e883dcd1 100755 --- a/cli-tests/t_v1_policy.sh +++ b/cli-tests/t_v1_policy.sh @@ -6,6 +6,7 @@ cd "$(dirname "$0")" . common.sh _setup_session_keyring +trap _cleanup_user_keyrings EXIT dir="$MNT/dir" mkdir "$dir" From 5c1f617c647eb0e9af5ce57758fa58f7e3f4db83 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sat, 13 Jun 2020 10:06:15 -0700 Subject: [PATCH 2/2] cmd/fscrypt: adjust status message for v1-encrypted dirs When 'fscrypt status DIR' detects that a v1-encrypted directory is still usable but its key seems to be absent, it shows the status as "Unlocked: Partially (incompletely locked)". But actually it can also be the case that the directory is unlocked by another user. Adjust the status message accordingly. This commit also fixes cli-tests/t_v1_policy. --- cli-tests/t_v1_policy.out | 4 ++-- cmd/fscrypt/status.go | 9 ++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/cli-tests/t_v1_policy.out b/cli-tests/t_v1_policy.out index b47bcca2..9adb00ae 100644 --- a/cli-tests/t_v1_policy.out +++ b/cli-tests/t_v1_policy.out @@ -42,7 +42,7 @@ desc2 No custom protector "prot" Policy: desc1 Options: padding:32 contents:AES_256_XTS filenames:AES_256_CTS policy_version:1 -Unlocked: Yes +Unlocked: Partially (incompletely locked, or unlocked by another user) Protected with 1 protector: PROTECTOR LINKED DESCRIPTION @@ -115,7 +115,7 @@ Then re-run: Policy: desc1 Options: padding:32 contents:AES_256_XTS filenames:AES_256_CTS policy_version:1 -Unlocked: Partially (incompletely locked) +Unlocked: Partially (incompletely locked, or unlocked by another user) Protected with 1 protector: PROTECTOR LINKED DESCRIPTION diff --git a/cmd/fscrypt/status.go b/cmd/fscrypt/status.go index 02fdc74f..255bb2be 100644 --- a/cmd/fscrypt/status.go +++ b/cmd/fscrypt/status.go @@ -68,13 +68,12 @@ 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. + // policies using the user keyring that are incompletely locked or are + // unlocked by another user, we'll get KeyAbsent. If we have a + // directory path, use a heuristic to try to detect these cases. if status == keyring.KeyAbsent && policy.NeedsUserKeyring() && path != "" && isDirUnlockedHeuristic(path) { - status = keyring.KeyAbsentButFilesBusy + return "Partially (incompletely locked, or unlocked by another user)" } switch status {