From 66374adaa7af6302ef739cbdf803305a735c381e Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Fri, 17 Aug 2012 14:59:01 +0200 Subject: [PATCH] Only create the SELinux login file if there are SELinux mappings on the IPA server --- ...move-the-temp-login-file-if-already-.patch | 44 ++++ ...SELinux-login-file-if-there-are-mapp.patch | 223 ++++++++++++++++++ sssd.spec | 8 +- 3 files changed, 274 insertions(+), 1 deletion(-) create mode 100644 0002-Do-not-try-to-remove-the-temp-login-file-if-already-.patch create mode 100644 0003-Only-create-the-SELinux-login-file-if-there-are-mapp.patch diff --git a/0002-Do-not-try-to-remove-the-temp-login-file-if-already-.patch b/0002-Do-not-try-to-remove-the-temp-login-file-if-already-.patch new file mode 100644 index 0000000..354d491 --- /dev/null +++ b/0002-Do-not-try-to-remove-the-temp-login-file-if-already-.patch @@ -0,0 +1,44 @@ +From 79402313dc0d7f854b4334dd427e03b7baf0b9db Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Sun, 5 Aug 2012 22:03:11 +0200 +Subject: [PATCH 1/2] Do not try to remove the temp login file if already + renamed + +write_selinux_string() would try to unlink the temporary file even after +it was renamed. Failure to unlink the file would not be fatal, but would +produce a confusing error message. + +Also don't use "0" for the default fd number, that's reserved for stdin. +Using -1 is safer. +--- + src/responder/pam/pamsrv_cmd.c | 5 +++-- + 1 file changed, 3 insertions(+), 2 deletions(-) + +diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c +index 8c9dd9b557982e04989bf9e63fd93ea294979252..944845a86dfa8166367029e6f7bddc478e5ebd03 100644 +--- a/src/responder/pam/pamsrv_cmd.c ++++ b/src/responder/pam/pamsrv_cmd.c +@@ -366,7 +366,7 @@ static errno_t write_selinux_string(const char *username, char *string) + char *tmp_path = NULL; + ssize_t written; + int len; +- int fd = 0; ++ int fd = -1; + mode_t oldmask; + TALLOC_CTX *tmp_ctx; + char *full_string = NULL; +@@ -437,9 +437,10 @@ static errno_t write_selinux_string(const char *username, char *string) + } else { + ret = EOK; + } ++ fd = -1; + + done: +- if (fd > 0) { ++ if (fd != -1) { + close(fd); + if (unlink(tmp_path) < 0) { + DEBUG(SSSDBG_MINOR_FAILURE, ("Could not remove file [%s]", +-- +1.7.11.2 + diff --git a/0003-Only-create-the-SELinux-login-file-if-there-are-mapp.patch b/0003-Only-create-the-SELinux-login-file-if-there-are-mapp.patch new file mode 100644 index 0000000..d1290e7 --- /dev/null +++ b/0003-Only-create-the-SELinux-login-file-if-there-are-mapp.patch @@ -0,0 +1,223 @@ +From f004e23af14fe020d81b8f97f30b448105b79606 Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Sun, 5 Aug 2012 22:37:09 +0200 +Subject: [PATCH 2/2] Only create the SELinux login file if there are mappings + on the server + +https://fedorahosted.org/sssd/ticket/1455 + +In case there are no rules on the IPA server, we must simply avoid generating +the login file. That would make us fall back to the system-wide default +defined in /etc/selinux/targeted/seusers. + +The IPA default must be only used if there *are* rules on the server, +but none matches. +--- + src/db/sysdb_selinux.c | 7 +-- + src/responder/pam/pamsrv_cmd.c | 120 ++++++++++++++++++++++++++--------------- + 2 files changed, 77 insertions(+), 50 deletions(-) + +diff --git a/src/db/sysdb_selinux.c b/src/db/sysdb_selinux.c +index eaf07b50a4435b11f937473e37c708edc6e2e0eb..976489503e0995b7e025146c23a7f4e7874c1643 100644 +--- a/src/db/sysdb_selinux.c ++++ b/src/db/sysdb_selinux.c +@@ -364,7 +364,7 @@ errno_t sysdb_search_selinux_usermap_by_username(TALLOC_CTX *mem_ctx, + struct ldb_message **msgs = NULL; + struct sysdb_attrs *user; + struct sysdb_attrs *tmp_attrs; +- struct ldb_message **usermaps; ++ struct ldb_message **usermaps = NULL; + struct sss_domain_info *domain; + struct ldb_dn *basedn; + size_t msgs_count = 0; +@@ -462,11 +462,6 @@ errno_t sysdb_search_selinux_usermap_by_username(TALLOC_CTX *mem_ctx, + } + } + +- if (usermaps[0] == NULL) { +- ret = ENOENT; +- goto done; +- } +- + *_usermaps = talloc_steal(mem_ctx, usermaps); + + ret = EOK; +diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c +index 944845a86dfa8166367029e6f7bddc478e5ebd03..238b4fa7feee666e3419eae55c4952e45fd7184c 100644 +--- a/src/responder/pam/pamsrv_cmd.c ++++ b/src/responder/pam/pamsrv_cmd.c +@@ -359,8 +359,10 @@ fail: + #ifdef HAVE_SELINUX + + #define ALL_SERVICES "*" ++#define selogin_path(mem_ctx, username) \ ++ talloc_asprintf(mem_ctx, "%s/logins/%s", selinux_policy_root(), username) + +-static errno_t write_selinux_string(const char *username, char *string) ++static errno_t write_selinux_login_file(const char *username, char *string) + { + char *path = NULL; + char *tmp_path = NULL; +@@ -383,8 +385,7 @@ static errno_t write_selinux_string(const char *username, char *string) + return ENOMEM; + } + +- path = talloc_asprintf(tmp_ctx, "%s/logins/%s", selinux_policy_root(), +- username); ++ path = selogin_path(tmp_ctx, username); + if (path == NULL) { + ret = ENOMEM; + goto done; +@@ -452,7 +453,33 @@ done: + return ret; + } + +-static errno_t get_selinux_string(struct pam_auth_req *preq) ++static errno_t remove_selinux_login_file(const char *username) ++{ ++ char *path; ++ errno_t ret; ++ ++ path = selogin_path(NULL, username); ++ if (!path) return ENOMEM; ++ ++ errno = 0; ++ ret = unlink(path); ++ if (ret < 0) { ++ ret = errno; ++ if (ret == ENOENT) { ++ /* Just return success if the file was not there */ ++ ret = EOK; ++ } else { ++ DEBUG(SSSDBG_OP_FAILURE, ++ ("Could not remove login file %s [%d]: %s\n", ++ path, ret, strerror(ret))); ++ } ++ } ++ ++ talloc_free(path); ++ return ret; ++} ++ ++static errno_t process_selinux_mappings(struct pam_auth_req *preq) + { + struct sysdb_ctx *sysdb; + TALLOC_CTX *tmp_ctx; +@@ -464,7 +491,7 @@ static errno_t get_selinux_string(struct pam_auth_req *preq) + const char *tmp_str; + char *order = NULL; + char **order_array; +- errno_t ret; ++ errno_t ret, err; + int i, j; + size_t order_count; + size_t len = 0; +@@ -550,53 +577,58 @@ static errno_t get_selinux_string(struct pam_auth_req *preq) + &usermaps); + if (ret != EOK && ret != ENOENT) { + goto done; ++ } else if (ret == ENOENT) { ++ DEBUG(SSSDBG_TRACE_FUNC, ("No maps defined on the server\n")); ++ ret = EOK; ++ goto done; + } + +- if (ret == ENOENT) { +- DEBUG(SSSDBG_TRACE_FUNC, ("No user maps found, using default!")); +- file_content = talloc_strdup(tmp_ctx, default_user); +- if (file_content == NULL) { +- ret = ENOMEM; +- goto done; +- } +- } else { +- /* Iterate through the order array and try to find SELinux users +- * in fetched maps. The order array contains all SELinux users +- * allowed in the domain in the same order they should appear +- * in the SELinux config file. If any user from the order array +- * is not in fetched user maps, it means it should not be allowed +- * for the user who is just logging in. +- * +- * Right now we have empty content of the SELinux config file, +- * we shall add only those SELinux users that are present both in +- * the order array and user maps applicable to the user who is +- * logging in. +- */ +- for (i = 0; i < order_count; i++) { +- for (j = 0; usermaps[j] != NULL; j++) { +- tmp_str = sss_selinux_map_get_seuser(usermaps[j]); ++ /* If no maps match, we'll use the default SELinux user from the ++ * config */ ++ file_content = talloc_strdup(tmp_ctx, default_user); ++ if (file_content == NULL) { ++ ret = ENOMEM; ++ goto done; ++ } ++ ++ /* Iterate through the order array and try to find SELinux users ++ * in fetched maps. The order array contains all SELinux users ++ * allowed in the domain in the same order they should appear ++ * in the SELinux config file. If any user from the order array ++ * is not in fetched user maps, it means it should not be allowed ++ * for the user who is just logging in. ++ * ++ * Right now we have empty content of the SELinux config file, ++ * we shall add only those SELinux users that are present both in ++ * the order array and user maps applicable to the user who is ++ * logging in. ++ */ ++ for (i = 0; i < order_count; i++) { ++ for (j = 0; usermaps[j] != NULL; j++) { ++ tmp_str = sss_selinux_map_get_seuser(usermaps[j]); + +- if (tmp_str && !strcasecmp(tmp_str, order_array[i])) { +- /* If file_content contained something, overwrite it. +- * This record has higher priority. +- */ +- talloc_zfree(file_content); +- file_content = talloc_strdup(tmp_ctx, tmp_str); +- if (file_content == NULL) { +- ret = ENOMEM; +- goto done; +- } +- break; ++ if (tmp_str && !strcasecmp(tmp_str, order_array[i])) { ++ /* If file_content contained something, overwrite it. ++ * This record has higher priority. ++ */ ++ talloc_zfree(file_content); ++ file_content = talloc_strdup(tmp_ctx, tmp_str); ++ if (file_content == NULL) { ++ ret = ENOMEM; ++ goto done; + } ++ break; + } + } + } + +- if (file_content) { +- ret = write_selinux_string(pd->user, file_content); +- } +- ++ ret = write_selinux_login_file(pd->user, file_content); + done: ++ if (!file_content) { ++ err = remove_selinux_login_file(pd->user); ++ /* Don't overwrite original error condition if there was one */ ++ if (ret == EOK) ret = err; ++ } + talloc_free(tmp_ctx); + return ret; + } +@@ -802,7 +834,7 @@ static void pam_reply(struct pam_auth_req *preq) + pd->pam_status == PAM_SUCCESS) { + /* Try to fetch data from sysdb + * (auth already passed -> we should have them) */ +- ret = get_selinux_string(preq); ++ ret = process_selinux_mappings(preq); + if (ret != EOK) { + pd->pam_status = PAM_SYSTEM_ERR; + } +-- +1.7.11.2 + diff --git a/sssd.spec b/sssd.spec index 0f53155..f6f85c2 100644 --- a/sssd.spec +++ b/sssd.spec @@ -16,7 +16,7 @@ Name: sssd Version: 1.9.0 -Release: 15%{?dist}.beta6 +Release: 16%{?dist}.beta6 Group: Applications/System Summary: System Security Services Daemon License: GPLv3+ @@ -26,6 +26,8 @@ BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) ### Patches ### Patch0001: 0001-Abort-PAM-access-phase-if-HBAC-does-not-return-PAM_S.patch +Patch0002: 0002-Do-not-try-to-remove-the-temp-login-file-if-already-.patch +Patch0003: 0003-Only-create-the-SELinux-login-file-if-there-are-mapp.patch ### Dependencies ### @@ -515,6 +517,10 @@ fi %postun -n libsss_sudo -p /sbin/ldconfig %changelog +* Fri Aug 17 2012 Jakub Hrozek - 1.9.0-16.beta6 +- Only create the SELinux login file if there are SELinux mappings on + the IPA server + * Fri Aug 10 2012 Jakub Hrozek - 1.9.0-14.beta6 - Don't discard HBAC rule processing result if SELinux is on Resolves: rhbz#846792 (CVE-2012-3462)