Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-06-02 Thread Tom Lane
Marko Kreen mark...@gmail.com writes:
 Here's my attempt for it.  As conditional port module seems trouble,
 I set up an unconditional pgGetpeereid() that is always defined.

-1 ... why would you think that a conditional substitution is trouble?
We have plenty of others.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-06-02 Thread Marko Kreen
On Wed, Jun 1, 2011 at 1:03 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Marko Kreen mark...@gmail.com writes:
 My suggestion would be to use getpeereid() everywhere.
 And just have compat getpeereid() implementation on non-BSD
 platforms.  This would minimize ifdeffery in core core.

 Hm, maybe.  I'd be for this if we had more than two call sites, but
 as things stand I'm not sure it's worth the trouble to set up a src/port
 module for it.

Here's my attempt for it.  As conditional port module seems trouble,
I set up an unconditional pgGetpeereid() that is always defined.

The result seems nice.  It also fixes broken ifdeffery where
#error missing implementation is unreachable, instead
pqGetpwuid() can be reached with undefined uid.

It does drop 2 error messages for HAVE_UNIX_SOCKET but no method
for getting peer id.  Now it will give plain ENOSYS in that case.
If really required, the message can be picked based on errno,
but it does not seem worth it.

-- 
marko
*** a/src/backend/libpq/auth.c
--- b/src/backend/libpq/auth.c
***
*** 17,28 
  
  #include sys/param.h
  #include sys/socket.h
- #ifdef HAVE_UCRED_H
- #include ucred.h
- #endif
- #ifdef HAVE_SYS_UCRED_H
- #include sys/ucred.h
- #endif
  #include netinet/in.h
  #include arpa/inet.h
  #include unistd.h
--- 17,22 
***
*** 1757,1839  auth_peer(hbaPort *port)
  {
  	char		ident_user[IDENT_USERNAME_MAX + 1];
  	uid_t		uid = 0;
  	struct passwd *pass;
  
! #if defined(HAVE_GETPEEREID)
! 	/* Most BSDen, including OS X: use getpeereid() */
! 	gid_t		gid;
! 
! 	errno = 0;
! 	if (getpeereid(port-sock, uid, gid) != 0)
  	{
- 		/* We didn't get a valid credentials struct. */
  		ereport(LOG,
  (errcode_for_socket_access(),
   errmsg(could not get peer credentials: %m)));
  		return STATUS_ERROR;
  	}
- #elif defined(SO_PEERCRED)
- 	/* Linux: use getsockopt(SO_PEERCRED) */
- 	struct ucred peercred;
- 	ACCEPT_TYPE_ARG3 so_len = sizeof(peercred);
- 
- 	errno = 0;
- 	if (getsockopt(port-sock, SOL_SOCKET, SO_PEERCRED, peercred, so_len) != 0 ||
- 		so_len != sizeof(peercred))
- 	{
- 		/* We didn't get a valid credentials struct. */
- 		ereport(LOG,
- (errcode_for_socket_access(),
-  errmsg(could not get peer credentials: %m)));
- 		return STATUS_ERROR;
- 	}
- 	uid = peercred.uid;
- #elif defined(LOCAL_PEERCRED)
- 	/* Debian with FreeBSD kernel: use getsockopt(LOCAL_PEERCRED) */
- 	struct xucred peercred;
- 	ACCEPT_TYPE_ARG3 so_len = sizeof(peercred);
- 
- 	errno = 0;
- 	if (getsockopt(port-sock, 0, LOCAL_PEERCRED, peercred, so_len) != 0 ||
- 		so_len != sizeof(peercred) ||
- 		peercred.cr_version != XUCRED_VERSION)
- 	{
- 		/* We didn't get a valid credentials struct. */
- 		ereport(LOG,
- (errcode_for_socket_access(),
-  errmsg(could not get peer credentials: %m)));
- 		return STATUS_ERROR;
- 	}
- 	uid = peercred.cr_uid;
- #elif defined(HAVE_GETPEERUCRED)
- 	/* Solaris: use getpeerucred() */
- 	ucred_t*ucred;
- 
- 	ucred = NULL;/* must be initialized to NULL */
- 	if (getpeerucred(port-sock, ucred) == -1)
- 	{
- 		ereport(LOG,
- (errcode_for_socket_access(),
-  errmsg(could not get peer credentials: %m)));
- 		return STATUS_ERROR;
- 	}
- 
- 	if ((uid = ucred_geteuid(ucred)) == -1)
- 	{
- 		ereport(LOG,
- (errcode_for_socket_access(),
- 		   errmsg(could not get effective UID from peer credentials: %m)));
- 		return STATUS_ERROR;
- 	}
- 
- 	ucred_free(ucred);
- #else
- 	ereport(LOG,
- 			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- 			 errmsg(Peer authentication is not supported on local connections on this platform)));
- 
- 	return STATUS_ERROR;
- #endif
  
  	pass = getpwuid(uid);
  
