Patch to include PAM support...

Started by Dominic J. Eidsonover 24 years ago33 messages
#1Dominic J. Eidson
sauron@the-infinite.org
1 attachment(s)

As mentioned on -hackers, I added the neccesary code to PostgreSQL to
allow for authentication through PAM... Attached is the patch.

-Dominic

--
Dominic J. Eidson
"Baruk Khazad! Khazad ai-menu!" - Gimli
-------------------------------------------------------------------------------
http://www.the-infinite.org/ http://www.the-infinite.org/~dominic/

Attachments:

pgsql-7.1+pam.difftext/plain; charset=US-ASCII; name=pgsql-7.1+pam.diffDownload
Index: configure.in
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/configure.in,v
retrieving revision 1.119
diff -u -r1.119 configure.in
--- configure.in	2001/04/23 15:14:58	1.119
+++ configure.in	2001/04/27 15:36:57
@@ -389,7 +389,6 @@
 AC_MSG_RESULT([$with_perl])
 AC_SUBST(with_perl)
 
-
 #
 # Optionally build Python interface module
 #
@@ -489,7 +488,30 @@
                    [The name of the PostgreSQL service principal in Kerberos])
 
 
+#
+# PAM
+#
+AC_MSG_CHECKING([whether to build with PAM support])
+PGAC_ARG_OPTARG(with, pam,
+                [  --with-pam[=DIR]        build with PAM support [/usr]],
+                [pam_prefix=/usr],
+                [pam_prefix=$withval],
+[
+  AC_MSG_RESULT([yes])
+  AC_DEFINE([USE_PAM], 1, [Define to build with PAM support])
 
+  if test -d "${pam_prefix}/include" ; then
+    INCLUDES="$INCLUDES -I${pam_prefix}/include/security"
+  fi
+  if test -d "${pam_prefix}/lib" ; then
+    LIBDIRS="$LIBDIRS -L${pam_prefix}/lib"
+  fi
+],
+[AC_MSG_RESULT(no)])
+
+AC_SUBST(with_pam)
+
+
 #
 # OpenSSL
 #
@@ -701,7 +723,11 @@
   AC_CHECK_LIB(ssl,    [SSL_library_init], [], [AC_MSG_ERROR([library 'ssl' is required for OpenSSL])])
 fi
 
+if test "$with_pam" = yes ; then
+  AC_CHECK_LIB(pam,    [pam_start], [], [AC_MSG_ERROR([library 'pam' is required for PAM])])
+fi
 
+
 ##
 ## Header files
 ##
@@ -737,6 +763,10 @@
 if test "$with_openssl" = yes ; then
   AC_CHECK_HEADER([openssl/ssl.h], [], [AC_MSG_ERROR([header file <openssl/ssl.h> is required for OpenSSL])])
   AC_CHECK_HEADER([openssl/err.h], [], [AC_MSG_ERROR([header file <openssl/err.h> is required for OpenSSL])])
+fi
+
+if test "$with_pam" = yes ; then
+  AC_CHECK_HEADER([security/pam_appl.h], [], [AC_MSG_ERROR([header file <security/pam_appl.h> is required for PAM])])
 fi
 
 
Index: doc/src/sgml/client-auth.sgml
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/doc/src/sgml/client-auth.sgml,v
retrieving revision 1.10
diff -u -r1.10 client-auth.sgml
--- doc/src/sgml/client-auth.sgml	2001/03/15 20:01:32	1.10
+++ doc/src/sgml/client-auth.sgml	2001/04/27 15:36:57
@@ -243,6 +243,19 @@
          </para>
         </listitem>
        </varlistentry>
+
+       <varlistentry>
+        <term>pam</term>
+        <listitem>
+         <para>
+          PAM is used to authenticate the user. If the
+          <replaceable>authentication option</replaceable>
+          is specified, <productname>PostgreSQL</productname> will use it
+          as service name, otherwise it will use the default, which is
+          <literal>postgresql</>.
+         </para>
+        </listitem>
+       </varlistentry>
       </variablelist>
 
       </para>
Index: src/backend/libpq/Makefile
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/libpq/Makefile,v
retrieving revision 1.24
diff -u -r1.24 Makefile
--- src/backend/libpq/Makefile	2000/08/25 10:00:30	1.24
+++ src/backend/libpq/Makefile	2001/04/27 15:36:57
@@ -18,7 +18,6 @@
 	auth.o crypt.o hba.o password.o \
 	pqcomm.o pqformat.o pqpacket.o pqsignal.o util.o
 
-
 all: SUBSYS.o
 
 SUBSYS.o: $(OBJS)
Index: src/backend/libpq/auth.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/libpq/auth.c,v
retrieving revision 1.52
diff -u -r1.52 auth.c
--- src/backend/libpq/auth.c	2001/03/22 03:59:30	1.52
+++ src/backend/libpq/auth.c	2001/04/27 15:36:57
@@ -30,6 +30,7 @@
 #include <arpa/inet.h>
 
 #include "postgres.h"
