Re: [SSSD] [PATCH] Use SO_PEERCRED on the PAM socket

2010-04-16 Thread Stephen Gallagher
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

2010-04-15 Thread Sumit Bose
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

2010-04-15 Thread Stephen Gallagher
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

2010-04-15 Thread Sumit Bose
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

2010-04-15 Thread Stephen Gallagher
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

2010-03-31 Thread Stephen Gallagher
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

2010-03-29 Thread Sumit Bose
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;
-