--- 1751,1766 
  {
  	char		ident_user[IDENT_USERNAME_MAX + 1];
  	uid_t		uid = 0;
+ 	gid_t		gid = 0;
  	struct passwd *pass;
  
! 	if (pgGetpeereid(port-sock, uid, gid) != 0)
  	{
  		ereport(LOG,
  (errcode_for_socket_access(),
   errmsg(could not get peer credentials: %m)));
  		return STATUS_ERROR;
  	}
  
  	pass = getpwuid(uid);
  
*** a/src/include/port.h
--- b/src/include/port.h
***
*** 470,473  extern int	pg_check_dir(const char *dir);
--- 470,476 
  /* port/pgmkdirp.c */
  extern int	pg_mkdir_p(char *path, int omode);
  
+ /* port/pggetpeereid.c */
+ extern int pgGetpeereid(int sock, uid_t *uid, gid_t *gid);
+ 
  #endif   /* PG_PORT_H */
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***
*** 21,32 
  #include ctype.h
  #include time.h
  #include unistd.h
- #ifdef HAVE_UCRED_H
- #include ucred.h
- #endif
- #ifdef HAVE_SYS_UCRED_H
- #include sys/ucred.h
- #endif
  
  #include libpq-fe.h
  #include libpq-int.h
--- 21,26 
***
*** 1866,1928  keep_going:		/* We will come back to here until there is
  if (conn-requirepeer  conn-requirepeer[0] 
  	IS_AF_UNIX(conn-raddr.addr.ss_family))
  {
- #if defined(HAVE_GETPEEREID) || defined(SO_PEERCRED) || defined(LOCAL_PEERCRED) || 

Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-06-02 Thread Marko Kreen
On Thu, Jun 2, 2011 at 5:49 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Marko Kreen mark...@gmail.com writes:
 Here's my attempt for it.  As conditional port module seems trouble,
 I set up an unconditional pgGetpeereid() that is always defined.

 -1 ... why would you think that a conditional substitution is trouble?
 We have plenty of others.

Because it required touching autoconf. ;)

So now I did it.  I hope it was that simple.

As there was no going back now, I even touched msvc.pm.

-- 
marko
*** a/configure.in
--- b/configure.in
***
*** 1191,1197  PGAC_VAR_INT_TIMEZONE
  AC_FUNC_ACCEPT_ARGTYPES
  PGAC_FUNC_GETTIMEOFDAY_1ARG
  
! AC_CHECK_FUNCS([cbrt dlopen fcvt fdatasync getifaddrs getpeereid getpeerucred getrlimit memmove poll pstat readlink scandir setproctitle setsid sigprocmask symlink sysconf towlower utime utimes waitpid wcstombs wcstombs_l])
  
  AC_REPLACE_FUNCS(fseeko)
  case $host_os in
--- 1191,1199 
  AC_FUNC_ACCEPT_ARGTYPES
  PGAC_FUNC_GETTIMEOFDAY_1ARG
  
! AC_CHECK_FUNCS([cbrt dlopen fcvt fdatasync getifaddrs getpeerucred getrlimit memmove poll pstat readlink scandir setproctitle setsid sigprocmask symlink sysconf towlower utime utimes waitpid wcstombs wcstombs_l])
! 
! AC_REPLACE_FUNCS(getpeereid)
  
  AC_REPLACE_FUNCS(fseeko)
  case $host_os in
*** a/src/backend/libpq/auth.c
--- b/src/backend/libpq/auth.c
***
*** 17,28 
  
  #include sys/param.h
  #include sys/socket.h
- #ifdef HAVE_UCRED_H
- #include ucred.h
- #endif
- #ifdef HAVE_SYS_UCRED_H
- #include sys/ucred.h
- #endif
  #include netinet/in.h
  #include arpa/inet.h
  #include unistd.h
--- 17,22 
***
*** 1757,1839  auth_peer(hbaPort *port)
  {
  	char		ident_user[IDENT_USERNAME_MAX + 1];
  	uid_t		uid = 0;
  	struct passwd *pass;
  
- #if defined(HAVE_GETPEEREID)
- 	/* Most BSDen, including OS X: use getpeereid() */
- 	gid_t		gid;
- 
- 	errno = 0;
  	if (getpeereid(port-sock, uid, gid) != 0)
  	{
- 		/* We didn't get a valid credentials struct. */
  		ereport(LOG,
  (errcode_for_socket_access(),
   errmsg(could not get peer credentials: %m)));
  		return STATUS_ERROR;
  	}
- #elif defined(SO_PEERCRED)
- 	/* Linux: use getsockopt(SO_PEERCRED) */
- 	struct ucred peercred;
- 	ACCEPT_TYPE_ARG3 so_len = sizeof(peercred);
- 
- 	errno = 0;
- 	if (getsockopt(port-sock, SOL_SOCKET, SO_PEERCRED, peercred, so_len) != 0 ||
- 		so_len != sizeof(peercred))
- 	{
- 		/* We didn't get a valid credentials struct. */
- 		ereport(LOG,
- (errcode_for_socket_access(),
-  errmsg(could not get peer credentials: %m)));
- 		return STATUS_ERROR;
- 	}
- 	uid = peercred.uid;
- #elif defined(LOCAL_PEERCRED)
- 	/* Debian with FreeBSD kernel: use getsockopt(LOCAL_PEERCRED) */
- 	struct xucred peercred;
- 	ACCEPT_TYPE_ARG3 so_len = sizeof(peercred);
- 
- 	errno = 0;
- 	if (getsockopt(port-sock, 0, LOCAL_PEERCRED, peercred, so_len) != 0 ||
- 		so_len != sizeof(peercred) ||
- 		peercred.cr_version != XUCRED_VERSION)
- 	{
- 		/* We didn't get a valid credentials struct. */
- 		ereport(LOG,
- (errcode_for_socket_access(),
-  errmsg(could not get peer credentials: %m)));
- 		return STATUS_ERROR;
- 	}
- 	uid = peercred.cr_uid;
- #elif defined(HAVE_GETPEERUCRED)
- 	/* Solaris: use getpeerucred() */
- 	ucred_t*ucred;
- 
- 	ucred = NULL;/* must be initialized to NULL */
- 	if (getpeerucred(port-sock, ucred) == -1)
- 	{
- 		ereport(LOG,
- (errcode_for_socket_access(),
-  errmsg(could not get peer credentials: %m)));
- 		return STATUS_ERROR;
- 	}
- 
- 	if ((uid = ucred_geteuid(ucred)) == -1)
- 	{
- 		ereport(LOG,
- (errcode_for_socket_access(),
- 		   errmsg(could not get effective UID from peer credentials: %m)));
- 		return STATUS_ERROR;
- 	}
- 
- 	ucred_free(ucred);
- #else
- 	ereport(LOG,
- 			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- 			 errmsg(Peer authentication is not supported on local connections on this platform)));
- 
- 	return STATUS_ERROR;
- #endif
  
  	pass = getpwuid(uid);
  