+#include "config.h"
 
 #include "libpq/auth.h"
 #include "libpq/crypt.h"
@@ -38,6 +39,27 @@
 #include "libpq/password.h"
 #include "miscadmin.h"
 
+#ifdef USE_PAM
+#include <security/pam_appl.h>
+
+#define PGSQL_PAM_SERVICE "postgresql"	/* Service name passed to PAM */
+
+static int	handle_pam_auth(void *arg, PacketLen len, void *pkt);
+static int      checkPamPasswd(Port *port, char *user, char *password);
+static int	pam_passw_conv_proc(int num_msg, const struct pam_message **msg,
+		struct pam_response **resp, void *appdata_ptr);
+static int	readPamPasswordPacket(void *arg, PacketLen len, void *pkt);
+
+
+static struct pam_conv pam_passw_conv = {
+    &pam_passw_conv_proc,
+    NULL
+};
+
+static char * pam_passwd = NULL; /* Workaround for Solaris 2.6 brokenness */
+
+#endif /* USE_PAM */
+
 static void sendAuthRequest(Port *port, AuthRequest areq, PacketDoneProc handler);
 static int	handle_done_auth(void *arg, PacketLen len, void *pkt);
 static int	handle_krb4_auth(void *arg, PacketLen len, void *pkt);
@@ -439,6 +461,11 @@
 		case uaCrypt:
 			authmethod = "Password";
 			break;
+#ifdef USE_PAM
+		case uaPam:
+			authmethod = "PAM";
+			break;
+#endif
 	}
 
 	sprintf(buffer, "%s authentication failed for user '%s'",
@@ -545,6 +572,12 @@
 				areq = AUTH_REQ_CRYPT;
 				auth_handler = handle_password_auth;
 				break;
+#ifdef USE_PAM
+			case uaPam:
+				areq = AUTH_REQ_PAM;
+				auth_handler = handle_pam_auth;
+				break;
+#endif
 		}
 
 		/* Tell the frontend what we want next. */
@@ -666,8 +699,158 @@
 	return STATUS_OK;
 }
 
+#ifdef USE_PAM
+/*
+ * Called when we have told the front end that it should use password
+ * authentication, however, we authenticate into PAM on the backend.
+ */
+
+static int
+handle_pam_auth(void *arg, PacketLen len, void *pkt)
+{
+	Port	   *port = (Port *) arg;
+
+	/* Set up the read of the password packet. */
+
+	PacketReceiveSetup(&port->pktInfo, readPamPasswordPacket, (void *) port);
+
+	return STATUS_OK;
+}
+
+/*
+ * Called when we have received the password packet, and check into PAM.
+ */
+static int
+readPamPasswordPacket(void *arg, PacketLen len, void *pkt)
+{
+	char		password[sizeof(PasswordPacket) + 1];
+	Port	   *port = (Port *) arg;
+
+	/* Silently truncate a password that is too big. */
+
+	if (len > sizeof(PasswordPacket))
+		len = sizeof(PasswordPacket);
+
+	StrNCpy(password, ((PasswordPacket *) pkt)->passwd, len);
+
+	if (checkPamPasswd(port, port->user, password) != STATUS_OK)
+		auth_failed(port);
+	else
+		sendAuthRequest(port, AUTH_REQ_OK, handle_done_auth);
+
+	return STATUS_OK;			/* don't close the connection yet */
+}
+
+/*
+ * PAM conversation function
+ */
+
+static int
+pam_passw_conv_proc (int num_msg, const struct pam_message **msg, struct pam_response **resp, void *appdata_ptr)
+{
+	if (num_msg != 1 || msg[0]->msg_style != PAM_PROMPT_ECHO_OFF) {
+		snprintf(PQerrormsg, PQERRORMSG_LENGTH,
+			"pam_passwd_conv: Unexpected PAM conversation '%d/%s'\n", 
+			msg[0]->msg_style, msg[0]->msg);
+		fputs(PQerrormsg, stderr);
+		pqdebug("%s", PQerrormsg);
+		return PAM_CONV_ERR;
+	}
+	if (!appdata_ptr) {
+		/* Workaround for Solaris 2.6 where the PAM library is broken
+		 * and does not pass appdata_ptr to the conversation routine
+		 */
+		appdata_ptr = pam_passwd;
+	}
+	if (!appdata_ptr) {
+		snprintf(PQerrormsg, PQERRORMSG_LENGTH,
+	        	"ERROR: No password available to pam_passw_conv_proc!\n");
+		fputs(PQerrormsg, stderr);
+		pqdebug("%s", PQerrormsg);
+	        return PAM_CONV_ERR;
+	}
+
+	/* Explicitly not using palloc here - PAM will free this memory in
+	 * pam_end()
+	 */
+	*resp = calloc(num_msg, sizeof(struct pam_response));
+	if (!*resp) {
+		snprintf(PQerrormsg, PQERRORMSG_LENGTH, "ERROR: Out of memory!\n");
+		fputs(PQerrormsg, stderr);
+		pqdebug("%s", PQerrormsg);
+        	return PAM_CONV_ERR;
+	}
+
+	(*resp)[0].resp = strdup((char *) appdata_ptr);
+	(*resp)[0].resp_retcode = 0;
+
+	return ((*resp)[0].resp ? PAM_SUCCESS : PAM_CONV_ERR);
+}
+
 
 /*
+ * Check authentication against PAM.
+ */
+static int
+checkPamPasswd(Port *port, char *user, char *password)
+{
+	int retval;
+	pam_handle_t *pamh = NULL;
+
+	/*
+	 * Apparently, Solaris 2.6 is broken, and needs ugly static 
+	 * variable workaround
+	 */
+	pam_passwd = password;
+
+	/* Set the application data portion of the conversation struct
+	 * This is later used inside the PAM conversation to pass the
+	 * password to the authentication module.
+	 */
+	pam_passw_conv.appdata_ptr = (char*) password;	/* from password above, not allocated */
+
+	/* Optionally, one can set the service name in pg_hba.conf */
+	if(port->auth_arg[0] == '\0') {
+		retval = pam_start(PGSQL_PAM_SERVICE, "pgsql@", &pam_passw_conv, &pamh);
+	} else {
+		retval = pam_start(port->auth_arg, "pgsql@", &pam_passw_conv, &pamh);
+	}
+
+	if (retval != PAM_SUCCESS) {
+		snprintf(PQerrormsg, PQERRORMSG_LENGTH,
+			"Failed to create PAM authenticator!\n");
+		fputs(PQerrormsg, stderr);
+		pqdebug("%s", PQerrormsg);  
+		pam_passwd = NULL;	/* Unset pam_passwd */
+		return STATUS_ERROR;
+	}
+
+	if (retval == PAM_SUCCESS)
+		retval = pam_set_item(pamh, PAM_USER, user);
+	if (retval == PAM_SUCCESS)
+		retval = pam_set_item(pamh, PAM_CONV, &pam_passw_conv);
+	if (retval == PAM_SUCCESS)
+		retval = pam_authenticate(pamh, 0);
+	if (retval == PAM_SUCCESS)
+		retval = pam_acct_mgmt(pamh, 0);
+	if (retval == PAM_SUCCESS) {
+		retval = pam_end(pamh, retval);
+		if(retval != PAM_SUCCESS) {
+			snprintf(PQerrormsg, PQERRORMSG_LENGTH,
+				"Failed to release PAM authenticator!\n");
+			fputs(PQerrormsg, stderr);
+			pqdebug("%s", PQerrormsg);  
+		}
+		pam_passwd = NULL;	/* Unset pam_passwd */
+
+		return (retval == PAM_SUCCESS ? STATUS_OK : STATUS_ERROR);
+	} else {
+		return STATUS_ERROR;
+	}
+}
+#endif /* PAM */
+
+/*
  * Called when we have received the password packet.
  */
 
