Hi Christian,

On Fri, Nov 09, 2018 at 06:53:07AM +0100, Salvatore Bonaccorso wrote:
> Hi Christian,
> 
> On Sat, Feb 10, 2018 at 10:15:48AM +0100, Julien Cristau wrote:
> > Control: tag -1 moreinfo
> > 
> > On Sat, Dec 23, 2017 at 13:40:43 +0100, Christian Seiler wrote:
> > 
> > > diff -Nru 
> > > open-iscsi-2.0.874/debian/patches/security/Check-for-root-peer-user-for-iscsiuio-IPC.patch
> > >  
> > > open-iscsi-2.0.874/debian/patches/security/Check-for-root-peer-user-for-iscsiuio-IPC.patch
> > > --- 
> > > open-iscsi-2.0.874/debian/patches/security/Check-for-root-peer-user-for-iscsiuio-IPC.patch
> > >     1970-01-01 01:00:00.000000000 +0100
> > > +++ 
> > > open-iscsi-2.0.874/debian/patches/security/Check-for-root-peer-user-for-iscsiuio-IPC.patch
> > >     2017-12-23 13:09:13.000000000 +0100
> > > @@ -0,0 +1,122 @@
> > > +From e313bd648a4c8a9526421e270eb597a5de1e0c7f Mon Sep 17 00:00:00 2001
> > > +From: Lee Duncan <ldun...@suse.com>
> > > +Date: Fri, 15 Dec 2017 10:36:11 -0800
> > > +Subject: [PATCH 1/8] Check for root peer user for iscsiuio IPC
> > > +
> > > +This fixes a possible vulnerability where a non-root
> > > +process could connect with iscsiuio. Fouund by Qualsys.
> > > +---
> > > + iscsiuio/src/unix/Makefile.am  |  3 ++-
> > > + iscsiuio/src/unix/iscsid_ipc.c | 47 
> > > ++++++++++++++++++++++++++++++++++++++++++
> > > + 2 files changed, 49 insertions(+), 1 deletion(-)
> > > +
> > [...]
> > > +@@ -1029,6 +1035,40 @@ static void iscsid_loop_close(void *arg)
> > > +         LOG_INFO(PFX "iSCSI daemon socket closed");
> > > + }
> > > + 
> > > ++/*
> > > ++ * check that the peer user is privilidged
> > > ++ *
> > 
> > This function doesn't actually do that.
> > 
> > > ++ * return 1 if peer is ok else 0
> > > ++ *
> > > ++ * XXX: this function is copied from iscsid_ipc.c and should be
> > > ++ * moved into a common library
> > > ++ */
> > > ++static int
> > > ++mgmt_peeruser(int sock, char *user)
> > > ++{
> > > ++        struct ucred peercred;
> > > ++        socklen_t so_len = sizeof(peercred);
> > > ++        struct passwd *pass;
> > > ++
> > > ++        errno = 0;
> > > ++        if (getsockopt(sock, SOL_SOCKET, SO_PEERCRED, &peercred,
> > > ++                &so_len) != 0 || so_len != sizeof(peercred)) {
> > > ++                /* We didn't get a valid credentials struct. */
> > > ++                LOG_ERR(PFX "peeruser_unux: error receiving 
> > > credentials: %m");
> > > ++                return 0;
> > > ++        }
> > > ++
> > > ++        pass = getpwuid(peercred.uid);
> > > ++        if (pass == NULL) {
> > > ++                LOG_ERR(PFX "peeruser_unix: unknown local user with uid 
> > > %d",
> > > ++                                (int) peercred.uid);
> > > ++                return 0;
> > > ++        }
> > > ++
> > > ++        strlcpy(user, pass->pw_name, PEERUSER_MAX);
> > > ++        return 1;
> > > ++}
> > > ++
> > > + /**
> > > +  *  iscsid_loop() - This is the function which will process the 
> > > broadcast
> > > +  *                  messages from iscsid
> > > +@@ -1038,6 +1078,7 @@ static void *iscsid_loop(void *arg)
> > > + {
> > > +         int rc;
> > > +         sigset_t set;
> > > ++        char user[PEERUSER_MAX];
> > > + 
> > > +         pthread_cleanup_push(iscsid_loop_close, arg);
> > > + 
> > > +@@ -1077,6 +1118,12 @@ static void *iscsid_loop(void *arg)
> > > +                         continue;
> > > +                 }
> > > + 
> > > ++                if (!mgmt_peeruser(iscsid_opts.fd, user) || 
> > > strncmp(user, "root", PEERUSER_MAX)) {
> > > ++                        close(s2);
> > > ++                        LOG_ERR(PFX "Access error: non-administrative 
> > > connection rejected");
> > > ++                        break;
> > > ++                }
> > > ++
> > > +                 process_iscsid_broadcast(s2);
> > > +                 close(s2);
> > > +         }
> > 
> > The above makes little sense to me.  We find out the peer uid, then
> > instead of just comparing that against 0 we turn it into a struct passwd
> > and compare pw_name against "root".  Why?
> 
> Did you had any chance to look at Julien's concerns/questions back on
> this proposed update for stretch?

Friendly ping :)

Regards,
Salvatore

Reply via email to