[Date Prev][Date Next]
[Chronological]
[Thread]
[Top]
Re: ACL processing: additive privs (using control continue)
Howard Chu wrote:
Kurt Zeilenga wrote:
On Aug 4, 2012, at 9:08 AM, Howard Chu<hyc@symas.com> wrote:
I haven't looked at the code yet but it's possible this is a bug.
Not a bug. As documented, every access statement ends implicitly with a "by * none" clause.
Ah right. The "continue" control is only useful if a following "by" clause
matches the subject *and* specifies incremental privileges.
Any following "by" clause specifying a group pattern obviously can also
specify incremental privileges *and* can (unforeseeable) match the subject.
Group entries and their members tend to be independent. Neither the
existence of a distinct group's entry nor its members are known in
advance (for example during slapd acl configuration time). Furthermore
existing group entries as well as memberships disappear without prior
notice.
- In general groups are managed by people that aren't and usually
don't need to be aware of slapd's internal acl engine dependencies.
- Modifying a group (e.g. removing members or the entry) that is
referenced within an acl probably results in the need to modify the
belonging "by" clauses.
- Statically pre-initializing all group entries referenced in the acls
is misleading especially regarding the idea behind variable groups and
memberships.
- ...
Thus, resetting previously given privileges just in case a group pattern
temporarily does not match seems to overshoot or even miss the target,
especially in case the group entry or the member
exists/disappears/exists/... during runtime.
My suggestion regarding a basis to start a hopefully fruitful discussion is:
Within "by" clauses using group patterns and specifying incremental
privileges the final implicit "by * none" should be ignored (or at least
be made ignoreable) for the current access-statement, while the controls
stay untouched (attached you'll find a prototype of a patch). If the
idea is acceptable, stable's behavior can be changed in general or
smoothly for example by introducing some kind of groupDn evaluation
specifier to make the evaluation of group pattern specific incremental
privileges customizable (ignoreable), e.g.
"group[/<objectclass>[/<attrname>]][.<groupstyle>][,<evalspecifier>]=<group>".
<evalspecifier> ::= "ignoreNoSuchObject" | "ignoreCompareFalse" |
"ignoreBoth" ...
The location of such an <evalspecifier>, e.g. at the groupDn specifier,
within the by clause, within the access clause or even globally is
intentionally left open here.
Another (at least to me much better) solution seems to be to get rid of
the implicit "by * none" code: It represents neither a reasonable
security functionality, nor a valueable convenience feature - the
opposite seems true: Implicit (hidden) "security" hides mission critical
settings and functionallity, causes headache that leads to
misinterpretation that can result in wrong asumptions (not only for
beginners, even for experts, see above). The (need for) discussion
represents just one sign for the inconvenience caused by such a kind of
"convenience feature".
With cn=config a missing "by * none" (as any other) acl statement can be
added during runtime without the past time (slapd.conf) need to restart
slapd. Although the implicit "by * none" seems outdated, specifying
adequate frontend acls "implicit (but documented in the configuration)
by * none" has been already achievable for some time in the past until
today.
In case of an "unwilling to perform situation", lets just try to get rid
of the two useless ("silly" and the "even more silly") continue control
examples described in slapd.access(5), either by
- Describing a appropriate "clever" continue example or if this should
be generally impossible, by
- Removing the obviously silly continue control code, or eventually by
- Starting a (openldap-devel?) discussion whether and if so, how to
integrate the above described (or similar) generally useful feature.
Thank you very much.
diff --git a/servers/slapd/acl.c b/servers/slapd/acl.c
index 92b4c43..35c3fb7 100644
--- a/servers/slapd/acl.c
+++ b/servers/slapd/acl.c
@@ -1121,7 +1121,7 @@ slap_acl_mask(
AccessControlState *state,
slap_access_t access )
{
- int i;
+ int i, implNone;
Access *b;
#ifdef LDAP_DEBUG
char accessmaskbuf[ACCESSMASK_MAXLEN];
@@ -1152,6 +1152,7 @@ slap_acl_mask(
b = a->acl_access;
i = 1;
+ implNone = 1;
for ( ; b != NULL; b = b->a_next, i++ ) {
slap_mask_t oldmask, modmask;
@@ -1648,6 +1649,21 @@ slap_acl_mask(
}
if ( rc != 0 ) {
+ Debug( LDAP_DEBUG_ACL, "<= check a_group_pat: membership evaluation failed: \"%s\" (rc: %d)\n",
+ ldap_err2string(rc), rc, 0 );
+
+ if( ACL_IS_ADDITIVE(b->a_access_mask) || ACL_IS_SUBTRACTIVE(b->a_access_mask) ) {
+ Debug( LDAP_DEBUG_ACL,
+ "<= acl_mask: [%d] ignoring incremental privileges %s (%s)\n",
+ i, accessmask2str( b->a_access_mask, accessmaskbuf, 1 ),
+ b->a_type == ACL_CONTINUE
+ ? "continue"
+ : b->a_type == ACL_BREAK
+ ? "break"
+ : "stop" );
+ implNone = 0;
+ goto byClauseDone;
+ }
continue;
}
}
@@ -1872,6 +1888,7 @@ slap_acl_mask(
"<= acl_mask: [%d] mask: %s\n",
i, accessmask2str(*mask, accessmaskbuf, 1), 0 );
+byClauseDone:
if( b->a_type == ACL_CONTINUE ) {
continue;
@@ -1884,7 +1901,8 @@ slap_acl_mask(
}
/* implicit "by * none" clause */
- ACL_INIT(*mask);
+ if ( implNone )
+ ACL_INIT(*mask);
Debug( LDAP_DEBUG_ACL,
"<= acl_mask: no more <who> clauses, returning %s (stop)\n",