--- 1751,1766 
  {
  	char		ident_user[IDENT_USERNAME_MAX + 1];
  	uid_t		uid = 0;
+ 	gid_t		gid = 0;
  	struct passwd *pass;
  
  	if (getpeereid(port-sock, uid, gid) != 0)
  	{
  		ereport(LOG,
  (errcode_for_socket_access(),
   errmsg(could not get peer credentials: %m)));
  		return STATUS_ERROR;
  	}
  
  	pass = getpwuid(uid);
  
*** a/src/include/port.h
--- b/src/include/port.h
***
*** 470,473  extern int	pg_check_dir(const char *dir);
--- 470,478 
  /* port/pgmkdirp.c */
  extern int	pg_mkdir_p(char *path, int omode);
  
+ /* port/getpeereid.c */
+ #ifndef HAVE_GETPEEREID
+ extern int getpeereid(int sock, uid_t *uid, gid_t *gid);
+ #endif
+ 
  #endif   /* PG_PORT_H */
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***
*** 21,32 
  #include ctype.h
  #include time.h
  #include unistd.h
- #ifdef HAVE_UCRED_H
- #include ucred.h
- #endif
- #ifdef HAVE_SYS_UCRED_H
- #include sys/ucred.h
- #endif
  
  

Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-06-02 Thread Andrew Dunstan



On 06/02/2011 11:29 AM, Marko Kreen wrote:


As there was no going back now, I even touched msvc.pm.


Why? Windows doesn't have Unix domain sockets at all.

cheers

andrew



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-06-02 Thread Andrew Dunstan



On 06/02/2011 12:04 PM, Marko Kreen wrote:

On Thu, Jun 2, 2011 at 6:59 PM, Andrew Dunstanand...@dunslane.net  wrote:

On 06/02/2011 11:29 AM, Marko Kreen wrote:

As there was no going back now, I even touched msvc.pm.

Why? Windows doesn't have Unix domain sockets at all.

Because the function is still referenced in the code.



Then maybe we need to use #ifndef WIN32 in those places. That's what 
we do for similar cases.


cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-06-02 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 On 06/02/2011 12:04 PM, Marko Kreen wrote:
 On Thu, Jun 2, 2011 at 6:59 PM, Andrew Dunstanand...@dunslane.net  wrote:
 On 06/02/2011 11:29 AM, Marko Kreen wrote:
 As there was no going back now, I even touched msvc.pm.
 Why? Windows doesn't have Unix domain sockets at all.
 Because the function is still referenced in the code.

 Then maybe we need to use #ifndef WIN32 in those places. That's what 
 we do for similar cases.

Seems reasonable, since the whole code chunk is within IS_AF_UNIX
anyway.  Will adjust and apply.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-06-02 Thread Marko Kreen
On Thu, Jun 2, 2011 at 7:20 PM, Andrew Dunstan and...@dunslane.net wrote:
 On 06/02/2011 12:04 PM, Marko Kreen wrote:
 On Thu, Jun 2, 2011 at 6:59 PM, Andrew Dunstanand...@dunslane.net
  wrote:
 On 06/02/2011 11:29 AM, Marko Kreen wrote:
 As there was no going back now, I even touched msvc.pm.

 Why? Windows doesn't have Unix domain sockets at all.

 Because the function is still referenced in the code.


 Then maybe we need to use #ifndef WIN32 in those places. That's what we do
 for similar cases.

No, that would be a bad idea - uglifies code for no good reason.

The function is referenced undef IS_AF_UNIX() check, so it would
not be run anyway.  Even if it would run somehow, there is only
2 lines to return ENOSYS.

With #ifdef you would need some additional error message under #ifdef WIN32,
just in case, so what exactly would be improved by that?

-- 
marko

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-06-02 Thread Marko Kreen
On Thu, Jun 2, 2011 at 6:59 PM, Andrew Dunstan and...@dunslane.net wrote:
 On 06/02/2011 11:29 AM, Marko Kreen wrote:
 As there was no going back now, I even touched msvc.pm.

 Why? Windows doesn't have Unix domain sockets at all.

Because the function is still referenced in the code.

-- 
marko

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-06-02 Thread Tom Lane
Marko Kreen mark...@gmail.com writes:
 -1 ... why would you think that a conditional substitution is trouble?
 We have plenty of others.

 Because it required touching autoconf. ;)
 So now I did it.  I hope it was that simple.

