Only create the SELinux login file if there are SELinux mappings on the IPA server

This commit is contained in:
Jakub Hrozek 2012-08-17 14:59:01 +02:00
parent a1657400dd
commit 66374adaa7
3 changed files with 274 additions and 1 deletions

View File

@ -0,0 +1,44 @@
From 79402313dc0d7f854b4334dd427e03b7baf0b9db Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
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

View File

@ -0,0 +1,223 @@
From f004e23af14fe020d81b8f97f30b448105b79606 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
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

View File

@ -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 <jhrozek@redhat.com> - 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 <jhrozek@redhat.com> - 1.9.0-14.beta6
- Don't discard HBAC rule processing result if SELinux is on
Resolves: rhbz#846792 (CVE-2012-3462)