Dne pátek 20 července 2012 14:32:10, Jakub Hrozek napsal(a):
> On Fri, Jul 20, 2012 at 01:55:59PM +0200, Jan Zelený wrote:
> > #156
> > Added some debug messages
>
> This debug message is wrong:
>
> + DEBUG(SSSDBG_TRACE_FUNC, ("HBAC rule [%s] matched, moving "
> + "SELinux user map [%s] to
> confirmed\n", + hbac_dn,
> seealso_dn));
>
> There is no "confirmed" list anymore. The rest of the patch looks good.You are right. Fixed > > #157 > > The original priority patch had this condition in the wrong place, > > resulting in hostCategory == all not being taken into account > > This code assigns SELINUX_PRIORITY_HOST_CAT to any rule that contains > anything in the "hostcat" attribute. It needs to specifically check for > strcasecmp(hostcat, "all") and error out saying that the category is not > supported on any other input. The same applies to user categories. > > Similar to hbac_get_category() for how the input validity check is performed > for the HBAC rules. On the contrary, the check is there. At least I can see the strcasecmp in my vim ;-) > > #158 > > The function ipa_selinux_map_merge() is no longer necessary since more > > generic function has been implemented and it is even used in the code > > Ack > > > #159 > > This patch provides the fix for HBAC - SELinux linking itself. I'm not > > sure > > about defining those two constants on top. If anyone has better idea where > > to put them in order to consolidate them with the same constants private > > for HBAC code, I'm open to suggestions. > > They are already stored in ipa_selinux_user_map[], see ipa_opts.h Yes they are but I don't think it's wise to extract the name from selinux user map and apply it to HBAC rule. It will work, sure. But IMO it would be even more confusing and potentially dangerous than defining the constant this way. Sending corrected patch #156 Thanks Jan
>From b11bb9de5f06337d92768965314033eb2ab56f8b Mon Sep 17 00:00:00 2001
From: Jan Zeleny <jzel...@redhat.com>
Date: Fri, 20 Jul 2012 11:10:48 -0400
Subject: [PATCH 4/4] Fix linking of HBAC rules and SELinux user maps
Translate manually memberHost and memberUser to originalMemberUser and
originalMemberHost. Without this, the HBAC rule won't be matched against
current user and/or host, meaning that no SELinux user map connected to
it will be matched againts any user on the system.
---
src/providers/ipa/ipa_session.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/src/providers/ipa/ipa_session.c b/src/providers/ipa/ipa_session.c
index 2c29e4c4ace1ef1cdb6969e4dd7d49187dcce3fc..380f7f9224828b2c6da0754edb74db40f5daaa8a 100644
--- a/src/providers/ipa/ipa_session.c
+++ b/src/providers/ipa/ipa_session.c
@@ -35,6 +35,9 @@
#include "providers/ipa/ipa_selinux_common.h"
#include "providers/ipa/ipa_selinux_maps.h"
+#define IPA_MEMBER_USER "memberUser"
+#define IPA_MEMBER_HOST "memberHost"
+
struct ipa_get_selinux_state {
struct be_req *be_req;
struct pam_data *pd;
@@ -472,6 +475,7 @@ static void ipa_get_selinux_hbac_done(struct tevent_req *subreq)
struct ipa_get_selinux_state);
struct sysdb_attrs **rules;
struct sysdb_attrs *usermap;
+ struct ldb_message_element *el;
const char *hbac_dn;
const char *seealso_dn;
size_t rule_count;
@@ -494,6 +498,17 @@ static void ipa_get_selinux_hbac_done(struct tevent_req *subreq)
goto done;
}
+ /* We need to do this translation for further processing. We have to
+ * do it manually because no map was used to retrieve HBAC rules.
+ */
+ ret = sysdb_attrs_get_el(rules[i], IPA_MEMBER_HOST, &el);
+ if (ret != EOK) goto done;
+ el->name = SYSDB_ORIG_MEMBER_HOST;
+
+ ret = sysdb_attrs_get_el(rules[i], IPA_MEMBER_USER, &el);
+ if (ret != EOK) goto done;
+ el->name = SYSDB_ORIG_MEMBER_USER;
+
DEBUG(SSSDBG_TRACE_ALL,
("Matching HBAC rule %s with SELinux mappings\n", hbac_dn));
--
1.7.7.6
>From 2f96c36d328d8963c4eb1c84a2f8b637383b1e24 Mon Sep 17 00:00:00 2001
From: Jan Zeleny <jzel...@redhat.com>
Date: Fri, 20 Jul 2012 11:05:24 -0400
Subject: [PATCH 1/4] Added some DEBUG statements into SELinux related code
---
src/providers/ipa/ipa_session.c | 18 ++++++++++++++----
src/util/sss_selinux.c | 28 ++++++++++++++++++++++++----
2 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/src/providers/ipa/ipa_session.c b/src/providers/ipa/ipa_session.c
index 3a87e957cacae8230f295b60459f865f02df77ff..51c785f5a396055a4c345fd8fe3729c154554cb4 100644
--- a/src/providers/ipa/ipa_session.c
+++ b/src/providers/ipa/ipa_session.c
@@ -481,21 +481,28 @@ static void ipa_get_selinux_hbac_done(struct tevent_req *subreq)
ret = ipa_hbac_rule_info_recv(subreq, state, &rule_count,
&rules);
+ DEBUG(SSSDBG_TRACE_INTERNAL,
+ ("Received %d HBAC rules\n", rule_count));
talloc_free(subreq);
if (ret != EOK) {
goto done;
}
for (i = 0; i < rule_count; i++) {
- if (!sss_selinux_match(rules[i], state->user, state->host, &priority)) {
- continue;
- }
-
ret = sysdb_attrs_get_string(rules[i], SYSDB_ORIG_DN, &hbac_dn);
if (ret != EOK) {
goto done;
}
+ DEBUG(SSSDBG_TRACE_ALL,
+ ("Matching HBAC rule %s with SELinux mappings\n", hbac_dn));
+
+ if (!sss_selinux_match(rules[i], state->user, state->host, &priority)) {
+ DEBUG(SSSDBG_TRACE_ALL, ("Rule did not match\n"));
+ continue;
+ }
+
+
/* HBAC rule matched, find if it is in the "possible" list */
for (j = 0; state->possible_match[j]; j++) {
usermap = state->possible_match[j];
@@ -509,6 +516,9 @@ static void ipa_get_selinux_hbac_done(struct tevent_req *subreq)
}
if (strcasecmp(hbac_dn, seealso_dn) == 0) {
+ DEBUG(SSSDBG_TRACE_FUNC, ("HBAC rule [%s] matched, copying its"
+ "attributes to SELinux user map [%s]\n",
+ hbac_dn, seealso_dn));
priority &= ~(SELINUX_PRIORITY_USER_NAME |
SELINUX_PRIORITY_USER_GROUP |
SELINUX_PRIORITY_USER_CAT);
diff --git a/src/util/sss_selinux.c b/src/util/sss_selinux.c
index 7b2417bbead70b110df7ee4a0974d1eaa2525b3f..b749b236da9c8b87c8525ee9e755896740597997 100644
--- a/src/util/sss_selinux.c
+++ b/src/util/sss_selinux.c
@@ -84,9 +84,17 @@ bool sss_selinux_match(struct sysdb_attrs *usermap,
if (user) {
ret = sysdb_attrs_get_el(user, SYSDB_ORIG_DN, &dn);
- if (ret != EOK) return false;
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE, ("User does not have origDN\n"));
+ return false;
+ }
ret = sysdb_attrs_get_el(user, SYSDB_ORIG_MEMBEROF, &memberof);
- if (ret != EOK) return false;
+ if (ret != EOK) {
+ DEBUG(SSSDBG_TRACE_ALL,
+ ("User does not have orig memberof, "
+ "therefore it can't match to any rule\n"));
+ return false;
+ }
/**
* The rule won't match if user category != "all" and user map doesn't
@@ -95,6 +103,7 @@ bool sss_selinux_match(struct sysdb_attrs *usermap,
if (usercat == NULL || usercat->num_values == 0 ||
strcasecmp((char *)usercat->values[0].data, "all") != 0) {
if (users_el == NULL) {
+ DEBUG(SSSDBG_TRACE_ALL, ("No users specified in the rule!\n"));
return false;
} else {
matched_name = match_entity(users_el, dn);
@@ -104,6 +113,7 @@ bool sss_selinux_match(struct sysdb_attrs *usermap,
} else if (matched_group) {
priority |= SELINUX_PRIORITY_USER_GROUP;
} else {
+ DEBUG(SSSDBG_TRACE_ALL, ("User did not match\n"));
return false;
}
}
@@ -114,9 +124,17 @@ bool sss_selinux_match(struct sysdb_attrs *usermap,
if (host) {
ret = sysdb_attrs_get_el(host, SYSDB_ORIG_DN, &dn);
- if (ret != EOK) return false;
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE, ("Host does not have origDN\n"));
+ return false;
+ }
ret = sysdb_attrs_get_el(host, SYSDB_ORIG_MEMBEROF, &memberof);
- if (ret != EOK) return false;
+ if (ret != EOK) {
+ DEBUG(SSSDBG_TRACE_ALL,
+ ("Host does not have orig memberof, "
+ "therefore it can't match to any rule\n"));
+ return false;
+ }
/**
* The rule won't match if host category != "all" and user map doesn't
@@ -125,6 +143,7 @@ bool sss_selinux_match(struct sysdb_attrs *usermap,
if (hostcat == NULL || hostcat->num_values == 0 ||
strcasecmp((char *)hostcat->values[0].data, "all") != 0) {
if (hosts_el == NULL) {
+ DEBUG(SSSDBG_TRACE_ALL, ("No users specified in the rule!\n"));
return false;
} else {
matched_name = match_entity(hosts_el, dn);
@@ -134,6 +153,7 @@ bool sss_selinux_match(struct sysdb_attrs *usermap,
} else if (matched_group) {
priority |= SELINUX_PRIORITY_HOST_GROUP;
} else {
+ DEBUG(SSSDBG_TRACE_ALL, ("Host did not match\n"));
return false;
}
}
--
1.7.7.6
>From e9614de4dfa107e05455da35820da9fb76279b14 Mon Sep 17 00:00:00 2001
From: Jan Zeleny <jzel...@redhat.com>
Date: Fri, 20 Jul 2012 11:06:20 -0400
Subject: [PATCH 2/4] Fixed semantically wrong evaluation of SELinux map
priority
---
src/util/sss_selinux.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/sss_selinux.c b/src/util/sss_selinux.c
index b749b236da9c8b87c8525ee9e755896740597997..b4919be71ba636438a0b613dcae32cea8b911771 100644
--- a/src/util/sss_selinux.c
+++ b/src/util/sss_selinux.c
@@ -157,9 +157,9 @@ bool sss_selinux_match(struct sysdb_attrs *usermap,
return false;
}
}
+ } else {
+ priority |= SELINUX_PRIORITY_HOST_CAT;
}
- } else {
- priority |= SELINUX_PRIORITY_HOST_CAT;
}
if (_priority != NULL) {
--
1.7.7.6
>From f7c73f2ed1fa5319119abae15e10dbfa81b5b4a4 Mon Sep 17 00:00:00 2001
From: Jan Zeleny <jzel...@redhat.com>
Date: Fri, 20 Jul 2012 11:09:30 -0400
Subject: [PATCH 3/4] Remove ipa_selinux_map_merge()
This function is no longer necessary since sysdb interface for copying
elements has been implemented.
---
src/providers/ipa/ipa_selinux_common.c | 41 --------------------------------
src/providers/ipa/ipa_selinux_common.h | 4 ---
src/providers/ipa/ipa_session.c | 10 -------
3 files changed, 0 insertions(+), 55 deletions(-)
diff --git a/src/providers/ipa/ipa_selinux_common.c b/src/providers/ipa/ipa_selinux_common.c
index 15f0b4588c1189061252f4db573dc2b87d9556e4..a01e0b6cb3f60348e0b0ad7d7d0a23b0ae0d3c56 100644
--- a/src/providers/ipa/ipa_selinux_common.c
+++ b/src/providers/ipa/ipa_selinux_common.c
@@ -27,47 +27,6 @@
#include "providers/ipa/ipa_selinux_common.h"
-errno_t ipa_selinux_map_merge(struct sysdb_attrs *map,
- struct sysdb_attrs *rule,
- const char *attr)
-{
- struct ldb_message_element *map_el;
- struct ldb_message_element *rule_el;
- size_t total_cnt;
- errno_t ret;
- int i = 0;
-
- ret = sysdb_attrs_get_el(map, attr, &map_el);
- if (ret != EOK) {
- goto done;
- }
-
- ret = sysdb_attrs_get_el(rule, attr, &rule_el);
- if (ret != EOK) {
- goto done;
- }
-
- total_cnt = map_el->num_values + rule_el->num_values;
- map_el->values = talloc_realloc(map->a, map_el->values,
- struct ldb_val, total_cnt);
- if (map_el->values == NULL) {
- ret = ENOMEM;
- goto done;
- }
-
- while (map_el->num_values < total_cnt)
- {
- map_el->values[map_el->num_values] = ldb_val_dup(map_el->values,
- &rule_el->values[i]);
- map_el->num_values++;
- i++;
- }
-
- ret = EOK;
-done:
- return ret;
-}
-
errno_t ipa_save_user_maps(struct sysdb_ctx *sysdb,
size_t map_count,
struct sysdb_attrs **maps)
diff --git a/src/providers/ipa/ipa_selinux_common.h b/src/providers/ipa/ipa_selinux_common.h
index 17ccb231282763b6cbe479eb31520def6d1a0fc5..d722136e9eb96350644bcaf29e69dbecca155815 100644
--- a/src/providers/ipa/ipa_selinux_common.h
+++ b/src/providers/ipa/ipa_selinux_common.h
@@ -25,10 +25,6 @@
#ifndef IPA_SELINUX_COMMON_H_
#define IPA_SELINUX_COMMON_H_
-errno_t ipa_selinux_map_merge(struct sysdb_attrs *map,
- struct sysdb_attrs *rule,
- const char *attr);
-
errno_t ipa_save_host(struct sysdb_ctx *sysdb,
struct sysdb_attrs *host);
diff --git a/src/providers/ipa/ipa_session.c b/src/providers/ipa/ipa_session.c
index 350f968524341ce61c957af0003e49cf833df20b..2c29e4c4ace1ef1cdb6969e4dd7d49187dcce3fc 100644
--- a/src/providers/ipa/ipa_session.c
+++ b/src/providers/ipa/ipa_session.c
@@ -541,16 +541,6 @@ static void ipa_get_selinux_hbac_done(struct tevent_req *subreq)
/* Just to boost the following lookup */
state->possible_match[j] = NULL;
-
- ret = ipa_selinux_map_merge(usermap, rules[i], SYSDB_ORIG_MEMBER_USER);
- if (ret != EOK) {
- goto done;
- }
-
- ret = ipa_selinux_map_merge(usermap, rules[i], SYSDB_ORIG_MEMBER_HOST);
- if (ret != EOK) {
- goto done;
- }
}
}
}
--
1.7.7.6
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
