From ec4a2c679d43db5e194434bdc7489e1a92599aa5 Mon Sep 17 00:00:00 2001 From: Chris Dunlap Date: Tue, 29 Dec 2015 16:40:27 -0800 Subject: [PATCH] Ignore group-writable permissions on LogDir Starting in Ubuntu 14.04, the permissions and ownership on the /var/log directory changed from 0755 root:root to 0775 root:syslog. This was done to allow rsyslogd to dynamically create files in that directory after having dropped privileges via "$PrivDropToUser syslog" and "$PrivDropToGroup syslog". The munged daemon enforces secure permissions and ownership on its files and directories. These directories must be owned by the user that the munged daemon will run as. They cannot allow write permissions for group or other (unless the sticky-bit is set). In addition, all of their parent directories in the path on up to the root directory must be owned by either root or the user that the munged daemon will run as. None of these directories can allow write permissions for group or other (unless the sticky-bit is set). With the group-writable permission set on the /var/log directory, munged fails to start with the following error: munged: Error: Logfile is insecure: group-writable permissions set on "/var/log" The directory checks are performed by path_is_secure(). This routine ensures the base directory cannot be modified by anyone other than the current user or root. It is called from multiple places within munged to check various paths. This commit adds a "flags" arg to path_is_secure(), and declares the path_security_flag_t enum for setting flags to control which security checks are performed. The PATH_SECURITY_IGNORE_GROUP_WRITE flag disables the group-writable permission check on the directory. This allows the group-writable permission check to be disabled on the LogDir while still enabling it for all other directories used by munged. References: - https://bugs.launchpad.net/ubuntu/+source/munge/+bug/1287624 - https://bugs.launchpad.net/ubuntu/+source/rsyslog/+bug/484336 - https://bugs.launchpad.net/ubuntu/+source/rsyslog/+bug/1256695 - http://www.rsyslog.com/doc/droppriv.html Tested on: - CentOS 7 - Fedora 23 - Ubuntu 15.10 Signed-off-by: Chris Dunlap Closes #31 --- src/munged/auth_recv.c | 4 ++-- src/munged/conf.c | 2 +- src/munged/munged.c | 7 ++++--- src/munged/path.c | 8 ++++++-- src/munged/path.h | 9 ++++++++- src/munged/random.c | 3 ++- 6 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/munged/auth_recv.c b/src/munged/auth_recv.c index 6f9cac2..8dba58c 100644 --- a/src/munged/auth_recv.c +++ b/src/munged/auth_recv.c @@ -116,7 +116,7 @@ _check_auth_server_dir (const char *dir, int got_force) } /* Ensure auth server dir is secure against modification by others. */ - n = path_is_secure (dir, ebuf, sizeof (ebuf)); + n = path_is_secure (dir, ebuf, sizeof (ebuf), PATH_SECURITY_NO_FLAGS); if (n < 0) { log_err (EMUNGE_SNAFU, LOG_ERR, "Failed to check auth server dir \"%s\": %s", dir, ebuf); @@ -202,7 +202,7 @@ _check_auth_client_dir (const char *dir, int got_force) log_err (EMUNGE_SNAFU, LOG_ERR, "Failed to determine dirname of auth client dir \"%s\"", dir); } - n = path_is_secure (dirdir, ebuf, sizeof (ebuf)); + n = path_is_secure (dirdir, ebuf, sizeof (ebuf), PATH_SECURITY_NO_FLAGS); if (n < 0) { log_err (EMUNGE_SNAFU, LOG_ERR, "Failed to check auth client parent dir \"%s\": %s", dirdir, ebuf); diff --git a/src/munged/conf.c b/src/munged/conf.c index 18350eb..6e86115 100644 --- a/src/munged/conf.c +++ b/src/munged/conf.c @@ -688,7 +688,7 @@ _conf_open_keyfile (const char *keyfile, int got_force) log_err (EMUNGE_SNAFU, LOG_ERR, "Failed to determine dirname of keyfile \"%s\"", keyfile); } - n = path_is_secure (keydir, ebuf, sizeof (ebuf)); + n = path_is_secure (keydir, ebuf, sizeof (ebuf), PATH_SECURITY_NO_FLAGS); if (n < 0) { log_err (EMUNGE_SNAFU, LOG_ERR, "Failed to check keyfile dir \"%s\": %s", keydir, ebuf); diff --git a/src/munged/munged.c b/src/munged/munged.c index 54eab5b..b12694a 100644 --- a/src/munged/munged.c +++ b/src/munged/munged.c @@ -397,7 +397,8 @@ open_logfile (const char *logfile, int priority, int got_force) log_err (EMUNGE_SNAFU, LOG_ERR, "Failed to determine dirname of logfile \"%s\"", logfile); } - n = path_is_secure (logdir, ebuf, sizeof (ebuf)); + n = path_is_secure (logdir, ebuf, sizeof (ebuf), + PATH_SECURITY_IGNORE_GROUP_WRITE); if (n < 0) { log_err (EMUNGE_SNAFU, LOG_ERR, "Failed to check logfile dir \"%s\": %s", logdir, ebuf); @@ -493,7 +494,7 @@ write_pidfile (const char *pidfile, int got_force) log_err (EMUNGE_SNAFU, LOG_ERR, "Failed to determine dirname of pidfile \"%s\"", pidfile); } - n = path_is_secure (piddir, ebuf, sizeof (ebuf)); + n = path_is_secure (piddir, ebuf, sizeof (ebuf), PATH_SECURITY_NO_FLAGS); if (n < 0) { log_err (EMUNGE_SNAFU, LOG_ERR, "Failed to check pidfile dir \"%s\": %s", piddir, ebuf); @@ -591,7 +592,7 @@ sock_create (conf_t conf) log_err (EMUNGE_SNAFU, LOG_ERR, "Failed to determine dirname of socket \"%s\"", conf->socket_name); } - n = path_is_secure (sockdir, ebuf, sizeof (ebuf)); + n = path_is_secure (sockdir, ebuf, sizeof (ebuf), PATH_SECURITY_NO_FLAGS); if (n < 0) { log_err (EMUNGE_SNAFU, LOG_ERR, "Failed to check socket dir \"%s\": %s", sockdir, ebuf); diff --git a/src/munged/path.c b/src/munged/path.c index 56cda4d..f70be3b 100644 --- a/src/munged/path.c +++ b/src/munged/path.c @@ -174,7 +174,8 @@ path_is_accessible (const char *path, char *errbuf, size_t errbuflen) int -path_is_secure (const char *path, char *errbuf, size_t errbuflen) +path_is_secure (const char *path, char *errbuf, size_t errbuflen, + path_security_flag_t flags) { int n; char buf [PATH_MAX]; @@ -217,7 +218,10 @@ path_is_secure (const char *path, char *errbuf, size_t errbuflen) return (_path_set_err (0, errbuf, errbuflen, "invalid ownership of \"%s\"", buf)); } - if ((st.st_mode & S_IWGRP) && !(st.st_mode & S_ISVTX)) { + if (!(flags & PATH_SECURITY_IGNORE_GROUP_WRITE) && + (st.st_mode & S_IWGRP) && + !(st.st_mode & S_ISVTX)) + { return (_path_set_err (0, errbuf, errbuflen, "group-writable permissions set on \"%s\"", buf)); } diff --git a/src/munged/path.h b/src/munged/path.h index f52162c..99ca2f5 100644 --- a/src/munged/path.h +++ b/src/munged/path.h @@ -42,6 +42,12 @@ #endif /* !PATH_MAX */ +typedef enum path_security_flags { + PATH_SECURITY_NO_FLAGS = 0x00, + PATH_SECURITY_IGNORE_GROUP_WRITE = 0x01, +} path_security_flag_t; + + int path_canonicalize (const char *src, char *dst, int dstlen); /* * Canonicalizes the path [src], returning an absolute pathname in the @@ -70,7 +76,8 @@ int path_is_accessible (const char *path, char *errbuf, size_t errbuflen); * will be written to the buffer [errbuf] of length [errbuflen]. */ -int path_is_secure (const char *path, char *errbuf, size_t errbuflen); +int path_is_secure (const char *path, char *errbuf, size_t errbuflen, + path_security_flag_t flags); /* * Checks if the specified [path] is secure, ensuring that the base directory * cannot be modified by anyone other than the current user or root. diff --git a/src/munged/random.c b/src/munged/random.c index 69c1f49..6d764a7 100644 --- a/src/munged/random.c +++ b/src/munged/random.c @@ -148,7 +148,8 @@ random_init (const char *seed) log_err (EMUNGE_SNAFU, LOG_ERR, "Failed to determine dirname of PRNG seed \"%s\"", seed); } - n = path_is_secure (seed_dir, ebuf, sizeof (ebuf)); + n = path_is_secure (seed_dir, ebuf, sizeof (ebuf), + PATH_SECURITY_NO_FLAGS); if (n < 0) { log_err (EMUNGE_SNAFU, LOG_ERR, "Failed to check PRNG seed dir \"%s\": %s", seed_dir, ebuf); -- 2.1.4