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?
Cheers,
Julien
