Merge pull request #281 from ebiggers/pam_fscrypt-updates

pam_fscrypt: eliminate unnecessary options and improve documentation

3 files changed, 73 insertions, 50 deletions

diff --git a/README.md b/README.md
index 73153ed..3cf20bb 100644
--- a/README.md
+++ b/README.md
@@ -40,7 +40,7 @@ native encryption.  See [Runtime Dependencies](#runtime-dependencies).
 - [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)
-    - [Enabling the PAM module on Ubuntu](#enabling-the-pam-module-on-ubuntu)
+    - [Enabling the PAM module on Debian or Ubuntu](#enabling-the-pam-module-on-debian-or-ubuntu)
 	- [Enabling the PAM module on Arch Linux](#enabling-the-pam-module-on-arch-linux)
 	- [Enabling the PAM module on other Linux distros](#enabling-the-pam-module-on-other-linux-distros)
   - [Allowing `fscrypt` to check your login passphrase](#allowing-fscrypt-to-check-your-login-passphrase)
@@ -104,9 +104,10 @@ secret or information used to protect the confidentiality of your data. The
 three currently supported protector types are:
 
 1. Your login passphrase, through [PAM](http://www.linux-pam.org/Linux-PAM-html).
-   The included PAM module (`pam_fscrypt.so`) can automatically unlock login
-   protectors when you log in.  __IMPORTANT:__ before using a login protector,
-   follow [Setting up for login protectors](#setting-up-for-login-protectors).
+   The included PAM module (`pam_fscrypt.so`) can automatically unlock
+   directories protected by your login passphrase when you log in, and lock them
+   when you log out.  __IMPORTANT:__ before using a login protector, follow
+   [Setting up for login protectors](#setting-up-for-login-protectors).
 
 2. A custom passphrase.  This passphrase is hashed with
    [Argon2id](https://en.wikipedia.org/wiki/Argon2), by default calibrated to
@@ -168,13 +169,14 @@ Running `sudo make install` installs `fscrypt` into `/usr/local/bin`,
 `pam_fscrypt.so` into `/usr/local/lib/security`, and `pam_fscrypt/config` into
 `/usr/local/share/pam-configs`.
 
-For Ubuntu, use `sudo make install PREFIX=/usr` to install into `/usr` instead
-of the default of `/usr/local`.  Ordinarily you shouldn't manually install
-software into `/usr`, since `/usr` is reserved for Ubuntu's own packages.
-However, Ubuntu only recognizes PAM configuration files in `/usr`, not in
-`/usr/local`.  This means that the PAM module will only work if you install into
-`/usr`.  Note: if you later decide to switch to using the Ubuntu package for
-`fscrypt`, you'll have to first manually run `sudo make uninstall PREFIX=/usr`.
+On Debian (and Debian derivatives such as Ubuntu), use `sudo make install
+PREFIX=/usr` to install into `/usr` instead of the default of `/usr/local`.
+Ordinarily you shouldn't manually install software into `/usr`, since `/usr` is
+reserved for Debian's own packages.  However, Debian's PAM configuration
+framework only recognizes configuration files in `/usr`, not in `/usr/local`.
+Therefore, the PAM module will only work if you install into `/usr`.  Note: if
+you later decide to switch to using the Debian package `libpam-fscrypt`, you'll
+have to first manually run `sudo make uninstall PREFIX=/usr`.
 
 It is also possible to use `make install-bin` to only install the `fscrypt`
 binary, or `make install-pam` to only install the PAM files.
@@ -363,22 +365,22 @@ existing passphrase with the new `rounds`.
 
 To enable the PAM module `pam_fscrypt.so`, follow the directions for your Linux
 distro below.  Enabling the PAM module is needed for login passphrase-protected
-directories to be automatically unlocked when you log in, and for login
-passphrase-protected directories to remain accessible when you change your login
-passphrase.
+directories to be automatically unlocked when you log in (and be automatically
+locked when you log out), and for login passphrase-protected directories to
+remain accessible when you change your login passphrase.
 
-#### Enabling the PAM module on Ubuntu
+#### Enabling the PAM module on Debian or Ubuntu
 
-The official `fscrypt` package for Ubuntu will install a configuration file for
-[Ubuntu's PAM configuration
+The official `libpam-fscrypt` package for Debian (and Debian derivatives such as
+Ubuntu) will install a configuration file for [Debian's PAM configuration
 framework](https://wiki.ubuntu.com/PAMConfigFrameworkSpec) to
 `/usr/share/pam-configs/fscrypt`.  This file contains reasonable defaults for
-the PAM module.  To automatically apply these defaults, run `sudo
-pam-auth-update` and follow the on-screen instructions.
+the PAM module.  To automatically apply these defaults, run
+`sudo pam-auth-update` and follow the on-screen instructions.
 
 This file also gets installed if you build and install `fscrypt` from source,
-but only if you use `make install PREFIX=/usr` to install into `/usr` instead of
-the default of `/usr/local`.
+but it is only installed to the correct location if you use `make install
+PREFIX=/usr` to install into `/usr` instead of the default of `/usr/local`.
 
 #### Enabling the PAM module on Arch Linux
 
@@ -407,23 +409,23 @@ password    optional    pam_fscrypt.so
 after `pam_unix.so` in `/etc/pam.d/common-password` or similar.
 
 The Auth and Session functionality of `pam_fscrypt.so` are used to automatically
-unlock directories when logging in as a user. An easy way to get this working is
-to add the line:
+unlock directories when logging in as a user, and lock them when logging out.
+An easy way to get this working is to add the line:
 ```
 auth        optional    pam_fscrypt.so
 ```
 after `pam_unix.so` in `/etc/pam.d/common-auth` or similar, and to add the
 line:
 ```
-session     optional    pam_fscrypt.so drop_caches lock_policies
+session     optional    pam_fscrypt.so
 ```
-after `pam_unix.so` in `/etc/pam.d/common-session` or similar. The
-`lock_policies` option locks the directories protected with the user's login
-passphrase when the last session ends. The `drop_caches` option tells `fscrypt`
-to clear the filesystem caches when the last session closes, ensuring all the
-locked data is inaccessible; this only needed for v1 encryption policies.  All
-the types also support the `debug` option which prints additional debug
-information to the syslog.
+after `pam_unix.so` in `/etc/pam.d/common-session` or similar, but before
+`pam_systemd.so` or any other module that accesses the user's home directory or
+which starts processes that access the user's home directory during their
+session.
+
+To make `pam_fscrypt.so` print debugging messages to the system log, add the
+`debug` option.  All hook types accept this option.
 
 ### Allowing `fscrypt` to check your login passphrase
 
diff --git a/pam_fscrypt/config b/pam_fscrypt/config
index 9b2eb8f..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 drop_caches 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 a7582cc..2e31af9 100644
--- a/pam_fscrypt/pam_fscrypt.go
+++ b/pam_fscrypt/pam_fscrypt.go
@@ -47,8 +47,16 @@ const (
 	authtokLabel = "fscrypt_authtok"
 	// 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 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
+	// 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 +221,22 @@ func CloseSession(handle *pam.Handle, args map[string]bool) error {
 		return err
 	}
 
-	var errLock, errCache error
+	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)")
+	}
+
 	// 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)
-	}
 
-	if args[cacheFlag] {
+	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()
 	}
@@ -232,11 +247,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 +262,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 +281,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 +297,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.