[Fedora-directory-devel] Resend: Please review: make PAM passthru plugin thread safe

[Rich Megginson]

Since PAM is not thread safe, only 1 thread at a time may access the PAM
API.  This fix adds a mutex around the critical section where we call
all of the PAM functions.  I've also added another init function which
is used to create the mutex.  This has been tested on RHEL4 under a
moderate load and seems to work fine.


Index: pam_passthru.h
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/plugins/pam_passthru/pam_passthru.h,v
retrieving revision 1.4
diff -u -8 -r1.4 pam_passthru.h
--- pam_passthru.h	19 Apr 2005 22:07:30 -0000	1.4
+++ pam_passthru.h	6 Jul 2005 22:20:33 -0000
@@ -126,11 +126,12 @@
  */
 int pam_passthru_config( Slapi_Entry *config_e );
 Pam_PassthruConfig *pam_passthru_get_config( void );
 int pam_passthru_check_suffix(Pam_PassthruConfig *cfg, char *binddn);
 
 /*
  * pam_ptimpl.c
  */
+int pam_passthru_pam_init( void );
 int pam_passthru_do_pam_auth(Slapi_PBlock *pb, Pam_PassthruConfig *cfg);
 
 #endif	/* _PAM_PASSTHRU_H_ */
Index: pam_ptimpl.c
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/plugins/pam_passthru/pam_ptimpl.c,v
retrieving revision 1.7
diff -u -8 -r1.7 pam_ptimpl.c
--- pam_ptimpl.c	19 Apr 2005 22:07:30 -0000	1.7
+++ pam_ptimpl.c	6 Jul 2005 22:20:33 -0000
@@ -34,16 +34,21 @@
  * Copyright (C) 2005 Red Hat, Inc.
  * All rights reserved.
  * END COPYRIGHT BLOCK **/
 
 #include <security/pam_appl.h>
 
 #include "pam_passthru.h"
 
+/*
+ * PAM is not thread safe.  We have to execute any PAM API calls in
+ * a critical section.  This is the lock that protects that code.
+ */
+static Slapi_Mutex *PAMLock;
 
 /* Utility struct to wrap strings to avoid mallocs if possible - use
    stack allocated string space */
 #define MY_STATIC_BUF_SIZE 256
 typedef struct my_str_buf {
 	char fixbuf[MY_STATIC_BUF_SIZE];
 	char *str;
 } MyStrBuf;
@@ -266,16 +271,18 @@
 	} else {
 		init_my_str_buf(&pam_id, binddn);
 	}
 
 	/* do the pam stuff */
 	my_data.pb = pb;
 	my_data.pam_identity = pam_id.str;
 	my_pam_conv.appdata_ptr = &my_data;
+	slapi_lock_mutex(PAMLock);
+	/* from this point on we are in the critical section */
 	rc = pam_start(pam_service, pam_id.str, &my_pam_conv, &pam_handle);
 	report_pam_error("during pam_start", rc, pam_handle);
 
 	if (rc == PAM_SUCCESS) {
 		/* use PAM_SILENT - there is no user interaction at this point */
 		rc = pam_authenticate(pam_handle, 0);
 		report_pam_error("during pam_authenticate", rc, pam_handle);
 		/* check different types of errors here */
@@ -346,16 +353,18 @@
 			errmsg = PR_smprintf("Unknown PAM error [%s] for user id [%s], bind DN
[%s]",
 								 pam_strerror(pam_handle, rc), pam_id.str, escape_string(binddn, buf));
 			retcode = LDAP_OPERATIONS_ERROR; /* unknown */
 		}
 	}
 
 	rc = pam_end(pam_handle, rc);
 	report_pam_error("during pam_end", rc, pam_handle);
+	slapi_unlock_mutex(PAMLock);
+	/* not in critical section any more */
 
 	delete_my_str_buf(&pam_id);
 
 	if ((retcode == LDAP_SUCCESS) && (rc != PAM_SUCCESS)) {
 		errmsg = PR_smprintf("Unknown PAM error [%d] for user id [%d], bind DN
[%s]",
 							 rc, pam_id.str, escape_string(binddn, buf));
 		retcode = LDAP_OPERATIONS_ERROR;
 	}
@@ -371,16 +380,30 @@
 	if (errmsg) {
 		PR_smprintf_free(errmsg);
 	}
 
 	return retcode;
 }
 
 /*
+ * Perform any PAM subsystem initialization that must be done at startup time.
+ * For now, this means only the PAM mutex since PAM is not thread safe.
+ */
+int
+pam_passthru_pam_init( void )
+{
+	if (!(PAMLock = slapi_new_mutex())) {
+		return LDAP_LOCAL_ERROR;
+	}
+
+	return 0;
+}
+
+/*
  * Entry point into the PAM auth code.  Shields the rest of the app
  * from PAM API code.  Get our config params, then call the actual
  * code that does the PAM auth.  Can call that code up to 3 times,
  * depending on what methods are set in the config.
  */
 int
 pam_passthru_do_pam_auth(Slapi_PBlock *pb, Pam_PassthruConfig *cfg)
 {
Index: pam_ptpreop.c
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/plugins/pam_passthru/pam_ptpreop.c,v
retrieving revision 1.4
diff -u -8 -r1.4 pam_ptpreop.c
--- pam_ptpreop.c	19 Apr 2005 22:07:30 -0000	1.4
+++ pam_ptpreop.c	6 Jul 2005 22:20:33 -0000
@@ -123,16 +123,22 @@
     }
 
     if (( rc = pam_passthru_config( config_e )) != LDAP_SUCCESS ) {
 		slapi_log_error( SLAPI_LOG_FATAL, PAM_PASSTHRU_PLUGIN_SUBSYSTEM,
 						 "configuration failed (%s)\n", ldap_err2string( rc ));
 		return( -1 );
     }
 
+    if (( rc = pam_passthru_pam_init()) != LDAP_SUCCESS ) {
+		slapi_log_error( SLAPI_LOG_FATAL, PAM_PASSTHRU_PLUGIN_SUBSYSTEM,
+						 "could not initialize PAM subsystem (%d)\n", rc);
+		return( -1 );
+    }
+
     return( 0 );
 }
 
 
 /*
  * Called right before the Directory Server shuts down.
  */
 static int