Applied with minor adjustments --- notably, I didn't agree with removing
the special-case error messages for platforms that lack support for
this.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-06-02 Thread Andrew Dunstan



On 06/02/2011 01:04 PM, Alvaro Herrera wrote:

Excerpts from Marko Kreen's message of jue jun 02 12:45:04 -0400 2011:

On Thu, Jun 2, 2011 at 7:31 PM, Alvaro Herrera
alvhe...@commandprompt.com  wrote:

Excerpts from Andrew Dunstan's message of jue jun 02 11:59:02 -0400 2011:

On 06/02/2011 11:29 AM, Marko Kreen wrote:

As there was no going back now, I even touched msvc.pm.

Why? Windows doesn't have Unix domain sockets at all.

So much for being thorough :-P

Well, there is 2 approaches to portable C code:
1) You #ifdef the main code portable
2) You #ifdef common platform in headers, then main code
is written against common platform, without ifdefs.

I'm from the camp #2.

I don't disagree, just saying that you seem to have gone out of your way
to produce something that doesn't seem to be necessary.


Yeah, I'm from the camp that says don't compile code that's guaranteed 
to be dead.


cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-06-02 Thread Alvaro Herrera
Excerpts from Andrew Dunstan's message of jue jun 02 11:59:02 -0400 2011:
 
 On 06/02/2011 11:29 AM, Marko Kreen wrote:
 
  As there was no going back now, I even touched msvc.pm.
 
 Why? Windows doesn't have Unix domain sockets at all.

