From 4b377041ef88f7c683b0a880e937e71a773c2c68 Mon Sep 17 00:00:00 2001 From: Eric Mackay Date: Fri, 29 Apr 2022 18:02:47 -0700 Subject: Add ability for MGMT IPC to check UID only The default behavior in iscsid, which is to check mgmt IPCs for UID==0 and user explicitly named "root", is unchanged. This option to perform only the UID check for management IPCs can be enabled in iscsid.conf. This can be useful for running iscsid in a constrained environment, e.g., initramfs. For instance, klibc initramfs in Ubuntu and Debian does not include a user DB. Attempting to run iscsid and iscsiadm in klibc initramfs would result in ISCSI_ERR_ACCESS for all MGMT IPCs. Specifying just the UID check allows iscsid and iscsiadm to run in klibc initramfs without a user DB. systemd can still start an iscsid process later in boot with the default full MGMT IPC auth checks, leaving the steady-state behavior of iscsid the same as before. The following setting in iscsid.conf enables this behavior: iscsid.ipc_auth_uid = Yes iscsistart now uses this mechanism to only perform the UID check, rather than statically linking against an alternate implementation of getpwuid(). This is effectively the same behavior as before. Signed-off-by: Eric Mackay Reviewed-by: Mike Christie --- etc/iscsid.conf | 6 ++++++ usr/Makefile | 2 +- usr/event_poll.c | 12 ++++++++++-- usr/iscsi_ipc.h | 13 +++++++++++++ usr/iscsid.c | 6 ++++++ usr/iscsistart.c | 2 ++ usr/mgmt_ipc.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++---- usr/mgmt_ipc.h | 1 + usr/statics.c | 19 ------------------- 9 files changed, 89 insertions(+), 26 deletions(-) delete mode 100644 usr/statics.c diff --git a/etc/iscsid.conf b/etc/iscsid.conf index 2dc412f..79d8127 100644 --- a/etc/iscsid.conf +++ b/etc/iscsid.conf @@ -31,6 +31,12 @@ # and refuse to logout if there are any. Defaults to "No". # iscsid.safe_logout = Yes +# Only require UID auth for MGMT IPCs, and not username. +# Useful if you want to run iscsid in a constrained environment. +# Note: Only do this if you are aware of the security implications. +# Defaults to "No". +# iscsid.ipc_auth_uid = Yes + ############################# # NIC/HBA and driver settings ############################# diff --git a/usr/Makefile b/usr/Makefile index c5571b6..79840fc 100644 --- a/usr/Makefile +++ b/usr/Makefile @@ -64,7 +64,7 @@ PROGRAMS_DEST = $(addprefix $(DESTDIR)$(SBINDIR)/,$(PROGRAMS)) ISCSID_OBJS = iscsid.o session_mgmt.o discoveryd.o mntcheck.o ISCSIADM_OBJS = iscsiadm.o session_mgmt.o mntcheck.o -ISCSISTART_OBJS = iscsistart.o statics.o +ISCSISTART_OBJS = iscsistart.o # libc compat files SYSDEPS_DIR = $(TOPDIR)/utils/sysdeps diff --git a/usr/event_poll.c b/usr/event_poll.c index ffd12a3..f39f899 100644 --- a/usr/event_poll.c +++ b/usr/event_poll.c @@ -195,8 +195,16 @@ void event_loop(struct iscsi_ipc *ipc, int control_fd, int mgmt_ipc_fd) if (poll_array[POLL_CTRL].revents) ipc->ctldev_handle(); - if (poll_array[POLL_IPC].revents) - mgmt_ipc_handle(mgmt_ipc_fd); + if (poll_array[POLL_IPC].revents) { + switch (ipc->auth_type) { + case ISCSI_IPC_AUTH_UID: + mgmt_ipc_handle_uid_only(mgmt_ipc_fd); + break; + default: + mgmt_ipc_handle(mgmt_ipc_fd); + break; + } + } if (poll_array[POLL_ALARM].revents) { struct signalfd_siginfo si; diff --git a/usr/iscsi_ipc.h b/usr/iscsi_ipc.h index 47857dd..78bd29a 100644 --- a/usr/iscsi_ipc.h +++ b/usr/iscsi_ipc.h @@ -52,6 +52,17 @@ struct iscsi_ipc_ev_clbk { extern void ipc_register_ev_callback(struct iscsi_ipc_ev_clbk *ipc_ev_clbk); +enum iscsi_ipc_auth_type { + /* UID must have valid entry in user db */ + ISCSI_IPC_AUTH_DEFAULT = 0, + + /* Check only that UID==0 */ + ISCSI_IPC_AUTH_UID, + + /* Must be last */ + ISCSI_IPC_AUTH_MAX, +}; + /** * struct iscsi_ipc - Open-iSCSI Interface for Kernel IPC * @@ -63,6 +74,8 @@ struct iscsi_ipc { int ctldev_bufmax; + enum iscsi_ipc_auth_type auth_type; + int (*ctldev_open) (void); void (*ctldev_close) (void); diff --git a/usr/iscsid.c b/usr/iscsid.c index d97738c..8441037 100644 --- a/usr/iscsid.c +++ b/usr/iscsid.c @@ -383,6 +383,7 @@ int main(int argc, char *argv[]) char *initiatorname_file = INITIATOR_NAME_FILE; char *pid_file = PID_FILE; char *safe_logout; + char *ipc_auth_uid; int ch, longindex; uid_t uid = 0; struct sigaction sa_old; @@ -583,6 +584,11 @@ int main(int argc, char *argv[]) daemon_config.safe_logout = 1; free(safe_logout); + ipc_auth_uid = cfg_get_string_param(config_file, "iscsid.ipc_auth_uid"); + if (ipc_auth_uid && !strcmp(ipc_auth_uid, "Yes")) + ipc->auth_type = ISCSI_IPC_AUTH_UID; + free(ipc_auth_uid); + /* see if we have any stale sessions to recover */ sessions_to_recover = iscsi_sysfs_count_sessions(); if (sessions_to_recover) { diff --git a/usr/iscsistart.c b/usr/iscsistart.c index b23751b..0fb1f56 100644 --- a/usr/iscsistart.c +++ b/usr/iscsistart.c @@ -527,6 +527,8 @@ int main(int argc, char *argv[]) log_debug(1, "TPGT=%d", config_rec.tpgt); log_debug(1, "IP Address=%s", config_rec.conn[0].address); + ipc->auth_type = ISCSI_IPC_AUTH_UID; + /* log the version, so that we can tell if the daemon and kernel module * match based on what shows up in the syslog. Tarballs releases * always install both, but Linux distributors may put the kernel module diff --git a/usr/mgmt_ipc.c b/usr/mgmt_ipc.c index c23bcc0..0ee513f 100644 --- a/usr/mgmt_ipc.c +++ b/usr/mgmt_ipc.c @@ -27,6 +27,7 @@ #include #include #include +#include #include "iscsid.h" #include "idbm.h" @@ -379,6 +380,32 @@ mgmt_peeruser(int sock, char *user) return 1; } +static bool +mgmt_authorized_uid(int sock) +{ + int authorized = false; + struct ucred peercred = {0}; + socklen_t so_len = sizeof(peercred); + + 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_error("Error receiving credentials: %m"); + goto ret_auth; + } + + /* Only UID==0 is authorized */ + authorized = peercred.uid ? false: true; + + if (!authorized) { + log_error("Unauthorized user with UID=%u", peercred.uid); + } + +ret_auth: + return authorized; +} + static void mgmt_ipc_destroy_queue_task(queue_task_t *qtask) { @@ -488,7 +515,7 @@ static mgmt_ipc_fn_t * mgmt_ipc_functions[__MGMT_IPC_MAX_COMMAND] = { [MGMT_IPC_NOTIFY_DEL_PORTAL] = mgmt_ipc_notify_del_portal, }; -void mgmt_ipc_handle(int accept_fd) +static void mgmt_ipc_handle_check_auth(int accept_fd, bool auth_uid_only) { unsigned int command; int fd, err; @@ -508,9 +535,16 @@ void mgmt_ipc_handle(int accept_fd) qtask->allocated = 1; qtask->mgmt_ipc_fd = fd; - if (!mgmt_peeruser(fd, user) || strncmp(user, "root", PEERUSER_MAX)) { - err = ISCSI_ERR_ACCESS; - goto err; + if (auth_uid_only) { + if (!mgmt_authorized_uid(fd)) { + err = ISCSI_ERR_ACCESS; + goto err; + } + } else { + if (!mgmt_peeruser(fd, user) || strncmp(user, "root", PEERUSER_MAX)) { + err = ISCSI_ERR_ACCESS; + goto err; + } } if (mgmt_ipc_read_req(qtask) < 0) { @@ -542,3 +576,15 @@ err: * connection and free the qtask */ mgmt_ipc_write_rsp(qtask, err); } + +void mgmt_ipc_handle(int accept_fd) +{ + /* Default behavior. Full auth check. */ + mgmt_ipc_handle_check_auth(accept_fd, false); +} + +void mgmt_ipc_handle_uid_only(int accept_fd) +{ + /* Check only originating UID. */ + mgmt_ipc_handle_check_auth(accept_fd, true); +} diff --git a/usr/mgmt_ipc.h b/usr/mgmt_ipc.h index 55972ed..cc6ef1b 100644 --- a/usr/mgmt_ipc.h +++ b/usr/mgmt_ipc.h @@ -115,5 +115,6 @@ int mgmt_ipc_listen(void); int mgmt_ipc_systemd(void); void mgmt_ipc_close(int fd); void mgmt_ipc_handle(int accept_fd); +void mgmt_ipc_handle_uid_only(int accept_fd); #endif /* MGMT_IPC_H */ diff --git a/usr/statics.c b/usr/statics.c deleted file mode 100644 index f59729b..0000000 --- a/usr/statics.c +++ /dev/null @@ -1,19 +0,0 @@ -#include -#include -#include -#include - -static struct passwd root_pw = { - .pw_name = "root", -}; - -struct passwd* -getpwuid(uid_t uid) -{ - if (uid == 0) - return &root_pw; - else { - errno = ENOENT; - return 0; - } -} -- cgit v1.2.1