Re: [PATCH] mod_authnz_ldap and satisfy all
Ryan Morgan said: Making this generic is a good idea, though you are correct in asserting it cannot be done without a major re-factoring. Even then the authz modules would need to be modified to respect the satisfy flag when multiple requires are given for a single authz module. The requirement I'm trying to fulfill is multiple group requires within ldap. I figured making it generic within ldap using satisfy would be a good idea, though this seems to be blowing up into a much bigger issue. Perhaps it would be easier if 'require ldap-group' could have multiple groups listed on a single require line? Something similar to ldap- attribute? The trouble is whether to interpret multiple groups as and or or - if you choose one, there is going to be people that want the other option. Or maybe just move the satisfy flag to an ldap specific directive like 'LDAPSatisfyAll' to remove any confusion on what it does? I would definitely like to avoid module specific directives like this, as it creates inconsistent configuration patterns in the server. A user could ask why can I specify multiple groups in LDAP, but not in other modules?, and that user would have a valid point. I think in the long run, supporting satisfy all generically would be an excellent option to have. Regards, Graham --
Re: [PATCH] mod_authnz_ldap and satisfy all
On Wednesday, September 07, 2005 at 5:47:10 pm, in message [EMAIL PROTECTED], [EMAIL PROTECTED] wrote: The requirement I'm trying to fulfill is multiple group requires within ldap. I figured making it generic within ldap using satisfy would be a good idea, though this seems to be blowing up into a much bigger issue. I haven't given this a lot of thought yet but have you tried using require ldap-filter to do what you want? You should be able to write an ldap filter that would satisfy multiple groups. Perhaps it would be easier if 'require ldap-group' could have multiple groups listed on a single require line? Something similar to ldap- attribute? Or maybe just move the satisfy flag to an ldap specific directive like 'LDAPSatisfyAll' to remove any confusion on what it does? LDAPSatisfyAll might be a possibility but I am a little concerned about heading down a road for one specific module that might be hard to come back from when we decide to implement it for all auth modules. I still like the concept and I would suggest that an enhancement be submitted in bugzilla for Apache 2.3. I don't think that we would be able to make it for 2.2. Brad
Re: [PATCH] mod_authnz_ldap and satisfy all
+1 in concept as well but it seems that this should be implemented at some lower level so that we don't have to touch each authz module to teach them how to deal with the satisfy directive. The problem is that the auth_checker hook is defined as AP_IMPLEMENT_HOOK_RUN_FIRST meaning that each registered hook will be called in turn until something other than DECLINE is returned. If something other than DECLINE is returned then the appropriate action takes place. Either authorization succeeds or fails with a specific error. Satisfy All would need to change the behavior of the hook so that each registered hook is called in turn as long as OK or DECLINE is returned. Then if something other than OK or DECLINE is returned, the authorization fails. This would probably require defining a new type of hook that would be defined as AP_IMPLEMENT_HOOK_RUN_ALL. At this point I am not sure how to make that happen without drastically altering the way authorization is currently configured and works. Currently Satisfy All | Any is very specific in how it is implemented inside of ap_process_request_internal(). It's purpose is to satisfy access control vs. (authentication and authorization). This would also need to be changed or use something other than the Satisfy directive. Brad On Monday, September 05, 2005 at 4:15:56 am, in message [EMAIL PROTECTED], [EMAIL PROTECTED] wrote: Ryan Morgan wrote: require ldap-group cn=Engineering,ou=Groups,o=SomeCompany,c=US require ldap-group cn=QA,ou=Groups,o=SomeCompany,c=US satisfy all Could someone provide feedback on whether this is a feature that could be added to the ldap module? Definitely a +1 in concept. Do the other authz modules handle satisfy all in the same way? A quick eyeball of the patch shows up some C++ comments - can you convert them to C comments? Regards, Graham --
Re: [PATCH] mod_authnz_ldap and satisfy all
Ryan Morgan wrote: Other than mod_access, none of the other authz modules handle the satisfy directive. mod_access uses it to specify how to handle authorization when both the require and allow directives are used. I thought that, in part, satisfy all is in the main request handling logic that's burried in the core (where we do the uri translation, loc walk, map_to_storage walks, again a loc walk and then begin the auth handling's outer wrapper.) Bill
Re: [PATCH] mod_authnz_ldap and satisfy all
On Wednesday, September 07, 2005 at 2:37:00 pm, in message [EMAIL PROTECTED], [EMAIL PROTECTED] wrote: I thought that, in part, satisfy all is in the main request handling logic that's burried in the core (where we do the uri translation, loc walk, map_to_storage walks, again a loc walk and then begin the auth handling's outer wrapper.) Bill It is. It is buried inside of ap_process_request_internal() and is code for a specific purpose. See my previous post. Brad
Re: [PATCH] mod_authnz_ldap and satisfy all
Making this generic is a good idea, though you are correct in asserting it cannot be done without a major re-factoring. Even then the authz modules would need to be modified to respect the satisfy flag when multiple requires are given for a single authz module. The requirement I'm trying to fulfill is multiple group requires within ldap. I figured making it generic within ldap using satisfy would be a good idea, though this seems to be blowing up into a much bigger issue. Perhaps it would be easier if 'require ldap-group' could have multiple groups listed on a single require line? Something similar to ldap- attribute? Or maybe just move the satisfy flag to an ldap specific directive like 'LDAPSatisfyAll' to remove any confusion on what it does? On Sep 7, 2005, at 9:02 AM, Brad Nicholes wrote: +1 in concept as well but it seems that this should be implemented at some lower level so that we don't have to touch each authz module to teach them how to deal with the satisfy directive. The problem is that the auth_checker hook is defined as AP_IMPLEMENT_HOOK_RUN_FIRST meaning that each registered hook will be called in turn until something other than DECLINE is returned. If something other than DECLINE is returned then the appropriate action takes place. Either authorization succeeds or fails with a specific error. Satisfy All would need to change the behavior of the hook so that each registered hook is called in turn as long as OK or DECLINE is returned. Then if something other than OK or DECLINE is returned, the authorization fails. This would probably require defining a new type of hook that would be defined as AP_IMPLEMENT_HOOK_RUN_ALL. At this point I am not sure how to make that happen without drastically altering the way authorization is currently configured and works. Currently Satisfy All | Any is very specific in how it is implemented inside of ap_process_request_internal(). It's purpose is to satisfy access control vs. (authentication and authorization). This would also need to be changed or use something other than the Satisfy directive. Brad On Monday, September 05, 2005 at 4:15:56 am, in message [EMAIL PROTECTED], [EMAIL PROTECTED] wrote: Ryan Morgan wrote: require ldap-group cn=Engineering,ou=Groups,o=SomeCompany,c=US require ldap-group cn=QA,ou=Groups,o=SomeCompany,c=US satisfy all Could someone provide feedback on whether this is a feature that could be added to the ldap module? Definitely a +1 in concept. Do the other authz modules handle satisfy all in the same way? A quick eyeball of the patch shows up some C++ comments - can you convert them to C comments? Regards, Graham --
Re: [PATCH] mod_authnz_ldap and satisfy all
On Sep 5, 2005, at 3:15 AM, Graham Leggett wrote:
Ryan Morgan wrote:
require ldap-group cn=Engineering,ou=Groups,o=SomeCompany,c=US
require ldap-group cn=QA,ou=Groups,o=SomeCompany,c=US
satisfy all
Could someone provide feedback on whether this is a feature that
could be
added to the ldap module?
Definitely a +1 in concept. Do the other authz modules handle
satisfy all in the same way?
Great.. Thanks for taking a look Graham.
Other than mod_access, none of the other authz modules handle the
satisfy
directive. mod_access uses it to specify how to handle authorization
when
both the require and allow directives are used.
This patch builds on that concept, but handles the case where
multiple require
lines are present. I figured using satisfy made more sense than
adding another
directive to the ldap module. It's entirely possible that satisfy
was not meant
to be used this way, but it seems to fit in nicely.
A quick eyeball of the patch shows up some C++ comments - can you
convert them to C comments?
Sorry about that, attached is an updated patch.
-Ryan
Index: modules/aaa/mod_authnz_ldap.c
===
--- modules/aaa/mod_authnz_ldap.c (revision 278618)
+++ modules/aaa/mod_authnz_ldap.c (working copy)
@@ -477,6 +477,9 @@
const apr_array_header_t *reqs_arr = ap_requires(r);
require_line *reqs = reqs_arr ? (require_line *)reqs_arr-elts : NULL;
+int satisfy = ap_satisfies(r);
+int require_failed = 0; /* flag for satisfy all */
+int req_user_success, req_group_success, req_attribute_success;
register int x;
const char *t;
@@ -602,9 +605,13 @@
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
[% APR_PID_T_FMT ] auth_ldap authorise:
require user: authorisation successful,
getpid());
-return OK;
+if (satisfy != SATISFY_ALL) {
+return OK;
+}
+break;
}
default: {
+require_failed = 1;
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
[% APR_PID_T_FMT ] auth_ldap authorise:
require user:
authorisation failed [%s][%s], getpid(),
@@ -614,24 +621,31 @@
/*
* Now break apart the line and compare each word on it
*/
+req_user_success = 0;
while (t[0]) {
w = ap_getword_conf(r-pool, t);
result = util_ldap_cache_compare(r, ldc, sec-url, req-dn,
sec-attribute, w);
-switch(result) {
-case LDAP_COMPARE_TRUE: {
-ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
- [% APR_PID_T_FMT ] auth_ldap
authorise:
- require user: authorisation
successful, getpid());
+
+if (result == LDAP_COMPARE_TRUE) {
+ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+ [% APR_PID_T_FMT ] auth_ldap authorise:
+ require user: authorisation successful,
getpid());
+if (satisfy != SATISFY_ALL) {
return OK;
}
-default: {
-ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
- [% APR_PID_T_FMT ] auth_ldap
authorise:
- require user: authorisation failed
[%s][%s],
- getpid(), ldc-reason,
ldap_err2string(result));
-}
+req_user_success = 1;
+break;
+} else {
+ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+ [% APR_PID_T_FMT ] auth_ldap authorise:
+ require user: authorisation failed
[%s][%s],
+ getpid(), ldc-reason,
ldap_err2string(result));
}
}
+if (!req_user_success) {
+/* all user checks failed for this require line */
+require_failed = 1;
+}
}
else if (strcmp(w, ldap-dn) == 0) {
if (req-dn == NULL || strlen(req-dn) == 0) {
@@ -648,9 +662,13 @@
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
[% APR_PID_T_FMT ] auth_ldap authorise:
require dn: authorisation successful,
getpid());
-return OK;
+if (satisfy != SATISFY_ALL) {
+return OK;
+}
+
Re: [PATCH] mod_authnz_ldap and satisfy all
Ryan Morgan wrote: require ldap-group cn=Engineering,ou=Groups,o=SomeCompany,c=US require ldap-group cn=QA,ou=Groups,o=SomeCompany,c=US satisfy all Could someone provide feedback on whether this is a feature that could be added to the ldap module? Definitely a +1 in concept. Do the other authz modules handle satisfy all in the same way? A quick eyeball of the patch shows up some C++ comments - can you convert them to C comments? Regards, Graham --
[PATCH] mod_authnz_ldap and satisfy all
Attached is a patch in response to my email earlier in the week.
This adds the ability for auth_ldap to check all require lines before
allowing access through use of the 'satisfy all' directive. The
previous
behavior of the module is grant access if any require line succeeds.
The main reason behind this patch is to allow administrators to require
users be in multiple groups. For example:
Location /
AuthType Basic
AuthName Authenticate Please
AuthBasicProvider ldap
AuthzLDAPAuthoritative off
AuthLDAPUrl ldap://localhost/o=SomeCompany,c=US?uid?sub?
require ldap-group cn=Engineering,ou=Groups,o=SomeCompany,c=US
require ldap-group cn=QA,ou=Groups,o=SomeCompany,c=US
satisfy all
/Location
Could someone provide feedback on whether this is a feature that
could be
added to the ldap module?
Thanks,
-Ryan
Index: modules/aaa/mod_authnz_ldap.c
===
--- modules/aaa/mod_authnz_ldap.c (revision 278618)
+++ modules/aaa/mod_authnz_ldap.c (working copy)
@@ -477,6 +477,9 @@
const apr_array_header_t *reqs_arr = ap_requires(r);
require_line *reqs = reqs_arr ? (require_line *)reqs_arr-elts : NULL;
+int satisfy = ap_satisfies(r);
+int require_failed = 0; // flag for 'satisfy all'
+int req_user_success, req_group_success, req_attribute_success;
register int x;
const char *t;
@@ -602,9 +605,13 @@
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
[% APR_PID_T_FMT ] auth_ldap authorise:
require user: authorisation successful,
getpid());
-return OK;
+if (satisfy != SATISFY_ALL) {
+return OK;
+}
+break;
}
default: {
+require_failed = 1;
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
[% APR_PID_T_FMT ] auth_ldap authorise:
require user:
authorisation failed [%s][%s], getpid(),
@@ -614,24 +621,30 @@
/*
* Now break apart the line and compare each word on it
*/
+req_user_success = 0;
while (t[0]) {
w = ap_getword_conf(r-pool, t);
result = util_ldap_cache_compare(r, ldc, sec-url, req-dn,
sec-attribute, w);
-switch(result) {
-case LDAP_COMPARE_TRUE: {
-ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
- [% APR_PID_T_FMT ] auth_ldap
authorise:
- require user: authorisation
successful, getpid());
+
+if (result == LDAP_COMPARE_TRUE) {
+ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+ [% APR_PID_T_FMT ] auth_ldap authorise:
+ require user: authorisation successful,
getpid());
+if (satisfy != SATISFY_ALL) {
return OK;
}
-default: {
-ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
- [% APR_PID_T_FMT ] auth_ldap
authorise:
- require user: authorisation failed
[%s][%s],
- getpid(), ldc-reason,
ldap_err2string(result));
-}
+req_user_success = 1;
+break;
+} else {
+ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+ [% APR_PID_T_FMT ] auth_ldap authorise:
+ require user: authorisation failed
[%s][%s],
+ getpid(), ldc-reason,
ldap_err2string(result));
}
}
+if (!req_user_success) {
+require_failed = 1; //all user checks failed for this require
+}
}
else if (strcmp(w, ldap-dn) == 0) {
if (req-dn == NULL || strlen(req-dn) == 0) {
@@ -648,9 +661,13 @@
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
[% APR_PID_T_FMT ] auth_ldap authorise:
require dn: authorisation successful,
getpid());
-return OK;
+if (satisfy != SATISFY_ALL) {
+return OK;
+}
+break;
}
default: {
+require_failed = 1;
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
[% APR_PID_T_FMT ] auth_ldap authorise:
require dn
