diff options
40 files changed, 1270 insertions, 302 deletions
@@ -37,6 +37,7 @@ dependencies](#runtime-dependencies). - [Building and installing](#building-and-installing) - [Runtime dependencies](#runtime-dependencies) - [Configuration file](#configuration-file) +- [Setting up `fscrypt` on a filesystem](#setting-up-fscrypt-on-a-filesystem) - [Setting up for login protectors](#setting-up-for-login-protectors) - [Securing your login passphrase](#securing-your-login-passphrase) - [Enabling the PAM module](#enabling-the-pam-module) @@ -319,7 +320,8 @@ that looks like the following: "filenames": "AES_256_CTS", "policy_version": "2" }, - "use_fs_keyring_for_v1_policies": false + "use_fs_keyring_for_v1_policies": false, + "allow_cross_user_metadata": false } ``` @@ -377,6 +379,54 @@ The fields are: kernels, it's better to not use this setting and instead (re-)create your encrypted directories with `"policy_version": "2"`. +* "allow\_cross\_user\_metadata" specifies whether `fscrypt` will allow + protectors and policies from other non-root users to be read, e.g. to be + offered as options by `fscrypt encrypt`. The default value is `false`, since + other users might be untrusted and could create malicious files. This can be + set to `true` to restore the old behavior on systems where `fscrypt` metadata + needs to be shared between multiple users. Note that this option is + independent from the permissions on the metadata files themselves, which are + set to 0600 by default; users who wish to share their metadata files with + other users would also need to explicitly change their mode to 0644. + +## Setting up `fscrypt` on a filesystem + +`fscrypt` needs some directories to exist on the filesystem on which encryption +will be used: + +* `MOUNTPOINT/.fscrypt/policies` +* `MOUNTPOINT/.fscrypt/protectors` + +(If login protectors are used, these must also exist on the root filesystem.) + +To create these directories, run `fscrypt setup MOUNTPOINT`. If MOUNTPOINT is +owned by root, as is usually the case, then this command will require root. + +There will be one decision you'll need to make: whether non-root users will be +allowed to create `fscrypt` metadata (policies and protectors). + +If you say `y`, then these directories will be made world-writable, with the +sticky bit set so that users can't delete each other's files -- just like +`/tmp`. If you say `N`, then these directories will be writable only by root. + +Saying `y` maximizes the usability of `fscrypt`, and on most systems it's fine +to say `y`. However, on some systems this may be inappropriate, as it will +allow malicious users to fill the entire filesystem unless filesystem quotas +have been configured -- similar to problems that have historically existed with +other world-writable directories, e.g. `/tmp`. If you are concerned about this, +say `N`. If you say `N`, then you'll only be able to run `fscrypt` as root to +set up encryption on users' behalf, unless you manually set custom permissions +on the metadata directories to grant write access to specific users or groups. + +If you chose the wrong mode at `fscrypt setup` time, you can change the +directory permissions at any time. To enable single-user writable mode, run: + + sudo chmod 0755 MOUNTPOINT/.fscrypt/* + +To enable world-writable mode, run: + + sudo chmod 1777 MOUNTPOINT/.fscrypt/* + ## Setting up for login protectors If you want any encrypted directories to be protected by your login passphrase, @@ -646,11 +696,15 @@ MOUNTPOINT DEVICE FILESYSTEM ENCRYPTION FSCRYPT Defaulting to policy_version 2 because kernel supports it. Customizing passphrase hashing difficulty for this system... Created global config file at "/etc/fscrypt.conf". -Metadata directories created at "/.fscrypt". +Allow users other than root to create fscrypt metadata on the root filesystem? +(See https://github.com/google/fscrypt#setting-up-fscrypt-on-a-filesystem) [y/N] y +Metadata directories created at "/.fscrypt", writable by everyone. # Start using fscrypt with our filesystem ->>>>> fscrypt setup /mnt/disk -Metadata directories created at "/mnt/disk/.fscrypt". +>>>>> sudo fscrypt setup /mnt/disk +Allow users other than root to create fscrypt metadata on this filesystem? (See +https://github.com/google/fscrypt#setting-up-fscrypt-on-a-filesystem) [y/N] y +Metadata directories created at "/mnt/disk/.fscrypt", writable by everyone. # Initialize encryption on a new empty directory >>>>> mkdir /mnt/disk/dir1 @@ -678,8 +732,8 @@ POLICY UNLOCKED PROTECTORS #### Quiet version ```bash ->>>>> sudo fscrypt setup --quiet --force ->>>>> fscrypt setup /mnt/disk --quiet +>>>>> sudo fscrypt setup --quiet --force --all-users +>>>>> sudo fscrypt setup /mnt/disk --quiet --all-users >>>>> echo "hunter2" | fscrypt encrypt /mnt/disk/dir1 --quiet --source=custom_passphrase --name="Super Secret" ``` diff --git a/actions/context.go b/actions/context.go index 26295ec..ac3f6d3 100644 --- a/actions/context.go +++ b/actions/context.go @@ -58,6 +58,12 @@ type Context struct { // the user for whom the keys are claimed in the filesystem keyring when // v2 policies are provisioned. TargetUser *user.User + // TrustedUser is the user for whom policies and protectors are allowed + // to be read. Specifically, if TrustedUser is set, then only + // policies and protectors owned by TrustedUser or by root will be + // allowed to be read. If it's nil, then all policies and protectors + // the process has filesystem-level read access to will be allowed. + TrustedUser *user.User } // NewContextFromPath makes a context for the filesystem containing the @@ -112,6 +118,16 @@ func newContextFromUser(targetUser *user.User) (*Context, error) { return nil, err } + // By default, when running as a non-root user we only read policies and + // protectors owned by the user or root. When running as root, we allow + // reading all policies and protectors. + if !ctx.Config.GetAllowCrossUserMetadata() && !util.IsUserRoot() { + ctx.TrustedUser, err = util.EffectiveUser() + if err != nil { + return nil, err + } + } + log.Printf("creating context for user %q", targetUser.Username) return ctx, nil } @@ -122,7 +138,7 @@ func (ctx *Context) checkContext() error { if err := ctx.Config.CheckValidity(); err != nil { return &ErrBadConfig{ctx.Config, err} } - return ctx.Mount.CheckSetup() + return ctx.Mount.CheckSetup(ctx.TrustedUser) } func (ctx *Context) getKeyringOptions() *keyring.Options { @@ -136,7 +152,7 @@ func (ctx *Context) getKeyringOptions() *keyring.Options { // getProtectorOption returns the ProtectorOption for the protector on the // context's mountpoint with the specified descriptor. func (ctx *Context) getProtectorOption(protectorDescriptor string) *ProtectorOption { - mnt, data, err := ctx.Mount.GetProtector(protectorDescriptor) + mnt, data, err := ctx.Mount.GetProtector(protectorDescriptor, ctx.TrustedUser) if err != nil { return &ProtectorOption{ProtectorInfo{}, nil, err} } @@ -155,7 +171,7 @@ func (ctx *Context) ProtectorOptions() ([]*ProtectorOption, error) { if err := ctx.checkContext(); err != nil { return nil, err } - descriptors, err := ctx.Mount.ListProtectors() + descriptors, err := ctx.Mount.ListProtectors(ctx.TrustedUser) if err != nil { return nil, err } diff --git a/actions/context_test.go b/actions/context_test.go index 7b56d92..6e28857 100644 --- a/actions/context_test.go +++ b/actions/context_test.go @@ -27,6 +27,7 @@ import ( "testing" "time" + "github.com/google/fscrypt/filesystem" "github.com/google/fscrypt/util" "github.com/pkg/errors" ) @@ -67,7 +68,7 @@ func setupContext() (ctx *Context, err error) { return nil, err } - return ctx, ctx.Mount.Setup() + return ctx, ctx.Mount.Setup(filesystem.WorldWritable) } // Cleans up the testing config file and testing filesystem data. diff --git a/actions/policy.go b/actions/policy.go index 7204380..3b20176 100644 --- a/actions/policy.go +++ b/actions/policy.go @@ -23,6 +23,7 @@ import ( "fmt" "log" "os" + "os/user" "github.com/golang/protobuf/proto" "github.com/pkg/errors" @@ -145,7 +146,7 @@ func PurgeAllPolicies(ctx *Context) error { if err := ctx.checkContext(); err != nil { return err } - policies, err := ctx.Mount.ListPolicies() + policies, err := ctx.Mount.ListPolicies(nil) if err != nil { return err } @@ -178,6 +179,7 @@ type Policy struct { data *metadata.PolicyData key *crypto.Key created bool + ownerIfCreating *user.User newLinkedProtectors []string } @@ -210,6 +212,12 @@ func CreatePolicy(ctx *Context, protector *Protector) (*Policy, error) { created: true, } + policy.ownerIfCreating, err = getOwnerOfMetadataForProtector(protector) + if err != nil { + policy.Lock() + return nil, err + } + if err = policy.AddProtector(protector); err != nil { policy.Lock() return nil, err @@ -225,7 +233,7 @@ func GetPolicy(ctx *Context, descriptor string) (*Policy, error) { if err := ctx.checkContext(); err != nil { return nil, err } - data, err := ctx.Mount.GetPolicy(descriptor) + data, err := ctx.Mount.GetPolicy(descriptor, ctx.TrustedUser) if err != nil { return nil, err } @@ -262,7 +270,7 @@ func GetPolicyFromPath(ctx *Context, path string) (*Policy, error) { descriptor := pathData.KeyDescriptor log.Printf("found policy %s for %q", descriptor, path) - mountData, err := ctx.Mount.GetPolicy(descriptor) + mountData, err := ctx.Mount.GetPolicy(descriptor, ctx.TrustedUser) if err != nil { log.Printf("getting policy metadata: %v", err) if _, ok := err.(*filesystem.ErrPolicyNotFound); ok { @@ -410,6 +418,25 @@ func (policy *Policy) UsesProtector(protector *Protector) bool { return ok } +// getOwnerOfMetadataForProtector returns the User to whom the owner of any new +// policies or protector links for the given protector should be set. +// +// This will return a non-nil value only when the protector is a login protector +// and the process is running as root. In this scenario, root is setting up +// encryption on the user's behalf, so we need to make new policies and +// protector links owned by the user (rather than root) to allow them to be read +// by the user, just like the login protector itself which is handled elsewhere. +func getOwnerOfMetadataForProtector(protector *Protector) (*user.User, error) { + if protector.data.Source == metadata.SourceType_pam_passphrase && util.IsUserRoot() { + owner, err := util.UserFromUID(protector.data.Uid) + if err != nil { + return nil, err + } + return owner, nil + } + return nil, nil +} + // AddProtector updates the data that is wrapping the Policy Key so that the // provided Protector is now protecting the specified Policy. If an error is // returned, no data has been changed. If the policy and protector are on @@ -427,8 +454,13 @@ func (policy *Policy) AddProtector(protector *Protector) error { // to it on the policy's filesystem. if policy.Context.Mount != protector.Context.Mount { log.Printf("policy on %s\n protector on %s\n", policy.Context.Mount, protector.Context.Mount) + ownerIfCreating, err := getOwnerOfMetadataForProtector(protector) + if err != nil { + return err + } isNewLink, err := policy.Context.Mount.AddLinkedProtector( - protector.Descriptor(), protector.Context.Mount) + protector.Descriptor(), protector.Context.Mount, + protector.Context.TrustedUser, ownerIfCreating) if err != nil { return err } @@ -554,7 +586,7 @@ func (policy *Policy) CanBeAppliedWithoutProvisioning() bool { // commitData writes the Policy's current data to the filesystem. func (policy *Policy) commitData() error { - return policy.Context.Mount.AddPolicy(policy.data) + return policy.Context.Mount.AddPolicy(policy.data, policy.ownerIfCreating) } // findWrappedPolicyKey returns the index of the wrapped policy key diff --git a/actions/policy_test.go b/actions/policy_test.go index 07943b8..8248862 100644 --- a/actions/policy_test.go +++ b/actions/policy_test.go @@ -27,7 +27,7 @@ import ( // Makes a protector and policy func makeBoth() (*Protector, *Policy, error) { - protector, err := CreateProtector(testContext, testProtectorName, goodCallback) + protector, err := CreateProtector(testContext, testProtectorName, goodCallback, nil) if err != nil { return nil, nil, err } @@ -68,7 +68,7 @@ func TestPolicyGoodAddProtector(t *testing.T) { t.Fatal(err) } - pro2, err := CreateProtector(testContext, testProtectorName2, goodCallback) + pro2, err := CreateProtector(testContext, testProtectorName2, goodCallback, nil) if err != nil { t.Fatal(err) } @@ -103,7 +103,7 @@ func TestPolicyGoodRemoveProtector(t *testing.T) { t.Fatal(err) } - pro2, err := CreateProtector(testContext, testProtectorName2, goodCallback) + pro2, err := CreateProtector(testContext, testProtectorName2, goodCallback, nil) if err != nil { t.Fatal(err) } @@ -129,7 +129,7 @@ func TestPolicyBadRemoveProtector(t *testing.T) { t.Fatal(err) } - pro2, err := CreateProtector(testContext, testProtectorName2, goodCallback) + pro2, err := CreateProtector(testContext, testProtectorName2, goodCallback, nil) if err != nil { t.Fatal(err) } diff --git a/actions/protector.go b/actions/protector.go index 3278e63..b986eb0 100644 --- a/actions/protector.go +++ b/actions/protector.go @@ -109,17 +109,18 @@ func checkIfUserHasLoginProtector(ctx *Context, uid int64) error { // to unlock policies and create new polices. As with the key struct, a // Protector should be wiped after use. type Protector struct { - Context *Context - data *metadata.ProtectorData - key *crypto.Key - created bool + Context *Context + data *metadata.ProtectorData + key *crypto.Key + created bool + ownerIfCreating *user.User } // CreateProtector creates an unlocked protector with a given name (name only // needed for custom and raw protector types). The keyFn provided to create the // Protector key will only be called once. If an error is returned, no data has // been changed on the filesystem. -func CreateProtector(ctx *Context, name string, keyFn KeyFunc) (*Protector, error) { +func CreateProtector(ctx *Context, name string, keyFn KeyFunc, owner *user.User) (*Protector, error) { if err := ctx.checkContext(); err != nil { return nil, err } @@ -147,7 +148,8 @@ func CreateProtector(ctx *Context, name string, keyFn KeyFunc) (*Protector, erro Name: name, Source: ctx.Config.Source, }, - created: true, + created: true, + ownerIfCreating: owner, } // Extra data is needed for some SourceTypes @@ -199,7 +201,7 @@ func GetProtector(ctx *Context, descriptor string) (*Protector, error) { } protector := &Protector{Context: ctx} - protector.data, err = ctx.Mount.GetRegularProtector(descriptor) + protector.data, err = ctx.Mount.GetRegularProtector(descriptor, ctx.TrustedUser) return protector, err } @@ -218,7 +220,7 @@ func GetProtectorFromOption(ctx *Context, option *ProtectorOption) (*Protector, // Replace the context if this is a linked protector if option.LinkedMount != nil { - ctx = &Context{ctx.Config, option.LinkedMount, ctx.TargetUser} + ctx = &Context{ctx.Config, option.LinkedMount, ctx.TargetUser, ctx.TrustedUser} } return &Protector{Context: ctx, data: option.data}, nil } @@ -294,5 +296,5 @@ func (protector *Protector) Rewrap(keyFn KeyFunc) error { return err } - return protector.Context.Mount.AddProtector(protector.data) + return protector.Context.Mount.AddProtector(protector.data, protector.ownerIfCreating) } diff --git a/actions/protector_test.go b/actions/protector_test.go index 6c81d49..f20dbcf 100644 --- a/actions/protector_test.go +++ b/actions/protector_test.go @@ -43,7 +43,7 @@ func badCallback(info ProtectorInfo, retry bool) (*crypto.Key, error) { // Tests that we can create a valid protector. func TestCreateProtector(t *testing.T) { - p, err := CreateProtector(testContext, testProtectorName, goodCallback) + p, err := CreateProtector(testContext, testProtectorName, goodCallback, nil) if err != nil { t.Error(err) } else { @@ -54,7 +54,7 @@ func TestCreateProtector(t *testing.T) { // Tests that a failure in the callback is relayed back to the caller. func TestBadCallback(t *testing.T) { - p, err := CreateProtector(testContext, testProtectorName, badCallback) + p, err := CreateProtector(testContext, testProtectorName, badCallback, nil) if err == nil { p.Lock() p.Destroy() diff --git a/actions/recovery.go b/actions/recovery.go index f533906..8a769cc 100644 --- a/actions/recovery.go +++ b/actions/recovery.go @@ -25,6 +25,7 @@ import ( "github.com/google/fscrypt/crypto" "github.com/google/fscrypt/metadata" + "github.com/google/fscrypt/util" ) // modifiedContextWithSource returns a copy of ctx with the protector source @@ -66,7 +67,7 @@ func AddRecoveryPassphrase(policy *Policy, dirname string) (*crypto.Key, *Protec if seq != 1 { name += " (" + strconv.Itoa(seq) + ")" } - recoveryProtector, err = CreateProtector(customCtx, name, getPassphraseFn) + recoveryProtector, err = CreateProtector(customCtx, name, getPassphraseFn, policy.ownerIfCreating) if err == nil { break } @@ -121,5 +122,10 @@ It is safe to keep it around though, as the recovery passphrase is high-entropy. if _, err = file.WriteString(str); err != nil { return err } + if recoveryProtector.ownerIfCreating != nil { + if err = util.Chown(file, recoveryProtector.ownerIfCreating); err != nil { + return err + } + } return file.Sync() } diff --git a/cli-tests/run.sh b/cli-tests/run.sh index dc17b5b..f6a4868 100755 --- a/cli-tests/run.sh +++ b/cli-tests/run.sh @@ -159,7 +159,7 @@ setup_for_test() # Give the tests their own fscrypt.conf. export FSCRYPT_CONF="$TMPDIR/fscrypt.conf" - fscrypt setup --time=1ms > /dev/null + fscrypt setup --time=1ms --quiet --all-users > /dev/null # The tests assume kernel support for v2 policies. if ! grep -q '"policy_version": "2"' "$FSCRYPT_CONF"; then @@ -171,7 +171,7 @@ EOF fi # Set up the test filesystems that aren't already set up. - fscrypt setup "$MNT" > /dev/null + fscrypt setup --quiet --all-users "$MNT" > /dev/null } run_test() diff --git a/cli-tests/t_encrypt.out b/cli-tests/t_encrypt.out index f067fc0..ecdc46b 100644 --- a/cli-tests/t_encrypt.out +++ b/cli-tests/t_encrypt.out @@ -1,7 +1,8 @@ # Try to encrypt a nonexistent directory [ERROR] fscrypt encrypt: no such file or directory -ext4 filesystem "MNT" has 0 protectors and 0 policies +ext4 filesystem "MNT" has 0 protectors and 0 policies. +All users can create fscrypt metadata on this filesystem. [ERROR] fscrypt status: file or directory "MNT/dir" is not encrypted @@ -23,7 +24,8 @@ files into it, and securely delete the original directory. For example: Caution: due to the nature of modern storage devices and filesystems, the original data may still be recoverable from disk. It's much better to encrypt your files from the start. -ext4 filesystem "MNT" has 0 protectors and 0 policies +ext4 filesystem "MNT" has 0 protectors and 0 policies. +All users can create fscrypt metadata on this filesystem. [ERROR] fscrypt status: file or directory "MNT/dir" is not encrypted @@ -45,13 +47,15 @@ files into it, and securely delete the original directory. For example: Caution: due to the nature of modern storage devices and filesystems, the original data may still be recoverable from disk. It's much better to encrypt your files from the start. -ext4 filesystem "MNT" has 0 protectors and 0 policies +ext4 filesystem "MNT" has 0 protectors and 0 policies. +All users can create fscrypt metadata on this filesystem. [ERROR] fscrypt status: file or directory "MNT/dir" is not encrypted # Encrypt a directory as non-root user -ext4 filesystem "MNT" has 1 protector and 1 policy +ext4 filesystem "MNT" has 1 protector and 1 policy. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc1 No custom protector "prot" @@ -67,7 +71,8 @@ Unlocked: Yes Protected with 1 protector: PROTECTOR LINKED DESCRIPTION desc1 No custom protector "prot" -ext4 filesystem "MNT" has 1 protector and 1 policy +ext4 filesystem "MNT" has 1 protector and 1 policy (only including ones owned by fscrypt-test-user or root). +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc1 No custom protector "prot" @@ -94,7 +99,8 @@ desc1 No custom protector "prot" Encryption can only be enabled on a directory you own, even if you have write access to the directory. -ext4 filesystem "MNT" has 0 protectors and 0 policies +ext4 filesystem "MNT" has 0 protectors and 0 policies. +All users can create fscrypt metadata on this filesystem. [ERROR] fscrypt status: file or directory "MNT/dir" is not encrypted diff --git a/cli-tests/t_encrypt_custom.out b/cli-tests/t_encrypt_custom.out index 8dd15e3..ac53d6f 100644 --- a/cli-tests/t_encrypt_custom.out +++ b/cli-tests/t_encrypt_custom.out @@ -1,6 +1,7 @@ # Encrypt with custom passphrase protector -ext4 filesystem "MNT" has 1 protector and 1 policy +ext4 filesystem "MNT" has 1 protector and 1 policy. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc1 No custom protector "prot" @@ -28,7 +29,8 @@ Enter a name for the new protector: prot Enter custom passphrase for protector "prot":
Confirm passphrase:
"MNT/dir" is now encrypted, unlocked, and ready for use.
-ext4 filesystem "MNT" has 1 protector and 1 policy +ext4 filesystem "MNT" has 1 protector and 1 policy. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc6 No custom protector "prot" @@ -49,7 +51,8 @@ desc6 No custom protector "prot" [ERROR] fscrypt encrypt: custom_passphrase protectors must be named Use --name=PROTECTOR_NAME to specify a protector name. -ext4 filesystem "MNT" has 0 protectors and 0 policies +ext4 filesystem "MNT" has 0 protectors and 0 policies. +All users can create fscrypt metadata on this filesystem. [ERROR] fscrypt status: file or directory "MNT/dir" is not encrypted diff --git a/cli-tests/t_encrypt_login.out b/cli-tests/t_encrypt_login.out index 269f597..bb91a46 100644 --- a/cli-tests/t_encrypt_login.out +++ b/cli-tests/t_encrypt_login.out @@ -7,7 +7,8 @@ IMPORTANT: See "MNT/dir/fscrypt_recovery_readme.txt" for will lose access to this directory if you reinstall the operating system or move this filesystem to another system. -ext4 filesystem "MNT" has 2 protectors and 1 policy +ext4 filesystem "MNT" has 2 protectors and 1 policy. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc1 Yes (MNT_ROOT) login protector for fscrypt-test-user @@ -15,7 +16,8 @@ desc2 No custom protector "Recovery passphrase POLICY UNLOCKED PROTECTORS desc3 Yes desc1, desc2 -ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies +ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc1 No login protector for fscrypt-test-user @@ -57,7 +59,8 @@ IMPORTANT: See "MNT/dir/fscrypt_recovery_readme.txt" for system or move this filesystem to another system.
"MNT/dir" is now encrypted, unlocked, and ready for use.
-ext4 filesystem "MNT" has 2 protectors and 1 policy +ext4 filesystem "MNT" has 2 protectors and 1 policy. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc10 Yes (MNT_ROOT) login protector for fscrypt-test-user @@ -65,7 +68,8 @@ desc11 No custom protector "Recovery passphras POLICY UNLOCKED PROTECTORS desc12 Yes desc10, desc11 -ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies +ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc10 No login protector for fscrypt-test-user @@ -88,7 +92,8 @@ IMPORTANT: See "MNT/dir/fscrypt_recovery_readme.txt" for will lose access to this directory if you reinstall the operating system or move this filesystem to another system. -ext4 filesystem "MNT" has 2 protectors and 1 policy +ext4 filesystem "MNT" has 2 protectors and 1 policy. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc19 Yes (MNT_ROOT) login protector for fscrypt-test-user @@ -96,7 +101,8 @@ desc20 No custom protector "Recovery passphras POLICY UNLOCKED PROTECTORS desc21 Yes desc19, desc20 -ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies +ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc19 No login protector for fscrypt-test-user @@ -112,16 +118,20 @@ desc19 Yes (MNT_ROOT) login protector for fscrypt-test-user desc20 No custom protector "Recovery passphrase for dir" Protector is owned by fscrypt-test-user:fscrypt-test-user +"MNT/dir" is now locked. +"MNT/dir" is now locked. # Encrypt with login protector with --no-recovery -ext4 filesystem "MNT" has 1 protector and 1 policy +ext4 filesystem "MNT" has 1 protector and 1 policy. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc28 Yes (MNT_ROOT) login protector for fscrypt-test-user POLICY UNLOCKED PROTECTORS desc29 Yes desc28 -ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies +ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc28 No login protector for fscrypt-test-user @@ -145,7 +155,8 @@ Unlocked: Yes Protected with 1 protector: PROTECTOR LINKED DESCRIPTION desc35 No login protector for fscrypt-test-user -ext4 filesystem "MNT_ROOT" has 1 protector and 1 policy +ext4 filesystem "MNT_ROOT" has 1 protector and 1 policy. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc35 No login protector for fscrypt-test-user @@ -159,18 +170,22 @@ desc34 Yes desc35 identified by user, not by name. To fix this, don't specify the --name=PROTECTOR_NAME option. -ext4 filesystem "MNT" has 0 protectors and 0 policies +ext4 filesystem "MNT" has 0 protectors and 0 policies. +All users can create fscrypt metadata on this filesystem. -ext4 filesystem "MNT_ROOT" has 0 protectors and 0 policies +ext4 filesystem "MNT_ROOT" has 0 protectors and 0 policies. +All users can create fscrypt metadata on this filesystem. [ERROR] fscrypt status: file or directory "MNT/dir" is not encrypted # Try to use the wrong login passphrase [ERROR] fscrypt encrypt: incorrect login passphrase -ext4 filesystem "MNT" has 0 protectors and 0 policies +ext4 filesystem "MNT" has 0 protectors and 0 policies. +All users can create fscrypt metadata on this filesystem. -ext4 filesystem "MNT_ROOT" has 0 protectors and 0 policies +ext4 filesystem "MNT_ROOT" has 0 protectors and 0 policies. +All users can create fscrypt metadata on this filesystem. [ERROR] fscrypt status: file or directory "MNT/dir" is not encrypted @@ -183,7 +198,8 @@ IMPORTANT: See "MNT/dir/fscrypt_recovery_readme.txt" for will lose access to this directory if you reinstall the operating system or move this filesystem to another system. -ext4 filesystem "MNT" has 2 protectors and 1 policy +ext4 filesystem "MNT" has 2 protectors and 1 policy. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc39 No custom protector "Recovery passphrase for dir" diff --git a/cli-tests/t_encrypt_login.sh b/cli-tests/t_encrypt_login.sh index 225a47d..b6ae2d8 100755 --- a/cli-tests/t_encrypt_login.sh +++ b/cli-tests/t_encrypt_login.sh @@ -58,9 +58,17 @@ 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. +# This is partly redundant with the below check, but we might as well test both. login_protector=$(_get_login_descriptor) owner=$(stat -c "%U:%G" "$MNT_ROOT/.fscrypt/protectors/$login_protector") echo -e "\nProtector is owned by $owner" +# The user should be able to lock and unlock the directory themselves. This +# tests that the fscrypt metadata file permissions got set appropriately when +# root set up the encryption on the user's behalf. +chown "$TEST_USER" "$dir" +_user_do "fscrypt lock $dir" +_user_do "echo TEST_USER_PASS | fscrypt unlock $dir --quiet --unlock-with=$MNT_ROOT:$login_protector" +_user_do "fscrypt lock $dir" begin "Encrypt with login protector with --no-recovery" chown "$TEST_USER" "$dir" diff --git a/cli-tests/t_encrypt_raw_key.out b/cli-tests/t_encrypt_raw_key.out index 1f51dc0..4cfc050 100644 --- a/cli-tests/t_encrypt_raw_key.out +++ b/cli-tests/t_encrypt_raw_key.out @@ -1,6 +1,7 @@ # Encrypt with raw_key protector from file -ext4 filesystem "MNT" has 1 protector and 1 policy +ext4 filesystem "MNT" has 1 protector and 1 policy. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc1 No raw key protector "prot" @@ -18,7 +19,8 @@ PROTECTOR LINKED DESCRIPTION desc1 No raw key protector "prot" # Encrypt with raw_key protector from stdin -ext4 filesystem "MNT" has 1 protector and 1 policy +ext4 filesystem "MNT" has 1 protector and 1 policy. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc6 No raw key protector "prot" @@ -37,21 +39,24 @@ desc6 No raw key protector "prot" # Try to encrypt with raw_key protector from file, using wrong key length [ERROR] fscrypt encrypt: TMPDIR/raw_key: key file must be 32 bytes -ext4 filesystem "MNT" has 0 protectors and 0 policies +ext4 filesystem "MNT" has 0 protectors and 0 policies. +All users can create fscrypt metadata on this filesystem. [ERROR] fscrypt status: file or directory "MNT/dir" is not encrypted # Try to encrypt with raw_key protector from stdin, using wrong key length [ERROR] fscrypt encrypt: unexpected EOF -ext4 filesystem "MNT" has 0 protectors and 0 policies +ext4 filesystem "MNT" has 0 protectors and 0 policies. +All users can create fscrypt metadata on this filesystem. [ERROR] fscrypt status: file or directory "MNT/dir" is not encrypted # Encrypt with raw_key protector from file, unlock from stdin "MNT/dir" is now locked. -ext4 filesystem "MNT" has 1 protector and 1 policy +ext4 filesystem "MNT" has 1 protector and 1 policy. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc11 No raw key protector "prot" diff --git a/cli-tests/t_lock.out b/cli-tests/t_lock.out index b8c8dcb..0da8964 100644 --- a/cli-tests/t_lock.out +++ b/cli-tests/t_lock.out @@ -76,7 +76,6 @@ cat: MNT/dir/file: No such file or directory mkdir: cannot create directory 'MNT/dir/subdir': Required key not available # Try to lock directory while other user has unlocked -Enter custom passphrase for protector "prot": "MNT/dir" is now unlocked and ready for use. [ERROR] fscrypt lock: Directory "MNT/dir" couldn't be fully locked because other user(s) have unlocked it. diff --git a/cli-tests/t_lock.sh b/cli-tests/t_lock.sh index 7ac1727..9b193fd 100755 --- a/cli-tests/t_lock.sh +++ b/cli-tests/t_lock.sh @@ -43,8 +43,11 @@ _expect_failure "cat '$dir/file'" _expect_failure "mkdir '$dir/subdir'" _print_header "Try to lock directory while other user has unlocked" +rm -rf "$dir" +mkdir "$dir" chown "$TEST_USER" "$dir" -_user_do "echo hunter2 | fscrypt unlock '$dir'" +_user_do "echo hunter2 | fscrypt encrypt --quiet --name=prot '$dir'" +_user_do "echo contents > $dir/file" _expect_failure "fscrypt lock '$dir'" cat "$dir/file" fscrypt lock --all-users "$dir" diff --git a/cli-tests/t_metadata.out b/cli-tests/t_metadata.out index fba816a..bbcc0f2 100644 --- a/cli-tests/t_metadata.out +++ b/cli-tests/t_metadata.out @@ -1,4 +1,5 @@ -ext4 filesystem "MNT" has 3 protectors and 1 policy +ext4 filesystem "MNT" has 3 protectors and 1 policy. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc1 No custom protector "foo" @@ -7,7 +8,8 @@ desc3 No custom protector "baz" POLICY UNLOCKED PROTECTORS desc4 No desc1, desc2, desc3 -ext4 filesystem "MNT" has 2 protectors and 1 policy +ext4 filesystem "MNT" has 2 protectors and 1 policy. +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc1 No custom protector "foo" diff --git a/cli-tests/t_not_supported.sh b/cli-tests/t_not_supported.sh index 9ff90e1..8b52392 100755 --- a/cli-tests/t_not_supported.sh +++ b/cli-tests/t_not_supported.sh @@ -10,7 +10,7 @@ umount "$MNT" mount tmpfs -t tmpfs -o size=128m "$MNT" _print_header "Try to create fscrypt metadata on tmpfs" -_expect_failure "fscrypt setup '$MNT'" +_expect_failure "fscrypt setup --quiet '$MNT'" _print_header "Try to encrypt a directory on tmpfs" mkdir "$MNT/dir" diff --git a/cli-tests/t_setup.out b/cli-tests/t_setup.out index 943a781..6ea03e3 100644 --- a/cli-tests/t_setup.out +++ b/cli-tests/t_setup.out @@ -9,7 +9,8 @@ Skipping creating MNT_ROOT/.fscrypt because it already exists. Defaulting to policy_version 2 because kernel supports it. Customizing passphrase hashing difficulty for this system... Created global config file at "FSCRYPT_CONF". -Metadata directories created at "MNT_ROOT/.fscrypt". +Allow users other than root to create fscrypt metadata on this filesystem? (See +https://github.com/google/fscrypt#setting-up-fscrypt-on-a-filesystem) [y/N] Metadata directories created at "MNT_ROOT/.fscrypt", writable by everyone. # fscrypt setup when fscrypt.conf already exists (cancel) Replace "FSCRYPT_CONF"? [y/N] [ERROR] fscrypt setup: operation canceled @@ -31,7 +32,8 @@ If desired, use --force to automatically run destructive operations. # fscrypt setup --quiet --force when fscrypt.conf already exists # fscrypt setup filesystem -Metadata directories created at "MNT/.fscrypt". +Allow users other than root to create fscrypt metadata on this filesystem? (See +https://github.com/google/fscrypt#setting-up-fscrypt-on-a-filesystem) [y/N] Metadata directories created at "MNT/.fscrypt", writable by everyone. # fscrypt setup filesystem (already set up) [ERROR] fscrypt setup: filesystem MNT is already setup for diff --git a/cli-tests/t_setup.sh b/cli-tests/t_setup.sh index a8a62a3..f7e302d 100755 --- a/cli-tests/t_setup.sh +++ b/cli-tests/t_setup.sh @@ -14,7 +14,7 @@ fscrypt setup --time=1ms _print_header "fscrypt setup creates fscrypt.conf and /.fscrypt" _rm_metadata "$MNT_ROOT" rm -f "$FSCRYPT_CONF" -fscrypt setup --time=1ms +echo y | fscrypt setup --time=1ms [ -e "$MNT_ROOT/.fscrypt" ] _print_header "fscrypt setup when fscrypt.conf already exists (cancel)" @@ -37,7 +37,7 @@ fscrypt setup --quiet --force --time=1ms _print_header "fscrypt setup filesystem" _rm_metadata "$MNT" -fscrypt setup "$MNT" +echo y | fscrypt setup "$MNT" [ -e "$MNT/.fscrypt" ] _print_header "fscrypt setup filesystem (already set up)" diff --git a/cli-tests/t_single_user.out b/cli-tests/t_single_user.out new file mode 100644 index 0000000..d038d52 --- /dev/null +++ b/cli-tests/t_single_user.out @@ -0,0 +1,30 @@ +ext4 filesystem "MNT" has 0 protectors and 0 policies. +Only root can create fscrypt metadata on this filesystem. + +ext4 filesystem "MNT" has 0 protectors and 0 policies (only including ones owned by fscrypt-test-user or root). +Only root can create fscrypt metadata on this filesystem. + + +# Encrypt, lock, and unlock as root +"MNT/dir" is now locked. + +# Encrypt as root with user's login protector + +IMPORTANT: See "MNT/dir/fscrypt_recovery_readme.txt" for + important recovery instructions. It is *strongly recommended* to + record the recovery passphrase in a secure location; otherwise you + will lose access to this directory if you reinstall the operating + system or move this filesystem to another system. + +Protector desc1 no longer protecting policy desc2. +"MNT/dir" is now locked. +Enter login passphrase for fscrypt-test-user: "MNT/dir" is now unlocked and ready for use. + +# Encrypt as user (should fail) +[ERROR] fscrypt encrypt: user lacks permission to create fscrypt metadata on + MNT + +For how to allow users to create fscrypt metadata on a filesystem, refer to +https://github.com/google/fscrypt#setting-up-fscrypt-on-a-filesystem + +# Encrypt as user if they set up filesystem (should succeed) diff --git a/cli-tests/t_single_user.sh b/cli-tests/t_single_user.sh new file mode 100755 index 0000000..c569f20 --- /dev/null +++ b/cli-tests/t_single_user.sh @@ -0,0 +1,55 @@ +#!/bin/bash + +# Test 'fscrypt setup' without --all-users. + +cd "$(dirname "$0")" +. common.sh + +_rm_metadata "$MNT_ROOT" +_rm_metadata "$MNT" +rm "$FSCRYPT_CONF" +fscrypt setup --time=1ms --quiet +fscrypt setup --time=1ms --quiet "$MNT" +fscrypt status "$MNT" +_user_do "fscrypt status \"$MNT\"" + +dir=$MNT/dir + +begin() +{ + _reset_filesystems + mkdir "$dir" + _print_header "$1" +} + +begin "Encrypt, lock, and unlock as root" +echo hunter2 | fscrypt encrypt --quiet --name=dir --skip-unlock "$dir" +echo hunter2 | fscrypt unlock --quiet "$dir" +fscrypt lock "$dir" + +begin "Encrypt as root with user's login protector" +echo TEST_USER_PASS | fscrypt encrypt --quiet --source=pam_passphrase --user="$TEST_USER" "$dir" +# The user should be able to update the policy and protectors created by the +# above command themselves. The easiest way to test this is by updating the +# policy to remove the auto-generated recovery protector. This verifies that +# (a) the policy was made owned by the user, and that (b) policy updates fall +# back to overwrites when the process cannot write to the containing directory. +# (It would be better to test updating the protectors too, but this is the +# easiest test to do here.) +policy=$(fscrypt status "$dir" | awk '/Policy/{print $2}') +recovery_protector=$(_get_protector_descriptor "$MNT" custom 'Recovery passphrase for dir') +_user_do "fscrypt metadata remove-protector-from-policy --force --protector=$MNT:$recovery_protector --policy=$MNT:$policy" +chown "$TEST_USER" "$dir" +_user_do "fscrypt lock $dir" +_user_do "echo TEST_USER_PASS | fscrypt unlock $dir" + +begin "Encrypt as user (should fail)" +chown "$TEST_USER" "$dir" +_user_do_and_expect_failure "echo hunter2 | fscrypt encrypt --quiet --name=dir --skip-unlock \"$dir\"" + +begin "Encrypt as user if they set up filesystem (should succeed)" +_rm_metadata "$MNT" +chown "$TEST_USER" "$MNT" +chown "$TEST_USER" "$dir" +_user_do "fscrypt setup --time=1ms --quiet $MNT" +_user_do "echo hunter2 | fscrypt encrypt --quiet --name=dir3 --skip-unlock \"$dir\"" diff --git a/cli-tests/t_status.out b/cli-tests/t_status.out index 0d478b5..058c62c 100644 --- a/cli-tests/t_status.out +++ b/cli-tests/t_status.out @@ -4,9 +4,11 @@ ext4 supported Yes ext4 supported Yes # Get status of setup mountpoint -ext4 filesystem "MNT" has 0 protectors and 0 policies +ext4 filesystem "MNT" has 0 protectors and 0 policies. +All users can create fscrypt metadata on this filesystem. -ext4 filesystem "MNT" has 0 protectors and 0 policies +ext4 filesystem "MNT" has 0 protectors and 0 policies (only including ones owned by fscrypt-test-user or root). +All users can create fscrypt metadata on this filesystem. # Get status of unencrypted directory on setup mountpoint diff --git a/cli-tests/t_v1_policy.out b/cli-tests/t_v1_policy.out index 9adb00a..2353527 100644 --- a/cli-tests/t_v1_policy.out +++ b/cli-tests/t_v1_policy.out @@ -120,7 +120,8 @@ Unlocked: Partially (incompletely locked, or unlocked by another user) Protected with 1 protector: PROTECTOR LINKED DESCRIPTION desc2 No custom protector "prot" -ext4 filesystem "MNT" has 1 protector and 1 policy +ext4 filesystem "MNT" has 1 protector and 1 policy (only including ones owned by fscrypt-test-user or root). +All users can create fscrypt metadata on this filesystem. PROTECTOR LINKED DESCRIPTION desc2 No custom protector "prot" diff --git a/cmd/fscrypt/commands.go b/cmd/fscrypt/commands.go index 023c0fa..30aa3a7 100644 --- a/cmd/fscrypt/commands.go +++ b/cmd/fscrypt/commands.go @@ -63,7 +63,7 @@ var Setup = cli.Command{ the README). This may require root privileges.`, mountpointArg, actions.ConfigFileLocation, shortDisplay(timeTargetFlag)), - Flags: []cli.Flag{timeTargetFlag, forceFlag}, + Flags: []cli.Flag{timeTargetFlag, forceFlag, allUsersSetupFlag}, Action: setupAction, } @@ -468,7 +468,7 @@ var Lock = cli.Command{ recoverable by an attacker who compromises system memory. To be fully safe, you must reboot with a power cycle.`, directoryArg, shortDisplay(dropCachesFlag)), - Flags: []cli.Flag{dropCachesFlag, userFlag, allUsersFlag}, + Flags: []cli.Flag{dropCachesFlag, userFlag, allUsersLockFlag}, Action: lockAction, } @@ -502,7 +502,7 @@ func lockAction(c *cli.Context) error { return newExitError(c, ErrDropCachesPerm) } - if err = policy.Deprovision(allUsersFlag.Value); err != nil { + if err = policy.Deprovision(allUsersLockFlag.Value); err != nil { switch err { case keyring.ErrKeyNotPresent: break diff --git a/cmd/fscrypt/errors.go b/cmd/fscrypt/errors.go index bcf5b59..2d76792 100644 --- a/cmd/fscrypt/errors.go +++ b/cmd/fscrypt/errors.go @@ -232,6 +232,10 @@ func getErrorSuggestions(err error) string { } } return "" + case *filesystem.ErrNoCreatePermission: + return `For how to allow users to create fscrypt metadata on a + filesystem, refer to + https://github.com/google/fscrypt#setting-up-fscrypt-on-a-filesystem` case *filesystem.ErrNotSetup: return fmt.Sprintf(`Run "sudo fscrypt setup %s" to use fscrypt on this filesystem.`, e.Mount.Path) diff --git a/cmd/fscrypt/flags.go b/cmd/fscrypt/flags.go index 044b71e..1b41839 100644 --- a/cmd/fscrypt/flags.go +++ b/cmd/fscrypt/flags.go @@ -116,7 +116,8 @@ var ( allFlags = []prettyFlag{helpFlag, versionFlag, verboseFlag, quietFlag, forceFlag, skipUnlockFlag, timeTargetFlag, sourceFlag, nameFlag, keyFileFlag, protectorFlag, - unlockWithFlag, policyFlag, allUsersFlag, noRecoveryFlag} + unlockWithFlag, policyFlag, allUsersLockFlag, allUsersSetupFlag, + noRecoveryFlag} // universalFlags contains flags that should be on every command universalFlags = []cli.Flag{verboseFlag, quietFlag, helpFlag} ) @@ -164,7 +165,7 @@ var ( privileges.`, Default: true, } - allUsersFlag = &boolFlag{ + allUsersLockFlag = &boolFlag{ Name: "all-users", Usage: `Lock the directory no matter which user(s) have unlocked it. Requires root privileges. This flag is only @@ -172,6 +173,15 @@ var ( different from the one you're locking it as. This flag is only implemented for v2 encryption policies.`, } + allUsersSetupFlag = &boolFlag{ + Name: "all-users", + Usage: `When setting up a filesystem for fscrypt, allow users + other than the calling user (typically root) to create + fscrypt policies and protectors on the filesystem. Note + that this will create a world-writable directory, which + users could use to fill up the entire filesystem. Hence, + this option may not be appropriate for some systems.`, + } noRecoveryFlag = &boolFlag{ Name: "no-recovery", Usage: `Don't generate a recovery passphrase.`, diff --git a/cmd/fscrypt/fscrypt_bash_completion b/cmd/fscrypt/fscrypt_bash_completion index 00ee490..110d2d4 100644 --- a/cmd/fscrypt/fscrypt_bash_completion +++ b/cmd/fscrypt/fscrypt_bash_completion @@ -15,25 +15,71 @@ # License for the specific language governing permissions and limitations under # the License. - -# Prefer completion script style COMPREPLY=($(...)) assignment. +# +# bash completion scripts require exercising some unusual shell script +# features/quirks, so we have to disable some shellcheck warnings: +# +# Disable SC2016 ("Expressions don't expand in single quotes, use double quotes +# for that") because the 'compgen' built-in expands the argument passed to -W, +# so that argument *must* be single-quoted to avoid command injection. +# shellcheck disable=SC2016 +# +# Disable SC2034 ("{Variable} appears unused. Verify use (or export if used +# externally)") because of the single quoting mentioned above as well as the +# fact that we have to declare "local" variables used only by a called function +# (_init_completion()) and not by the function itself. +# shellcheck disable=SC2034 +# +# Disable SC2207 ("Prefer mapfile or read -a to split command output (or quote +# to avoid splitting)") because bash completion scripts conventionally use +# COMPREPLY=($(...)) assignments. # shellcheck disable=SC2207 -true # To apply shellcheck directive to all file +# +true # To apply the above shellcheck directives to the entire file -# Output list of possible mount points -_fscrypt_mountpoints() +# Generate the completion list for possible mountpoints. +# +# We need to be super careful here because mountpoints can contain whitespace +# and shell meta-characters. To avoid most problems, we do the following: +# +# 1.) To avoid parsing ambiguities, 'fscrypt status' replaces the space, tab, +# newline, and backslash characters with octal escape sequences -- like +# what /proc/self/mountinfo does. To properly process its output, we need +# to split lines on space only (and not on other whitespace which might +# not be escaped), and unescape these characters. Exception: we don't +# unescape newlines, as we need to reserve newline as the separator for +# the words passed to compgen. (This causes mountpoints containing +# newlines to not be completed correctly, which we have to tolerate.) +# +# 2.) We backslash-escape all shell meta-characters, and single-quote the +# argument passed to compgen -W. Without either step, command injection +# would be possible. Without both steps, completions would be incorrect. +# The list of shell meta-characters used comes from that used by the +# completion script for umount, which has to solve this same problem. +# +_fscrypt_compgen_mountpoints() { - # shellcheck disable=SC2016 - fscrypt status 2>/dev/null | \ - command awk 'substr($0, 1, 1) == "/" && $5 == "Yes" { print $1 }' + local IFS=$'\n' + compgen -W '$(_fscrypt_mountpoints_internal)' -- "${cur}" } +_fscrypt_mountpoints_internal() +{ + fscrypt status 2>/dev/null | command awk -F " " \ + 'substr($0, 1, 1) == "/" && $5 == "Yes" { + gsub(/\\040/, " ", $1) + gsub(/\\011/, "\t", $1) + gsub(/\\134/, "\\", $1) + gsub(/[\]\[(){}<>",:;^&!$=?`|'\''\\ \t\f\n\r\v]/, "\\\\&", $1) + print $1 + }' +} # Complete with all possible mountpoints _fscrypt_complete_mountpoint() { - COMPREPLY=($(compgen -W "$(_fscrypt_mountpoints)" -- "${cur}")) + COMPREPLY=($(_fscrypt_compgen_mountpoints)) } @@ -43,7 +89,6 @@ _fscrypt_complete_mountpoint() _fscrypt_status_section() { local section=${2^^} - # shellcheck disable=SC2016 fscrypt status "$1" 2>/dev/null | \ command awk '/^[[:xdigit:]]{16}/ && section == "'"$section"'" { print $1; next; } { section = $1 }' @@ -57,13 +102,13 @@ _fscrypt_complete_policy_or_protector() if [[ $cur = *:* ]]; then # Complete with IDs of the given mountpoint local mountpoint="${cur%:*}" id="${cur#*:}" + # Note: compgen expands the argument to -W, so it *must* be single-quoted. COMPREPLY=($(compgen \ - -W "$(_fscrypt_status_section "${mountpoint}" "${status_section}")" \ + -W '$(_fscrypt_status_section "${mountpoint}" "${status_section}")' \ -- "${id}")) else # Complete with mountpoints, with colon and without ending space - COMPREPLY=($(compgen -W "$(_fscrypt_mountpoints)" \ - -- "${cur}" | sed s/\$/:/)) + COMPREPLY=($(_fscrypt_compgen_mountpoints | sed s/\$/:/)) compopt -o nospace fi } @@ -72,7 +117,8 @@ _fscrypt_complete_policy_or_protector() # Complete with all arguments of that function _fscrypt_complete_word() { - COMPREPLY=($(compgen -W "$*" -- "${cur}")) + # Note: compgen expands the argument to -W, so it *must* be single-quoted. + COMPREPLY=($(compgen -W '$*' -- "${cur}")) } @@ -82,7 +128,8 @@ _fscrypt_complete_option() local additional_opts=( "$@" ) # Add global options, always correct additional_opts+=( --verbose --quiet --help ) - COMPREPLY=($(compgen -W "${additional_opts[*]}" -- "${cur}")) + # Note: compgen expands the argument to -W, so it *must* be single-quoted. + COMPREPLY=($(compgen -W '${additional_opts[*]}' -- "${cur}")) } @@ -95,7 +142,6 @@ _fscrypt() # # `split` is set by `_init_completion -s`, we must declare it local # even if we don't use it, not to modify the environment. - # shellcheck disable=SC2034 local cur prev words cword split _init_completion -s -n : || return diff --git a/cmd/fscrypt/protector.go b/cmd/fscrypt/protector.go index ac864dd..186ca7a 100644 --- a/cmd/fscrypt/protector.go +++ b/cmd/fscrypt/protector.go @@ -23,6 +23,7 @@ package main import ( "fmt" "log" + "os/user" "github.com/google/fscrypt/actions" "github.com/google/fscrypt/filesystem" @@ -38,7 +39,6 @@ func createProtectorFromContext(ctx *actions.Context) (*actions.Protector, error return nil, err } log.Printf("using source: %s", ctx.Config.Source.String()) - if ctx.Config.Source == metadata.SourceType_pam_passphrase { if userFlag.Value == "" && util.IsUserRoot() { return nil, ErrSpecifyUser @@ -70,7 +70,11 @@ IMPORTANT: Before continuing, ensure you have properly set up your system for } } - return actions.CreateProtector(ctx, name, createKeyFn) + var owner *user.User + if ctx.Config.Source == metadata.SourceType_pam_passphrase && util.IsUserRoot() { + owner = ctx.TargetUser + } + return actions.CreateProtector(ctx, name, createKeyFn, owner) } // selectExistingProtector returns a locked Protector which corresponds to an diff --git a/cmd/fscrypt/setup.go b/cmd/fscrypt/setup.go index 7b9bebb..b9a16e8 100644 --- a/cmd/fscrypt/setup.go +++ b/cmd/fscrypt/setup.go @@ -26,6 +26,7 @@ import ( "os" "github.com/google/fscrypt/actions" + "github.com/google/fscrypt/filesystem" "github.com/google/fscrypt/util" ) @@ -80,11 +81,47 @@ func setupFilesystem(w io.Writer, path string) error { if err != nil { return err } + username := ctx.TargetUser.Username - if err = ctx.Mount.Setup(); err != nil { + err = ctx.Mount.CheckSetup(ctx.TrustedUser) + if err == nil { + return &filesystem.ErrAlreadySetup{Mount: ctx.Mount} + } + if _, ok := err.(*filesystem.ErrNotSetup); !ok { return err } - fmt.Fprintf(w, "Metadata directories created at %q.\n", ctx.Mount.BaseDir()) + allUsers := allUsersSetupFlag.Value + if !allUsers { + thisFilesystem := "this filesystem" + if ctx.Mount.Path == "/" { + thisFilesystem = "the root filesystem" + } + prompt := fmt.Sprintf(`Allow users other than %s to create +fscrypt metadata on %s? (See +https://github.com/google/fscrypt#setting-up-fscrypt-on-a-filesystem)`, + username, thisFilesystem) + allUsers, err = askQuestion(wrapText(prompt, 0), false) + if err != nil { + return err + } + } + var setupMode filesystem.SetupMode + if allUsers { + setupMode = filesystem.WorldWritable + } else { + setupMode = filesystem.SingleUserWritable + } + if err = ctx.Mount.Setup(setupMode); err != nil { + return err + } + + if allUsers { + fmt.Fprintf(w, "Metadata directories created at %q, writable by everyone.\n", + ctx.Mount.BaseDir()) + } else { + fmt.Fprintf(w, "Metadata directories created at %q, writable by %s only.\n", + ctx.Mount.BaseDir(), username) + } return nil } diff --git a/cmd/fscrypt/status.go b/cmd/fscrypt/status.go index 255bb2b..bc8f1ee 100644 --- a/cmd/fscrypt/status.go +++ b/cmd/fscrypt/status.go @@ -101,7 +101,7 @@ func writeGlobalStatus(w io.Writer) error { t := makeTableWriter(w, "MOUNTPOINT\tDEVICE\tFILESYSTEM\tENCRYPTION\tFSCRYPT") for _, mount := range mounts { // Only print mountpoints backed by devices or using fscrypt. - usingFscrypt := mount.CheckSetup() == nil + usingFscrypt := mount.CheckSetup(nil) == nil if !usingFscrypt && mount.Device == "" { continue } @@ -114,8 +114,11 @@ func writeGlobalStatus(w io.Writer) error { continue } - fmt.Fprintf(t, "%s\t%s\t%s\t%s\t%s\n", mount.Path, mount.Device, - mount.FilesystemType, supportString, yesNoString(usingFscrypt)) + fmt.Fprintf(t, "%s\t%s\t%s\t%s\t%s\n", + filesystem.EscapeString(mount.Path), + filesystem.EscapeString(mount.Device), + filesystem.EscapeString(mount.FilesystemType), + supportString, yesNoString(usingFscrypt)) if supportErr == nil { supportCount++ @@ -157,14 +160,27 @@ func writeFilesystemStatus(w io.Writer, ctx *actions.Context) error { return err } - policyDescriptors, err := ctx.Mount.ListPolicies() + policyDescriptors, err := ctx.Mount.ListPolicies(ctx.TrustedUser) if err != nil { return err } - fmt.Fprintf(w, "%s filesystem %q has %s and %s\n\n", ctx.Mount.FilesystemType, + filterDescription := "" + if ctx.TrustedUser != nil { + filterDescription = fmt.Sprintf(" (only including ones owned by %s or root)", ctx.TrustedUser.Username) + } + fmt.Fprintf(w, "%s filesystem %q has %s and %s%s.\n", ctx.Mount.FilesystemType, ctx.Mount.Path, pluralize(len(options), "protector"), - pluralize(len(policyDescriptors), "policy")) + pluralize(len(policyDescriptors), "policy"), filterDescription) + if setupMode, user, err := ctx.Mount.GetSetupMode(); err == nil { + switch setupMode { + case filesystem.WorldWritable: + fmt.Fprintf(w, "All users can create fscrypt metadata on this filesystem.\n") + case filesystem.SingleUserWritable: + fmt.Fprintf(w, "Only %s can create fscrypt metadata on this filesystem.\n", user.Username) + } + } + fmt.Fprintf(w, "\n") if len(options) > 0 { writeOptions(w, options) diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go index da6b9cc..27bfa24 100644 --- a/filesystem/filesystem.go +++ b/filesystem/filesystem.go @@ -34,6 +34,7 @@ package filesystem import ( "fmt" + "io" "io/ioutil" "log" "os" @@ -41,6 +42,7 @@ import ( "path/filepath" "sort" "strings" + "syscall" "time" "github.com/golang/protobuf/proto" @@ -83,6 +85,17 @@ func (err *ErrFollowLink) Error() string { err.Link, err.UnderlyingError) } +// ErrInsecurePermissions indicates that a filesystem is not considered to be +// setup for fscrypt because a metadata directory has insecure permissions. +type ErrInsecurePermissions struct { + Path string +} + +func (err *ErrInsecurePermissions) Error() string { + return fmt.Sprintf("%q has insecure permissions (world-writable without sticky bit)", + err.Path) +} + // ErrMakeLink indicates that a protector link can't be created. type ErrMakeLink struct { Target *Mount @@ -94,6 +107,27 @@ func (err *ErrMakeLink) Error() string { err.Target.Path, err.UnderlyingError) } +// ErrMountOwnedByAnotherUser indicates that the mountpoint root directory is +// owned by a user that isn't trusted in the current context, so we don't +// consider fscrypt to be properly setup on the filesystem. +type ErrMountOwnedByAnotherUser struct { + Mount *Mount +} + +func (err *ErrMountOwnedByAnotherUser) Error() string { + return fmt.Sprintf("another non-root user owns the root directory of %s", err.Mount.Path) +} + +// ErrNoCreatePermission indicates that the current user lacks permission to +// create fscrypt metadata on the given filesystem. +type ErrNoCreatePermission struct { + Mount *Mount +} + +func (err *ErrNoCreatePermission) Error() string { + return fmt.Sprintf("user lacks permission to create fscrypt metadata on %s", err.Mount.Path) +} + // ErrNotAMountpoint indicates that a path is not a mountpoint. type ErrNotAMountpoint struct { Path string @@ -112,6 +146,17 @@ func (err *ErrNotSetup) Error() string { return fmt.Sprintf("filesystem %s is not setup for use with fscrypt", err.Mount.Path) } +// ErrSetupByAnotherUser indicates that one or more of the fscrypt metadata +// directories is owned by a user that isn't trusted in the current context, so +// we don't consider fscrypt to be properly setup on the filesystem. +type ErrSetupByAnotherUser struct { + Mount *Mount +} + +func (err *ErrSetupByAnotherUser) Error() string { + return fmt.Sprintf("another non-root user owns fscrypt metadata directories on %s", err.Mount.Path) +} + // ErrSetupNotSupported indicates that the given filesystem type is not // supported for fscrypt setup. type ErrSetupNotSupported struct { @@ -207,12 +252,32 @@ const ( // The base directory should be read-only (except for the creator) basePermissions = 0755 - // The subdirectories should be writable to everyone, but they have the - // sticky bit set so users cannot delete other users' metadata. - dirPermissions = os.ModeSticky | 0777 - // The metadata files are globally visible, but can only be deleted by - // the user that created them - filePermissions = 0644 + + // The metadata files shouldn't be readable or writable by other users. + // Having them be world-readable wouldn't necessarily be a huge issue, + // but given that some of these files contain (strong) password hashes, + // we error on the side of caution -- similar to /etc/shadow. + // Note: existing files on-disk might have mode 0644, as that was the + // mode used by fscrypt v0.3.2 and earlier. + filePermissions = os.FileMode(0600) + + // Maximum size of a metadata file. This value is arbitrary, and it can + // be changed. We just set a reasonable limit that shouldn't be reached + // in practice, except by users trying to cause havoc by creating + // extremely large files in the metadata directories. + maxMetadataFileSize = 16384 +) + +// SetupMode is a mode for creating the fscrypt metadata directories. +type SetupMode int + +const ( + // SingleUserWritable specifies to make the fscrypt metadata directories + // writable by a single user (usually root) only. + SingleUserWritable SetupMode = iota + // WorldWritable specifies to make the fscrypt metadata directories + // world-writable (with the sticky bit set). + WorldWritable ) func (m *Mount) String() string { @@ -343,26 +408,94 @@ func (m *Mount) CheckSupport() error { return m.EncryptionSupportError(metadata.CheckSupport(m.Path)) } -// CheckSetup returns an error if all the fscrypt metadata directories do not +func checkOwnership(path string, info os.FileInfo, trustedUser *user.User) bool { + if trustedUser == nil { + return true + } + trustedUID := uint32(util.AtoiOrPanic(trustedUser.Uid)) + actualUID := info.Sys().(*syscall.Stat_t).Uid + if actualUID != 0 && actualUID != trustedUID { + log.Printf("WARNING: %q is owned by uid %d, but expected %d or 0", + path, actualUID, trustedUID) + return false + } + return true +} + +// CheckSetup returns an error if any of the fscrypt metadata directories do not // exist. Will log any unexpected errors or incorrect permissions. -func (m *Mount) CheckSetup() error { +func (m *Mount) CheckSetup(trustedUser *user.User) error { if !m.isFscryptSetupAllowed() { return &ErrNotSetup{m} } - // Run all the checks so we will always get all the warnings - baseGood := isDirCheckPerm(m.BaseDir(), basePermissions) - policyGood := isDirCheckPerm(m.PolicyDir(), dirPermissions) - protectorGood := isDirCheckPerm(m.ProtectorDir(), dirPermissions) + // Check that the mountpoint directory itself is not a symlink and has + // proper ownership, as otherwise we can't trust anything beneath it. + info, err := loggedLstat(m.Path) + if err != nil { + return &ErrNotSetup{m} + } + if (info.Mode() & os.ModeSymlink) != 0 { + log.Printf("mountpoint directory %q cannot be a symlink", m.Path) + return &ErrNotSetup{m} + } + if !info.IsDir() { + log.Printf("mountpoint %q is not a directory", m.Path) + return &ErrNotSetup{m} + } + if !checkOwnership(m.Path, info, trustedUser) { + return &ErrMountOwnedByAnotherUser{m} + } - if baseGood && policyGood && protectorGood { - return nil + // Check BaseDir similarly. However, unlike the other directories, we + // allow BaseDir to be a symlink, to support the use case of metadata + // for a read-only filesystem being redirected to a writable location. + info, err = loggedStat(m.BaseDir()) + if err != nil { + return &ErrNotSetup{m} + } + if !info.IsDir() { + log.Printf("%q is not a directory", m.BaseDir()) + return &ErrNotSetup{m} } - return &ErrNotSetup{m} + if !checkOwnership(m.Path, info, trustedUser) { + return &ErrMountOwnedByAnotherUser{m} + } + + // Check that the policies and protectors directories aren't symlinks and + // have proper ownership. + subdirs := []string{m.PolicyDir(), m.ProtectorDir()} + for _, path := range subdirs { + info, err := loggedLstat(path) + if err != nil { + return &ErrNotSetup{m} + } + if (info.Mode() & os.ModeSymlink) != 0 { + log.Printf("directory %q cannot be a symlink", path) + return &ErrNotSetup{m} + } + if !info.IsDir() { + log.Printf("%q is not a directory", path) + return &ErrNotSetup{m} + } + // We are no longer too picky about the mode, given that + // 'fscrypt setup' now offers a choice of two different modes, + // and system administrators could customize it further. + // However, we can at least verify that if the directory is + // world-writable, then the sticky bit is also set. + if info.Mode()&(os.ModeSticky|0002) == 0002 { + log.Printf("%q is world-writable but doesn't have sticky bit set", path) + return &ErrInsecurePermissions{path} + } + if !checkOwnership(path, info, trustedUser) { + return &ErrSetupByAnotherUser{m} + } + } + return nil } // makeDirectories creates the three metadata directories with the correct // permissions. Note that this function overrides the umask. -func (m *Mount) makeDirectories() error { +func (m *Mount) makeDirectories(setupMode SetupMode) error { // Zero the umask so we get the permissions we want oldMask := unix.Umask(0) defer func() { @@ -372,18 +505,52 @@ func (m *Mount) makeDirectories() error { if err := os.Mkdir(m.BaseDir(), basePermissions); err != nil { return err } - if err := os.Mkdir(m.PolicyDir(), dirPermissions); err != nil { + + var dirMode os.FileMode + switch setupMode { + case SingleUserWritable: + dirMode = 0755 + case WorldWritable: + dirMode = os.ModeSticky | 0777 + } + if err := os.Mkdir(m.PolicyDir(), dirMode); err != nil { return err } - return os.Mkdir(m.ProtectorDir(), dirPermissions) + return os.Mkdir(m.ProtectorDir(), dirMode) +} + +// GetSetupMode returns the current mode for fscrypt metadata creation on this +// filesystem. +func (m *Mount) GetSetupMode() (SetupMode, *user.User, error) { + info1, err1 := os.Stat(m.PolicyDir()) + info2, err2 := os.Stat(m.ProtectorDir()) + + if err1 == nil && err2 == nil { + mask := os.ModeSticky | 0777 + mode1 := info1.Mode() & mask + mode2 := info2.Mode() & mask + uid1 := info1.Sys().(*syscall.Stat_t).Uid + uid2 := info2.Sys().(*syscall.Stat_t).Uid + user, err := util.UserFromUID(int64(uid1)) + if err == nil && mode1 == mode2 && uid1 == uid2 { + switch mode1 { + case mask: + return WorldWritable, nil, nil + case 0755: + return SingleUserWritable, user, nil + } + } + log.Printf("filesystem %s uses custom permissions on metadata directories", m.Path) + } + return -1, nil, errors.New("unable to determine setup mode") } // Setup sets up the filesystem for use with fscrypt. Note that this merely // creates the appropriate files on the filesystem. It does not actually modify // the filesystem's feature flags. This operation is atomic; it either succeeds // or no files in the baseDir are created. -func (m *Mount) Setup() error { - if m.CheckSetup() == nil { +func (m *Mount) Setup(mode SetupMode) error { + if m.CheckSetup(nil) == nil { return &ErrAlreadySetup{m} } if !m.isFscryptSetupAllowed() { @@ -396,7 +563,7 @@ func (m *Mount) Setup() error { } defer os.RemoveAll(temp.Path) - if err = temp.makeDirectories(); err != nil { + if err = temp.makeDirectories(mode); err != nil { return err } @@ -410,7 +577,7 @@ func (m *Mount) Setup() error { // WARNING: Will cause data loss if the metadata is used to encrypt // directories (this could include directories on other filesystems). func (m *Mount) RemoveAllMetadata() error { - if err := m.CheckSetup(); err != nil { + if err := m.CheckSetup(nil); err != nil { return err } // temp will hold the old metadata temporarily @@ -436,23 +603,59 @@ func syncDirectory(dirPath string) error { return dirFile.Close() } -// writeDataAtomic writes the data to the path such that the data is either -// written to stable storage or an error is returned. -func (m *Mount) writeDataAtomic(path string, data []byte, owner *user.User) error { +func (m *Mount) overwriteDataNonAtomic(path string, data []byte) error { + file, err := os.OpenFile(path, os.O_WRONLY|os.O_TRUNC|unix.O_NOFOLLOW, 0) + if err != nil { + return err + } + if _, err = file.Write(data); err != nil { + log.Printf("WARNING: overwrite of %q failed; file will be corrupted!", path) + file.Close() + return err + } + if err = file.Sync(); err != nil { + file.Close() + return err + } + if err = file.Close(); err != nil { + return err + } + log.Printf("successfully overwrote %q non-atomically", path) + return nil +} + +// writeData writes the given data to the given path such that, if possible, the +// data is either written to stable storage or an error is returned. If a file +// already exists at the path, it will be replaced. +// +// However, if the process doesn't have write permission to the directory but +// does have write permission to the file itself, then as a fallback the file is +// overwritten in-place rather than replaced. Note that this may be non-atomic. +func (m *Mount) writeData(path string, data []byte, owner *user.User, mode os.FileMode) error { // Write the data to a temporary file, sync it, then rename into place // so that the operation will be atomic. dirPath := filepath.Dir(path) tempFile, err := ioutil.TempFile(dirPath, tempPrefix) if err != nil { + log.Print(err) + if os.IsPermission(err) { + if _, err = os.Lstat(path); err == nil { + log.Printf("trying non-atomic overwrite of %q", path) + return m.overwriteDataNonAtomic(path, data) + } + return &ErrNoCreatePermission{m} + } return err } defer os.Remove(tempFile.Name()) - // TempFile() creates the file with mode 0600. Change it to 0644. - if err = tempFile.Chmod(filePermissions); err != nil { + // Ensure the new file has the right permissions mask. + if err = tempFile.Chmod(mode); err != nil { tempFile.Close() return err } + // Override the file owner if one was specified. This happens when root + // needs to create files owned by a particular user. if owner != nil { if err = util.Chown(tempFile, owner); err != nil { log.Printf("could not set owner of %q to %v: %v", @@ -492,29 +695,107 @@ func (m *Mount) addMetadata(path string, md metadata.Metadata, owner *user.User) return err } - log.Printf("writing metadata to %q", path) - return m.writeDataAtomic(path, data, owner) + mode := filePermissions + // If the file already exists, then preserve its owner and mode if + // possible. This is necessary because by default, for atomicity + // reasons we'll replace the file rather than overwrite it. + info, err := os.Lstat(path) + if err == nil { + if owner == nil && util.IsUserRoot() { + uid := info.Sys().(*syscall.Stat_t).Uid + if owner, err = util.UserFromUID(int64(uid)); err != nil { + log.Print(err) + } + } + mode = info.Mode() & 0777 + } else if !os.IsNotExist(err) { + log.Print(err) + } + + if owner != nil { + log.Printf("writing metadata to %q and setting owner to %s", path, owner.Username) + } else { + log.Printf("writing metadata to %q", path) + } + return m.writeData(path, data, owner, mode) +} + +// readMetadataFileSafe gets the contents of a metadata file extra-carefully, +// considering that it could be a malicious file created to cause a +// denial-of-service. Specifically, the following checks are done: +// +// - It must be a regular file, not another type of file like a symlink or FIFO. +// (Symlinks aren't bad by themselves, but given that a malicious user could +// point one to absolutely anywhere, and there is no known use case for the +// metadata files themselves being symlinks, it seems best to disallow them.) +// - It must have a reasonable size (<= maxMetadataFileSize). +// - If trustedUser is non-nil, then the file must be owned by the given user +// or by root. +// +// Take care to avoid TOCTOU (time-of-check-time-of-use) bugs when doing these +// tests. Notably, we must open the file before checking the file type, as the +// file type could change between any previous checks and the open. When doing +// this, O_NOFOLLOW is needed to avoid following a symlink (this applies to the +// last path component only), and O_NONBLOCK is needed to avoid blocking if the +// file is a FIFO. +// +// This function returns the data read as well as the UID of the user who owns +// the file. The returned UID is needed for login protectors, where the UID +// needs to be cross-checked with the UID stored in the file itself. +func readMetadataFileSafe(path string, trustedUser *user.User) ([]byte, int64, error) { + file, err := os.OpenFile(path, os.O_RDONLY|unix.O_NOFOLLOW|unix.O_NONBLOCK, 0) + if err != nil { + return nil, -1, err + } + defer file.Close() + + info, err := file.Stat() + if err != nil { + return nil, -1, err + } + if !info.Mode().IsRegular() { + return nil, -1, &ErrCorruptMetadata{path, errors.New("not a regular file")} + } + if !checkOwnership(path, info, trustedUser) { + return nil, -1, &ErrCorruptMetadata{path, errors.New("metadata file belongs to another user")} + } + // Clear O_NONBLOCK, since it has served its purpose when opening the + // file, and the behavior of reading from a regular file with O_NONBLOCK + // is technically unspecified. + if _, err = unix.FcntlInt(file.Fd(), unix.F_SETFL, 0); err != nil { + return nil, -1, &os.PathError{Op: "clearing O_NONBLOCK", Path: path, Err: err} + } + // Read the file contents, allowing at most maxMetadataFileSize bytes. + reader := &io.LimitedReader{R: file, N: maxMetadataFileSize + 1} + data, err := ioutil.ReadAll(reader) + if err != nil { + return nil, -1, err + } + if reader.N == 0 { + return nil, -1, &ErrCorruptMetadata{path, errors.New("metadata file size limit exceeded")} + } + return data, int64(info.Sys().(*syscall.Stat_t).Uid), nil } // getMetadata reads the metadata structure from the file with the specified // path. Only reads normal metadata files, not linked metadata. -func (m *Mount) getMetadata(path string, md metadata.Metadata) error { - data, err := ioutil.ReadFile(path) +func (m *Mount) getMetadata(path string, trustedUser *user.User, md metadata.Metadata) (int64, error) { + data, owner, err := readMetadataFileSafe(path, trustedUser) if err != nil { log.Printf("could not read metadata from %q: %v", path, err) - return err + return -1, err } if err := proto.Unmarshal(data, md); err != nil { - return &ErrCorruptMetadata{path, err} + return -1, &ErrCorruptMetadata{path, err} } if err := md.CheckValidity(); err != nil { - return &ErrCorruptMetadata{path, err} + return -1, &ErrCorruptMetadata{path, err} } log.Printf("successfully read metadata from %q", path) - return nil + return owner, nil } // removeMetadata deletes the metadata struct from the file with the specified @@ -533,9 +814,9 @@ func (m *Mount) removeMetadata(path string) error { // will overwrite the value of an existing protector with this descriptor. This // will fail with ErrLinkedProtector if a linked protector with this descriptor // already exists on the filesystem. -func (m *Mount) AddProtector(data *metadata.ProtectorData) error { +func (m *Mount) AddProtector(data *metadata.ProtectorData, owner *user.User) error { var err error - if err = m.CheckSetup(); err != nil { + if err = m.CheckSetup(nil); err != nil { return err } if isRegularFile(m.linkedProtectorPath(data.ProtectorDescriptor)) { @@ -543,33 +824,26 @@ func (m *Mount) AddProtector(data *metadata.ProtectorData) error { data.ProtectorDescriptor, m.Path) } path := m.protectorPath(data.ProtectorDescriptor) - - var owner *user.User - if data.Source == metadata.SourceType_pam_passphrase && util.IsUserRoot() { - owner, err = util.UserFromUID(data.Uid) - if err != nil { - return err - } - } return m.addMetadata(path, data, owner) } // AddLinkedProtector adds a link in this filesystem to the protector metadata // in the dest filesystem, if one doesn't already exist. On success, the return // value is a nil error and a bool that is true iff the link is newly created. -func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount) (bool, error) { - if err := m.CheckSetup(); err != nil { +func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount, trustedUser *user.User, + ownerIfCreating *user.User) (bool, error) { + if err := m.CheckSetup(trustedUser); err != nil { return false, err } // Check that the link is good (descriptor exists, filesystem has UUID). - if _, err := dest.GetRegularProtector(descriptor); err != nil { + if _, err := dest.GetRegularProtector(descriptor, trustedUser); err != nil { return false, err } linkPath := m.linkedProtectorPath(descriptor) // Check whether the link already exists. - existingLink, err := ioutil.ReadFile(linkPath) + existingLink, _, err := readMetadataFileSafe(linkPath, trustedUser) if err == nil { existingLinkedMnt, err := getMountFromLink(string(existingLink)) if err != nil { @@ -590,38 +864,52 @@ func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount) (bool, error) if err != nil { return false, err } - return true, m.writeDataAtomic(linkPath, []byte(newLink), nil) + return true, m.writeData(linkPath, []byte(newLink), ownerIfCreating, filePermissions) } // GetRegularProtector looks up the protector metadata by descriptor. This will -// fail with ErrNoMetadata if the descriptor is a linked protector. -func (m *Mount) GetRegularProtector(descriptor string) (*metadata.ProtectorData, error) { - if err := m.CheckSetup(); err != nil { +// fail with ErrProtectorNotFound if the descriptor is a linked protector. +func (m *Mount) GetRegularProtector(descriptor string, trustedUser *user.User) (*metadata.ProtectorData, error) { + if err := m.CheckSetup(trustedUser); err != nil { return nil, err } data := new(metadata.ProtectorData) path := m.protectorPath(descriptor) - err := m.getMetadata(path, data) + owner, err := m.getMetadata(path, trustedUser, data) if os.IsNotExist(err) { err = &ErrProtectorNotFound{descriptor, m} } - return data, err + if err != nil { + return nil, err + } + // Login protectors have their UID stored in the file. Since normally + // any user can create files in the fscrypt metadata directories, for a + // login protector to be considered valid it *must* be owned by the + // claimed user or by root. Note: fscrypt v0.3.2 and later always makes + // login protectors owned by the user, but previous versions could + // create them owned by root -- that is the main reason we allow root. + if data.Source == metadata.SourceType_pam_passphrase && owner != 0 && owner != data.Uid { + log.Printf("WARNING: %q claims to be the login protector for uid %d, but it is owned by uid %d. Needs to be %d or 0.", + path, data.Uid, owner, data.Uid) + return nil, &ErrCorruptMetadata{path, errors.New("login protector belongs to wrong user")} + } + return data, nil } // GetProtector returns the Mount of the filesystem containing the information // and that protector's data. If the descriptor is a regular (not linked) // protector, the mount will return itself. -func (m *Mount) GetProtector(descriptor string) (*Mount, *metadata.ProtectorData, error) { - if err := m.CheckSetup(); err != nil { +func (m *Mount) GetProtector(descriptor string, trustedUser *user.User) (*Mount, *metadata.ProtectorData, error) { + if err := m.CheckSetup(trustedUser); err != nil { return nil, nil, err } // Get the link data from the link file path := m.linkedProtectorPath(descriptor) - link, err := ioutil.ReadFile(path) + link, _, err := readMetadataFileSafe(path, trustedUser) if err != nil { // If the link doesn't exist, try for a regular protector. if os.IsNotExist(err) { - data, err := m.GetRegularProtector(descriptor) + data, err := m.GetRegularProtector(descriptor, trustedUser) return m, data, err } return nil, nil, err @@ -631,7 +919,7 @@ func (m *Mount) GetProtector(descriptor string) (*Mount, *metadata.ProtectorData if err != nil { return nil, nil, errors.Wrap(err, path) } - data, err := linkedMnt.GetRegularProtector(descriptor) + data, err := linkedMnt.GetRegularProtector(descriptor, trustedUser) if err != nil { return nil, nil, &ErrFollowLink{string(link), err} } @@ -641,7 +929,7 @@ func (m *Mount) GetProtector(descriptor string) (*Mount, *metadata.ProtectorData // RemoveProtector deletes the protector metadata (or a link to another // filesystem's metadata) from the filesystem storage. func (m *Mount) RemoveProtector(descriptor string) error { - if err := m.CheckSetup(); err != nil { + if err := m.CheckSetup(nil); err != nil { return err } // We first try to remove the linkedProtector. If that metadata does not @@ -657,30 +945,28 @@ func (m *Mount) RemoveProtector(descriptor string) error { } // ListProtectors lists the descriptors of all protectors on this filesystem. -// This does not include linked protectors. -func (m *Mount) ListProtectors() ([]string, error) { - if err := m.CheckSetup(); err != nil { - return nil, err - } - return m.listDirectory(m.ProtectorDir()) +// This does not include linked protectors. If trustedUser is non-nil, then +// the protectors are restricted to those owned by the given user or by root. +func (m *Mount) ListProtectors(trustedUser *user.User) ([]string, error) { + return m.listMetadata(m.ProtectorDir(), "protectors", trustedUser) } // AddPolicy adds the policy metadata to the filesystem storage. -func (m *Mount) AddPolicy(data *metadata.PolicyData) error { - if err := m.CheckSetup(); err != nil { +func (m *Mount) AddPolicy(data *metadata.PolicyData, owner *user.User) error { + if err := m.CheckSetup(nil); err != nil { return err } - return m.addMetadata(m.PolicyPath(data.KeyDescriptor), data, nil) + return m.addMetadata(m.PolicyPath(data.KeyDescriptor), data, owner) } // GetPolicy looks up the policy metadata by descriptor. -func (m *Mount) GetPolicy(descriptor string) (*metadata.PolicyData, error) { - if err := m.CheckSetup(); err != nil { +func (m *Mount) GetPolicy(descriptor string, trustedUser *user.User) (*metadata.PolicyData, error) { + if err := m.CheckSetup(trustedUser); err != nil { return nil, err } data := new(metadata.PolicyData) - err := m.getMetadata(m.PolicyPath(descriptor), data) + _, err := m.getMetadata(m.PolicyPath(descriptor), trustedUser, data) if os.IsNotExist(err) { err = &ErrPolicyNotFound{descriptor, m} } @@ -689,7 +975,7 @@ func (m *Mount) GetPolicy(descriptor string) (*metadata.PolicyData, error) { // RemovePolicy deletes the policy metadata from the filesystem storage. func (m *Mount) RemovePolicy(descriptor string) error { - if err := m.CheckSetup(); err != nil { + if err := m.CheckSetup(nil); err != nil { return err } err := m.removeMetadata(m.PolicyPath(descriptor)) @@ -699,12 +985,11 @@ func (m *Mount) RemovePolicy(descriptor string) error { return err } -// ListPolicies lists the descriptors of all policies on this filesystem. -func (m *Mount) ListPolicies() ([]string, error) { - if err := m.CheckSetup(); err != nil { - return nil, err - } - return m.listDirectory(m.PolicyDir()) +// ListPolicies lists the descriptors of all policies on this filesystem. If +// trustedUser is non-nil, then the policies are restricted to those owned by +// the given user or by root. +func (m *Mount) ListPolicies(trustedUser *user.User) ([]string, error) { + return m.listMetadata(m.PolicyDir(), "policies", trustedUser) } type namesAndTimes struct { @@ -741,7 +1026,6 @@ func sortFileListByLastMtime(directoryPath string, names []string) error { // listDirectory returns a list of descriptors for a metadata directory, // including files which are links to other filesystem's metadata. func (m *Mount) listDirectory(directoryPath string) ([]string, error) { - log.Printf("listing descriptors in %q", directoryPath) dir, err := os.Open(directoryPath) if err != nil { return nil, err @@ -764,7 +1048,41 @@ func (m *Mount) listDirectory(directoryPath string) ([]string, error) { // Be sure to include links as well descriptors = append(descriptors, strings.TrimSuffix(name, linkFileExtension)) } - - log.Printf("found %d descriptor(s)", len(descriptors)) return descriptors, nil } + +func (m *Mount) listMetadata(dirPath string, metadataType string, owner *user.User) ([]string, error) { + log.Printf("listing %s in %q", metadataType, dirPath) + if err := m.CheckSetup(owner); err != nil { + return nil, err + } + names, err := m.listDirectory(dirPath) + if err != nil { + return nil, err + } + filesIgnoredDescription := "" + if owner != nil { + filteredNames := make([]string, 0, len(names)) + uid := uint32(util.AtoiOrPanic(owner.Uid)) + for _, name := range names { + info, err := os.Lstat(filepath.Join(dirPath, name)) + if err != nil { + continue + } + fileUID := info.Sys().(*syscall.Stat_t).Uid + if fileUID != uid && fileUID != 0 { + continue + } + filteredNames = append(filteredNames, name) + } + numIgnored := len(names) - len(filteredNames) + if numIgnored != 0 { + filesIgnoredDescription = + fmt.Sprintf(" (ignored %d %s not owned by %s or root)", + numIgnored, metadataType, owner.Username) + } + names = filteredNames + } + log.Printf("found %d %s%s", len(names), metadataType, filesIgnoredDescription) + return names, nil +} diff --git a/filesystem/filesystem_test.go b/filesystem/filesystem_test.go index 92726b2..0e15256 100644 --- a/filesystem/filesystem_test.go +++ b/filesystem/filesystem_test.go @@ -21,12 +21,14 @@ package filesystem import ( "io/ioutil" - "log" "os" + "os/user" "path/filepath" + "syscall" "testing" "github.com/golang/protobuf/proto" + "golang.org/x/sys/unix" "github.com/google/fscrypt/crypto" "github.com/google/fscrypt/metadata" @@ -58,6 +60,19 @@ func getFakeProtector() *metadata.ProtectorData { } } +func getFakeLoginProtector(uid int64) *metadata.ProtectorData { + protector := getFakeProtector() + protector.Source = metadata.SourceType_pam_passphrase + protector.Uid = uid + protector.Costs = &metadata.HashingCosts{ + Time: 1, + Memory: 1 << 8, + Parallelism: 1, + } + protector.Salt = make([]byte, 16) + return protector +} + func getFakePolicy() *metadata.PolicyData { return &metadata.PolicyData{ KeyDescriptor: "0123456789abcdef", @@ -77,7 +92,7 @@ func getSetupMount(t *testing.T) (*Mount, error) { if err != nil { return nil, err } - return mnt, mnt.Setup() + return mnt, mnt.Setup(WorldWritable) } // Tests that the setup works and creates the correct files @@ -87,7 +102,7 @@ func TestSetup(t *testing.T) { t.Fatal(err) } - if err := mnt.CheckSetup(); err != nil { + if err := mnt.CheckSetup(nil); err != nil { t.Error(err) } @@ -110,16 +125,6 @@ func TestRemoveAllMetadata(t *testing.T) { } } -// loggedLstat runs os.Lstat (doesn't dereference trailing symlink), but it logs -// the error if lstat returns any error other than nil or IsNotExist. -func loggedLstat(name string) (os.FileInfo, error) { - info, err := os.Lstat(name) - if err != nil && !os.IsNotExist(err) { - log.Print(err) - } - return info, err -} - // isSymlink returns true if the path exists and is that of a symlink. func isSymlink(path string) bool { info, err := loggedLstat(path) @@ -138,11 +143,11 @@ func testSetupWithSymlink(t *testing.T, mnt *Mount, symlinkTarget string, realDi } defer os.Remove(rawBaseDir) - if err := mnt.Setup(); err != nil { + if err := mnt.Setup(WorldWritable); err != nil { t.Fatal(err) } defer mnt.RemoveAllMetadata() - if err := mnt.CheckSetup(); err != nil { + if err := mnt.CheckSetup(nil); err != nil { t.Fatal(err) } if !isSymlink(rawBaseDir) { @@ -188,6 +193,57 @@ func TestSetupWithRelativeSymlink(t *testing.T) { testSetupWithSymlink(t, mnt, ".fscrypt-real", realDir) } +func testSetupMode(t *testing.T, mnt *Mount, setupMode SetupMode, expectedPerms os.FileMode) { + mnt.RemoveAllMetadata() + if err := mnt.Setup(setupMode); err != nil { + t.Fatal(err) + } + dirNames := []string{"policies", "protectors"} + for _, dirName := range dirNames { + fi, err := os.Stat(filepath.Join(mnt.Path, ".fscrypt", dirName)) + if err != nil { + t.Fatal(err) + } + if fi.Mode()&(os.ModeSticky|0777) != expectedPerms { + t.Errorf("directory %s doesn't have permissions %o", dirName, expectedPerms) + } + } +} + +// Tests that the supported setup modes (WorldWritable and SingleUserWritable) +// work as intended. +func TestSetupModes(t *testing.T) { + mnt, err := getTestMount(t) + if err != nil { + t.Fatal(err) + } + defer mnt.RemoveAllMetadata() + testSetupMode(t, mnt, WorldWritable, os.ModeSticky|0777) + testSetupMode(t, mnt, SingleUserWritable, 0755) +} + +// Tests that fscrypt refuses to use metadata directories that are +// world-writable but don't have the sticky bit set. +func TestInsecurePermissions(t *testing.T) { + mnt, err := getTestMount(t) + if err != nil { + t.Fatal(err) + } + defer mnt.RemoveAllMetadata() + + if err = mnt.Setup(WorldWritable); err != nil { + t.Fatal(err) + } + if err = os.Chmod(mnt.PolicyDir(), 0777); err != nil { + t.Fatal(err) + } + defer os.Chmod(mnt.PolicyDir(), os.ModeSticky|0777) + err = mnt.CheckSetup(nil) + if _, ok := err.(*ErrInsecurePermissions); !ok { + t.Fatal("expected ErrInsecurePermissions") + } +} + // Adding a good Protector should succeed, adding a bad one should fail func TestAddProtector(t *testing.T) { mnt, err := getSetupMount(t) @@ -197,31 +253,31 @@ func TestAddProtector(t *testing.T) { defer mnt.RemoveAllMetadata() protector := getFakeProtector() - if err = mnt.AddProtector(protector); err != nil { + if err = mnt.AddProtector(protector, nil); err != nil { t.Error(err) } // Change the source to bad one, or one that requires hashing costs protector.Source = metadata.SourceType_default - if mnt.AddProtector(protector) == nil { + if mnt.AddProtector(protector, nil) == nil { t.Error("bad source for a descriptor should make metadata invalid") } protector.Source = metadata.SourceType_custom_passphrase - if mnt.AddProtector(protector) == nil { + if mnt.AddProtector(protector, nil) == nil { t.Error("protectors using passphrases should require hashing costs") } protector.Source = metadata.SourceType_raw_key // Use a bad wrapped key protector.WrappedKey = wrappedPolicyKey - if mnt.AddProtector(protector) == nil { + if mnt.AddProtector(protector, nil) == nil { t.Error("bad length for protector keys should make metadata invalid") } protector.WrappedKey = wrappedProtectorKey // Change the descriptor (to a bad length) protector.ProtectorDescriptor = "abcde" - if mnt.AddProtector(protector) == nil { + if mnt.AddProtector(protector, nil) == nil { t.Error("bad descriptor length should make metadata invalid") } @@ -236,32 +292,32 @@ func TestAddPolicy(t *testing.T) { defer mnt.RemoveAllMetadata() policy := getFakePolicy() - if err = mnt.AddPolicy(policy); err != nil { + if err = mnt.AddPolicy(policy, nil); err != nil { t.Error(err) } // Bad encryption options should make policy invalid policy.Options.Padding = 7 - if mnt.AddPolicy(policy) == nil { + if mnt.AddPolicy(policy, nil) == nil { t.Error("padding not a power of 2 should make metadata invalid") } policy.Options.Padding = 16 policy.Options.Filenames = metadata.EncryptionOptions_default - if mnt.AddPolicy(policy) == nil { + if mnt.AddPolicy(policy, nil) == nil { t.Error("encryption mode not set should make metadata invalid") } policy.Options.Filenames = metadata.EncryptionOptions_AES_256_CTS // Use a bad wrapped key policy.WrappedPolicyKeys[0].WrappedKey = wrappedProtectorKey - if mnt.AddPolicy(policy) == nil { + if mnt.AddPolicy(policy, nil) == nil { t.Error("bad length for policy keys should make metadata invalid") } policy.WrappedPolicyKeys[0].WrappedKey = wrappedPolicyKey // Change the descriptor (to a bad length) policy.KeyDescriptor = "abcde" - if mnt.AddPolicy(policy) == nil { + if mnt.AddPolicy(policy, nil) == nil { t.Error("bad descriptor length should make metadata invalid") } } @@ -275,11 +331,11 @@ func TestSetPolicy(t *testing.T) { defer mnt.RemoveAllMetadata() policy := getFakePolicy() - if err = mnt.AddPolicy(policy); err != nil { + if err = mnt.AddPolicy(policy, nil); err != nil { t.Fatal(err) } - realPolicy, err := mnt.GetPolicy(policy.KeyDescriptor) + realPolicy, err := mnt.GetPolicy(policy.KeyDescriptor, nil) if err != nil { t.Fatal(err) } @@ -299,11 +355,11 @@ func TestSetProtector(t *testing.T) { defer mnt.RemoveAllMetadata() protector := getFakeProtector() - if err = mnt.AddProtector(protector); err != nil { + if err = mnt.AddProtector(protector, nil); err != nil { t.Fatal(err) } - realProtector, err := mnt.GetRegularProtector(protector.ProtectorDescriptor) + realProtector, err := mnt.GetRegularProtector(protector.ProtectorDescriptor, nil) if err != nil { t.Fatal(err) } @@ -313,6 +369,85 @@ func TestSetProtector(t *testing.T) { } } +// Tests that a login protector whose embedded UID doesn't match the file owner +// is considered invalid. (Such a file could be created by a malicious user to +// try to confuse fscrypt into processing the wrong file.) +func TestSpoofedLoginProtector(t *testing.T) { + myUID := int64(os.Geteuid()) + badUID := myUID + 1 // anything different from myUID + mnt, err := getSetupMount(t) + if err != nil { + t.Fatal(err) + } + defer mnt.RemoveAllMetadata() + + // Control case: protector with matching UID should be accepted. + protector := getFakeLoginProtector(myUID) + if err = mnt.AddProtector(protector, nil); err != nil { + t.Fatal(err) + } + _, err = mnt.GetRegularProtector(protector.ProtectorDescriptor, nil) + if err != nil { + t.Fatal(err) + } + if err = mnt.RemoveProtector(protector.ProtectorDescriptor); err != nil { + t.Fatal(err) + } + + // The real test: protector with mismatching UID should rejected, + // *unless* the process running the tests (and hence the file owner) is + // root in which case it should be accepted. + protector = getFakeLoginProtector(badUID) + if err = mnt.AddProtector(protector, nil); err != nil { + t.Fatal(err) + } + _, err = mnt.GetRegularProtector(protector.ProtectorDescriptor, nil) + if myUID == 0 { + if err != nil { + t.Fatal(err) + } + } else { + if err == nil { + t.Fatal("reading protector with bad UID unexpectedly succeeded") + } + } +} + +// Tests that the fscrypt metadata files are given mode 0600. +func TestMetadataFileMode(t *testing.T) { + mnt, err := getSetupMount(t) + if err != nil { + t.Fatal(err) + } + defer mnt.RemoveAllMetadata() + + // Policy + policy := getFakePolicy() + if err = mnt.AddPolicy(policy, nil); err != nil { + t.Fatal(err) + } + fi, err := os.Stat(filepath.Join(mnt.Path, ".fscrypt/policies/", policy.KeyDescriptor)) + if err != nil { + t.Fatal(err) + } + if fi.Mode()&0777 != 0600 { + t.Error("Policy file has wrong mode") + } + + // Protector + protector := getFakeProtector() + if err = mnt.AddProtector(protector, nil); err != nil { + t.Fatal(err) + } + fi, err = os.Stat(filepath.Join(mnt.Path, ".fscrypt/protectors", protector.ProtectorDescriptor)) + if err != nil { + t.Fatal(err) + } + if fi.Mode()&0777 != 0600 { + t.Error("Protector file has wrong mode") + } +} + // Gets a setup mount and a fake second mount func getTwoSetupMounts(t *testing.T) (realMnt, fakeMnt *Mount, err error) { if realMnt, err = getSetupMount(t); err != nil { @@ -325,7 +460,7 @@ func getTwoSetupMounts(t *testing.T) (realMnt, fakeMnt *Mount, err error) { return } fakeMnt = &Mount{Path: fakeMountpoint, FilesystemType: realMnt.FilesystemType} - err = fakeMnt.Setup() + err = fakeMnt.Setup(WorldWritable) return } @@ -345,19 +480,19 @@ func TestLinkedProtector(t *testing.T) { // Add the protector to the first filesystem protector := getFakeProtector() - if err = realMnt.AddProtector(protector); err != nil { + if err = realMnt.AddProtector(protector, nil); err != nil { t.Fatal(err) } // Add the link to the second filesystem var isNewLink bool - if isNewLink, err = fakeMnt.AddLinkedProtector(protector.ProtectorDescriptor, realMnt); err != nil { + if isNewLink, err = fakeMnt.AddLinkedProtector(protector.ProtectorDescriptor, realMnt, nil, nil); err != nil { t.Fatal(err) } if !isNewLink { t.Fatal("Link was not new") } - if isNewLink, err = fakeMnt.AddLinkedProtector(protector.ProtectorDescriptor, realMnt); err != nil { + if isNewLink, err = fakeMnt.AddLinkedProtector(protector.ProtectorDescriptor, realMnt, nil, nil); err != nil { t.Fatal(err) } if isNewLink { @@ -365,12 +500,12 @@ func TestLinkedProtector(t *testing.T) { } // Get the protector though the second system - _, err = fakeMnt.GetRegularProtector(protector.ProtectorDescriptor) + _, err = fakeMnt.GetRegularProtector(protector.ProtectorDescriptor, nil) if _, ok := err.(*ErrProtectorNotFound); !ok { t.Fatal(err) } - retMnt, retProtector, err := fakeMnt.GetProtector(protector.ProtectorDescriptor) + retMnt, retProtector, err := fakeMnt.GetProtector(protector.ProtectorDescriptor, nil) if err != nil { t.Fatal(err) } @@ -382,3 +517,95 @@ func TestLinkedProtector(t *testing.T) { t.Errorf("protector %+v does not equal expected protector %+v", retProtector, protector) } } + +func createFile(path string, size int64) error { + if err := ioutil.WriteFile(path, []byte{}, 0600); err != nil { + return err + } + return os.Truncate(path, size) +} + +// Tests the readMetadataFileSafe() function. +func TestReadMetadataFileSafe(t *testing.T) { + currentUser, err := util.EffectiveUser() + otherUser := &user.User{Uid: "-1"} + if err != nil { + t.Fatal(err) + } + tempDir, err := ioutil.TempDir("", "fscrypt") + if err != nil { + t.Fatal(err) + } + filePath := filepath.Join(tempDir, "file") + defer os.RemoveAll(tempDir) + + // Good file (control case) + if err = createFile(filePath, 1000); err != nil { + t.Fatal(err) + } + _, owner, err := readMetadataFileSafe(filePath, nil) + if err != nil { + t.Fatal("failed to read file") + } + if owner != int64(os.Geteuid()) { + t.Fatal("got wrong owner") + } + // Also try it with the trustedUser argument set to the current user. + if _, _, err = readMetadataFileSafe(filePath, currentUser); err != nil { + t.Fatal("failed to read file") + } + os.Remove(filePath) + + // File owned by another user. We might not have permission to actually + // change the file's ownership, so we simulate this by passing in a bad + // value for the trustedUser argument. + if err = createFile(filePath, 1000); err != nil { + t.Fatal(err) + } + _, _, err = readMetadataFileSafe(filePath, otherUser) + if util.IsUserRoot() { + if err != nil { + t.Fatal("root-owned file didn't pass owner validation") + } + } else { + if err == nil { + t.Fatal("unexpectedly could read file owned by another user") + } + } + os.Remove(filePath) + + // Nonexistent file + _, _, err = readMetadataFileSafe(filePath, nil) + if !os.IsNotExist(err) { + t.Fatal("trying to read nonexistent file didn't fail with expected error") + } + + // Symlink + if err = os.Symlink("target", filePath); err != nil { + t.Fatal(err) + } + _, _, err = readMetadataFileSafe(filePath, nil) + if err.(*os.PathError).Err != syscall.ELOOP { + t.Fatal("trying to read symlink didn't fail with ELOOP") + } + os.Remove(filePath) + + // FIFO + if err = unix.Mkfifo(filePath, 0600); err != nil { + t.Fatal(err) + } + _, _, err = readMetadataFileSafe(filePath, nil) + if _, ok := err.(*ErrCorruptMetadata); !ok { + t.Fatal("trying to read FIFO didn't fail with expected error") + } + os.Remove(filePath) + + // Very large file + if err = createFile(filePath, 1000000); err != nil { + t.Fatal(err) + } + _, _, err = readMetadataFileSafe(filePath, nil) + if _, ok := err.(*ErrCorruptMetadata); !ok { + t.Fatal("trying to read very large file didn't fail with expected error") + } +} diff --git a/filesystem/mountpoint.go b/filesystem/mountpoint.go index 182cafa..0b0693b 100644 --- a/filesystem/mountpoint.go +++ b/filesystem/mountpoint.go @@ -77,6 +77,21 @@ func unescapeString(str string) string { return sb.String() } +// EscapeString is the reverse of unescapeString. Use this to avoid injecting +// spaces or newlines into output that uses these characters as separators. +func EscapeString(str string) string { + var sb strings.Builder + for _, b := range []byte(str) { + switch b { + case ' ', '\t', '\n', '\\': + sb.WriteString(fmt.Sprintf("\\%03o", b)) + default: + sb.WriteByte(b) + } + } + return sb.String() +} + // We get the device name via the device number rather than use the mount source // field directly. This is necessary to handle a rootfs that was mounted via // the kernel command line, since mountinfo always shows /dev/root for that. diff --git a/filesystem/mountpoint_test.go b/filesystem/mountpoint_test.go index 749e5e3..99d4b55 100644 --- a/filesystem/mountpoint_test.go +++ b/filesystem/mountpoint_test.go @@ -179,6 +179,21 @@ func TestLoadMountWithSpecialCharacters(t *testing.T) { } } +// Tests the EscapeString() and unescapeString() functions. +func TestStringEscaping(t *testing.T) { + charsNeedEscaping := " \t\n\\" + charsDontNeedEscaping := "ABCDEF\u2603\xff\xff\v" + + orig := charsNeedEscaping + charsDontNeedEscaping + escaped := `\040\011\012\134` + charsDontNeedEscaping + if EscapeString(orig) != escaped { + t.Fatal("EscapeString gave wrong result") + } + if unescapeString(escaped) != orig { + t.Fatal("unescapeString gave wrong result") + } +} + // Test parsing some invalid mountinfo lines. func TestLoadBadMountInfo(t *testing.T) { mountinfos := []string{"a", diff --git a/filesystem/path.go b/filesystem/path.go index fa38701..8cfb235 100644 --- a/filesystem/path.go +++ b/filesystem/path.go @@ -38,9 +38,6 @@ func OpenFileOverridingUmask(name string, flag int, perm os.FileMode) (*os.File, return os.OpenFile(name, flag, perm) } -// We only check the unix permissions and the sticky bit -const permMask = os.ModeSticky | os.ModePerm - // canonicalizePath turns path into an absolute path without symlinks. func canonicalizePath(path string) (string, error) { path, err := filepath.Abs(path) @@ -67,28 +64,22 @@ func loggedStat(name string) (os.FileInfo, error) { return info, err } +// loggedLstat runs os.Lstat (doesn't dereference trailing symlink), but it logs +// the error if lstat returns any error other than nil or IsNotExist. +func loggedLstat(name string) (os.FileInfo, error) { + info, err := os.Lstat(name) + if err != nil && !os.IsNotExist(err) { + log.Print(err) + } + return info, err +} + // isDir returns true if the path exists and is that of a directory. func isDir(path string) bool { info, err := loggedStat(path) return err == nil && info.IsDir() } -// isDirCheckPerm returns true if the path exists and is a directory. If the -// specified permissions and sticky bit of mode do not match the path, an error -// is logged. -func isDirCheckPerm(path string, mode os.FileMode) bool { - info, err := loggedStat(path) - // Check if directory - if err != nil || !info.IsDir() { - return false - } - // Check for bad permissions - if info.Mode()&permMask != mode&permMask { - log.Printf("directory %s has incorrect permissions", path) - } - return true -} - // isRegularFile returns true if the path exists and is that of a regular file. func isRegularFile(path string) bool { info, err := loggedStat(path) diff --git a/metadata/config_test.go b/metadata/config_test.go index 52f83f2..2874bb8 100644 --- a/metadata/config_test.go +++ b/metadata/config_test.go @@ -49,7 +49,8 @@ var testConfigString = `{ "filenames": "AES_256_CTS", "policy_version": "1" }, - "use_fs_keyring_for_v1_policies": false + "use_fs_keyring_for_v1_policies": false, + "allow_cross_user_metadata": false } ` diff --git a/metadata/metadata.pb.go b/metadata/metadata.pb.go index a2148ce..6709804 100644 --- a/metadata/metadata.pb.go +++ b/metadata/metadata.pb.go @@ -45,7 +45,7 @@ func (x SourceType) String() string { return proto.EnumName(SourceType_name, int32(x)) } func (SourceType) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_metadata_20fa0d9b7a38c428, []int{0} + return fileDescriptor_metadata_31965d2849cb292a, []int{0} } // Type of encryption; should match declarations of unix.FSCRYPT_MODE @@ -87,7 +87,7 @@ func (x EncryptionOptions_Mode) String() string { return proto.EnumName(EncryptionOptions_Mode_name, int32(x)) } func (EncryptionOptions_Mode) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_metadata_20fa0d9b7a38c428, []int{3, 0} + return fileDescriptor_metadata_31965d2849cb292a, []int{3, 0} } // Cost parameters to be used in our hashing functions. @@ -104,7 +104,7 @@ func (m *HashingCosts) Reset() { *m = HashingCosts{} } func (m *HashingCosts) String() string { return proto.CompactTextString(m) } func (*HashingCosts) ProtoMessage() {} func (*HashingCosts) Descriptor() ([]byte, []int) { - return fileDescriptor_metadata_20fa0d9b7a38c428, []int{0} + return fileDescriptor_metadata_31965d2849cb292a, []int{0} } func (m *HashingCosts) XXX_Unmarshal(b []byte) error { return xxx_messageInfo_HashingCosts.Unmarshal(m, b) @@ -159,7 +159,7 @@ func (m *WrappedKeyData) Reset() { *m = WrappedKeyData{} } func (m *WrappedKeyData) String() string { return proto.CompactTextString(m) } func (*WrappedKeyData) ProtoMessage() {} func (*WrappedKeyData) Descriptor() ([]byte, []int) { - return fileDescriptor_metadata_20fa0d9b7a38c428, []int{1} + return fileDescriptor_metadata_31965d2849cb292a, []int{1} } func (m *WrappedKeyData) XXX_Unmarshal(b []byte) error { return xxx_messageInfo_WrappedKeyData.Unmarshal(m, b) @@ -219,7 +219,7 @@ func (m *ProtectorData) Reset() { *m = ProtectorData{} } func (m *ProtectorData) String() string { return proto.CompactTextString(m) } func (*ProtectorData) ProtoMessage() {} func (*ProtectorData) Descriptor() ([]byte, []int) { - return fileDescriptor_metadata_20fa0d9b7a38c428, []int{2} + return fileDescriptor_metadata_31965d2849cb292a, []int{2} } func (m *ProtectorData) XXX_Unmarshal(b []byte) error { return xxx_messageInfo_ProtectorData.Unmarshal(m, b) @@ -303,7 +303,7 @@ func (m *EncryptionOptions) Reset() { *m = EncryptionOptions{} } func (m *EncryptionOptions) String() string { return proto.CompactTextString(m) } func (*EncryptionOptions) ProtoMessage() {} func (*EncryptionOptions) Descriptor() ([]byte, []int) { - return fileDescriptor_metadata_20fa0d9b7a38c428, []int{3} + return fileDescriptor_metadata_31965d2849cb292a, []int{3} } func (m *EncryptionOptions) XXX_Unmarshal(b []byte) error { return xxx_messageInfo_EncryptionOptions.Unmarshal(m, b) @@ -363,7 +363,7 @@ func (m *WrappedPolicyKey) Reset() { *m = WrappedPolicyKey{} } func (m *WrappedPolicyKey) String() string { return proto.CompactTextString(m) } func (*WrappedPolicyKey) ProtoMessage() {} func (*WrappedPolicyKey) Descriptor() ([]byte, []int) { - return fileDescriptor_metadata_20fa0d9b7a38c428, []int{4} + return fileDescriptor_metadata_31965d2849cb292a, []int{4} } func (m *WrappedPolicyKey) XXX_Unmarshal(b []byte) error { return xxx_messageInfo_WrappedPolicyKey.Unmarshal(m, b) @@ -411,7 +411,7 @@ func (m *PolicyData) Reset() { *m = PolicyData{} } func (m *PolicyData) String() string { return proto.CompactTextString(m) } func (*PolicyData) ProtoMessage() {} func (*PolicyData) Descriptor() ([]byte, []int) { - return fileDescriptor_metadata_20fa0d9b7a38c428, []int{5} + return fileDescriptor_metadata_31965d2849cb292a, []int{5} } func (m *PolicyData) XXX_Unmarshal(b []byte) error { return xxx_messageInfo_PolicyData.Unmarshal(m, b) @@ -458,6 +458,7 @@ type Config struct { HashCosts *HashingCosts `protobuf:"bytes,2,opt,name=hash_costs,json=hashCosts,proto3" json:"hash_costs,omitempty"` Options *EncryptionOptions `protobuf:"bytes,4,opt,name=options,proto3" json:"options,omitempty"` UseFsKeyringForV1Policies bool `protobuf:"varint,5,opt,name=use_fs_keyring_for_v1_policies,json=useFsKeyringForV1Policies,proto3" json:"use_fs_keyring_for_v1_policies,omitempty"` + AllowCrossUserMetadata bool `protobuf:"varint,6,opt,name=allow_cross_user_metadata,json=allowCrossUserMetadata,proto3" json:"allow_cross_user_metadata,omitempty"` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` @@ -467,7 +468,7 @@ func (m *Config) Reset() { *m = Config{} } func (m *Config) String() string { return proto.CompactTextString(m) } func (*Config) ProtoMessage() {} func (*Config) Descriptor() ([]byte, []int) { - return fileDescriptor_metadata_20fa0d9b7a38c428, []int{6} + return fileDescriptor_metadata_31965d2849cb292a, []int{6} } func (m *Config) XXX_Unmarshal(b []byte) error { return xxx_messageInfo_Config.Unmarshal(m, b) @@ -515,6 +516,13 @@ func (m *Config) GetUseFsKeyringForV1Policies() bool { return false } +func (m *Config) GetAllowCrossUserMetadata() bool { + if m != nil { + return m.AllowCrossUserMetadata + } + return false +} + func init() { proto.RegisterType((*HashingCosts)(nil), "metadata.HashingCosts") proto.RegisterType((*WrappedKeyData)(nil), "metadata.WrappedKeyData") @@ -527,53 +535,55 @@ func init() { proto.RegisterEnum("metadata.EncryptionOptions_Mode", EncryptionOptions_Mode_name, EncryptionOptions_Mode_value) } -func init() { proto.RegisterFile("metadata/metadata.proto", fileDescriptor_metadata_20fa0d9b7a38c428) } +func init() { proto.RegisterFile("metadata/metadata.proto", fileDescriptor_metadata_31965d2849cb292a) } -var fileDescriptor_metadata_20fa0d9b7a38c428 = []byte{ - // 716 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x9c, 0x54, 0xdd, 0x6a, 0xdb, 0x48, - 0x14, 0x5e, 0x49, 0x8e, 0x7f, 0x8e, 0x7f, 0x56, 0x99, 0x64, 0xb3, 0xda, 0x5d, 0x58, 0x8c, 0x97, - 0x40, 0x58, 0x42, 0x16, 0x7b, 0x49, 0x69, 0xa1, 0x14, 0x52, 0x27, 0x69, 0x93, 0x10, 0x9a, 0x8e, - 0x8d, 0xdb, 0x42, 0x41, 0x4c, 0xa4, 0xb1, 0x3d, 0x58, 0xd2, 0x88, 0x99, 0x71, 0x8c, 0xee, 0x7a, - 0xd7, 0x07, 0xe8, 0xbb, 0xf4, 0x69, 0xfa, 0x28, 0xbd, 0x28, 0x1a, 0xc9, 0x7f, 0x09, 0x84, 0xa4, - 0x37, 0xe2, 0x9c, 0x6f, 0xce, 0xef, 0x77, 0xce, 0x11, 0xfc, 0x1e, 0x52, 0x45, 0x7c, 0xa2, 0xc8, - 0x7f, 0x73, 0xe1, 0x20, 0x16, 0x5c, 0x71, 0x54, 0x9e, 0xeb, 0xad, 0x8f, 0x50, 0x7b, 0x4d, 0xe4, - 0x98, 0x45, 0xa3, 0x2e, 0x97, 0x4a, 0x22, 0x04, 0x05, 0xc5, 0x42, 0xea, 0x98, 0x4d, 0x63, 0xcf, - 0xc2, 0x5a, 0x46, 0x3b, 0x50, 0x0c, 0x69, 0xc8, 0x45, 0xe2, 0x58, 0x1a, 0xcd, 0x35, 0xd4, 0x84, - 0x6a, 0x4c, 0x04, 0x09, 0x02, 0x1a, 0x30, 0x19, 0x3a, 0x05, 0xfd, 0xb8, 0x0a, 0xb5, 0x3e, 0x40, - 0xe3, 0x9d, 0x20, 0x71, 0x4c, 0xfd, 0x0b, 0x9a, 0x1c, 0x13, 0x45, 0x50, 0x03, 0xcc, 0xb3, 0x81, - 0x63, 0x34, 0x8d, 0xbd, 0x1a, 0x36, 0xcf, 0x06, 0xe8, 0x1f, 0xa8, 0xd3, 0xc8, 0x13, 0x49, 0xac, - 0xa8, 0xef, 0x4e, 0x68, 0xa2, 0x13, 0xd7, 0x70, 0x6d, 0x01, 0x5e, 0xd0, 0x24, 0x2d, 0x6a, 0x1c, - 0x12, 0x4f, 0xa7, 0xaf, 0x61, 0x2d, 0xb7, 0xbe, 0x98, 0x50, 0xbf, 0x12, 0x5c, 0x51, 0x4f, 0x71, - 0xa1, 0x43, 0xb7, 0x61, 0x3b, 0x9e, 0x03, 0xae, 0x4f, 0xa5, 0x27, 0x58, 0xac, 0xb8, 0xd0, 0xc9, - 0x2a, 0x78, 0x6b, 0xf1, 0x76, 0xbc, 0x78, 0x42, 0xfb, 0x50, 0x94, 0x7c, 0x2a, 0xbc, 0xac, 0xdf, - 0x46, 0x67, 0xfb, 0x60, 0x41, 0x54, 0x4f, 0xe3, 0xfd, 0x24, 0xa6, 0x38, 0xb7, 0x49, 0xcb, 0x88, - 0x48, 0x48, 0x75, 0x19, 0x15, 0xac, 0x65, 0xb4, 0x0f, 0x1b, 0x5e, 0x4a, 0x9c, 0xee, 0xbe, 0xda, - 0xd9, 0x59, 0x06, 0x58, 0xa5, 0x15, 0x67, 0x46, 0x69, 0x04, 0x49, 0x02, 0xe5, 0x6c, 0x64, 0x8d, - 0xa4, 0x32, 0xb2, 0xc1, 0x9a, 0x32, 0xdf, 0x29, 0x6a, 0xf6, 0x52, 0x11, 0x3d, 0x83, 0xea, 0x2c, - 0x63, 0x4d, 0x33, 0x52, 0xd2, 0x91, 0x9d, 0x65, 0xe4, 0x75, 0x4a, 0x31, 0xcc, 0x16, 0x7a, 0xeb, - 0x9b, 0x09, 0x9b, 0x27, 0x19, 0x75, 0x8c, 0x47, 0x6f, 0xf4, 0x57, 0x22, 0x07, 0x4a, 0x31, 0xf1, - 0x7d, 0x16, 0x8d, 0x34, 0x19, 0x16, 0x9e, 0xab, 0xe8, 0x39, 0x94, 0x3d, 0x1e, 0x29, 0x1a, 0x29, - 0x99, 0x53, 0xd0, 0x5c, 0xe6, 0xb9, 0x13, 0xe8, 0xe0, 0x92, 0xfb, 0x14, 0x2f, 0x3c, 0xd0, 0x0b, - 0xa8, 0x0c, 0x59, 0x40, 0x53, 0x22, 0xa4, 0x66, 0xe5, 0x21, 0xee, 0x4b, 0x17, 0xb4, 0x0b, 0x8d, - 0x98, 0x07, 0xcc, 0x4b, 0xdc, 0x1b, 0x2a, 0x24, 0xe3, 0x51, 0xbe, 0x43, 0xf5, 0x0c, 0x1d, 0x64, - 0x60, 0xeb, 0xb3, 0x01, 0x85, 0xd4, 0x15, 0x55, 0xa1, 0xe4, 0xd3, 0x21, 0x99, 0x06, 0xca, 0xfe, - 0x05, 0xfd, 0x0a, 0xd5, 0xa3, 0x93, 0x9e, 0xdb, 0x39, 0x7c, 0xe2, 0xbe, 0xef, 0xf7, 0x6c, 0x63, - 0x15, 0x78, 0xd5, 0xbd, 0xb4, 0xcd, 0x55, 0xa0, 0xfb, 0xb2, 0x6b, 0x5b, 0x6b, 0x40, 0xbf, 0x67, - 0x17, 0xe6, 0x40, 0xbb, 0xf3, 0x54, 0x5b, 0x6c, 0xac, 0x01, 0xfd, 0x9e, 0x5d, 0x44, 0x35, 0x28, - 0x1f, 0xf9, 0x8c, 0x44, 0x6a, 0x1a, 0xda, 0x95, 0xd6, 0x27, 0x03, 0xec, 0x9c, 0xfd, 0x2b, 0x5d, - 0x62, 0xba, 0x9d, 0x3f, 0xb1, 0x77, 0xb7, 0x26, 0x6c, 0x3e, 0x62, 0xc2, 0x5f, 0x0d, 0x80, 0x2c, - 0xb7, 0x5e, 0xfa, 0x5d, 0x68, 0x4c, 0x68, 0x72, 0x37, 0x6d, 0x7d, 0x42, 0x93, 0x95, 0x84, 0x87, - 0x50, 0xe2, 0xd9, 0x10, 0xf2, 0x64, 0x7f, 0xdd, 0x33, 0x27, 0x3c, 0xb7, 0x45, 0xe7, 0xb0, 0x35, - 0xaf, 0x33, 0x1f, 0xd4, 0x84, 0x26, 0xe9, 0xa8, 0xad, 0xbd, 0x6a, 0xe7, 0xcf, 0x3b, 0xf5, 0x2e, - 0x38, 0xc1, 0x9b, 0xb3, 0x5b, 0x88, 0x6c, 0x7d, 0x37, 0xa0, 0xd8, 0xe5, 0xd1, 0x90, 0x8d, 0x56, - 0xce, 0xce, 0x78, 0xc0, 0xd9, 0x1d, 0x02, 0x8c, 0x89, 0x1c, 0xbb, 0xd9, 0x9d, 0x99, 0xf7, 0xde, - 0x59, 0x25, 0xb5, 0xcc, 0xfe, 0x64, 0x2b, 0x2d, 0x17, 0x1e, 0xd1, 0xf2, 0x11, 0xfc, 0x3d, 0x95, - 0xd4, 0x1d, 0xca, 0xb4, 0x55, 0xc1, 0xa2, 0x91, 0x3b, 0xe4, 0xc2, 0xbd, 0x69, 0x67, 0x04, 0x30, - 0x2a, 0xf5, 0xf1, 0x96, 0xf1, 0x1f, 0x53, 0x49, 0x4f, 0xe5, 0x45, 0x66, 0x73, 0xca, 0xc5, 0xa0, - 0x7d, 0x95, 0x1b, 0x9c, 0x17, 0xca, 0x96, 0x5d, 0xc0, 0x75, 0x8f, 0x87, 0x31, 0x51, 0xec, 0x9a, - 0x05, 0x4c, 0x25, 0xff, 0xbe, 0x05, 0x58, 0xf6, 0xb6, 0xbe, 0xc9, 0x08, 0x1a, 0x31, 0x09, 0xdd, - 0x98, 0x48, 0x19, 0x8f, 0x05, 0x91, 0xd4, 0x36, 0xd0, 0x6f, 0xb0, 0xe9, 0x4d, 0xa5, 0xe2, 0x6b, - 0xb0, 0x99, 0xfa, 0x09, 0x32, 0x4b, 0x4b, 0xb3, 0xad, 0xeb, 0xa2, 0xfe, 0x99, 0xff, 0xff, 0x23, - 0x00, 0x00, 0xff, 0xff, 0x3d, 0x33, 0x9f, 0x0d, 0xe7, 0x05, 0x00, 0x00, +var fileDescriptor_metadata_31965d2849cb292a = []byte{ + // 748 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x9c, 0x55, 0xdb, 0x6a, 0xf3, 0x46, + 0x10, 0xae, 0x24, 0xc7, 0x87, 0xf1, 0xa1, 0xca, 0xfe, 0x69, 0xaa, 0xb4, 0x50, 0x8c, 0x4b, 0x20, + 0x94, 0x90, 0x62, 0x97, 0x94, 0x06, 0x4a, 0x21, 0x75, 0x92, 0x36, 0x09, 0xa1, 0xe9, 0xda, 0x75, + 0x5b, 0x28, 0x88, 0x8d, 0xb4, 0xb6, 0x17, 0x4b, 0x5a, 0xb1, 0xbb, 0x8a, 0xd1, 0x5d, 0xef, 0xfa, + 0x00, 0x7d, 0x97, 0xf6, 0x65, 0xfa, 0x30, 0x45, 0x2b, 0xc9, 0x87, 0x04, 0x42, 0xf2, 0xdf, 0x98, + 0xd9, 0x6f, 0x67, 0xe6, 0x9b, 0xf9, 0x66, 0xc7, 0x82, 0x8f, 0x43, 0xaa, 0x88, 0x4f, 0x14, 0xf9, + 0xb2, 0x34, 0x4e, 0x62, 0xc1, 0x15, 0x47, 0xf5, 0xf2, 0xdc, 0xfb, 0x03, 0x5a, 0x3f, 0x12, 0x39, + 0x67, 0xd1, 0x6c, 0xc8, 0xa5, 0x92, 0x08, 0x41, 0x45, 0xb1, 0x90, 0x3a, 0x66, 0xd7, 0x38, 0xb2, + 0xb0, 0xb6, 0xd1, 0x3e, 0x54, 0x43, 0x1a, 0x72, 0x91, 0x3a, 0x96, 0x46, 0x8b, 0x13, 0xea, 0x42, + 0x33, 0x26, 0x82, 0x04, 0x01, 0x0d, 0x98, 0x0c, 0x9d, 0x8a, 0xbe, 0xdc, 0x84, 0x7a, 0xbf, 0x43, + 0xe7, 0x57, 0x41, 0xe2, 0x98, 0xfa, 0xb7, 0x34, 0xbd, 0x20, 0x8a, 0xa0, 0x0e, 0x98, 0xd7, 0x13, + 0xc7, 0xe8, 0x1a, 0x47, 0x2d, 0x6c, 0x5e, 0x4f, 0xd0, 0xe7, 0xd0, 0xa6, 0x91, 0x27, 0xd2, 0x58, + 0x51, 0xdf, 0x5d, 0xd0, 0x54, 0x13, 0xb7, 0x70, 0x6b, 0x05, 0xde, 0xd2, 0x34, 0x2b, 0x6a, 0x1e, + 0x12, 0x4f, 0xd3, 0xb7, 0xb0, 0xb6, 0x7b, 0x7f, 0x9b, 0xd0, 0xbe, 0x17, 0x5c, 0x51, 0x4f, 0x71, + 0xa1, 0x53, 0xf7, 0x61, 0x2f, 0x2e, 0x01, 0xd7, 0xa7, 0xd2, 0x13, 0x2c, 0x56, 0x5c, 0x68, 0xb2, + 0x06, 0x7e, 0xb7, 0xba, 0xbb, 0x58, 0x5d, 0xa1, 0x63, 0xa8, 0x4a, 0x9e, 0x08, 0x2f, 0xef, 0xb7, + 0x33, 0xd8, 0x3b, 0x59, 0x09, 0x35, 0xd2, 0xf8, 0x38, 0x8d, 0x29, 0x2e, 0x7c, 0xb2, 0x32, 0x22, + 0x12, 0x52, 0x5d, 0x46, 0x03, 0x6b, 0x1b, 0x1d, 0xc3, 0x8e, 0x97, 0x09, 0xa7, 0xbb, 0x6f, 0x0e, + 0xf6, 0xd7, 0x09, 0x36, 0x65, 0xc5, 0xb9, 0x53, 0x96, 0x41, 0x92, 0x40, 0x39, 0x3b, 0x79, 0x23, + 0x99, 0x8d, 0x6c, 0xb0, 0x12, 0xe6, 0x3b, 0x55, 0xad, 0x5e, 0x66, 0xa2, 0x33, 0x68, 0x2e, 0x73, + 0xd5, 0xb4, 0x22, 0x35, 0x9d, 0xd9, 0x59, 0x67, 0xde, 0x96, 0x14, 0xc3, 0x72, 0x75, 0xee, 0xfd, + 0x67, 0xc2, 0xee, 0x65, 0x2e, 0x1d, 0xe3, 0xd1, 0x4f, 0xfa, 0x57, 0x22, 0x07, 0x6a, 0x31, 0xf1, + 0x7d, 0x16, 0xcd, 0xb4, 0x18, 0x16, 0x2e, 0x8f, 0xe8, 0x5b, 0xa8, 0x7b, 0x3c, 0x52, 0x34, 0x52, + 0xb2, 0x90, 0xa0, 0xbb, 0xe6, 0x79, 0x96, 0xe8, 0xe4, 0x8e, 0xfb, 0x14, 0xaf, 0x22, 0xd0, 0x77, + 0xd0, 0x98, 0xb2, 0x80, 0x66, 0x42, 0x48, 0xad, 0xca, 0x6b, 0xc2, 0xd7, 0x21, 0xe8, 0x10, 0x3a, + 0x31, 0x0f, 0x98, 0x97, 0xba, 0x8f, 0x54, 0x48, 0xc6, 0xa3, 0xe2, 0x0d, 0xb5, 0x73, 0x74, 0x92, + 0x83, 0xbd, 0xbf, 0x0c, 0xa8, 0x64, 0xa1, 0xa8, 0x09, 0x35, 0x9f, 0x4e, 0x49, 0x12, 0x28, 0xfb, + 0x03, 0xf4, 0x21, 0x34, 0xcf, 0x2f, 0x47, 0xee, 0xe0, 0xf4, 0x6b, 0xf7, 0xb7, 0xf1, 0xc8, 0x36, + 0x36, 0x81, 0x1f, 0x86, 0x77, 0xb6, 0xb9, 0x09, 0x0c, 0xbf, 0x1f, 0xda, 0xd6, 0x16, 0x30, 0x1e, + 0xd9, 0x95, 0x12, 0xe8, 0x0f, 0xbe, 0xd1, 0x1e, 0x3b, 0x5b, 0xc0, 0x78, 0x64, 0x57, 0x51, 0x0b, + 0xea, 0xe7, 0x3e, 0x23, 0x91, 0x4a, 0x42, 0xbb, 0xd1, 0xfb, 0xd3, 0x00, 0xbb, 0x50, 0xff, 0x5e, + 0x97, 0x98, 0xbd, 0xce, 0xf7, 0x78, 0x77, 0x4f, 0x26, 0x6c, 0xbe, 0x61, 0xc2, 0xff, 0x18, 0x00, + 0x39, 0xb7, 0x7e, 0xf4, 0x87, 0xd0, 0x59, 0xd0, 0xf4, 0x39, 0x6d, 0x7b, 0x41, 0xd3, 0x0d, 0xc2, + 0x53, 0xa8, 0xf1, 0x7c, 0x08, 0x05, 0xd9, 0xa7, 0x2f, 0xcc, 0x09, 0x97, 0xbe, 0xe8, 0x06, 0xde, + 0x95, 0x75, 0x16, 0x83, 0x5a, 0xd0, 0x34, 0x1b, 0xb5, 0x75, 0xd4, 0x1c, 0x7c, 0xf2, 0xac, 0xde, + 0x95, 0x26, 0x78, 0x77, 0xf9, 0x04, 0x91, 0xbd, 0x7f, 0x4d, 0xa8, 0x0e, 0x79, 0x34, 0x65, 0xb3, + 0x8d, 0xb5, 0x33, 0x5e, 0xb1, 0x76, 0xa7, 0x00, 0x73, 0x22, 0xe7, 0x6e, 0xbe, 0x67, 0xe6, 0x8b, + 0x7b, 0xd6, 0xc8, 0x3c, 0xf3, 0x7f, 0xb2, 0x8d, 0x96, 0x2b, 0x6f, 0x68, 0xf9, 0x1c, 0x3e, 0x4b, + 0x24, 0x75, 0xa7, 0x32, 0x6b, 0x55, 0xb0, 0x68, 0xe6, 0x4e, 0xb9, 0x70, 0x1f, 0xfb, 0xb9, 0x00, + 0x8c, 0x4a, 0xbd, 0xbc, 0x75, 0x7c, 0x90, 0x48, 0x7a, 0x25, 0x6f, 0x73, 0x9f, 0x2b, 0x2e, 0x26, + 0xfd, 0xfb, 0xc2, 0x01, 0x9d, 0xc1, 0x01, 0x09, 0x02, 0xbe, 0x74, 0x3d, 0xc1, 0xa5, 0x74, 0x13, + 0x49, 0x85, 0x5b, 0x52, 0xeb, 0x3d, 0xaf, 0xe3, 0x7d, 0xed, 0x30, 0xcc, 0xee, 0x7f, 0x91, 0x54, + 0xdc, 0x15, 0xb7, 0x37, 0x95, 0xba, 0x65, 0x57, 0x70, 0xdb, 0xe3, 0x61, 0x4c, 0x14, 0x7b, 0x60, + 0x01, 0x53, 0xe9, 0x17, 0x3f, 0x03, 0xac, 0x65, 0xd9, 0x5e, 0x02, 0x04, 0x9d, 0x98, 0x84, 0x6e, + 0x4c, 0xa4, 0x8c, 0xe7, 0x82, 0x48, 0x6a, 0x1b, 0xe8, 0x23, 0xd8, 0xf5, 0x12, 0xa9, 0xf8, 0x16, + 0x6c, 0x66, 0x71, 0x82, 0x2c, 0xb3, 0xae, 0x6c, 0xeb, 0xa1, 0xaa, 0xbf, 0x03, 0x5f, 0xfd, 0x1f, + 0x00, 0x00, 0xff, 0xff, 0xe2, 0x78, 0x9e, 0x2e, 0x22, 0x06, 0x00, 0x00, } diff --git a/metadata/metadata.proto b/metadata/metadata.proto index 8ffb4f6..84245e0 100644 --- a/metadata/metadata.proto +++ b/metadata/metadata.proto @@ -99,6 +99,7 @@ message Config { HashingCosts hash_costs = 2; EncryptionOptions options = 4; bool use_fs_keyring_for_v1_policies = 5; + bool allow_cross_user_metadata = 6; // reserve the removed field 'string compatibility = 3;' reserved 3; diff --git a/pam_fscrypt/run_fscrypt.go b/pam_fscrypt/run_fscrypt.go index ef7ff92..6b40854 100644 --- a/pam_fscrypt/run_fscrypt.go +++ b/pam_fscrypt/run_fscrypt.go @@ -35,6 +35,7 @@ import ( "log" "log/syslog" "os" + "os/user" "path/filepath" "runtime/debug" "unsafe" @@ -57,6 +58,10 @@ const ( countDirectoryPermissions = 0700 countFilePermissions = 0600 countFileFormat = "%d\n" + // uidMin is the first UID that can be used for a regular user (as + // opposed to a system user or root). This value is fairly standard + // across Linux distros, but it can be adjusted if needed. + uidMin = 1000 ) // PamFunc is used to define the various actions in the PAM module. @@ -67,6 +72,14 @@ type PamFunc struct { impl func(handle *pam.Handle, args map[string]bool) error } +// isSystemUser checks if a user is a system user. pam_fscrypt should never +// need to do anything for system users since they should never have login +// protectors. Therefore, we detect them early to avoid wasting resources. +func isSystemUser(user *user.User) bool { + uid := util.AtoiOrPanic(user.Uid) + return uid < uidMin && uid != 0 +} + // Run is used to convert between the Go functions and exported C funcs. func (f *PamFunc) Run(pamh unsafe.Pointer, argc C.int, argv **C.char) (ret C.int) { args := parseArgs(argc, argv) @@ -85,7 +98,13 @@ func (f *PamFunc) Run(pamh unsafe.Pointer, argc C.int, argv **C.char) (ret C.int log.Printf("%s(%v) starting", f.name, args) handle, err := pam.NewHandle(pamh) if err == nil { - err = f.impl(handle, args) + if isSystemUser(handle.PamUser) { + log.Printf("invoked for system user %q (%s), doing nothing", + handle.PamUser.Username, handle.PamUser.Uid) + err = nil + } else { + err = f.impl(handle, args) + } } if err != nil { fmt.Fprintf(errorWriter, "%s(%v) failed: %s", f.name, args, err) @@ -137,6 +156,13 @@ func loginProtector(handle *pam.Handle) (*actions.Protector, error) { if err != nil { return nil, err } + // Ensure that pam_fscrypt only processes metadata files owned by the + // user or root, even if the user is root themselves. (Normally, when + // fscrypt is run as root it is allowed to process all metadata files. + // This implements stricter behavior for pam_fscrypt.) + if !ctx.Config.GetAllowCrossUserMetadata() { + ctx.TrustedUser = handle.PamUser + } // Find the user's PAM protector. options, err := ctx.ProtectorOptions() @@ -164,10 +190,14 @@ func policiesUsingProtector(protector *actions.Protector) []*actions.Policy { var policies []*actions.Policy for _, mount := range mounts { // Skip mountpoints that do not use the protector. - if _, _, err := mount.GetProtector(protector.Descriptor()); err != nil { + if _, _, err := mount.GetProtector(protector.Descriptor(), + protector.Context.TrustedUser); err != nil { + if _, ok := err.(*filesystem.ErrNotSetup); !ok { + log.Print(err) + } continue } - policyDescriptors, err := mount.ListPolicies() + policyDescriptors, err := mount.ListPolicies(protector.Context.TrustedUser) if err != nil { log.Printf("listing policies: %s", err) continue |