guilhem@fripost.org wrote:
> Full_Name: Guilhem Moulin
> Version: 2.4.44
> OS: Debian GNU/Linux (Stretch)
> URL: ftp://ftp.openldap.org/incoming/
> Submission from: (NULL) (109.225.112.70)
> 
> 
> slapd.conf(5) manpage (in both 2.4.44 and in current — 0f320b3 — master)
> mentions that authz-policy's "all" flag requires both source and destinations
> authorizations rules to succeed.  However if the source rule (the authentication
> identity's "authzTo" attribute) fails but the destination rule (the
> authorization identity's "authzFrom" attribute) succeeds, then the authorization
> is granted, violating the intended semantics and possibly yielding unauthorized
> access.  See the following log excerpt:

Thanks for the report. Looks like this has been present since commit 113727ba.
Fixed now in git master

> 
> SASL proxy authorize [conn=1019]: authcid="authcid"
> authzid="dn:uid=authzid,dc=example,dc=net"
> ==>slap_sasl_authorized: can uid=authcid,dc=example,dc=net become
> uid=authzid,dc=example,dc=net?
> ==>slap_sasl_check_authz: does uid=authzid,dc=example,dc=net match authzTo rule
> in uid=authcid,dc=example,dc=net?
> <==slap_sasl_check_authz: authzTo check returning 50
> ==>slap_sasl_check_authz: does uid=authcid,dc=example,dc=net match authzFrom
> rule in uid=authzid,dc=example,dc=net?
> <===slap_sasl_match: comparison returned 0
> <==slap_sasl_check_authz: authzFrom check returning 0
> <== slap_sasl_authorized: return 0
> conn=1019 op=1 BIND authcid="authcid"
> authzid="dn:uid=authzid,dc=example,dc=net"
> SASL Authorize [conn=1019]:  proxy authorization allowed
> authzDN="uid=authzid,dc=example,dc=net"
> 
> AFAICT the problem is in servers/slapd/saslauthz.c:slap_sasl_authorized(), and
> is also present in master.  Here is a naive patch that fails the authorization
> if the source rules doesn't verify and SASL_AUTHZ_AND is set in authz_policy.
> 
> --- a/servers/slapd/saslauthz.c
> +++ b/servers/slapd/saslauthz.c
> @@ -2077,6 +2077,10 @@ int slap_sasl_authorized( Operation *op,
>          if( rc == LDAP_SUCCESS && !(authz_policy & SASL_AUTHZ_AND) ) {
>              goto DONE;
>          }
> +        else if( rc != LDAP_SUCCESS && (authz_policy & SASL_AUTHZ_AND) ) {
> +         rc = LDAP_INAPPROPRIATE_AUTH;
> +         goto DONE;
> +     }
>      }
>   
>      /* Check destination rules */
> 
> 


-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

changed notes
changed state Open to Test
moved from Incoming to Software Bugs

On Wed, 29 Aug 2018 at 01:14:51 +0100, Howard Chu wrote:
> Thanks for the report. Looks like this has been present since commit
> 113727ba.  Fixed now in git master

Thanks for the quick fix!  Not sure why rc's value is preserved here but
set to LDAP_INAPPROPRIATE_AUTH in all other failing cases, though.  But
that doesn't seem to matter beside debug logs now showing a return value
other than 48, disclosing the actual reason of the failure; for instance

    <== slap_sasl_authorized: return 16
    SASL Proxy Authorize [conn=1022]: proxy authorization disallowed (16)

for a missing authTo under authz-policy "all".

-- 
Guilhem.

Guilhem Moulin wrote:
> On Wed, 29 Aug 2018 at 01:14:51 +0100, Howard Chu wrote:
>> Thanks for the report. Looks like this has been present since commit
>> 113727ba.  Fixed now in git master
> 
> Thanks for the quick fix!  Not sure why rc's value is preserved here but
> set to LDAP_INAPPROPRIATE_AUTH in all other failing cases, though.  But
> that doesn't seem to matter beside debug logs now showing a return value
> other than 48, disclosing the actual reason of the failure; for instance
> 
>      <== slap_sasl_authorized: return 16
>      SASL Proxy Authorize [conn=1022]: proxy authorization disallowed (16)
> 
> for a missing authTo under authz-policy "all".
> 
Probably not a bad thing overall, but for consistency it's now patched
to set INAPPROPRIATE_AUTH as with the other cases.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

changed notes
changed state Test to Release

changed notes

fixed in master
fixed in RE24 (2.4.47)

changed notes
changed state Release to Closed