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