Merge pull request #346 from google/fixes

Metadata validation and other security improvements

1 files changed, 33 insertions, 3 deletions

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