From 9eefc8e774c484f745344cef6b3d15576502f0efeb818a0b59b8760c9ff2cbff Mon Sep 17 00:00:00 2001 From: Christian Goll Date: Wed, 12 Dec 2018 09:28:26 +0000 Subject: [PATCH] Accepting request 657422 from home:mslacken:slurm18 - restarting services on update only when activated - added rotation of logs - Added backported patches which harden the pam module pam_slurm_adopt (BOO#1116758) which will be in slurm 19.05.x * added pam_slurm_adopt-avoid-running-outside-of-the-sshd-PA.patch [PATCH 1/3] pam_slurm_adopt: avoid running outside of the sshd PAM * added pam_slurm_adopt-send_user_msg-don-t-copy-undefined-d.patch [PATCH 2/3] pam_slurm_adopt: send_user_msg: don't copy undefined data * added pam_slurm_adopt-use-uid-to-determine-whether-root-is.patch [PATCH 3/3] pam_slurm_adopt: use uid to determine whether root is logging on - package slurm-pam_slurm now depends on slurm-node and not on slurm OBS-URL: https://build.opensuse.org/request/show/657422 OBS-URL: https://build.opensuse.org/package/show/network:cluster/slurm?expand=0&rev=79 --- ...avoid-running-outside-of-the-sshd-PA.patch | 137 ++++++++++++++++++ ...send_user_msg-don-t-copy-undefined-d.patch | 32 ++++ ...use-uid-to-determine-whether-root-is.patch | 56 +++++++ slurm.changes | 17 +++ slurm.spec | 36 ++++- 5 files changed, 274 insertions(+), 4 deletions(-) create mode 100644 pam_slurm_adopt-avoid-running-outside-of-the-sshd-PA.patch create mode 100644 pam_slurm_adopt-send_user_msg-don-t-copy-undefined-d.patch create mode 100644 pam_slurm_adopt-use-uid-to-determine-whether-root-is.patch diff --git a/pam_slurm_adopt-avoid-running-outside-of-the-sshd-PA.patch b/pam_slurm_adopt-avoid-running-outside-of-the-sshd-PA.patch new file mode 100644 index 0000000..f16ee26 --- /dev/null +++ b/pam_slurm_adopt-avoid-running-outside-of-the-sshd-PA.patch @@ -0,0 +1,137 @@ +From 9f13f7450cb38ac099d2887ab42f588f9dd35306 Mon Sep 17 00:00:00 2001 +From: Matthias Gerstner +Date: Wed, 5 Dec 2018 15:03:19 +0100 +Subject: [PATCH 1/3] pam_slurm_adopt: avoid running outside of the sshd PAM + service context + +This pam module is tailored towards running in the context of remote ssh +logins. When running in a different context like a local sudo call then +the module could be influenced by e.g. passing environment variables +like SLURM_CONF. + +By limiting the module to only perform its actions when running in the +sshd context by default this situation can be avoided. An additional pam +module argument service= allows an Administrator to control +this behaviour, if different behaviour is explicitly desired. + +Signed-off-by: Christian Goll +--- + contribs/pam_slurm_adopt/README | 9 ++++++ + contribs/pam_slurm_adopt/pam_slurm_adopt.c | 46 ++++++++++++++++++++++++++++++ + 2 files changed, 55 insertions(+) + +diff --git a/contribs/pam_slurm_adopt/README b/contribs/pam_slurm_adopt/README +index a84480c1a6..a2d61a977b 100644 +--- a/contribs/pam_slurm_adopt/README ++++ b/contribs/pam_slurm_adopt/README +@@ -97,6 +97,15 @@ This module has the following options (* = default): + 0* = If the step the job is adopted into has X11 enabled, set + the DISPLAY variable in the processes environment accordingly. + ++ service - The pam service name for which this module should run. By default ++ it only runs for sshd for which it was designed for. A ++ different service name can be specified like "login" or "*" to ++ allow the module to in any service context. For local pam logins ++ this module could cause unexpected behaviour or even security ++ issues. Therefore if the service name does not match then this ++ module will not perform the adoption logic and returns ++ PAM_IGNORE immediately. ++ + SLURM.CONF CONFIGURATION + PrologFlags=contain must be set in slurm.conf. This sets up the "extern" step + into which ssh-launched processes will be adopted. +diff --git a/contribs/pam_slurm_adopt/pam_slurm_adopt.c b/contribs/pam_slurm_adopt/pam_slurm_adopt.c +index 3f23c2ec77..da21479f61 100644 +--- a/contribs/pam_slurm_adopt/pam_slurm_adopt.c ++++ b/contribs/pam_slurm_adopt/pam_slurm_adopt.c +@@ -94,6 +94,7 @@ static struct { + log_level_t log_level; + char *node_name; + bool disable_x11; ++ char *pam_service; + } opts; + + static void _init_opts(void) +@@ -107,6 +108,7 @@ static void _init_opts(void) + opts.log_level = LOG_LEVEL_INFO; + opts.node_name = NULL; + opts.disable_x11 = false; ++ opts.pam_service = NULL; + } + + static slurm_cgroup_conf_t *slurm_cgroup_conf = NULL; +@@ -576,6 +578,9 @@ static void _parse_opts(pam_handle_t *pamh, int argc, const char **argv) + opts.node_name = xstrdup(v); + } else if (!xstrncasecmp(*argv, "disable_x11=1", 13)) { + opts.disable_x11 = true; ++ } else if (!xstrncasecmp(*argv, "service=", 8)) { ++ v = (char *)(8 + *argv); ++ opts.pam_service = xstrdup(v); + } + } + +@@ -601,6 +606,40 @@ static int _load_cgroup_config() + return SLURM_SUCCESS; + } + ++/* Make sure to only continue if we're running in the sshd context ++ * ++ * If this module is used locally e.g. via sudo then unexpected things might ++ * happen (e.g. passing environment variables interpreted by slurm code like ++ * SLURM_CONF or inheriting file descriptors that are used by _try_rpc()). ++ */ ++static int check_pam_service(pam_handle_t *pamh) ++{ ++ const char *allowed = opts.pam_service ? opts.pam_service : "sshd"; ++ char *service = NULL; ++ int rc; ++ ++ if (!strcmp(allowed, "*")) ++ // any service name is allowed ++ return PAM_SUCCESS; ++ ++ rc = pam_get_item(pamh, PAM_SERVICE, (void*)&service); ++ ++ if (rc != PAM_SUCCESS) { ++ pam_syslog(pamh, LOG_ERR, "failed to obtain PAM_SERVICE name"); ++ return rc; ++ } ++ else if (service == NULL) { ++ // this shouldn't actually happen ++ return PAM_BAD_ITEM; ++ } ++ ++ if (!strcmp(service, allowed)) { ++ return PAM_SUCCESS; ++ } ++ ++ pam_syslog(pamh, LOG_INFO, "Not adopting process since this is not an allowed pam service"); ++ return PAM_IGNORE; ++} + + /* Parse arguments, etc then get my socket address/port information. Attempt to + * adopt this process into a job in the following order: +@@ -622,6 +661,12 @@ PAM_EXTERN int pam_sm_acct_mgmt(pam_handle_t *pamh, int flags + + _init_opts(); + _parse_opts(pamh, argc, argv); ++ ++ retval = check_pam_service(pamh); ++ if (retval != PAM_SUCCESS) { ++ return retval; ++ } ++ + _log_init(opts.log_level); + + switch (opts.action_generic_failure) { +@@ -762,6 +807,7 @@ cleanup: + xfree(buf); + xfree(slurm_cgroup_conf); + xfree(opts.node_name); ++ xfree(opts.pam_service); + return rc; + } + +-- +2.16.4 + diff --git a/pam_slurm_adopt-send_user_msg-don-t-copy-undefined-d.patch b/pam_slurm_adopt-send_user_msg-don-t-copy-undefined-d.patch new file mode 100644 index 0000000..53a178c --- /dev/null +++ b/pam_slurm_adopt-send_user_msg-don-t-copy-undefined-d.patch @@ -0,0 +1,32 @@ +From 33d78f2db60d3a86c38512f0502df559782cbdf6 Mon Sep 17 00:00:00 2001 +From: Matthias Gerstner +Date: Wed, 5 Dec 2018 14:08:07 +0100 +Subject: [PATCH 2/3] pam_slurm_adopt: send_user_msg: don't copy undefined data + into message + +Using memcpy, an amount of undefined data from the stack will be copied +into the target buffer. While pam_conv probably doesn't evalute the +extra data it still unclean to do that. It could lead up to an +information leak somewhen. + +Signed-off-by: Christian Goll +--- + contribs/pam_slurm_adopt/helper.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/contribs/pam_slurm_adopt/helper.c b/contribs/pam_slurm_adopt/helper.c +index 9c3e202a87..1bac0a0fcf 100644 +--- a/contribs/pam_slurm_adopt/helper.c ++++ b/contribs/pam_slurm_adopt/helper.c +@@ -128,7 +128,7 @@ send_user_msg(pam_handle_t *pamh, const char *mesg) + + /* Construct msg to send to app. + */ +- memcpy(str, mesg, sizeof(str)); ++ strncpy(str, mesg, sizeof(str)); + msg[0].msg_style = PAM_ERROR_MSG; + msg[0].msg = str; + pmsg[0] = &msg[0]; +-- +2.16.4 + diff --git a/pam_slurm_adopt-use-uid-to-determine-whether-root-is.patch b/pam_slurm_adopt-use-uid-to-determine-whether-root-is.patch new file mode 100644 index 0000000..5286328 --- /dev/null +++ b/pam_slurm_adopt-use-uid-to-determine-whether-root-is.patch @@ -0,0 +1,56 @@ +From 86f74afb04f2f8f40751ccc0bdbfd77b99035d8d Mon Sep 17 00:00:00 2001 +From: Matthias Gerstner +Date: Wed, 5 Dec 2018 15:08:53 +0100 +Subject: [PATCH 3/3] pam_slurm_adopt: use uid to determine whether root is + logging on + +In some systems there can be multiple user accounts for uid 0, therefore +the check for literal user name "root" might be insufficient. + +Signed-off-by: Christian Goll +--- + contribs/pam_slurm_adopt/pam_slurm_adopt.c | 21 ++++++++++----------- + 1 file changed, 10 insertions(+), 11 deletions(-) + +diff --git a/contribs/pam_slurm_adopt/pam_slurm_adopt.c b/contribs/pam_slurm_adopt/pam_slurm_adopt.c +index da21479f61..c4635b4693 100644 +--- a/contribs/pam_slurm_adopt/pam_slurm_adopt.c ++++ b/contribs/pam_slurm_adopt/pam_slurm_adopt.c +@@ -708,17 +708,6 @@ PAM_EXTERN int pam_sm_acct_mgmt(pam_handle_t *pamh, int flags + opts.ignore_root = 1; + } + +- /* Ignoring root is probably best but the admin can allow it */ +- if (!strcmp(user_name, "root")) { +- if (opts.ignore_root) { +- info("Ignoring root user"); +- return PAM_IGNORE; +- } else { +- /* This administrator is crazy */ +- info("Danger!!! This is a connection attempt by root and ignore_root=0 is set! Hope for the best!"); +- } +- } +- + /* Calculate buffer size for getpwnam_r */ + bufsize = sysconf(_SC_GETPW_R_SIZE_MAX); + if (bufsize == -1) +@@ -740,6 +729,16 @@ PAM_EXTERN int pam_sm_acct_mgmt(pam_handle_t *pamh, int flags + + if (_load_cgroup_config() != SLURM_SUCCESS) + return rc; ++ /* Ignoring root is probably best but the admin can allow it */ ++ if (pwd.pw_uid == 0) { ++ if (opts.ignore_root) { ++ info("Ignoring root user"); ++ return PAM_IGNORE; ++ } else { ++ /* This administrator is crazy */ ++ info("Danger!!! This is a connection attempt by root (user id 0) and ignore_root=0 is set! Hope for the best!"); ++ } ++ } + + /* Check if there are any steps on the node from any user. A failure here + * likely means failures everywhere so exit on failure or if no local jobs +-- +2.16.4 + diff --git a/slurm.changes b/slurm.changes index 0be850f..b5750d4 100644 --- a/slurm.changes +++ b/slurm.changes @@ -1,3 +1,20 @@ +------------------------------------------------------------------- +Mon Dec 10 10:49:14 UTC 2018 - cgoll@suse.com + +- restarting services on update only when activated +- added rotation of logs +- Added backported patches which harden the pam module pam_slurm_adopt + (BOO#1116758) which will be in slurm 19.05.x + * added pam_slurm_adopt-avoid-running-outside-of-the-sshd-PA.patch + [PATCH 1/3] pam_slurm_adopt: avoid running outside of the sshd PAM + * added pam_slurm_adopt-send_user_msg-don-t-copy-undefined-d.patch + [PATCH 2/3] pam_slurm_adopt: send_user_msg: don't copy undefined data + * added pam_slurm_adopt-use-uid-to-determine-whether-root-is.patch + [PATCH 3/3] pam_slurm_adopt: use uid to determine whether root is + logging on +- package slurm-pam_slurm now depends on slurm-node and not on slurm + + ------------------------------------------------------------------- Wed Dec 5 16:00:50 UTC 2018 - Christian Goll diff --git a/slurm.spec b/slurm.spec index 5f78d78..2156827 100644 --- a/slurm.spec +++ b/slurm.spec @@ -12,7 +12,7 @@ # license that conforms to the Open Source Definition (Version 1.9) # published by the Open Source Initiative. -# Please submit bugfixes or comments via https://bugs.opensuse.org/ +# Please submit bugfixes or comments via http://bugs.opensuse.org/ # @@ -74,6 +74,9 @@ Patch6: slurmdbd-uses-xdaemon_-for-systemd.patch Patch7: slurmsmwd-uses-xdaemon_-for-systemd.patch Patch8: removed-deprecated-xdaemon.patch Patch9: slurmctld-rerun-agent_init-when-backup-controller-takes-over.patch +Patch10: pam_slurm_adopt-avoid-running-outside-of-the-sshd-PA.patch +Patch11: pam_slurm_adopt-send_user_msg-don-t-copy-undefined-d.patch +Patch12: pam_slurm_adopt-use-uid-to-determine-whether-root-is.patch Requires: slurm-config = %{version} Requires: slurm-node = %{version} @@ -280,7 +283,7 @@ This package contains a Perl tool to print SLURM job state information. %package pam_slurm Summary: PAM module for restricting access to compute nodes via SLURM Group: Productivity/Clustering/Computing -Requires: slurm = %{version} +Requires: slurm-node = %{version} BuildRequires: pam-devel %description pam_slurm @@ -349,6 +352,9 @@ Man pages for the SLURM cluster managment software config files. %patch7 -p1 %patch8 -p1 %patch9 -p1 +%patch10 -p1 +%patch11 -p1 +%patch12 -p1 %build %configure --enable-shared \ @@ -483,6 +489,27 @@ Name: %{name} Version: %{version} EOF +# Enable rotation of log files + +mkdir -p %{buildroot}/%{_sysconfdir}/logrotate.d/ +for service in slurmd slurmctld slurmdbd ; do +cat < %{buildroot}/%{_sysconfdir}/logrotate.d/${service}.conf +/var/log/${service}.log { + compress + dateext + missingok + nocreate + notifempty + maxage 365 + rotate 99 + copytruncate + postrotate + pgrep ${service} && killall -SIGUSR2 ${service} || exit 0 + endscript +} +EOF +done + %fdupes -s %{buildroot} %pre @@ -593,12 +620,12 @@ exit 0 } %define _test_rest() %{?with_systemd: os.remove("/run/%{1}.rst") if os.execute() and os.getenv("YAST_IS_RUNNING") ~= "instsys" then - local handle = io.popen("systemctl is-enabled %{1} 2>&1") + local handle = io.popen("systemctl is-active %{1} 2>&1") local str = handle:read("*a"); handle:close() str = string.gsub(str, '^%%s+', '') str = string.gsub(str, '%%s+$', '') str = string.gsub(str, '[\\n\\r]+', ' ') - if str == "enabled" then + if str == "active" then local file = io.open("/run/%{1}.rst","w"); file:close() end end @@ -903,6 +930,7 @@ exit 0 %{?OHPC_BUILD:%attr(0755, %slurm_u, %slurm_g) %_localstatedir/lib/slurm} %{?with_systemd:%{_tmpfilesdir}/%{name}.conf} %dir %{_var}/spool/slurm +%config(noreplace) %{_sysconfdir}/logrotate.d/slurm* %files config-man %{_mandir}/man5/acct_gather.conf.*