Bug#885069: stretch-pu: package open-iscsi/2.0.874-3~deb9u1

[Salvatore Bonaccorso <carnil@debian.org>]

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 <lduncan@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