From 57be034ce4700fb07c10b771628c1c63d8483d09 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 23 Dec 2021 11:39:08 -0600 Subject: cli-tests: add helper functions to get protector descriptors --- cli-tests/common.sh | 34 ++++++++++++++++++++++++++++++++++ cli-tests/t_change_passphrase.sh | 2 +- cli-tests/t_encrypt_login.sh | 11 +++-------- 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/cli-tests/common.sh b/cli-tests/common.sh index 0ace2b9..1d7b17b 100644 --- a/cli-tests/common.sh +++ b/cli-tests/common.sh @@ -72,6 +72,40 @@ _get_enabled_fs_count() echo "$count" } +# Gets the descriptor of the given protector. +_get_protector_descriptor() +{ + local mnt=$1 + local source=$2 + + case $source in + custom) + local name=$3 + local description="custom protector \\\"$name\\\"" + ;; + login) + local user=$3 + local description="login protector for $user" + ;; + *) + _fail "Unknown protector source $source" + esac + + local descriptor + descriptor=$(fscrypt status "$mnt" | + awk -F ' *' '{ if ($3 == "'"$description"'") print $1 }') + if [ -z "$descriptor" ]; then + _fail "Can't find $description on $mnt" + fi + echo "$descriptor" +} + +# Gets the descriptor of the login protector for $TEST_USER. +_get_login_descriptor() +{ + _get_protector_descriptor "$MNT_ROOT" login "$TEST_USER" +} + # Prints the number of filesystems that have fscrypt metadata. _get_setup_fs_count() { diff --git a/cli-tests/t_change_passphrase.sh b/cli-tests/t_change_passphrase.sh index 204512d..1360bc2 100755 --- a/cli-tests/t_change_passphrase.sh +++ b/cli-tests/t_change_passphrase.sh @@ -14,7 +14,7 @@ echo pass1 | fscrypt encrypt --quiet --name=prot --skip-unlock "$dir" _print_header "Try to unlock with wrong passphrase" _expect_failure "echo pass2 | fscrypt unlock --quiet '$dir'" _expect_failure "mkdir '$dir/subdir'" -protector=$(fscrypt status "$dir" | awk '/custom protector/{print $1}') +protector=$(_get_protector_descriptor "$dir" custom prot) _print_header "Change passphrase" echo $'pass1\npass2' | \ diff --git a/cli-tests/t_encrypt_login.sh b/cli-tests/t_encrypt_login.sh index e03122d..c42fec7 100755 --- a/cli-tests/t_encrypt_login.sh +++ b/cli-tests/t_encrypt_login.sh @@ -27,18 +27,13 @@ show_status() fi } -get_login_protector() -{ - fscrypt status "$dir" | awk '/login protector/{print $1}' -} - begin "Encrypt with login protector" chown "$TEST_USER" "$dir" _user_do "echo TEST_USER_PASS | fscrypt encrypt --quiet --source=pam_passphrase '$dir'" show_status true recovery_passphrase=$(grep -E '^ +[a-z]{20}$' "$dir/fscrypt_recovery_readme.txt" | sed 's/^ +//') -recovery_protector=$(fscrypt status "$dir" | awk '/Recovery passphrase/{print $1}') -login_protector=$(get_login_protector) +recovery_protector=$(_get_protector_descriptor "$MNT" custom 'Recovery passphrase for dir') +login_protector=$(_get_login_descriptor) _print_header "=> Lock, then unlock with login passphrase" _user_do "fscrypt lock '$dir'" # FIXME: should we be able to use $MNT:$login_protector here? @@ -63,7 +58,7 @@ begin "Encrypt with login protector as root" echo TEST_USER_PASS | fscrypt encrypt --quiet --source=pam_passphrase --user="$TEST_USER" "$dir" show_status true # The newly-created login protector should be owned by the user, not root. -login_protector=$(get_login_protector) +login_protector=$(_get_login_descriptor) owner=$(stat -c "%U:%G" "$MNT_ROOT/.fscrypt/protectors/$login_protector") echo -e "\nProtector is owned by $owner" -- cgit v1.2.3 From 6ebd5a54eae2dfb16b66da649e75848fe6030b7f Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 23 Dec 2021 11:39:08 -0600 Subject: cmd/fscrypt: don't load protector in remove-protector-from-policy Make remove-protector-from-policy work even if the protector cannot be loaded (for example, due to having been deleted already). Fixes https://github.com/google/fscrypt/issues/258 Fixes https://github.com/google/fscrypt/issues/272 --- actions/policy.go | 13 +++++++------ actions/policy_test.go | 6 +++--- cli-tests/t_metadata.out | 17 +++++++++++++++++ cli-tests/t_metadata.sh | 36 ++++++++++++++++++++++++++++++++++++ cmd/fscrypt/commands.go | 13 +++++++------ 5 files changed, 70 insertions(+), 15 deletions(-) create mode 100644 cli-tests/t_metadata.out create mode 100755 cli-tests/t_metadata.sh diff --git a/actions/policy.go b/actions/policy.go index 6c48117..7204380 100644 --- a/actions/policy.go +++ b/actions/policy.go @@ -461,14 +461,15 @@ func (policy *Policy) AddProtector(protector *Protector) error { } // RemoveProtector updates the data that is wrapping the Policy Key so that the -// provided Protector is no longer protecting the specified Policy. If an error -// is returned, no data has been changed. Note that no protector links are +// protector with the given descriptor is no longer protecting the specified +// Policy. If an error is returned, no data has been changed. Note that the +// protector itself won't be removed, nor will a link to the protector be // removed (in the case where the protector and policy are on different -// filesystems). The policy and protector can be locked or unlocked. -func (policy *Policy) RemoveProtector(protector *Protector) error { - idx, ok := policy.findWrappedKeyIndex(protector.Descriptor()) +// filesystems). The policy can be locked or unlocked. +func (policy *Policy) RemoveProtector(protectorDescriptor string) error { + idx, ok := policy.findWrappedKeyIndex(protectorDescriptor) if !ok { - return &ErrNotProtected{policy.Descriptor(), protector.Descriptor()} + return &ErrNotProtected{policy.Descriptor(), protectorDescriptor} } if len(policy.data.WrappedPolicyKeys) == 1 { diff --git a/actions/policy_test.go b/actions/policy_test.go index 11c9c3e..07943b8 100644 --- a/actions/policy_test.go +++ b/actions/policy_test.go @@ -114,7 +114,7 @@ func TestPolicyGoodRemoveProtector(t *testing.T) { t.Fatal(err) } - err = pol.RemoveProtector(pro1) + err = pol.RemoveProtector(pro1.Descriptor()) if err != nil { t.Error(err) } @@ -135,11 +135,11 @@ func TestPolicyBadRemoveProtector(t *testing.T) { } defer cleanupProtector(pro2) - if pol.RemoveProtector(pro2) == nil { + if pol.RemoveProtector(pro2.Descriptor()) == nil { t.Error("we should not be able to remove a protector we did not add") } - if pol.RemoveProtector(pro1) == nil { + if pol.RemoveProtector(pro1.Descriptor()) == nil { t.Error("we should not be able to remove all the protectors from a policy") } } diff --git a/cli-tests/t_metadata.out b/cli-tests/t_metadata.out new file mode 100644 index 0000000..fba816a --- /dev/null +++ b/cli-tests/t_metadata.out @@ -0,0 +1,17 @@ +ext4 filesystem "MNT" has 3 protectors and 1 policy + +PROTECTOR LINKED DESCRIPTION +desc1 No custom protector "foo" +desc2 No custom protector "bar" +desc3 No custom protector "baz" + +POLICY UNLOCKED PROTECTORS +desc4 No desc1, desc2, desc3 +ext4 filesystem "MNT" has 2 protectors and 1 policy + +PROTECTOR LINKED DESCRIPTION +desc1 No custom protector "foo" +desc2 No custom protector "bar" + +POLICY UNLOCKED PROTECTORS +desc4 No desc1 diff --git a/cli-tests/t_metadata.sh b/cli-tests/t_metadata.sh new file mode 100755 index 0000000..e688eda --- /dev/null +++ b/cli-tests/t_metadata.sh @@ -0,0 +1,36 @@ +#!/bin/bash + +# Test 'fscrypt metadata'. + +cd "$(dirname "$0")" +. common.sh + +# Create three protectors, and a policy protected by them. +echo foo | fscrypt metadata create protector "$MNT" \ + --quiet --name=foo --source=custom_passphrase +echo bar | fscrypt metadata create protector "$MNT" \ + --quiet --name=bar --source=custom_passphrase +echo baz | fscrypt metadata create protector "$MNT" \ + --quiet --name=baz --source=custom_passphrase +prot_foo=$MNT:$(_get_protector_descriptor "$MNT" custom foo) +prot_bar=$MNT:$(_get_protector_descriptor "$MNT" custom bar) +desc_baz=$(_get_protector_descriptor "$MNT" custom baz) +prot_baz=$MNT:$desc_baz +echo foo | fscrypt metadata create policy "$MNT" --quiet \ + --protector="$prot_foo" +policy=$MNT:$(fscrypt status "$MNT" | grep -A10 "^POLICY" | \ + tail -1 | awk '{print $1}') +echo -e "bar\nfoo" | fscrypt metadata add-protector-to-policy --quiet \ + --policy="$policy" --protector="$prot_bar" +echo -e "baz\nfoo" | fscrypt metadata add-protector-to-policy --quiet \ + --policy="$policy" --protector="$prot_baz" --unlock-with="$prot_foo" +fscrypt status "$MNT" + +# Remove two of the protectors from the policy. +# Make sure that this works even if the protector was already deleted. +fscrypt metadata remove-protector-from-policy --quiet --force \ + --policy="$policy" --protector="$prot_bar" +rm "$MNT/.fscrypt/protectors/$desc_baz" +fscrypt metadata remove-protector-from-policy --quiet --force \ + --policy="$policy" --protector="$prot_baz" +fscrypt status "$MNT" diff --git a/cmd/fscrypt/commands.go b/cmd/fscrypt/commands.go index 0b382a6..023c0fa 100644 --- a/cmd/fscrypt/commands.go +++ b/cmd/fscrypt/commands.go @@ -1083,29 +1083,30 @@ func removeProtectorAction(c *cli.Context) error { return err } - // We do not need to unlock anything for this operation - protector, err := getProtectorFromFlag(protectorFlag.Value, nil) + // We only need the protector descriptor, not the protector itself. + ctx, protectorDescriptor, err := parseMetadataFlag(protectorFlag.Value, nil) if err != nil { return newExitError(c, err) } - policy, err := getPolicyFromFlag(policyFlag.Value, protector.Context.TargetUser) + // We don't need to unlock the policy for this operation. + policy, err := getPolicyFromFlag(policyFlag.Value, ctx.TargetUser) if err != nil { return newExitError(c, err) } prompt := fmt.Sprintf("Stop protecting policy %s with protector %s?", - policy.Descriptor(), protector.Descriptor()) + policy.Descriptor(), protectorDescriptor) warning := "All files using this policy will NO LONGER be accessible with this protector!!" if err := askConfirmation(prompt, false, warning); err != nil { return newExitError(c, err) } - if err := policy.RemoveProtector(protector); err != nil { + if err := policy.RemoveProtector(protectorDescriptor); err != nil { return newExitError(c, err) } fmt.Fprintf(c.App.Writer, "Protector %s no longer protecting policy %s.\n", - protector.Descriptor(), policy.Descriptor()) + protectorDescriptor, policy.Descriptor()) return nil } -- cgit v1.2.3