Merge pull request #103 from google/pam

Cleanup privilege dropping/raising in pam_fscrypt

2 files changed, 27 insertions, 18 deletions

diff --git a/pam_fscrypt/pam_fscrypt.go b/pam_fscrypt/pam_fscrypt.go
index e611bc6..85bd934 100644
--- a/pam_fscrypt/pam_fscrypt.go
+++ b/pam_fscrypt/pam_fscrypt.go
@@ -50,10 +50,17 @@ const (
 	cacheFlag = "drop_caches"
 )
 
+var (
+	// PamFuncs for our 4 provided methods
+	authenticateFunc = PamFunc{"Authenticate", Authenticate}
+	openSessionFunc  = PamFunc{"OpenSession", OpenSession}
+	closeSessionFunc = PamFunc{"CloseSession", CloseSession}
+	chauthtokFunc    = PamFunc{"Chauthtok", Chauthtok}
+)
+
 // Authenticate copies the AUTHTOK (if necessary) into the PAM data so it can be
 // used in pam_sm_open_session.
 func Authenticate(handle *pam.Handle, _ map[string]bool) error {
-	log.Print("Authenticate()")
 	if err := handle.StartAsPamUser(); err != nil {
 		return err
 	}
@@ -76,7 +83,6 @@ func Authenticate(handle *pam.Handle, _ map[string]bool) error {
 
 // OpenSession provisions any policies protected with the login protector.
 func OpenSession(handle *pam.Handle, _ map[string]bool) error {
-	log.Print("OpenSession()")
 	// We will always clear the the AUTHTOK data
 	defer handle.ClearData(authtokLabel)
 	// Increment the count as we add a session
@@ -150,7 +156,6 @@ func OpenSession(handle *pam.Handle, _ map[string]bool) error {
 // CloseSession can deprovision all keys provisioned at the start of the
 // session. It can also clear the cache so these changes take effect.
 func CloseSession(handle *pam.Handle, args map[string]bool) error {
-	log.Printf("CloseSession(%v)", args)
 	// Only do stuff on session close when we are the last session
 	if count, err := AdjustCount(handle, -1); err != nil || count != 0 {
 		log.Printf("count is %d and we are not locking", count)
@@ -212,7 +217,6 @@ func lockLoginPolicies(handle *pam.Handle) error {
 
 // Chauthtok rewraps the login protector when the passphrase changes.
 func Chauthtok(handle *pam.Handle, _ map[string]bool) error {
-	log.Print("Chauthtok()")
 	if err := handle.StartAsPamUser(); err != nil {
 		return err
 	}
@@ -257,7 +261,7 @@ func Chauthtok(handle *pam.Handle, _ map[string]bool) error {
 
 //export pam_sm_authenticate
 func pam_sm_authenticate(pamh unsafe.Pointer, flags, argc C.int, argv **C.char) C.int {
-	return RunPamFunc(Authenticate, pamh, argc, argv)
+	return authenticateFunc.Run(pamh, argc, argv)
 }
 
 // pam_sm_stecred needed because we use pam_sm_authenticate.
@@ -268,12 +272,12 @@ func pam_sm_setcred(pamh unsafe.Pointer, flags, argc C.int, argv **C.char) C.int
 
 //export pam_sm_open_session
 func pam_sm_open_session(pamh unsafe.Pointer, flags, argc C.int, argv **C.char) C.int {
-	return RunPamFunc(OpenSession, pamh, argc, argv)
+	return openSessionFunc.Run(pamh, argc, argv)
 }
 
 //export pam_sm_close_session
 func pam_sm_close_session(pamh unsafe.Pointer, flags, argc C.int, argv **C.char) C.int {
-	return RunPamFunc(CloseSession, pamh, argc, argv)
+	return closeSessionFunc.Run(pamh, argc, argv)
 }
 
 //export pam_sm_chauthtok
@@ -282,8 +286,7 @@ func pam_sm_chauthtok(pamh unsafe.Pointer, flags, argc C.int, argv **C.char) C.i
 	if pam.Flag(flags)&pam.PrelimCheck != 0 {
 		return C.PAM_SUCCESS
 	}
-
-	return RunPamFunc(Chauthtok, pamh, argc, argv)
+	return chauthtokFunc.Run(pamh, argc, argv)
 }
 
 // main() is needed to make a shared library compile
diff --git a/pam_fscrypt/run_fscrypt.go b/pam_fscrypt/run_fscrypt.go
index da336df..8622a64 100644
--- a/pam_fscrypt/run_fscrypt.go
+++ b/pam_fscrypt/run_fscrypt.go
@@ -59,11 +59,16 @@ const (
 	countFileFormat           = "%d\n"
 )
 
-// PamFunc is used to define the various actions in the PAM module
-type PamFunc func(handle *pam.Handle, args map[string]bool) error
+// PamFunc is used to define the various actions in the PAM module.
+type PamFunc struct {
+	// Name of the function being executed
+	name string
+	// Go implementation of this function
+	impl func(handle *pam.Handle, args map[string]bool) error
+}
 
-// RunPamFunc is used to convert between the Go functions and exported C funcs.
-func RunPamFunc(f PamFunc, pamh unsafe.Pointer, argc C.int, argv **C.char) (ret C.int) {
+// 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)
 	errorWriter := setupLogging(args)
 
@@ -72,20 +77,21 @@ func RunPamFunc(f PamFunc, pamh unsafe.Pointer, argc C.int, argv **C.char) (ret
 		if r := recover(); r != nil {
 			ret = C.PAM_SERVICE_ERR
 			fmt.Fprintf(errorWriter,
-				"pam func panicked: %s\nPlease open an issue.\n%s",
-				r, debug.Stack())
+				"%s(%v) panicked: %s\nPlease open a bug.\n%s",
+				f.name, args, r, debug.Stack())
 		}
 	}()
 
+	log.Printf("%s(%v) starting", f.name, args)
 	handle, err := pam.NewHandle(pamh)
 	if err == nil {
-		err = f(handle, args)
+		err = f.impl(handle, args)
 	}
 	if err != nil {
-		fmt.Fprintf(errorWriter, "pam func failed: %s", err)
+		fmt.Fprintf(errorWriter, "%s(%v) failed: %s", f.name, args, err)
 		return C.PAM_SERVICE_ERR
 	}
-	log.Print("pam func succeeded")
+	log.Printf("%s(%v) succeeded", f.name, args)
 	return C.PAM_SUCCESS
 }