On Thu, Nov 23, 2023 at 3:00 PM Ruediger Pluem <rpl...@apache.org> wrote:
>
> On 11/23/23 2:28 PM, Yann Ylavic wrote:
> > 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..
>
> I think for ldap-search things depend on the configured filter expression. It
> may involve a previously authenticated user, in
> which case it has similar approaches like ldap-filter or even require
> valid-user or it does not include that and may remind more
> of a mod_authz_host with a dynamic IP list stored in LDAP. Or the certificate
> example Graham gave.
>
> >
> >> 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?
>
> Have a look at line 2117 and following in modules/ldap/util_ldap.c. We call
> it in line 1480 of modules/aaa/mod_authnz_ldap.c via
> util_ldap_cache_getuserdn. Result will be LDAP_NO_SUCH_OBJECT and dn will
> stay the same as before the call.
Thanks, got it.
Regards;
Yann.
