On Thu, Nov 23, 2023 at 1:33 PM Ruediger Pluem <rpl...@apache.org> wrote:
>
>
>
> On 11/23/23 12:52 PM, Graham Leggett via dev wrote:
> > On 23 Nov 2023, at 11:25, Ruediger Pluem <rpl...@apache.org> wrote:
> >
> >>> - if (req->dn == NULL || !*req->dn) {
> >>> - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02636)
> >>> - "auth_ldap authorize: require ldap-filter: user's
> >>> DN "
> >>> - "has not been defined; failing authorization");
> >>> - return AUTHZ_DENIED;
> >>> + if (req->dn == NULL || !*req->dn) {
> >>> + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02636)
> >>> + "auth_ldap authorize: require ldap-search:
> >>> user's DN "
> >>> + "has not been defined; failing authorization");
> >>> + return AUTHZ_DENIED;
> >>> + }
> >>
> >> Why do we need to get the dn in case that r->user is not NULL and why is
> >> it a reason to fail if we don't get a dn for this user?
> >
> > This message is misleading, the DN we care about is not the DN of the user,
> > but rather the DN of the object returned in the
> > ldap-search, which may or may not bear a relation to the user.
>
> Unfortunately the message is correct and we do just that via
> get_dn_for_nonldap_authn (line 1451). The DN you mention we look for
> later in line 1480 via util_ldap_cache_getuserdn.
My guess was that if r->user is set we always want to use it for ldap
requests/authn? Or maybe there should be another Require ldap-!search
configured to achieve that (if wanted) since ldap-search is special..
> BTW: I guess the following patch is missing as we don't store the dn / vals
> we get from util_ldap_cache_getuserdn but just
> "forget" it once we leave the function.
>
>
> Index: modules/aaa/mod_authnz_ldap.c
> ===================================================================
> --- modules/aaa/mod_authnz_ldap.c (revision 1914069)
> +++ modules/aaa/mod_authnz_ldap.c (working copy)
> @@ -1482,10 +1482,12 @@
>
> /* Make sure that the filtered search returned a single dn */
> if (result == LDAP_SUCCESS && dn) {
> + req->dn = dn;
> + req->vals = vals;
> ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02631)
> "auth_ldap authorize: require ldap-search: "
> "authorization successful");
> - set_request_vars(r, LDAP_AUTHZ, req->vals);
> + set_request_vars(r, LDAP_AUTHZ, vals);
> return AUTHZ_GRANTED;
> }
> else {
>
>
> >
> > For example the ldap-search might be doing a lookup on (sucks thumb) the
> > issuer of a certificate, which if it matches some LDAP
> > object means it is allowed. The DN will not refer to the user, but
> > something else.
>
> I get that. Hence I wonder why we care of the dn fitting to r->user set
> before.
>
> >
> > On a side note, ldap-search requires that exactly one LDAP object matches,
> > we might want to support many matches, or a specific
> > number of matches.
I still don't get where "Make sure that the filtered search returned a
single dn" is enforced, is it because result != LDAP_SUCCESS
otherwise, or dn == NULL if there are multiple objects/DNs?
Regards;
Yann.
