Re: Enforcing attribute ACL on add operations

[Howard Chu <hyc@symas.com>]

Kurt Zeilenga wrote:
> On Sep 27, 2008, at 8:59 AM, Emmanuel Dreyfus wrote:
>
> > Hello
> >
> > Right now, slapd ignore attribute ACL when performing an add
> > operation.
>
> I note that this is the expected behavior, been so for many, many years.

Yes, but it never really made much sense - it means you can be forbidden from 
modifying an existing record to contain certain privileged data, but not 
forbidden from creating records with privileged data. It makes sense to me 
that your ability to create particular data values should not depend on 
whether you're creating it for the very first time, or some subsequent time.

(Coming at it from the opposite direction, Delete of course requires you to 
permit the Delete even if certain attributes are read-only; since every entry 
contains read-only operational attributes, deletes would be impossible without 
this provision.)
> > If you have privilegied users that can add entries, it means that
> > you can
> > prevent them from modifying attributes in existing entries, but you
> > cannot
> > prevent them from adding an entry with a read-only attribute.
> >
> > The problem can be interesting with an attribute such as authzTo,
> > where the
> > whole access control can be circumvented by any user that can create
> > an
> > entry in the tree. IMO this behavior was not intended, but if it was,
>
> It was.   Likewise the behavior of rename.
>
> > then it should be clearly documented.
>
> I recall it being noted somewhere in the documents, but likely not as
> clear as it should be.  I recall discussing this ACL/authzTo issue
> long ago.
>
> > Below is a patch that cause attribute ACL to be checked for add
> > operations.
> > It is done in the backend, so if it is acceptable, then I will have
> > to do it
> > for other backends. I wonder if the modrdn operation shoulnd't be
> > subject to
> > the same sanity checks.
> >
> > Any thought? Does it look right?
>
> I have no opinion as whether this change should be made or not.  I'm
> merely providing some background information....
>
> -- Kurt
>
> > diff -U2 -r1.174 add.c
> > --- servers/slapd/back-bdb/add.c        26 Aug 2008 23:45:35
> > -0000     1.174
> > +++ servers/slapd/back-bdb/add.c        27 Sep 2008 15:54:58 -0000
> > @@ -300,4 +300,22 @@
> >        }
> >
> > +       /*
> > +        * Check ACL for attribute write access
> > +        */
> > +       if (!acl_check_modlist(op, oe, op->ora_modlist)) {
> > +               switch( opinfo.boi_err ) {
> > +               case DB_LOCK_DEADLOCK:
> > +               case DB_LOCK_NOTGRANTED:
> > +                       goto retry;
> > +               }
> > +
> > +               Debug( LDAP_DEBUG_TRACE,
> > +                       LDAP_XSTRING(bdb_add) ": no write access to
> > attribute\n",
> > +                       0, 0, 0 );
> > +               rs->sr_err = LDAP_INSUFFICIENT_ACCESS;
> > +               rs->sr_text = "no write access to attribute";
> > +               goto return_results;;
> > +       }
> > +
> >        if ( eid == NOID ) {
> >                rs->sr_err = bdb_next_id( op->o_bd,&eid );
> >
> > --
> > Emmanuel Dreyfus
> > http://hcpnet.free.fr/pubz
> > manu@netbsd.org


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