|
|
dc8c34 |
From ac35b0d68dce1742f0d32a6fefc4e9d38cd3114c Mon Sep 17 00:00:00 2001
|
|
|
dc8c34 |
From: "Thierry bordaz (tbordaz)" <tbordaz@redhat.com>
|
|
|
dc8c34 |
Date: Mon, 22 Apr 2013 14:15:33 +0200
|
|
|
dc8c34 |
Subject: [PATCH 224/225] Ticket 47331 - Self entry access ACI not working
|
|
|
dc8c34 |
properly
|
|
|
dc8c34 |
|
|
|
dc8c34 |
Bug Description:
|
|
|
dc8c34 |
|
|
|
dc8c34 |
There are two issues in that bug.
|
|
|
dc8c34 |
|
|
|
dc8c34 |
The first one is that for a given entry, the rights related to an attribute are evaluated and cached. Reusing this evaluation for a different entry is erronous.
|
|
|
dc8c34 |
|
|
|
dc8c34 |
The second one is that for each deny/allow aci, the results of the evaluation of the aci is cached. These results
|
|
|
dc8c34 |
are reset for aci type that are entry related. The parsing of the rule self entry access miss the setting
|
|
|
dc8c34 |
of ACI_USERDN_SELFRULE.
|
|
|
dc8c34 |
This flag allows to reset (in result cache) a result obtained on a previous entry. The consequence is that
|
|
|
dc8c34 |
a previous result was erronously reused.
|
|
|
dc8c34 |
|
|
|
dc8c34 |
Fix Description:
|
|
|
dc8c34 |
|
|
|
dc8c34 |
The fix for the first issue, is to prevent acl__match_handlesFromCache to reuse already evaluated attributes.
|
|
|
dc8c34 |
A new flag make acl__match_handlesFromCache to return if the evaluation is entry related.
|
|
|
dc8c34 |
|
|
|
dc8c34 |
The second fix is to set ACI_USERDN_SELFRULE, when we have a rule like 'userdn = ldap:///self'
|
|
|
dc8c34 |
|
|
|
dc8c34 |
https://fedorahosted.org/389/ticket/47331
|
|
|
dc8c34 |
|
|
|
dc8c34 |
Reviewed by: Noriko Hosoi, Ludwig Krispenz
|
|
|
dc8c34 |
|
|
|
dc8c34 |
Platforms tested: fedora 17
|
|
|
dc8c34 |
|
|
|
dc8c34 |
Flag Day: no
|
|
|
dc8c34 |
|
|
|
dc8c34 |
Doc impact: no
|
|
|
dc8c34 |
(cherry picked from commit 79346deb255ca8d7889d7590534d308d4e3a78da)
|
|
|
dc8c34 |
(cherry picked from commit 1580bcd71cbb60f0e97a36bf83faca6e079cd861)
|
|
|
dc8c34 |
(cherry picked from commit 46b06bb3f12be8df973579971c5cdf4312456974)
|
|
|
dc8c34 |
---
|
|
|
dc8c34 |
ldap/servers/plugins/acl/acl.c | 28 ++++++++++++++++++++++++++--
|
|
|
dc8c34 |
ldap/servers/plugins/acl/acl.h | 1 +
|
|
|
dc8c34 |
ldap/servers/plugins/acl/aclparse.c | 19 +++++++++++++++++++
|
|
|
dc8c34 |
3 files changed, 46 insertions(+), 2 deletions(-)
|
|
|
dc8c34 |
|
|
|
dc8c34 |
diff --git a/ldap/servers/plugins/acl/acl.c b/ldap/servers/plugins/acl/acl.c
|
|
|
dc8c34 |
index 09f28ee..d27c0e1 100644
|
|
|
dc8c34 |
--- a/ldap/servers/plugins/acl/acl.c
|
|
|
dc8c34 |
+++ b/ldap/servers/plugins/acl/acl.c
|
|
|
dc8c34 |
@@ -2799,6 +2799,11 @@ acl__TestRights(Acl_PBlock *aclpb,int access, char **right, char ** map_generic,
|
|
|
dc8c34 |
|
|
|
dc8c34 |
if (access & ( SLAPI_ACL_SEARCH | SLAPI_ACL_READ)) {
|
|
|
dc8c34 |
|
|
|
dc8c34 |
+ /* We can not reused results obtained on a other entry */
|
|
|
dc8c34 |
+ if (aci->aci_type & ACI_CACHE_RESULT_PER_ENTRY) {
|
|
|
dc8c34 |
+ aclpb->aclpb_state |= ACLPB_CACHE_RESULT_PER_ENTRY_SKIP;
|
|
|
dc8c34 |
+ }
|
|
|
dc8c34 |
+
|
|
|
dc8c34 |
/*
|
|
|
dc8c34 |
* aclpb->aclpb_cache_result[0..aclpb->aclpb_last_cache_result] is
|
|
|
dc8c34 |
* a cache of info about whether applicable acis
|
|
|
dc8c34 |
@@ -3010,6 +3015,10 @@ acl__TestRights(Acl_PBlock *aclpb,int access, char **right, char ** map_generic,
|
|
|
dc8c34 |
|
|
|
dc8c34 |
if (access & ( SLAPI_ACL_SEARCH | SLAPI_ACL_READ)) {
|
|
|
dc8c34 |
|
|
|
dc8c34 |
+ /* We can not reused results obtained on a other entry */
|
|
|
dc8c34 |
+ if (aci->aci_type & ACI_CACHE_RESULT_PER_ENTRY) {
|
|
|
dc8c34 |
+ aclpb->aclpb_state |= ACLPB_CACHE_RESULT_PER_ENTRY_SKIP;
|
|
|
dc8c34 |
+ }
|
|
|
dc8c34 |
/*
|
|
|
dc8c34 |
* aclpb->aclpb_cache_result[0..aclpb->aclpb_last_cache_result] is
|
|
|
dc8c34 |
* a cache of info about whether applicable acis
|
|
|
dc8c34 |
@@ -3794,8 +3803,23 @@ acl__match_handlesFromCache ( Acl_PBlock *aclpb, char *attr, int access)
|
|
|
dc8c34 |
} else {
|
|
|
dc8c34 |
context_type = ACLPB_EVALCONTEXT_PREV;
|
|
|
dc8c34 |
c_evalContext = &aclpb->aclpb_prev_entryEval_context;
|
|
|
dc8c34 |
- }
|
|
|
dc8c34 |
-
|
|
|
dc8c34 |
+ }
|
|
|
dc8c34 |
+
|
|
|
dc8c34 |
+ /* we can not reused access evaluation done on a previous entry
|
|
|
dc8c34 |
+ * so just skip that cache
|
|
|
dc8c34 |
+ */
|
|
|
dc8c34 |
+ if (aclpb->aclpb_state & ACLPB_CACHE_RESULT_PER_ENTRY_SKIP) {
|
|
|
dc8c34 |
+ aclpb->aclpb_state &= ~ACLPB_MATCHES_ALL_ACLS;
|
|
|
dc8c34 |
+ aclpb->aclpb_state |= ACLPB_UPD_ACLCB_CACHE;
|
|
|
dc8c34 |
+ /* Did not match */
|
|
|
dc8c34 |
+ if (context_type == ACLPB_EVALCONTEXT_ACLCB) {
|
|
|
dc8c34 |
+ aclpb->aclpb_state &= ~ACLPB_HAS_ACLCB_EVALCONTEXT;
|
|
|
dc8c34 |
+ } else {
|
|
|
dc8c34 |
+ aclpb->aclpb_state |= ACLPB_COPY_EVALCONTEXT;
|
|
|
dc8c34 |
+ c_evalContext->acle_numof_tmatched_handles = 0;
|
|
|
dc8c34 |
+ }
|
|
|
dc8c34 |
+ return -1;
|
|
|
dc8c34 |
+ }
|
|
|
dc8c34 |
|
|
|
dc8c34 |
if ( aclpb->aclpb_res_type & (ACLPB_NEW_ENTRY | ACLPB_EFFECTIVE_RIGHTS) ) {
|
|
|
dc8c34 |
aclpb->aclpb_state |= ACLPB_MATCHES_ALL_ACLS;
|
|
|
dc8c34 |
diff --git a/ldap/servers/plugins/acl/acl.h b/ldap/servers/plugins/acl/acl.h
|
|
|
dc8c34 |
index c61ee70..dbe5c11 100644
|
|
|
dc8c34 |
--- a/ldap/servers/plugins/acl/acl.h
|
|
|
dc8c34 |
+++ b/ldap/servers/plugins/acl/acl.h
|
|
|
dc8c34 |
@@ -468,6 +468,7 @@ struct acl_pblock {
|
|
|
dc8c34 |
#define ACLPB_UPD_ACLCB_CACHE 0x100000
|
|
|
dc8c34 |
#define ACLPB_ATTR_RULE_EVALUATED 0x200000
|
|
|
dc8c34 |
#define ACLPB_DONOT_EVALUATE_PROXY 0x400000
|
|
|
dc8c34 |
+#define ACLPB_CACHE_RESULT_PER_ENTRY_SKIP 0x800000
|
|
|
dc8c34 |
|
|
|
dc8c34 |
|
|
|
dc8c34 |
#define ACLPB_RESET_MASK ( ACLPB_ACCESS_ALLOWED_ON_A_ATTR | ACLPB_ACCESS_DENIED_ON_ALL_ATTRS | \
|
|
|
dc8c34 |
diff --git a/ldap/servers/plugins/acl/aclparse.c b/ldap/servers/plugins/acl/aclparse.c
|
|
|
dc8c34 |
index 8b11471..26d57c4 100644
|
|
|
dc8c34 |
--- a/ldap/servers/plugins/acl/aclparse.c
|
|
|
dc8c34 |
+++ b/ldap/servers/plugins/acl/aclparse.c
|
|
|
dc8c34 |
@@ -784,6 +784,8 @@ normalize_nextACERule:
|
|
|
dc8c34 |
goto error;
|
|
|
dc8c34 |
}
|
|
|
dc8c34 |
} else if ( 0 == strncmp ( s, DS_LAS_USERDN, 6 )) {
|
|
|
dc8c34 |
+ char *prefix;
|
|
|
dc8c34 |
+
|
|
|
dc8c34 |
p = PL_strnchr (s, '=', end - s);
|
|
|
dc8c34 |
if (NULL == p) {
|
|
|
dc8c34 |
goto error;
|
|
|
dc8c34 |
@@ -808,6 +810,23 @@ normalize_nextACERule:
|
|
|
dc8c34 |
goto error;
|
|
|
dc8c34 |
}
|
|
|
dc8c34 |
|
|
|
dc8c34 |
+ /* skip the ldap prefix */
|
|
|
dc8c34 |
+ prefix = PL_strncasestr(p, LDAP_URL_prefix, end - p);
|
|
|
dc8c34 |
+ if (prefix) {
|
|
|
dc8c34 |
+ prefix += strlen(LDAP_URL_prefix);
|
|
|
dc8c34 |
+ } else {
|
|
|
dc8c34 |
+ prefix = PL_strncasestr(p, LDAPS_URL_prefix, end - p);
|
|
|
dc8c34 |
+ if (prefix) {
|
|
|
dc8c34 |
+ prefix += strlen(LDAPS_URL_prefix);
|
|
|
dc8c34 |
+ }
|
|
|
dc8c34 |
+ }
|
|
|
dc8c34 |
+ if (prefix == NULL) {
|
|
|
dc8c34 |
+ /* userdn value does not starts with LDAP(S)_URL_prefix */
|
|
|
dc8c34 |
+ goto error;
|
|
|
dc8c34 |
+ }
|
|
|
dc8c34 |
+ p = prefix;
|
|
|
dc8c34 |
+
|
|
|
dc8c34 |
+
|
|
|
dc8c34 |
/* we have a rule like userdn = "ldap:///blah". s points to blah now.
|
|
|
dc8c34 |
** let's find if we have a SELF rule like userdn = "ldap:///self".
|
|
|
dc8c34 |
** Since the resource changes on entry basis, we can't cache the
|
|
|
dc8c34 |
--
|
|
|
dc8c34 |
1.8.1.4
|
|
|
dc8c34 |
|