[SSSD] [PATCHES] SUDO Integration - responder command for cn=defaults

Stephen Gallagher sgallagh at redhat.com
Fri Jan 27 14:11:44 UTC 2012


On Fri, 2012-01-27 at 01:48 +0100, Jakub Hrozek wrote:
> On Thu, Jan 26, 2012 at 05:12:01PM +0100, Pavel Březina wrote:
> > Dne 25.1.2012 18:55, Jakub Hrozek napsal(a):
> > >On Tue, Jan 24, 2012 at 10:42:14AM +0100, Pavel Březina wrote:
> > >>Dne 19.1.2012 12:31, Jakub Hrozek napsal(a):
> > >>>On Tue, Jan 17, 2012 at 05:55:49PM +0100, Pavel Březina wrote:
> > >>>>https://fedorahosted.org/sssd/ticket/1143
> > >>>>
> > >>>>Add a new responder command that will return to the caller just the
> > >>>>rule named cn=defaults.
> > >>>
> > >>>The patches look fine conceptually but I would like to have them rebased on
> > >>>top of the upcoming patches for https://fedorahosted.org/sssd/ticket/1115
> > >>>not the other way around, it would be much easier. I'd like to send patches
> > >>>for #1115 quite soon, probably today.
> > >>
> > >>Rebased patches attached.
> > >
> > >0001: Ack
> > >
> > >0002: Nack, please use "uid_t" for uid in "struct be_sudo_req"
> > 
> > Done.
> > 
> > >I'm also not very fond of using BE_REQ_* inside the sdap_* request and
> > >especially in the periodic update.
> > 
> > I though of this as well but I chose it as a lesser evil. We have
> > already BE_REQ_* and SSS_DP_*, I don't want to create another name
> > for sdap and eventually for IPA that would have literally the same
> > meaning.
> > 
> >  I think we should rather try to
> > >construct the filters on one place, maybe even before the _send
> > >function. The "ALL" case could then be handled by "username=*" similarly
> > >to what we do in the LDAP provider.
> > 
> > This is a good idea. Unfortunately as you have found that the way we
> > are clearing the cache is entirely wrong*, this would be possible
> > only for the LDAP filter. I suggest to leave it as it is for the
> > moment and deal with it after the cache purging is reworked. Than we
> > will know what options do we have.
> > 
> 
> Yeah, I can't believe this design bug made it through my review during
> development and then the peer-review on the list..
> 
> > *
> > - DP downloads rules for the user, searching for the keyword ALL,
> > username, #uid, %group, +* (all netgroups)
> > - before it stores these rules into the sysdb, it deletes the
> > entries using the same filter
> > 
> > This can cause problems in following situation:
> > - we have users A and B
> > - rule R will match for both users by their name or uid (there is no
> > problem for the groups or netgroups)
> > - the rule is stored in the sysdb
> > - A is removed from R
> > - A runs sudo which causes deletion of R
> > - DP goes offline
> > - B is unable to run sudo beacuse R is deleted
> > 
> > Jakub suggested than instead of deleting entire rule we will just
> > delete the username/uid/group from the sudoUser attribute and delete
> > the rule only in case the sudoUser is empty.
> > 
> 
> Because the update step would also download all the rules that contain
> any netgroups, we can purge rules that contain "sudoUser=+*" as well.
> 
> > >0003: Ack
> > >
> > >0004: Nack, you are leaking reply_buf in sss_sudo_send_recv_generic()
> > 
> > Fixed.
> > 
> > >0005: Ack
> > 
> > Thank you for the review.
> > 
> 
> This round of patches looks good to me.
> 
> Ack to all five of them.

Pushed to master.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120127/2b70e03b/attachment.sig>


More information about the sssd-devel mailing list