So much for being thorough :-P

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-06-02 Thread Alvaro Herrera
Excerpts from Marko Kreen's message of jue jun 02 12:45:04 -0400 2011:
 On Thu, Jun 2, 2011 at 7:31 PM, Alvaro Herrera
 alvhe...@commandprompt.com wrote:
  Excerpts from Andrew Dunstan's message of jue jun 02 11:59:02 -0400 2011:
  On 06/02/2011 11:29 AM, Marko Kreen wrote:
   As there was no going back now, I even touched msvc.pm.
 
  Why? Windows doesn't have Unix domain sockets at all.
 
  So much for being thorough :-P
 
 Well, there is 2 approaches to portable C code:
 1) You #ifdef the main code portable
 2) You #ifdef common platform in headers, then main code
 is written against common platform, without ifdefs.
 
 I'm from the camp #2.

I don't disagree, just saying that you seem to have gone out of your way
to produce something that doesn't seem to be necessary.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-06-02 Thread Marko Kreen
On Thu, Jun 2, 2011 at 7:44 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Marko Kreen mark...@gmail.com writes:
 On Thu, Jun 2, 2011 at 7:20 PM, Andrew Dunstan and...@dunslane.net wrote:
 Then maybe we need to use #ifndef WIN32 in those places. That's what we do
 for similar cases.

 No, that would be a bad idea - uglifies code for no good reason.

 The function is referenced undef IS_AF_UNIX() check, so it would
 not be run anyway.  Even if it would run somehow, there is only
 2 lines to return ENOSYS.

 Yeah, but not compiling thirty lines in fe-connect.c is worthwhile.

 The auth_peer code in the backend is #ifdef HAVE_UNIX_SOCKETS, and
 I see no reason why this chunk in libpq shouldn't be as well.

ip.h:

#ifdef  HAVE_UNIX_SOCKETS
#define IS_AF_UNIX(fam) ((fam) == AF_UNIX)
#else
#define IS_AF_UNIX(fam) (0)
#endif

This the #ifdefs-in-headers-only approach to the problem...

