Re: [SSSD] [PATCH] Use SO_PEERCRED on the PAM socket
On 04/15/2010 09:32 AM, Stephen Gallagher wrote: On 04/15/2010 09:06 AM, Sumit Bose wrote: ah, sorry, looks like it is time to upgrade :-) New version attached. bye, Sumit Ack. Pushed to master and sssd-1-2. -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Use SO_PEERCRED on the PAM socket
On Wed, Mar 31, 2010 at 03:29:58PM -0400, Stephen Gallagher wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 03/29/2010 05:23 AM, Sumit Bose wrote: Hi, please find attached my second attempt to exchange uid, gid and pid between PAM client and responder. This new apporoach does not require any communication between the client and the responder and should behave much better than the previous one based on SO_PASSCRED and SCM_CREDENTIALS. To make the review easier the first patch reverts the previous attempt. bye, Sumit Nack. Please make the strings in ssscli_err2string() translatable. strerror() is already internationalized, we should be consistent. (Also, we may eventually use these in places other than logger()) Otherwise, this looks great (and much less intrusive than the first attempt). New version attached. bye, Sumit - -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkuzorYACgkQeiVVYja6o6O7hwCfYgyIzEYx7HrSOudn6KXEGT4I sMYAn2UKunxU2JGP8w3PBJAhxX7gaL+J =4PZ7 -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel From 0be18996d1c72be1a8f4bd7954b6412124986029 Mon Sep 17 00:00:00 2001 From: Sumit Bose sb...@redhat.com Date: Fri, 26 Mar 2010 10:11:22 +0100 Subject: [PATCH 1/2] Revert Add better checks on PAM socket This reverts commit 5a88e963744e5da453e88b5c36499f04712df097. --- src/external/platform.m4| 12 --- src/responder/common/responder.h|4 - src/responder/common/responder_common.c | 137 +-- src/sss_client/common.c | 126 +--- 4 files changed, 5 insertions(+), 274 deletions(-) diff --git a/src/external/platform.m4 b/src/external/platform.m4 index ee00937..71b4f2c 100644 --- a/src/external/platform.m4 +++ b/src/external/platform.m4 @@ -27,15 +27,3 @@ fi AM_CONDITIONAL([HAVE_FEDORA], [test x$osname == xfedora]) AM_CONDITIONAL([HAVE_REDHAT], [test x$osname == xredhat]) AM_CONDITIONAL([HAVE_SUSE], [test x$osname == xsuse]) - -AC_CHECK_MEMBERS([struct ucred.pid, struct ucred.uid, struct ucred.gid], , , - [[#define _GNU_SOURCE - #include sys/socket.h]]) - -if test x$ac_cv_member_struct_ucred_pid = xyes -a \ -x$ac_cv_member_struct_ucred_uid = xyes -a \ -x$ac_cv_member_struct_ucred_gid = xyes ; then -AC_DEFINE([HAVE_UCRED], [1], [Define if struct ucred is available]) -else -AC_MSG_WARN([struct ucred is not available]) -fi diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h index 6391fcf..ea6ba58 100644 --- a/src/responder/common/responder.h +++ b/src/responder/common/responder.h @@ -101,10 +101,6 @@ struct cli_ctx { struct cli_request *creq; struct cli_protocol_version *cli_protocol_version; int priv; -int creds_exchange_done; -int client_uid; -int client_gid; -int client_pid; }; struct sss_cmd_table { diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c index 315eb73..48ec4bd 100644 --- a/src/responder/common/responder_common.c +++ b/src/responder/common/responder_common.c @@ -19,9 +19,6 @@ along with this program. If not, see http://www.gnu.org/licenses/. */ -/* for struct ucred */ -#define _GNU_SOURCE - #include stdio.h #include unistd.h #include fcntl.h @@ -32,8 +29,7 @@ #include string.h #include sys/time.h #include errno.h -#include popt.h -#include config.h +#include popt.h #include util/util.h #include db/sysdb.h #include confdb/confdb.h @@ -148,134 +144,12 @@ static void client_recv(struct cli_ctx *cctx) return; } -static void cred_handler(struct cli_ctx *cctx, char action) -{ -#ifdef HAVE_UCRED -int ret; -int fd; -struct msghdr msg; -struct iovec iov; -struct cmsghdr *cmsg; -struct ucred *creds; -/* buf must be aligned on some architectures. */ -union ubuf { -int align; -char buf[CMSG_SPACE(sizeof(struct ucred))]; -} u; -char dummy='s'; -int enable=1; - -if (cctx-creds_exchange_done != 0) { -DEBUG(1, (cred_handler called, but creds are already exchanged.\n)); -goto failed; -} - -fd = cctx-cfd; - -iov.iov_base = dummy; -iov.iov_len = 1; - -memset (msg, 0, sizeof(msg)); - -msg.msg_name = NULL; -msg.msg_namelen = 0; -msg.msg_iov = iov; -msg.msg_iovlen = 1; - -msg.msg_control = u.buf; -msg.msg_controllen = sizeof(u.buf); - -switch (action) { -case 'r': -ret = setsockopt(fd, SOL_SOCKET, SO_PASSCRED, enable,
Re: [SSSD] [PATCH] Use SO_PEERCRED on the PAM socket
On 04/15/2010 07:49 AM, Sumit Bose wrote: New version attached. bye, Sumit Nack. Warnings seen while compiling on Fedora 13: ../../src/responder/common/responder_common.c: In function ‘get_client_cred’: ../../src/responder/common/responder_common.c:80: warning: passing argument 5 of ‘getsockopt’ from incompatible pointer type /usr/include/sys/socket.h:190: note: expected ‘socklen_t * __restrict__’ but argument is of type ‘size_t *’ ../../src/sss_client/common.c: In function ‘check_server_cred’: ../../src/sss_client/common.c:650: warning: passing argument 5 of ‘getsockopt’ from incompatible pointer type /usr/include/sys/socket.h:190: note: expected ‘socklen_t * __restrict__’ but argument is of type ‘size_t *’ -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Use SO_PEERCRED on the PAM socket
On Thu, Apr 15, 2010 at 08:24:59AM -0400, Stephen Gallagher wrote: On 04/15/2010 07:49 AM, Sumit Bose wrote: New version attached. bye, Sumit Nack. Warnings seen while compiling on Fedora 13: ../../src/responder/common/responder_common.c: In function ‘get_client_cred’: ../../src/responder/common/responder_common.c:80: warning: passing argument 5 of ‘getsockopt’ from incompatible pointer type /usr/include/sys/socket.h:190: note: expected ‘socklen_t * __restrict__’ but argument is of type ‘size_t *’ ../../src/sss_client/common.c: In function ‘check_server_cred’: ../../src/sss_client/common.c:650: warning: passing argument 5 of ‘getsockopt’ from incompatible pointer type /usr/include/sys/socket.h:190: note: expected ‘socklen_t * __restrict__’ but argument is of type ‘size_t *’ ah, sorry, looks like it is time to upgrade :-) New version attached. bye, Sumit -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel From 0be18996d1c72be1a8f4bd7954b6412124986029 Mon Sep 17 00:00:00 2001 From: Sumit Bose sb...@redhat.com Date: Fri, 26 Mar 2010 10:11:22 +0100 Subject: [PATCH 1/2] Revert Add better checks on PAM socket This reverts commit 5a88e963744e5da453e88b5c36499f04712df097. --- src/external/platform.m4| 12 --- src/responder/common/responder.h|4 - src/responder/common/responder_common.c | 137 +-- src/sss_client/common.c | 126 +--- 4 files changed, 5 insertions(+), 274 deletions(-) diff --git a/src/external/platform.m4 b/src/external/platform.m4 index ee00937..71b4f2c 100644 --- a/src/external/platform.m4 +++ b/src/external/platform.m4 @@ -27,15 +27,3 @@ fi AM_CONDITIONAL([HAVE_FEDORA], [test x$osname == xfedora]) AM_CONDITIONAL([HAVE_REDHAT], [test x$osname == xredhat]) AM_CONDITIONAL([HAVE_SUSE], [test x$osname == xsuse]) - -AC_CHECK_MEMBERS([struct ucred.pid, struct ucred.uid, struct ucred.gid], , , - [[#define _GNU_SOURCE - #include sys/socket.h]]) - -if test x$ac_cv_member_struct_ucred_pid = xyes -a \ -x$ac_cv_member_struct_ucred_uid = xyes -a \ -x$ac_cv_member_struct_ucred_gid = xyes ; then -AC_DEFINE([HAVE_UCRED], [1], [Define if struct ucred is available]) -else -AC_MSG_WARN([struct ucred is not available]) -fi diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h index 6391fcf..ea6ba58 100644 --- a/src/responder/common/responder.h +++ b/src/responder/common/responder.h @@ -101,10 +101,6 @@ struct cli_ctx { struct cli_request *creq; struct cli_protocol_version *cli_protocol_version; int priv; -int creds_exchange_done; -int client_uid; -int client_gid; -int client_pid; }; struct sss_cmd_table { diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c index 315eb73..48ec4bd 100644 --- a/src/responder/common/responder_common.c +++ b/src/responder/common/responder_common.c @@ -19,9 +19,6 @@ along with this program. If not, see http://www.gnu.org/licenses/. */ -/* for struct ucred */ -#define _GNU_SOURCE - #include stdio.h #include unistd.h #include fcntl.h @@ -32,8 +29,7 @@ #include string.h #include sys/time.h #include errno.h -#include popt.h -#include config.h +#include popt.h #include util/util.h #include db/sysdb.h #include confdb/confdb.h @@ -148,134 +144,12 @@ static void client_recv(struct cli_ctx *cctx) return; } -static void cred_handler(struct cli_ctx *cctx, char action) -{ -#ifdef HAVE_UCRED -int ret; -int fd; -struct msghdr msg; -struct iovec iov; -struct cmsghdr *cmsg; -struct ucred *creds; -/* buf must be aligned on some architectures. */ -union ubuf { -int align; -char buf[CMSG_SPACE(sizeof(struct ucred))]; -} u; -char dummy='s'; -int enable=1; - -if (cctx-creds_exchange_done != 0) { -DEBUG(1, (cred_handler called, but creds are already exchanged.\n)); -goto failed; -} - -fd = cctx-cfd; - -iov.iov_base = dummy; -iov.iov_len = 1; - -memset (msg, 0, sizeof(msg)); - -msg.msg_name = NULL; -msg.msg_namelen = 0; -msg.msg_iov = iov; -msg.msg_iovlen = 1; - -msg.msg_control = u.buf; -msg.msg_controllen = sizeof(u.buf); - -switch (action) { -case 'r': -ret = setsockopt(fd, SOL_SOCKET, SO_PASSCRED, enable, sizeof(int)); -if (ret == -1) { -DEBUG(1, (setsockopt failed: [%d][%s].\n, errno, -strerror(errno))); -goto failed; -} - -ret
Re: [SSSD] [PATCH] Use SO_PEERCRED on the PAM socket
On 04/15/2010 09:06 AM, Sumit Bose wrote: ah, sorry, looks like it is time to upgrade :-) New version attached. bye, Sumit Ack. -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Use SO_PEERCRED on the PAM socket
This goes to syslog, not debug logs. On Mar 31, 2010, at 4:38 PM, Simo Sorce sso...@redhat.com wrote: On Wed, 31 Mar 2010 15:29:58 -0400 Stephen Gallagher sgall...@redhat.com wrote: Nack. Please make the strings in ssscli_err2string() translatable. strerror() is already internationalized, we should be consistent. (Also, we may eventually use these in places other than logger()) Otherwise, this looks great (and much less intrusive than the first attempt). I thought we said we would translate only log messages not debug messages, what am I missing ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
[SSSD] [PATCH] Use SO_PEERCRED on the PAM socket
Hi, please find attached my second attempt to exchange uid, gid and pid between PAM client and responder. This new apporoach does not require any communication between the client and the responder and should behave much better than the previous one based on SO_PASSCRED and SCM_CREDENTIALS. To make the review easier the first patch reverts the previous attempt. bye, Sumit From 480d2e7f822256f1bd871b34007b261047a2bb46 Mon Sep 17 00:00:00 2001 From: Sumit Bose sb...@redhat.com Date: Fri, 26 Mar 2010 10:11:22 +0100 Subject: [PATCH 1/2] Revert Add better checks on PAM socket This reverts commit 5a88e963744e5da453e88b5c36499f04712df097. --- src/external/platform.m4| 12 --- src/responder/common/responder.h|4 - src/responder/common/responder_common.c | 137 +-- src/sss_client/common.c | 126 +--- 4 files changed, 5 insertions(+), 274 deletions(-) diff --git a/src/external/platform.m4 b/src/external/platform.m4 index ee00937..71b4f2c 100644 --- a/src/external/platform.m4 +++ b/src/external/platform.m4 @@ -27,15 +27,3 @@ fi AM_CONDITIONAL([HAVE_FEDORA], [test x$osname == xfedora]) AM_CONDITIONAL([HAVE_REDHAT], [test x$osname == xredhat]) AM_CONDITIONAL([HAVE_SUSE], [test x$osname == xsuse]) - -AC_CHECK_MEMBERS([struct ucred.pid, struct ucred.uid, struct ucred.gid], , , - [[#define _GNU_SOURCE - #include sys/socket.h]]) - -if test x$ac_cv_member_struct_ucred_pid = xyes -a \ -x$ac_cv_member_struct_ucred_uid = xyes -a \ -x$ac_cv_member_struct_ucred_gid = xyes ; then -AC_DEFINE([HAVE_UCRED], [1], [Define if struct ucred is available]) -else -AC_MSG_WARN([struct ucred is not available]) -fi diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h index 6391fcf..ea6ba58 100644 --- a/src/responder/common/responder.h +++ b/src/responder/common/responder.h @@ -101,10 +101,6 @@ struct cli_ctx { struct cli_request *creq; struct cli_protocol_version *cli_protocol_version; int priv; -int creds_exchange_done; -int client_uid; -int client_gid; -int client_pid; }; struct sss_cmd_table { diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c index 315eb73..48ec4bd 100644 --- a/src/responder/common/responder_common.c +++ b/src/responder/common/responder_common.c @@ -19,9 +19,6 @@ along with this program. If not, see http://www.gnu.org/licenses/. */ -/* for struct ucred */ -#define _GNU_SOURCE - #include stdio.h #include unistd.h #include fcntl.h @@ -32,8 +29,7 @@ #include string.h #include sys/time.h #include errno.h -#include popt.h -#include config.h +#include popt.h #include util/util.h #include db/sysdb.h #include confdb/confdb.h @@ -148,134 +144,12 @@ static void client_recv(struct cli_ctx *cctx) return; } -static void cred_handler(struct cli_ctx *cctx, char action) -{ -#ifdef HAVE_UCRED -int ret; -int fd; -struct msghdr msg; -struct iovec iov; -struct cmsghdr *cmsg; -struct ucred *creds; -/* buf must be aligned on some architectures. */ -union ubuf { -int align; -char buf[CMSG_SPACE(sizeof(struct ucred))]; -} u; -char dummy='s'; -int enable=1; - -if (cctx-creds_exchange_done != 0) { -DEBUG(1, (cred_handler called, but creds are already exchanged.\n)); -goto failed; -} - -fd = cctx-cfd; - -iov.iov_base = dummy; -iov.iov_len = 1; - -memset (msg, 0, sizeof(msg)); - -msg.msg_name = NULL; -msg.msg_namelen = 0; -msg.msg_iov = iov; -msg.msg_iovlen = 1; - -msg.msg_control = u.buf; -msg.msg_controllen = sizeof(u.buf); - -switch (action) { -case 'r': -ret = setsockopt(fd, SOL_SOCKET, SO_PASSCRED, enable, sizeof(int)); -if (ret == -1) { -DEBUG(1, (setsockopt failed: [%d][%s].\n, errno, -strerror(errno))); -goto failed; -} - -ret = recvmsg(fd, msg, 0); -if (ret == -1) { -DEBUG(1, (recvmsg failed.[%d][%s]\n, errno, strerror(errno))); -goto failed; -} - -cmsg = CMSG_FIRSTHDR(msg); - -if (cmsg-cmsg_level == SOL_SOCKET -cmsg-cmsg_type == SCM_CREDENTIALS) { -creds = (struct ucred *) CMSG_DATA(cmsg); -DEBUG(1, (creds: [%d][%d][%d]\n,creds-uid, creds-gid, - creds-pid)); -cctx-client_uid = creds-uid; -cctx-client_gid = creds-gid; -cctx-client_pid = creds-pid; -} - -TEVENT_FD_WRITEABLE(cctx-cfde); - -return; -break; -case 's': -cmsg = CMSG_FIRSTHDR(msg); -cmsg-cmsg_level = SOL_SOCKET; -