From cbd2472b7cfe927b6e74aececcb60f0f9de4925a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Tue, 15 Oct 2019 23:33:54 +0200 Subject: [PATCH] migrate to new SELinux api MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using hard-coded access vector ids is deprecated and can lead to issues with custom SELinux policies. Switch to `selinux_check_access()`. Also use the libselinux log callback and log if available to audit. This makes it easier for users to catch SELinux denials. Drop legacy shortcut logic for passwd, which avoided a SELinux check if uid 0 changes a password of a user which username equals the current SELinux user identifier. Nowadays usernames rarely match SELinux user identifiers and the benefit of skipping a SELinux check is negligible. Signed-off-by: Christian Göttsche --- lib/prototypes.h | 1 + lib/selinux.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++ src/Makefile.am | 30 +++++++------- src/chage.c | 8 +--- src/chfn.c | 7 +--- src/chsh.c | 7 +--- src/passwd.c | 79 +++---------------------------------- 7 files changed, 125 insertions(+), 107 deletions(-) diff --git a/lib/prototypes.h b/lib/prototypes.h index 24825f60..22603b98 100644 --- a/lib/prototypes.h +++ b/lib/prototypes.h @@ -336,6 +336,7 @@ extern /*@observer@*/const char *crypt_make_salt (/*@null@*//*@observer@*/const #ifdef WITH_SELINUX extern int set_selinux_file_context (const char *dst_name); extern int reset_selinux_file_context (void); +extern int check_selinux_permit (const char *perm_name); #endif /* semanage.c */ diff --git a/lib/selinux.c b/lib/selinux.c index d7513298..4ab85a36 100644 --- a/lib/selinux.c +++ b/lib/selinux.c @@ -34,6 +34,7 @@ #include "defines.h" #include +#include #include "prototypes.h" @@ -98,6 +99,105 @@ int reset_selinux_file_context (void) return 0; } +/* + * Log callback for libselinux internal error reporting. + */ +__attribute__((__format__ (printf, 2, 3))) +static int selinux_log_cb (int type, const char *fmt, ...) { + va_list ap; + char *buf; + int r; +#ifdef WITH_AUDIT + static int selinux_audit_fd = -2; +#endif + + va_start (ap, fmt); + r = vasprintf (&buf, fmt, ap); + va_end (ap); + + if (r < 0) { + return 0; + } + +#ifdef WITH_AUDIT + if (-2 == selinux_audit_fd) { + selinux_audit_fd = audit_open (); + + if (-1 == selinux_audit_fd) { + /* You get these only when the kernel doesn't have + * audit compiled in. */ + if ( (errno != EINVAL) + && (errno != EPROTONOSUPPORT) + && (errno != EAFNOSUPPORT)) { + + (void) fputs (_("Cannot open audit interface.\n"), + stderr); + SYSLOG ((LOG_WARN, "Cannot open audit interface.")); + } + } + } + + if (-1 != selinux_audit_fd) { + if (SELINUX_AVC == type) { + if (audit_log_user_avc_message (selinux_audit_fd, + AUDIT_USER_AVC, buf, NULL, NULL, + NULL, 0) > 0) { + goto skip_syslog; + } + } else if (SELINUX_ERROR == type) { + if (audit_log_user_avc_message (selinux_audit_fd, + AUDIT_USER_SELINUX_ERR, buf, NULL, NULL, + NULL, 0) > 0) { + goto skip_syslog; + } + } + } +#endif + + SYSLOG ((LOG_WARN, "libselinux: %s", buf)); + +skip_syslog: + free (buf); + + return 0; +} + +/* + * check_selinux_permit - Check whether SELinux grants the given + * operation + * + * Parameter is the SELinux permission name, e.g. rootok + * + * Returns 0 when permission is granted + * or something failed but running in + * permissive mode + */ +int check_selinux_permit (const char *perm_name) +{ + char *user_context_str; + int r; + + if (0 == is_selinux_enabled ()) { + return 0; + } + + selinux_set_callback (SELINUX_CB_LOG, (union selinux_callback) selinux_log_cb); + + if (getprevcon (&user_context_str) != 0) { + fprintf (stderr, + _("%s: can not get previous SELinux process context: %s\n"), + Prog, strerror (errno)); + SYSLOG ((LOG_WARN, + "can not get previous SELinux process context: %s", + strerror (errno))); + return (security_getenforce () != 0); + } + + r = selinux_check_access (user_context_str, user_context_str, "passwd", perm_name, NULL); + freecon (user_context_str); + return r; +} + #else /* !WITH_SELINUX */ extern int errno; /* warning: ANSI C forbids an empty source file */ #endif /* !WITH_SELINUX */ diff --git a/src/Makefile.am b/src/Makefile.am index 451816d7..1b6aaab1 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -89,33 +89,33 @@ LIBCRYPT_NOPAM = $(LIBCRYPT) endif chage_LDADD = $(LDADD) $(LIBPAM_SUID) $(LIBAUDIT) $(LIBSELINUX) $(LIBECONF) -newuidmap_LDADD = $(LDADD) $(LIBSELINUX) $(LIBCAP) -newgidmap_LDADD = $(LDADD) $(LIBSELINUX) $(LIBCAP) -chfn_LDADD = $(LDADD) $(LIBPAM) $(LIBSELINUX) $(LIBCRYPT_NOPAM) $(LIBSKEY) $(LIBMD) $(LIBECONF) -chgpasswd_LDADD = $(LDADD) $(LIBPAM_SUID) $(LIBSELINUX) $(LIBCRYPT) $(LIBECONF) -chsh_LDADD = $(LDADD) $(LIBPAM) $(LIBSELINUX) $(LIBCRYPT_NOPAM) $(LIBSKEY) $(LIBMD) $(LIBECONF) -chpasswd_LDADD = $(LDADD) $(LIBPAM) $(LIBSELINUX) $(LIBCRYPT) $(LIBECONF) +newuidmap_LDADD = $(LDADD) $(LIBAUDIT) $(LIBSELINUX) $(LIBCAP) +newgidmap_LDADD = $(LDADD) $(LIBAUDIT) $(LIBSELINUX) $(LIBCAP) +chfn_LDADD = $(LDADD) $(LIBPAM) $(LIBAUDIT) $(LIBSELINUX) $(LIBCRYPT_NOPAM) $(LIBSKEY) $(LIBMD) $(LIBECONF) +chgpasswd_LDADD = $(LDADD) $(LIBPAM_SUID) $(LIBAUDIT) $(LIBSELINUX) $(LIBCRYPT) $(LIBECONF) +chsh_LDADD = $(LDADD) $(LIBPAM) $(LIBAUDIT) $(LIBSELINUX) $(LIBCRYPT_NOPAM) $(LIBSKEY) $(LIBMD) $(LIBECONF) +chpasswd_LDADD = $(LDADD) $(LIBPAM) $(LIBAUDIT) $(LIBSELINUX) $(LIBCRYPT) $(LIBECONF) expiry_LDADD = $(LDADD) $(LIBECONF) gpasswd_LDADD = $(LDADD) $(LIBAUDIT) $(LIBSELINUX) $(LIBCRYPT) $(LIBECONF) groupadd_LDADD = $(LDADD) $(LIBPAM_SUID) $(LIBAUDIT) $(LIBSELINUX) $(LIBECONF) groupdel_LDADD = $(LDADD) $(LIBPAM_SUID) $(LIBAUDIT) $(LIBSELINUX) $(LIBECONF) -groupmems_LDADD = $(LDADD) $(LIBPAM) $(LIBSELINUX) $(LIBECONF) +groupmems_LDADD = $(LDADD) $(LIBPAM) $(LIBAUDIT) $(LIBSELINUX) $(LIBECONF) groupmod_LDADD = $(LDADD) $(LIBPAM_SUID) $(LIBAUDIT) $(LIBSELINUX) $(LIBECONF) -grpck_LDADD = $(LDADD) $(LIBSELINUX) $(LIBECONF) -grpconv_LDADD = $(LDADD) $(LIBSELINUX) $(LIBECONF) -grpunconv_LDADD = $(LDADD) $(LIBSELINUX) $(LIBECONF) +grpck_LDADD = $(LDADD) $(LIBAUDIT) $(LIBSELINUX) $(LIBECONF) +grpconv_LDADD = $(LDADD) $(LIBAUDIT) $(LIBSELINUX) $(LIBECONF) +grpunconv_LDADD = $(LDADD) $(LIBAUDIT) $(LIBSELINUX) $(LIBECONF) lastlog_LDADD = $(LDADD) $(LIBAUDIT) $(LIBECONF) login_SOURCES = \ login.c \ login_nopam.c login_LDADD = $(LDADD) $(LIBPAM) $(LIBAUDIT) $(LIBCRYPT_NOPAM) $(LIBSKEY) $(LIBMD) $(LIBECONF) newgrp_LDADD = $(LDADD) $(LIBAUDIT) $(LIBCRYPT) $(LIBECONF) -newusers_LDADD = $(LDADD) $(LIBPAM) $(LIBSELINUX) $(LIBCRYPT) $(LIBECONF) +newusers_LDADD = $(LDADD) $(LIBPAM) $(LIBAUDIT) $(LIBSELINUX) $(LIBCRYPT) $(LIBECONF) nologin_LDADD = passwd_LDADD = $(LDADD) $(LIBPAM) $(LIBCRACK) $(LIBAUDIT) $(LIBSELINUX) $(LIBCRYPT_NOPAM) $(LIBECONF) -pwck_LDADD = $(LDADD) $(LIBSELINUX) $(LIBECONF) -pwconv_LDADD = $(LDADD) $(LIBSELINUX) $(LIBECONF) -pwunconv_LDADD = $(LDADD) $(LIBSELINUX) $(LIBECONF) +pwck_LDADD = $(LDADD) $(LIBAUDIT) $(LIBSELINUX) $(LIBECONF) +pwconv_LDADD = $(LDADD) $(LIBAUDIT) $(LIBSELINUX) $(LIBECONF) +pwunconv_LDADD = $(LDADD) $(LIBAUDIT) $(LIBSELINUX) $(LIBECONF) su_SOURCES = \ su.c \ suauth.c @@ -124,7 +124,7 @@ sulogin_LDADD = $(LDADD) $(LIBCRYPT) $(LIBECONF) useradd_LDADD = $(LDADD) $(LIBPAM_SUID) $(LIBAUDIT) $(LIBSELINUX) $(LIBSEMANAGE) $(LIBACL) $(LIBATTR) $(LIBECONF) userdel_LDADD = $(LDADD) $(LIBPAM_SUID) $(LIBAUDIT) $(LIBSELINUX) $(LIBSEMANAGE) $(LIBECONF) usermod_LDADD = $(LDADD) $(LIBPAM_SUID) $(LIBAUDIT) $(LIBSELINUX) $(LIBSEMANAGE) $(LIBACL) $(LIBATTR) $(LIBECONF) -vipw_LDADD = $(LDADD) $(LIBSELINUX) $(LIBECONF) +vipw_LDADD = $(LDADD) $(LIBAUDIT) $(LIBSELINUX) $(LIBECONF) install-am: all-am $(MAKE) $(AM_MAKEFLAGS) install-exec-am install-data-am diff --git a/src/chage.c b/src/chage.c index 23622e24..bcc58c95 100644 --- a/src/chage.c +++ b/src/chage.c @@ -48,10 +48,6 @@ #endif /* USE_PAM */ #endif /* ACCT_TOOLS_SETUID */ #include -#ifdef WITH_SELINUX -#include -#include -#endif #include "prototypes.h" #include "defines.h" #include "pwio.h" @@ -832,8 +828,8 @@ int main (int argc, char **argv) rgid = getgid (); amroot = (ruid == 0); #ifdef WITH_SELINUX - if (amroot && (is_selinux_enabled () > 0)) { - amroot = (selinux_check_passwd_access (PASSWD__ROOTOK) == 0); + if (amroot) { + amroot = (check_selinux_permit ("rootok") == 0); } #endif diff --git a/src/chfn.c b/src/chfn.c index 0725e1c7..b2658fcf 100644 --- a/src/chfn.c +++ b/src/chfn.c @@ -40,10 +40,6 @@ #include #include #include -#ifdef WITH_SELINUX -#include -#include -#endif #include "defines.h" #include "getdef.h" #include "nscd.h" @@ -379,8 +375,7 @@ static void check_perms (const struct passwd *pw) * check if the change is allowed by SELinux policy. */ if ((pw->pw_uid != getuid ()) - && (is_selinux_enabled () > 0) - && (selinux_check_passwd_access (PASSWD__CHFN) != 0)) { + && (check_selinux_permit ("chfn") != 0)) { fprintf (stderr, _("%s: Permission denied.\n"), Prog); closelog (); exit (E_NOPERM); diff --git a/src/chsh.c b/src/chsh.c index 910e3dd4..06edf407 100644 --- a/src/chsh.c +++ b/src/chsh.c @@ -39,10 +39,6 @@ #include #include #include -#ifdef WITH_SELINUX -#include -#include -#endif #include "defines.h" #include "getdef.h" #include "nscd.h" @@ -286,8 +282,7 @@ static void check_perms (const struct passwd *pw) * check if the change is allowed by SELinux policy. */ if ((pw->pw_uid != getuid ()) - && (is_selinux_enabled () > 0) - && (selinux_check_passwd_access (PASSWD__CHSH) != 0)) { + && (check_selinux_permit("chsh") != 0)) { SYSLOG ((LOG_WARN, "can't change shell for '%s'", pw->pw_name)); fprintf (stderr, _("You may not change the shell for '%s'.\n"), diff --git a/src/passwd.c b/src/passwd.c index 5bea2765..8dca4455 100644 --- a/src/passwd.c +++ b/src/passwd.c @@ -41,12 +41,6 @@ #include #include #include -#ifdef WITH_SELINUX -#include -#include -#include -#include -#endif /* WITH_SELINUX */ #include #include "defines.h" #include "getdef.h" @@ -149,11 +143,6 @@ static char *update_crypt_pw (char *); static void update_noshadow (void); static void update_shadow (void); -#ifdef WITH_SELINUX -static int check_selinux_access (const char *changed_user, - uid_t changed_uid, - access_vector_t requested_access); -#endif /* WITH_SELINUX */ /* * usage - print command usage and exit @@ -710,55 +699,6 @@ static void update_shadow (void) spw_locked = false; } -#ifdef WITH_SELINUX -static int check_selinux_access (const char *changed_user, - uid_t changed_uid, - access_vector_t requested_access) -{ - int status = -1; - security_context_t user_context; - context_t c; - const char *user; - - /* if in permissive mode then allow the operation */ - if (security_getenforce() == 0) { - return 0; - } - - /* get the context of the process which executed passwd */ - if (getprevcon(&user_context) != 0) { - return -1; - } - - /* get the "user" portion of the context (the part before the first - colon) */ - c = context_new(user_context); - user = context_user_get(c); - - /* if changing a password for an account with UID==0 or for an account - where the identity matches then return success */ - if (changed_uid != 0 && strcmp(changed_user, user) == 0) { - status = 0; - } else { - struct av_decision avd; - int retval; - retval = security_compute_av(user_context, - user_context, - SECCLASS_PASSWD, - requested_access, - &avd); - if ((retval == 0) && - ((requested_access & avd.allowed) == requested_access)) { - status = 0; - } - } - context_free(c); - freecon(user_context); - return status; -} - -#endif /* WITH_SELINUX */ - /* * passwd - change a user's password file information * @@ -1034,22 +974,13 @@ int main (int argc, char **argv) #ifdef WITH_SELINUX /* only do this check when getuid()==0 because it's a pre-condition for changing a password without entering the old one */ - if ((is_selinux_enabled() > 0) && (getuid() == 0) && - (check_selinux_access (name, pw->pw_uid, PASSWD__PASSWD) != 0)) { - security_context_t user_context = NULL; - const char *user = "Unknown user context"; - if (getprevcon (&user_context) == 0) { - user = user_context; /* FIXME: use context_user_get? */ - } + if (amroot && (check_selinux_permit ("passwd") != 0)) { SYSLOG ((LOG_ALERT, - "%s is not authorized to change the password of %s", - user, name)); + "root is not authorized by SELinux to change the password of %s", + name)); (void) fprintf(stderr, - _("%s: %s is not authorized to change the password of %s\n"), - Prog, user, name); - if (NULL != user_context) { - freecon (user_context); - } + _("%s: root is not authorized by SELinux to change the password of %s\n"), + Prog, name); exit (E_NOPERM); } #endif /* WITH_SELINUX */