-- 
marko

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-06-02 Thread Marko Kreen
On Thu, Jun 2, 2011 at 7:31 PM, Alvaro Herrera
alvhe...@commandprompt.com wrote:
 Excerpts from Andrew Dunstan's message of jue jun 02 11:59:02 -0400 2011:
 On 06/02/2011 11:29 AM, Marko Kreen wrote:
  As there was no going back now, I even touched msvc.pm.

 Why? Windows doesn't have Unix domain sockets at all.

 So much for being thorough :-P

Well, there is 2 approaches to portable C code:
1) You #ifdef the main code portable
2) You #ifdef common platform in headers, then main code
is written against common platform, without ifdefs.

I'm from the camp #2.

-- 
marko

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-06-02 Thread Tom Lane
Marko Kreen mark...@gmail.com writes:
 On Thu, Jun 2, 2011 at 7:20 PM, Andrew Dunstan and...@dunslane.net wrote:
 Then maybe we need to use #ifndef WIN32 in those places. That's what we do
 for similar cases.

 No, that would be a bad idea - uglifies code for no good reason.

 The function is referenced undef IS_AF_UNIX() check, so it would
 not be run anyway.  Even if it would run somehow, there is only
 2 lines to return ENOSYS.

Yeah, but not compiling thirty lines in fe-connect.c is worthwhile.

The auth_peer code in the backend is #ifdef HAVE_UNIX_SOCKETS, and
I see no reason why this chunk in libpq shouldn't be as well.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-05-31 Thread Andrew Dunstan



On 05/30/2011 07:57 PM, Tom Lane wrote:

I've applied patches to fix Martin Pitt's report of peer auth failing on
FreeBSD-amd64 kernels.  I tested it with FreeBSD but do not have the
resources to check every other platform that uses the same code branch
in auth_peer.  The buildfarm will soon tell us if the patches fail to
compile anywhere, but since the buildfarm doesn't test that
authentication path, it's not going to be as obvious whether it works.

So, if you have a BSD-ish machine, please try HEAD and see if peer auth
(or ident auth in older branches) still works for you.  Extra points
if you find out it used to be broken on your machine.  (Hey Stefan, did
you ever try that on spoonbill?)




There's actually no reason we couldn't test this in the buildfarm. 
Turning it on unconditionally is a one-line change. Making it happen 
just on the right platforms would be a few more lines, but nothing much. 
It breaks the dblink regression tests, so we'd either have to get around 
that or turn it off when checking contrib. I can add this if you think 
it's worth it.


But I did try it on my FBSD/x86_64 VM and it passed the tests up till it 
got to dblink, so there's another data point for you anyway.


cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-05-31 Thread Tom Lane
I wrote:
 BTW, after looking more closely at the buildfarm configure logs, it
 appears that both OpenBSD and NetBSD have getpeereid(), which means
 that they don't use this code at all.  It is currently looking to me
 like the HAVE_STRUCT_FCRED and HAVE_STRUCT_SOCKCRED variants are dead
 code.

Further research discloses that:

1. FreeBSD has has getpeereid() since 2001; their CVS logs show that it
was implemented mostly for compatibility with OpenBSD, so OpenBSD has
had it since even further back.

2. NetBSD implemented getpeereid() as of 5.0.

3. Mac OS X has had getpeereid() since 10.3 or thereabouts.

This means that in fact the control-message variant of auth_peer is dead
code on EVERY modern *BSD variant.  So far as I can find, the only
platform on which it is still used is Debian/kFreeBSD, that is Linux
userland running on a FreeBSD kernel: glibc naturally lacks getpeereid,
but the kernel doesn't have SO_PEERCRED, so you end up with the control
message stuff.  (This doubtless explains why the portability issues in
the control-message code escaped notice for so long.)

However, FreeBSD does have, and Debian/kFreeBSD does expose,
getsockopt(LOCAL_PEERCRED), which turns out to be functionally
equivalent to SO_PEERCRED: in particular, you can just call it and get
the answer without having to fool with getting the far end to send a
message.  This is not only a whole lot cleaner than what we have, but
also could be used to implement libpq's requirepeer option, which is
currently unsupported on such platforms.

