From 28e4999ebd9221a71488d715d9f1182b494216d8 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 8 Mar 2021 15:20:08 -0800 Subject: pam_fscrypt: decide cache dropping behavior automatically Configuring whether pam_fscrypt drops caches or not isn't really something the user should have to do, and it's also irrelevant for v2 encryption policies (the default on newer systems). It's better to have pam_fscrypt automatically decide whether it needs to drop caches or not. Do this by making pam_fscrypt check whether any encryption policy keys are being removed from a user keyring (rather than from a filesystem keyring). If so, it drops caches; otherwise it doesn't. This supersedes the "drop_caches" option, which won't do anything anymore. --- pam_fscrypt/config | 2 +- pam_fscrypt/pam_fscrypt.go | 42 +++++++++++++++++++++++++++++------------- 2 files changed, 30 insertions(+), 14 deletions(-) (limited to 'pam_fscrypt') diff --git a/pam_fscrypt/config b/pam_fscrypt/config index 9b2eb8f..d2fbf68 100644 --- a/pam_fscrypt/config +++ b/pam_fscrypt/config @@ -7,7 +7,7 @@ Auth-Final: Session-Type: Additional Session-Interactive-Only: yes Session-Final: - optional PAM_INSTALL_PATH drop_caches lock_policies + optional PAM_INSTALL_PATH lock_policies Password-Type: Additional Password-Final: optional PAM_INSTALL_PATH diff --git a/pam_fscrypt/pam_fscrypt.go b/pam_fscrypt/pam_fscrypt.go index a7582cc..195ba43 100644 --- a/pam_fscrypt/pam_fscrypt.go +++ b/pam_fscrypt/pam_fscrypt.go @@ -48,7 +48,12 @@ const ( // These flags are used to toggle behavior of the PAM module. debugFlag = "debug" lockFlag = "lock_policies" - cacheFlag = "drop_caches" + + // This option is accepted for compatibility with existing config files, + // but it no longer does anything. pam_fscrypt now drops caches if and + // only if it is needed. (Usually it is not needed anymore, as the + // FS_IOC_REMOVE_ENCRYPTION_KEY ioctl handles this automatically.) + dropCachesFlag = "drop_caches" ) var ( @@ -213,15 +218,20 @@ func CloseSession(handle *pam.Handle, args map[string]bool) error { return err } + if args[dropCachesFlag] { + log.Print("ignoring deprecated 'drop_caches' option (now auto-detected)") + } + + needDropCaches := false var errLock, errCache error // Don't automatically drop privileges, since we may need them to // deprovision policies or to drop caches. if args[lockFlag] { log.Print("locking polices protected with login protector") - errLock = lockLoginPolicies(handle) + needDropCaches, errLock = lockLoginPolicies(handle) } - if args[cacheFlag] { + if needDropCaches { log.Print("dropping appropriate filesystem caches at session close") errCache = security.DropFilesystemCache() } @@ -232,11 +242,14 @@ func CloseSession(handle *pam.Handle, args map[string]bool) error { return errCache } -// lockLoginPolicies deprovisions all policy keys that are protected by -// the user's login protector. -func lockLoginPolicies(handle *pam.Handle) error { +// lockLoginPolicies deprovisions all policy keys that are protected by the +// user's login protector. It returns true if dropping filesystem caches will +// be needed afterwards to completely lock the relevant directories. +func lockLoginPolicies(handle *pam.Handle) (bool, error) { + needDropCaches := false + if err := handle.StartAsPamUser(); err != nil { - return err + return needDropCaches, err } defer handle.StopAsPamUser() @@ -244,16 +257,16 @@ func lockLoginPolicies(handle *pam.Handle) error { protector, err := loginProtector(handle) if err != nil { log.Printf("nothing to lock: %s", err) - return nil + return needDropCaches, nil } policies := policiesUsingProtector(protector) if len(policies) == 0 { log.Print("no policies to lock") - return nil + return needDropCaches, nil } if err = setupUserKeyringIfNeeded(handle, policies); err != nil { - return errors.Wrapf(err, "setting up user keyring") + return needDropCaches, errors.Wrapf(err, "setting up user keyring") } // We will try to deprovision all of the policies. @@ -263,12 +276,15 @@ func lockLoginPolicies(handle *pam.Handle) error { policy.Descriptor(), handle.PamUser.Username) continue } + if policy.NeedsUserKeyring() { + needDropCaches = true + } if err := beginProvisioningOp(handle, policy); err != nil { - return err + return needDropCaches, err } deprovisionErr := policy.Deprovision(false) if err := endProvisioningOp(handle, policy); err != nil { - return err + return needDropCaches, err } if deprovisionErr != nil { log.Printf("deprovisioning policy %s: %s", policy.Descriptor(), deprovisionErr) @@ -276,7 +292,7 @@ func lockLoginPolicies(handle *pam.Handle) error { } log.Printf("policy %s deprovisioned by %v", policy.Descriptor(), handle.PamUser.Username) } - return nil + return needDropCaches, nil } // Chauthtok rewraps the login protector when the passphrase changes. -- cgit v1.2.3 From b7e898f01bcae17174fcd928599d0d933655db9b Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 8 Mar 2021 15:20:08 -0800 Subject: pam_fscrypt: make "lock_policies" the default behavior All pam_fscrypt configuration guides that I'm aware of say to use the "lock_policies" option for the pam_fscrypt.so session hook. The Debian/Ubuntu pam-config-framework config file has it too. Make locking the default behavior, since this is what everyone wants. Existing configuration files that contain the "lock_policies" option will continue to work, but that option won't do anything anymore. (We could add an option "unlock_only" to restore the old default behavior, but it's not clear that it would be useful. So for simplicity, leave it out for now.) --- pam_fscrypt/config | 2 +- pam_fscrypt/pam_fscrypt.go | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) (limited to 'pam_fscrypt') diff --git a/pam_fscrypt/config b/pam_fscrypt/config index d2fbf68..f83dab2 100644 --- a/pam_fscrypt/config +++ b/pam_fscrypt/config @@ -7,7 +7,7 @@ Auth-Final: Session-Type: Additional Session-Interactive-Only: yes Session-Final: - optional PAM_INSTALL_PATH lock_policies + optional PAM_INSTALL_PATH Password-Type: Additional Password-Final: optional PAM_INSTALL_PATH diff --git a/pam_fscrypt/pam_fscrypt.go b/pam_fscrypt/pam_fscrypt.go index 195ba43..2e31af9 100644 --- a/pam_fscrypt/pam_fscrypt.go +++ b/pam_fscrypt/pam_fscrypt.go @@ -47,7 +47,10 @@ const ( authtokLabel = "fscrypt_authtok" // These flags are used to toggle behavior of the PAM module. debugFlag = "debug" - lockFlag = "lock_policies" + + // This option is accepted for compatibility with existing config files, + // but now we lock policies unconditionally and this option is a no-op. + lockPoliciesFlag = "lock_policies" // This option is accepted for compatibility with existing config files, // but it no longer does anything. pam_fscrypt now drops caches if and @@ -218,19 +221,21 @@ func CloseSession(handle *pam.Handle, args map[string]bool) error { return err } + if args[lockPoliciesFlag] { + log.Print("ignoring deprecated 'lock_policies' option (now the default)") + } + if args[dropCachesFlag] { log.Print("ignoring deprecated 'drop_caches' option (now auto-detected)") } - needDropCaches := false - var errLock, errCache error // Don't automatically drop privileges, since we may need them to // deprovision policies or to drop caches. - if args[lockFlag] { - log.Print("locking polices protected with login protector") - needDropCaches, errLock = lockLoginPolicies(handle) - } + log.Print("locking policies protected with login protector") + needDropCaches, errLock := lockLoginPolicies(handle) + + var errCache error if needDropCaches { log.Print("dropping appropriate filesystem caches at session close") errCache = security.DropFilesystemCache() -- cgit v1.2.3