[SSSD] [PATCH][PRELIMINARY] sudo: send username and uid while requesting default options

Jakub Hrozek jhrozek at redhat.com
Tue Mar 27 02:00:53 UTC 2012


On Mon, Mar 26, 2012 at 06:35:08PM -0400, Simo Sorce wrote:
> On Mon, 2012-03-26 at 15:47 -0400, Jakub Hrozek wrote:
> > On Mon, Mar 19, 2012 at 02:46:02PM +0100, Pavel Březina wrote:
> > > https://fedorahosted.org/sssd/ticket/1239
> > > 
> > > [PATCH 1/2]
> > > Finally removes EOK constant from sudo api header. It is not used in
> > > the SUDO code so it does not require their changes.
> > > 
> > 
> > Looks good to me, it'w weird to use EOK in an external header (even
> > though it is correctly defined).
> > 
> > (Note: this patch shouldn't be pushed even though I don't have any
> > comments. It is an ABI break and we need to coordinate with Daniel K.)
> 
> I do not see any ABI change in the first patch, what are you referring
> to ?
> 

I misread, I thought Pavel also changed a definiton of a function.
Please ignore me.

> > > [PATCH 2/2]
> > > This does what is requested in the ticket. It seems to be very huge but
> > > in fact it is mainly changing the variable. Basically I tried to get
> > > rid of domain ctx where possible, leave it only in initgroups part and
> > > use command ctx elsewhere.
> > > 
> > 
> > Still, it is hard to review the huge patch. Can you split it into
> > smaller ones? What about creating one that removes the duplication
> > between _get_sudorules and _get_defaults, one that converts to using
> > cmdctx and one that adds the uid support?
> > 
> > What is the rationale for removing the dctx? It would seem logical to me
> > to continue using it, the user is found in a particular domain anyway..
> > 
> > sudosrv_cmd: we should free cmd_ctx if retreiving cmd_ctx->sudo_ctx
> > fails (I know the problem was there before)
> > 
> > sudosrv_get_user: can you add a DEBUG() message when the UID number does
> > not match?
> > 
> > You shouldn't add add _GNU_SOURCE to sss_sudo.c, it is part of config.h
> > in SSSD (see AC_GNU_SOURCE)
> > 
> > While we are touching sss_sudo_create_query, can you use
> > safealign_memcpy? It does virtually the same as your code, but would be
> > more error prone when/if we extend the protocol again.
> > 
> > > The in-memory cache is not yet implemented, I want to discuss the
> > > possible ways of doing it.
> > > 
> > > The basic problem is that we need to get the domain during the request
> > > for default options. How will we do it? I think there are two options:
> > > 
> > > 1. always try to perform the initgroups - find the domain and the
> > > check the in-memory cache (which may be slow if the user is in the
> > > last domain, but that will be probably handled as part of
> > > https://fedorahosted.org/sssd/ticket/1126
> > > 
> > 
> > I'm not sure if this is what are you proposing in 1), but when I look
> > at sudo_sssd_display_defaults() in the sudo tree, I see that it knows
> > the user we are performing sudo on behalf on already. So can we extend
> > it to send username/uid and perform initgroups during the "get default
> > options operation?". I don't think there is much slowness here, as you
> > said, ticket #1226 would bring some improvement and with the current
> > model we need to search all the domains anyway. Also, only the first
> > request would perform searches, the other would be caught by the
> > in-memory cache, right?
> > 
> > > 2. store uid:username = domain in the in-memory cache (same cache as
> > > results or a new one?)
> > > 
> 
> May I ask what 'in-memory' cache are we talking about ?
> I would like to understand why looking up into LDB is not considered a
> cache look-up.
> 
> Simo.
> 

This cache works much like the in-memory cache in the PAM responder that
caches initgroups results and the actual sudo rules. It's a short-lived
cache, 180 seconds by default.

In general, we want to always go to the remote directory and look up the
sudoers rules online to make sure we are not allowing something we
shouldn't be. At the same time, it's quite common to run a couple of
sudo commands in quick succession, so we implemented this cache.

Here's the original design mini-thread:
https://fedorahosted.org/pipermail/sssd-devel/2011-December/007712.html



More information about the sssd-devel mailing list