So what I'm now thinking is we should rip out the control-message
implementation altogether, and instead use LOCAL_PEERCRED.  This is
probably not something to back-patch, but it would make things a lot
cleaner going forward.

Comments?

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-05-31 Thread Peter Eisentraut
On tis, 2011-05-31 at 11:59 -0400, Tom Lane wrote:
 However, FreeBSD does have, and Debian/kFreeBSD does expose,
 getsockopt(LOCAL_PEERCRED), which turns out to be functionally
 equivalent to SO_PEERCRED: in particular, you can just call it and get
 the answer without having to fool with getting the far end to send a
 message.  This is not only a whole lot cleaner than what we have, but
 also could be used to implement libpq's requirepeer option, which is
 currently unsupported on such platforms.
 
 So what I'm now thinking is we should rip out the control-message
 implementation altogether, and instead use LOCAL_PEERCRED.  This is
 probably not something to back-patch, but it would make things a lot
 cleaner going forward.

Oh yes, no point in having complicated code that doesn't get exercised.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-05-31 Thread Greg Stark
On Tue, May 31, 2011 at 12:38 PM, Peter Eisentraut pete...@gmx.net wrote:
 So what I'm now thinking is we should rip out the control-message
 implementation altogether, and instead use LOCAL_PEERCRED.  This is
 probably not something to back-patch, but it would make things a lot
 cleaner going forward.

 Oh yes, no point in having complicated code that doesn't get exercised.


This does amount to desupporting old versions of those OSes in newer
versions of Postgres, at least for this one feature. Since you're
saying you don't want to backport it that doesn't seem like a big deal
to me. Probably something worth mentioning in release notes though.

-- 
greg

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-05-31 Thread Tom Lane
Greg Stark gsst...@mit.edu writes:
 On Tue, May 31, 2011 at 12:38 PM, Peter Eisentraut pete...@gmx.net wrote:
 Oh yes, no point in having complicated code that doesn't get exercised.

 This does amount to desupporting old versions of those OSes in newer
 versions of Postgres, at least for this one feature. Since you're
 saying you don't want to backport it that doesn't seem like a big deal
 to me. Probably something worth mentioning in release notes though.

Yeah, possibly.  So far as I can tell, both FreeBSD and OpenBSD have had
getpeereid for so long that it couldn't be an issue.  I guess there
might still be some people running pre-5.0 versions of NetBSD though.

(BTW, in both FreeBSD and NetBSD, it turns out that getpeereid is just a
thin wrapper around SO_PEERCRED-equivalent getsockopt calls.  However,
there doesn't seem to be any point in supporting NetBSD's getsockopt
call directly, because it was added at the same time as the getpeereid
function.  Unless maybe there's a kFreeBSD-like project out there with
NetBSD as the kernel?)

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-05-31 Thread Marko Kreen
On Wed, Jun 1, 2011 at 12:22 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Greg Stark gsst...@mit.edu writes:
 On Tue, May 31, 2011 at 12:38 PM, Peter Eisentraut pete...@gmx.net wrote:
 Oh yes, no point in having complicated code that doesn't get exercised.

 This does amount to desupporting old versions of those OSes in newer
 versions of Postgres, at least for this one feature. Since you're
 saying you don't want to backport it that doesn't seem like a big deal
 to me. Probably something worth mentioning in release notes though.

 Yeah, possibly.  So far as I can tell, both FreeBSD and OpenBSD have had
 getpeereid for so long that it couldn't be an issue.  I guess there
 might still be some people running pre-5.0 versions of NetBSD though.

 (BTW, in both FreeBSD and NetBSD, it turns out that getpeereid is just a
 thin wrapper around SO_PEERCRED-equivalent getsockopt calls.  However,
 there doesn't seem to be any point in supporting NetBSD's getsockopt
 call directly, because it was added at the same time as the getpeereid
 function.  Unless maybe there's a kFreeBSD-like project out there with
 NetBSD as the kernel?)