@@ -760,8 +943,11 @@
 {
 	switch (port->auth_method)
 	{
-			case uaCrypt:
-			case uaReject:
+		case uaCrypt:
+		case uaReject:
+#ifdef USE_PAM
+		case uaPam:
+#endif
 			status = STATUS_ERROR;
 			break;
 
Index: src/backend/libpq/hba.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/libpq/hba.c,v
retrieving revision 1.55
diff -u -r1.55 hba.c
--- src/backend/libpq/hba.c	2001/02/10 02:31:26	1.55
+++ src/backend/libpq/hba.c	2001/04/27 15:36:57
@@ -125,6 +125,10 @@
 		*userauth_p = uaReject;
 	else if (strcmp(buf, "crypt") == 0)
 		*userauth_p = uaCrypt;
+#ifdef USE_PAM
+	else if (strcmp(buf, "pam") == 0)
+		*userauth_p = uaPam;
+#endif
 	else
 	{
 		*error_p = true;
Index: src/backend/libpq/pg_hba.conf.sample
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/libpq/pg_hba.conf.sample,v
retrieving revision 1.17
diff -u -r1.17 pg_hba.conf.sample
--- src/backend/libpq/pg_hba.conf.sample	2000/11/21 20:44:32	1.17
+++ src/backend/libpq/pg_hba.conf.sample	2001/04/27 15:36:57
@@ -121,10 +121,15 @@
 #
 #   krb5:   	Kerberos V5 authentication is used.
 #
+#   pam:	Authentication is passed off to PAM (PostgreSQL must be
+#               configured --with-pam), using the default service name
+#               "postgresql" - you can specify your own service name, by
+#               setting AUTH_ARGUMENT to the desired service name.
+#
 #   reject: 	Reject the connection.
 #
 # Local (UNIX socket) connections support only AUTHTYPEs "trust",
-# "password", "crypt", and "reject".
+# "password", "crypt", "pam", and "reject".
 
 
 # Examples
Index: src/include/config.h.in
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/config.h.in,v
retrieving revision 1.162
diff -u -r1.162 config.h.in
--- src/include/config.h.in	2001/04/14 22:55:02	1.162
+++ src/include/config.h.in	2001/04/27 15:36:57
@@ -63,6 +63,9 @@
 /* Define to build with (Open)SSL support (--with-openssl[=DIR]) */
 #undef USE_SSL
 
+/* Define to build with PAM Support */
+#undef USE_PAM
+
 /* 
  * DEF_PGPORT is the TCP port number on which the Postmaster listens and
  * which clients will try to connect to.  This is just a default value;
Index: src/include/libpq/hba.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/libpq/hba.h,v
retrieving revision 1.19
diff -u -r1.19 hba.h
--- src/include/libpq/hba.h	2001/03/22 04:00:47	1.19
+++ src/include/libpq/hba.h	2001/04/27 15:36:57
@@ -35,6 +35,9 @@
 	uaTrust,
 	uaIdent,
 	uaPassword,
+#ifdef USE_PAM
+	uaPam,
+#endif
 	uaCrypt
 } UserAuth;
 
Index: src/include/libpq/pqcomm.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/libpq/pqcomm.h,v
retrieving revision 1.55
diff -u -r1.55 pqcomm.h
--- src/include/libpq/pqcomm.h	2001/03/22 04:00:48	1.55
+++ src/include/libpq/pqcomm.h	2001/04/27 15:36:57
@@ -132,6 +132,9 @@
 #define AUTH_REQ_KRB5		2	/* Kerberos V5 */
 #define AUTH_REQ_PASSWORD	3	/* Password */
 #define AUTH_REQ_CRYPT		4	/* Encrypted password */
+#ifdef USE_PAM
+#define AUTH_REQ_PAM		5	/* Password, handed off to PAM */
+#endif
 
 typedef uint32 AuthRequest;
 
Index: src/interfaces/libpq/fe-auth.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/libpq/fe-auth.c,v
retrieving revision 1.47
diff -u -r1.47 fe-auth.c
--- src/interfaces/libpq/fe-auth.c	2001/03/22 04:01:25	1.47
+++ src/interfaces/libpq/fe-auth.c	2001/04/27 15:36:58
@@ -493,6 +493,9 @@
 
 		case AUTH_REQ_PASSWORD:
 		case AUTH_REQ_CRYPT:
+#ifdef USE_PAM
+		case AUTH_REQ_PAM:
+#endif
 			if (password == NULL || *password == '\0')
 			{
 				(void) sprintf(PQerrormsg,
#2Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Dominic J. Eidson (#1)
Re: Patch to include PAM support...

Saved for 7.2 in the patches mailbox.

As mentioned on -hackers, I added the neccesary code to PostgreSQL to
allow for authentication through PAM... Attached is the patch.

-Dominic

--
Dominic J. Eidson
"Baruk Khazad! Khazad ai-menu!" - Gimli
-------------------------------------------------------------------------------
http://www.the-infinite.org/ http://www.the-infinite.org/~dominic/

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/users-lounge/docs/faq.html

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#3Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Dominic J. Eidson (#1)
Re: Patch to include PAM support...

Is there any interest in PAM support? If not, I will reject this patch.

As mentioned on -hackers, I added the neccesary code to PostgreSQL to
allow for authentication through PAM... Attached is the patch.

-Dominic

--
Dominic J. Eidson
"Baruk Khazad! Khazad ai-menu!" - Gimli
-------------------------------------------------------------------------------
http://www.the-infinite.org/ http://www.the-infinite.org/~dominic/

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/users-lounge/docs/faq.html

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#4Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#3)
Re: Patch to include PAM support...

Bruce Momjian writes:

Is there any interest in PAM support? If not, I will reject this patch.

Sure there is.

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

#5Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Peter Eisentraut (#4)
Re: Patch to include PAM support...

Bruce Momjian writes:

Is there any interest in PAM support? If not, I will reject this patch.

Sure there is.

OK, care to give a thumbs up on the patch?

http://candle.pha.pa.us/cgi-bin/pgpatches

It is enabled with a compile-time option.

Tom objected to it.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#3)
Re: Patch to include PAM support...

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Is there any interest in PAM support? If not, I will reject this patch.

We have seen multiple requests for PAM support, so there's interest out
there. But IIRC, I had some serious concerns about this proposed
implementation.

regards, tom lane

#7Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#6)
Re: Patch to include PAM support...

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Is there any interest in PAM support? If not, I will reject this patch.

We have seen multiple requests for PAM support, so there's interest out
there. But IIRC, I had some serious concerns about this proposed
implementation.

I know there was concerns about blocking but is that problem any more so
than other interfaces we already support?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#7)
Re: Patch to include PAM support...

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I know there was concerns about blocking but is that problem any more so
than other interfaces we already support?

We don't need to make it worse. We've already had trouble reports about
postmaster hangups with broken IDENT servers; PAM will hugely expand the
scope of potential troubles. Can you say "denial of service"?

regards, tom lane

#9Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#8)
Re: Patch to include PAM support...

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I know there was concerns about blocking but is that problem any more so
than other interfaces we already support?

We don't need to make it worse. We've already had trouble reports about
postmaster hangups with broken IDENT servers; PAM will hugely expand the
scope of potential troubles. Can you say "denial of service"?

Does it really? You are saying PAM can make "denial of service" attacks
even easier than ident?

If it is the same risk, I think it is OK, but if it is worse, I see your
point. (I don't know much about PAM except it allows authentication.)

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#10Mathijs Brands
mathijs@ilse.nl
In reply to: Bruce Momjian (#3)
Re: Patch to include PAM support...

On Tue, Jun 12, 2001 at 11:56:12AM -0400, Bruce Momjian allegedly wrote:

Is there any interest in PAM support? If not, I will reject this patch.

Yeah, I would be interested. It would enable me to pull passwords from
LDAP, which should enable our helpdesk to support PostgreSQL better
(change requests for passwords now go via sysadmin, while all other
passwords (ftp, staging, accounts) are stored in a central repository).

However, to me it's more of a handy featured than a critical one.

Regards,

Mathijs
--
"Books constitute capital."
Thomas Jefferson

#11Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#5)
Re: Patch to include PAM support...

Bruce Momjian writes:

OK, care to give a thumbs up on the patch?

http://candle.pha.pa.us/cgi-bin/pgpatches

From static inspection I have some doubts about whether this patch would
operate correctly. The way it is implemented is that if the backend is
instructed to use PAM authentication it pretends to the frontend that
password authentication is going on. This would probably work correctly
if your PAM setup is that you require exactly one password from the user.
But if the PAM setup does not require a password (Kerberos, rhosts
modules?) it would involve a useless exchange (and possibly prompt) for a
password. More importantly, though, if the PAM configuration requires
more than one password (perhaps the password is due to be changed), this
implementation will fail (to authenticate).

Dominic, any comments?

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

#12Dominic J. Eidson
sauron@the-infinite.org
In reply to: Bruce Momjian (#9)
Re: Patch to include PAM support...

On Tue, 12 Jun 2001, Bruce Momjian wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I know there was concerns about blocking but is that problem any more so
than other interfaces we already support?

We don't need to make it worse. We've already had trouble reports about
postmaster hangups with broken IDENT servers; PAM will hugely expand the
scope of potential troubles. Can you say "denial of service"?

Does it really? You are saying PAM can make "denial of service" attacks
even easier than ident?

If anything, then "possibly as easy as ident" - but that's a worst case
scenario. And the reason for that is because they both potentially use
outside server/services. PAM doesn't _have_ to authenticate into external
devices, the LDAP example is just an example from my/our situation. You
could use PAM to authenticate into the local system password file, and/or
use it to create user limits (Only 3 connections per user, as example..)

If it is the same risk, I think it is OK, but if it is worse, I see your
point. (I don't know much about PAM except it allows authentication.)

My apologies if PAM has somehow been equated to "remote server
authentication piece" - there is a lot more to PAM than the abillity to
easily do remote authentication.

http://www.kernel.org/pub/linux/libs/pam/whatispam.html
http://www.kernel.org/pub/linux/libs/pam/FAQ

--
Dominic J. Eidson
"Baruk Khazad! Khazad ai-menu!" - Gimli
-------------------------------------------------------------------------------
http://www.the-infinite.org/ http://www.the-infinite.org/~dominic/

#13Dominic J. Eidson
sauron@the-infinite.org
In reply to: Peter Eisentraut (#11)
Re: Patch to include PAM support...

On Tue, 12 Jun 2001, Peter Eisentraut wrote:

Bruce Momjian writes:

OK, care to give a thumbs up on the patch?

http://candle.pha.pa.us/cgi-bin/pgpatches

From static inspection I have some doubts about whether this patch would

operate correctly. The way it is implemented is that if the backend is
instructed to use PAM authentication it pretends to the frontend that
password authentication is going on. This would probably work correctly

Correct - this was to save code duplication - since the frontend steps for
password authentication are the same, whether you're authenticating to
global/pg_pwd, or handing off the username/password processing to PAM.

if your PAM setup is that you require exactly one password from the user.
But if the PAM setup does not require a password (Kerberos, rhosts
modules?) it would involve a useless exchange (and possibly prompt) for a

This works fine - if it doesn't require a password, it won't get to the
"password prompt" step inside the conversation function, and ends up just
returning "success".

password. More importantly, though, if the PAM configuration requires
more than one password (perhaps the password is due to be changed), this
implementation will fail (to authenticate).

Typical use of a database, is from a non-interactive interface (script,
application, et al), where you aren't given the abillity to enter a second
password in the first place. Granted, this could be implemented - but my
goal was to emulate the existing libpq authentication process (which only
allows for the transmission of one password for all (the one?) of the
existing authentication methods that utilize passwords.

In all of the other remote authentication pieces that I have worked
with/used (radius, tacacs, etc) - if your password is in need to be
changed and/or expired - your authentication just fails.

Dominic, any comments?

--
Dominic J. Eidson
"Baruk Khazad! Khazad ai-menu!" - Gimli
-------------------------------------------------------------------------------
http://www.the-infinite.org/ http://www.the-infinite.org/~dominic/

#14Tom Lane
tgl@sss.pgh.pa.us
In reply to: Dominic J. Eidson (#12)
Re: Patch to include PAM support...

"Dominic J. Eidson" <sauron@the-infinite.org> writes:

My apologies if PAM has somehow been equated to "remote server
authentication piece" - there is a lot more to PAM than the abillity to
easily do remote authentication.

Right. Part of the reason I'm concerned is that if we support PAM,
then we don't *know* exactly what it is we are buying into or which
authentication protocol will be used. This doesn't bother me as long
as any PAM-induced failure is confined to the connection trying to use
a particular PAM auth mechanism. But it does bother me if such a problem
can cause denial of service for all clients.

We have this problem already with IDENT, and we know we need to fix it.
I'm just saying that we'd better fix it before we add PAM support, not
after.

regards, tom lane

#15Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#14)
Re: Patch to include PAM support...

"Dominic J. Eidson" <sauron@the-infinite.org> writes:

My apologies if PAM has somehow been equated to "remote server
authentication piece" - there is a lot more to PAM than the abillity to
easily do remote authentication.

Right. Part of the reason I'm concerned is that if we support PAM,
then we don't *know* exactly what it is we are buying into or which
authentication protocol will be used. This doesn't bother me as long
as any PAM-induced failure is confined to the connection trying to use
a particular PAM auth mechanism. But it does bother me if such a problem
can cause denial of service for all clients.

We have this problem already with IDENT, and we know we need to fix it.
I'm just saying that we'd better fix it before we add PAM support, not
after.

It is has the same problems as IDENT, and it doesn't add any new
problems, and it meets people's needs, why not add it? When we get
IDENT fixed we can fix PAM at the same time.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#16Peter Eisentraut
peter_e@gmx.net
In reply to: Dominic J. Eidson (#13)
Re: Patch to include PAM support...

Dominic J. Eidson writes:

if your PAM setup is that you require exactly one password from the user.
But if the PAM setup does not require a password (Kerberos, rhosts
modules?) it would involve a useless exchange (and possibly prompt) for a

This works fine - if it doesn't require a password, it won't get to the
"password prompt" step inside the conversation function, and ends up just
returning "success".

In the patch I'm looking at, the conversation function doesn't do any
actual "prompting", it looks at the password that has previously been
obtained by way of the password packet exchange. If no password is
required, the password is never looked at, but still obtained. That by
itself causes psql to print a password prompt.

Perhaps this could work: In the switch in be_recvauth(), you call the
pam_authenticate() and friends and if the sequence passes you report back
"OK". In the conversation function -- if it gets called -- send a
password packet and store the answer packet. You might have to play some
tricks here to obtain the answer packet, though.

In all of the other remote authentication pieces that I have worked
with/used (radius, tacacs, etc) - if your password is in need to be
changed and/or expired - your authentication just fails.

Alright.

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#15)
Re: Patch to include PAM support...

Bruce Momjian <pgman@candle.pha.pa.us> writes:

It is has the same problems as IDENT, and it doesn't add any new
problems, and it meets people's needs, why not add it?

Because (a) it greatly increases the scope of the vulnerability,
and (b) it adds more code that will need to be rewritten to fix the
problem. I want to fix the blocking problem first, then solicit a
PAM patch that fits into the rewritten postmaster.

regards, tom lane

#18Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#17)
Re: Patch to include PAM support...

Bruce Momjian <pgman@candle.pha.pa.us> writes:

It is has the same problems as IDENT, and it doesn't add any new
problems, and it meets people's needs, why not add it?

Because (a) it greatly increases the scope of the vulnerability,

How? It is just a new authentication method with the same problems as
our current ones.

and (b) it adds more code that will need to be rewritten to fix the
problem. I want to fix the blocking problem first, then solicit a
PAM patch that fits into the rewritten postmaster.

This seems to fit into the "wait for the perfect fix" solution which I
don't think applies here. There is no saying that a PAM patch will even
be around once we get the rest working.

Basically, we have some people who want it. Now we need to hear from
people who don't want it. I have a "no" from Tom and a "yes" from
"Peter E" (and the author).

We need more votes to decide.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#19Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#18)
Re: Patch to include PAM support...

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Because (a) it greatly increases the scope of the vulnerability,

How? It is just a new authentication method with the same problems as
our current ones.

No, it is not *a* new authentication method, it is an open interface
that could be plugged into almost anything. We need the top-level
postmaster process to be absolutely reliable; plugging into "almost
anything" is not conducive to reliability.

Besides, an hour ago you were ready to reject this patch for lack of
interest. Why are you suddenly so eager to ignore the risks and apply
it anyway?

regards, tom lane

#20Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#19)
Re: Patch to include PAM support...

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Because (a) it greatly increases the scope of the vulnerability,

How? It is just a new authentication method with the same problems as
our current ones.

No, it is not *a* new authentication method, it is an open interface
that could be plugged into almost anything. We need the top-level
postmaster process to be absolutely reliable; plugging into "almost
anything" is not conducive to reliability.

But isn't that the responsibility of the administrator? They are
already responsible for the IDENT servers they use. Isn't this the same
thing.

Besides, an hour ago you were ready to reject this patch for lack of
interest. Why are you suddenly so eager to ignore the risks and apply
it anyway?

Because some have now said they want it and I do not see the _new_ risks.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#21Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#20)
Re: Patch to include PAM support...

Bruce Momjian <pgman@candle.pha.pa.us> writes:

But isn't that the responsibility of the administrator? They are
already responsible for the IDENT servers they use.

Doesn't stop us from getting questions/complaints when it doesn't work.

regards, tom lane

#22Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#21)
Re: Patch to include PAM support...

Bruce Momjian <pgman@candle.pha.pa.us> writes:

But isn't that the responsibility of the administrator? They are
already responsible for the IDENT servers they use.

Doesn't stop us from getting questions/complaints when it doesn't work.

We are not getting flooded with IDENT problem reports, are we?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#23Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#11)
Re: Patch to include PAM support...

Peter Eisentraut <peter_e@gmx.net> writes:

... More importantly, though, if the PAM configuration requires
more than one password (perhaps the password is due to be changed), this
implementation will fail (to authenticate).

I *think* that the FE protocol will support more than one round of
password challenge, although given the lack of any way for the PAM
module to direct what prompt is given, that is unlikely to work
pleasantly.

The larger issue is how a PAM auth method of unknown characteristics
is going to fit into our existing FE/BE protocol. It would seem to me
that a protocol extension will be required. Lying to the frontend about
what is happening is very unlikely to prove workable in the long run.
What if the selected PAM auth method requires the client side to respond
in some special way?

regards, tom lane

#24Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#23)
Re: Patch to include PAM support...

Tom Lane writes:

The larger issue is how a PAM auth method of unknown characteristics
is going to fit into our existing FE/BE protocol. It would seem to me
that a protocol extension will be required. Lying to the frontend about
what is happening is very unlikely to prove workable in the long run.
What if the selected PAM auth method requires the client side to respond
in some special way?

The interaction that a PAM stack can initiate is limited to prompting for
one or more values and getting strings as an answer. The PAM-using
application registers a "conversation function" callback, which is
responsible for issuing the prompt and getting at the data in an
application-specific manner. Ideally, the libpq protocol and API would be
extended to support this generality, but based on Dominic's comments the
password exchange would work to support the useful subset of this
functionality without any protocol or API changes.

Most of the time, PAM is used as a wrapper around some password database
like NIS or LDAP (or maybe even PostgreSQL).

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

#25Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#24)
Re: Patch to include PAM support...

Peter Eisentraut <peter_e@gmx.net> writes:

The interaction that a PAM stack can initiate is limited to prompting for
one or more values and getting strings as an answer.

We could do that full-up, if only the FE/BE protocol included a prompt
string in the outgoing password request. However, given the difficulty
of reprogramming clients to cope with multiple password challenges,
you're probably right that handling the single-password case without
any protocol or client API change is the wiser course.

However, I'm still quite concerned about letting the postmaster ignore
its other clients while it's executing a PAM auth cycle that will
invoke who-knows-what processing. What's your take on that point?

regards, tom lane

#26Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Peter Eisentraut (#24)
Re: Patch to include PAM support...

The interaction that a PAM stack can initiate is limited to prompting for
one or more values and getting strings as an answer. The PAM-using
application registers a "conversation function" callback, which is
responsible for issuing the prompt and getting at the data in an
application-specific manner. Ideally, the libpq protocol and API would be
extended to support this generality, but based on Dominic's comments the
password exchange would work to support the useful subset of this
functionality without any protocol or API changes.

Most of the time, PAM is used as a wrapper around some password database
like NIS or LDAP (or maybe even PostgreSQL).

We now have enough "yes" votes to apply this patch. I will give another
day for comments on the patch's contents.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#27Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#18)
Re: Patch to include PAM support...

Bruce Momjian writes:

This seems to fit into the "wait for the perfect fix" solution which I
don't think applies here. There is no saying that a PAM patch will even
be around once we get the rest working.

Basically, we have some people who want it. Now we need to hear from
people who don't want it. I have a "no" from Tom and a "yes" from
"Peter E" (and the author).

Not in the current form.

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

#28Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#27)
Re: Patch to include PAM support...

Peter Eisentraut <peter_e@gmx.net> writes:

Basically, we have some people who want it. Now we need to hear from
people who don't want it. I have a "no" from Tom and a "yes" from
"Peter E" (and the author).

Not in the current form.

I think Peter's main objection was that it'd always prompt for a
password whether needed or not.

Could we change the PAM code so that it tries to run the PAM auth cycle
immediately on receipt of a connection request? If it gets a callback
for a password, it abandons the PAM conversation, sends off a password
request packet, and then tries again when the password comes back.

Of course, this would be hugely simpler if the work were being done in
a dedicated forked child of the postmaster ;-) ;-) ... just send the
request packet when PAM asks for a password, and sleep till it comes
back.

regards, tom lane

#29Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#28)
Re: Patch to include PAM support...

Peter Eisentraut <peter_e@gmx.net> writes:

Basically, we have some people who want it. Now we need to hear from
people who don't want it. I have a "no" from Tom and a "yes" from
"Peter E" (and the author).

Not in the current form.

I think Peter's main objection was that it'd always prompt for a
password whether needed or not.

OK, let's let Dominic work on that. Now that there is a strong chance
that the patch will be applied, I am sure he can try it.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#30Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#28)
Re: Patch to include PAM support...

Peter Eisentraut <peter_e@gmx.net> writes:

Basically, we have some people who want it. Now we need to hear from
people who don't want it. I have a "no" from Tom and a "yes" from
"Peter E" (and the author).

Not in the current form.

I think Peter's main objection was that it'd always prompt for a
password whether needed or not.

I am on IRC with the author now and he is working on it. It is actually
pretty nice to be on IRC while working on a patch because you can ask
questions and stuff. Channel #postgresql on EfNet.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#31Dominic J. Eidson
sauron@the-infinite.org
In reply to: Tom Lane (#28)
Re: [PATCHES] Patch to include PAM support...

On Wed, 13 Jun 2001, Tom Lane wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

Basically, we have some people who want it. Now we need to hear from
people who don't want it. I have a "no" from Tom and a "yes" from
"Peter E" (and the author).

Not in the current form.

I think Peter's main objection was that it'd always prompt for a
password whether needed or not.

Okay, after many months of lurking, I've finally set aside some time this
last week to actually finish up the code. (It's been mostly-merged/working
since about a week after Tom sent the mail I'm replying to - but then my
employer decided it would be good for us (read: me) to finish working on a
project which has consumed 99% of any programming motivation I could
muster.

Could we change the PAM code so that it tries to run the PAM auth cycle
immediately on receipt of a connection request? If it gets a callback
for a password, it abandons the PAM conversation, sends off a password
request packet, and then tries again when the password comes back.

I am attempting to do this in a way that's relatively elegant, and the
code should get sent to -patches tomorrow sometime , after I've had time
to do some testing.

--
Dominic J. Eidson
"Baruk Khazad! Khazad ai-menu!" - Gimli
-------------------------------------------------------------------------------
http://www.the-infinite.org/ http://www.the-infinite.org/~dominic/

#32Tom Lane
tgl@sss.pgh.pa.us
In reply to: Dominic J. Eidson (#31)
Re: Re: [PATCHES] Patch to include PAM support...

"Dominic J. Eidson" <sauron@the-infinite.org> writes:

Could we change the PAM code so that it tries to run the PAM auth cycle
immediately on receipt of a connection request? If it gets a callback
for a password, it abandons the PAM conversation, sends off a password
request packet, and then tries again when the password comes back.

I am attempting to do this in a way that's relatively elegant, and the
code should get sent to -patches tomorrow sometime , after I've had time
to do some testing.

I think that the main objection to the original form of the PAM patch
was that it would lock up the postmaster until the client responded.
However, that is *not* a concern any longer, since the current code
forks first and authenticates after. Accordingly, you shouldn't be
complexifying the PAM code to avoid waits.

regards, tom lane

#33Dominic J. Eidson
sauron@the-infinite.org
In reply to: Tom Lane (#32)
Re: Re: [PATCHES] Patch to include PAM support...

On Sat, 25 Aug 2001, Tom Lane wrote:

"Dominic J. Eidson" <sauron@the-infinite.org> writes:

Could we change the PAM code so that it tries to run the PAM auth cycle
immediately on receipt of a connection request? If it gets a callback
for a password, it abandons the PAM conversation, sends off a password
request packet, and then tries again when the password comes back.

I am attempting to do this in a way that's relatively elegant, and the
code should get sent to -patches tomorrow sometime , after I've had time
to do some testing.

I think that the main objection to the original form of the PAM patch
was that it would lock up the postmaster until the client responded.
However, that is *not* a concern any longer, since the current code
forks first and authenticates after. Accordingly, you shouldn't be
complexifying the PAM code to avoid waits.

The complexity comes from getting PAM to only send a password request to
the frontend if the PAM authentication needs a password, and not
otherwise. As I'd mentioned to Bruce before, I think PAM authentication
should be treated like password authentication - if there's a potential
that a password might be required, request a password, whether it's needed
or not. But PeterE asked that it only request a password if a password is
needed, so I'm fighting to get it to do exactly that.

(I already knew auth is done in the backend, and therefor can be blocking :)

--
Dominic J. Eidson
"Baruk Khazad! Khazad ai-menu!" - Gimli
-------------------------------------------------------------------------------
http://www.the-infinite.org/ http://www.the-infinite.org/~dominic/