My suggestion would be to use getpeereid() everywhere.
And just have compat getpeereid() implementation on non-BSD
platforms.  This would minimize ifdeffery in core core.

-- 
marko

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-05-31 Thread Tom Lane
Marko Kreen mark...@gmail.com writes:
 My suggestion would be to use getpeereid() everywhere.
 And just have compat getpeereid() implementation on non-BSD
 platforms.  This would minimize ifdeffery in core core.

Hm, maybe.  I'd be for this if we had more than two call sites, but
as things stand I'm not sure it's worth the trouble to set up a src/port
module for it.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-05-31 Thread Nicolas Barbier
2011/5/31, Tom Lane t...@sss.pgh.pa.us:

 Unless maybe there's a kFreeBSD-like project out there with NetBSD as
 the kernel?)

There used to be an attempt by Debian (called GNU/NetBSD), but that
has since long been abandoned. I don't know of any other similar
projects.

URL:http://www.debian.org/ports/netbsd/

Wikipedia doesn't list any other similar projects either:

URL:http://en.wikipedia.org/wiki/GNU_variants#BSD_variants

Nicolas

-- 
A. Because it breaks the logical sequence of discussion.
Q. Why is top posting bad?

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-05-31 Thread Marko Kreen
On Wed, Jun 1, 2011 at 1:03 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Marko Kreen mark...@gmail.com writes:
 My suggestion would be to use getpeereid() everywhere.
 And just have compat getpeereid() implementation on non-BSD
 platforms.  This would minimize ifdeffery in core core.

 Hm, maybe.  I'd be for this if we had more than two call sites, but
 as things stand I'm not sure it's worth the trouble to set up a src/port
 module for it.

It would remove ~50 lines of low-level code from otherwise
high-level function.  So even with one call site it would be improvement.

If the src/port is trouble, how about putting it as 'static inline' into .h?  :)

-- 
marko

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Please test peer (socket ident) auth on *BSD

2011-05-30 Thread Tom Lane
I've applied patches to fix Martin Pitt's report of peer auth failing on
FreeBSD-amd64 kernels.  I tested it with FreeBSD but do not have the
resources to check every other platform that uses the same code branch
in auth_peer.  The buildfarm will soon tell us if the patches fail to
compile anywhere, but since the buildfarm doesn't test that
authentication path, it's not going to be as obvious whether it works.

So, if you have a BSD-ish machine, please try HEAD and see if peer auth
(or ident auth in older branches) still works for you.  Extra points
if you find out it used to be broken on your machine.  (Hey Stefan, did
you ever try that on spoonbill?)

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Please test peer (socket ident) auth on *BSD

2011-05-30 Thread Tom Lane
I wrote:
 I've applied patches to fix Martin Pitt's report of peer auth failing on
 FreeBSD-amd64 kernels.  I tested it with FreeBSD but do not have the
 resources to check every other platform that uses the same code branch
 in auth_peer.  The buildfarm will soon tell us if the patches fail to
 compile anywhere, but since the buildfarm doesn't test that
 authentication path, it's not going to be as obvious whether it works.

 So, if you have a BSD-ish machine, please try HEAD and see if peer auth
 (or ident auth in older branches) still works for you.  Extra points
 if you find out it used to be broken on your machine.  (Hey Stefan, did
 you ever try that on spoonbill?)

BTW, after looking more closely at the buildfarm configure logs, it
appears that both OpenBSD and NetBSD have getpeereid(), which means
that they don't use this code at all.  It is currently looking to me
like the HAVE_STRUCT_FCRED and HAVE_STRUCT_SOCKCRED variants are dead
code.  They've been in there since before we had the getpeereid() code
path, and presumably were needed at one time ... but does anyone know
of a platform where they're still needed?

I'm a bit inclined to rip that code out of HEAD, if we can't point to a
platform where it'd be needed, just to reduce the #ifdef spaghetti.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers