[PATCHES] SUDO integration - first iteration

[Pavel Březina]

https://fedorahosted.org/sssd/ticket/623

Helpful links:
  - SUDO plugin API
    http://www.gratisoft.us/sudo/man/1.8.1/sudo_plugin.man.html

What does it do currently?
  1. sudo plugin sends input data to responder
  2. responder sends back to plugin that user is allowed to run the
     command

What does it not do?
  - Doesn't require any PAM authentication
  - Doesn't read anything from LDAP
  - Doesn't set any environment variables (and reset current environment)
    This actually can cause some troubles to applications, so far I've
    encountered an error message in VIM (no $HOME specified)

How to test it?
  1. Install SUDO version 1.8 or greater
     I am running 1.8.2 built from source:
       ftp://ftp.sudo.ws/pub/sudo/sudo-1.8.2.tar.gz

  2. Enable SUDO plugin in /etc/sudo.conf
     Plugin sss_sudo_policy /usr/lib/sudo/libsss_sudoplugin.so

     Unfortunately, SUDO doesn't allow to have more than one policy
     plugin activated, so comment out standard sudoers plugin.

  3. Enable it in sssd.conf ;-)
     services += sudo

  4. Run sssd

  5. Run sudo

I left there some debug messages, so don't worry when you see:
   CMD Return code: 0
   errnop: 0
   Command exited with status 0

in the output. But tell me if there is anything else than 0 :-)

[PATCH 1/5]
Adds a client function that allows client to send data to responder.
And set sudo responder protocol version.

[PATCH 2/5]
SUDO plugin.

[PATCH 3/5]
SUDO responder.

[PATCH 4/5]
Configure and Makefile updates so we can build plugin and responder.

[PATCH 5/5]
Just a little update that adds sudo responder to known services of monitor.

[Jakub Hrozek]

On 10/10/2011 03:20 PM, Pavel Březina wrote:
>  https://fedorahosted.org/sssd/ticket/623
>  
>  Helpful links:
>   - SUDO plugin API
>     http://www.gratisoft.us/sudo/man/1.8.1/sudo_plugin.man.html
>  
>  What does it do currently?
>   1. sudo plugin sends input data to responder
>   2. responder sends back to plugin that user is allowed to run the
>      command
>  
>  What does it not do?
>   - Doesn't require any PAM authentication
>   - Doesn't read anything from LDAP
>   - Doesn't set any environment variables (and reset current environment)
>     This actually can cause some troubles to applications, so far I've
>     encountered an error message in VIM (no $HOME specified)
>  
>  How to test it?
>   1. Install SUDO version 1.8 or greater
>      I am running 1.8.2 built from source:
>        ftp://ftp.sudo.ws/pub/sudo/sudo-1.8.2.tar.gz
>  
>   2. Enable SUDO plugin in /etc/sudo.conf
>      Plugin sss_sudo_policy /usr/lib/sudo/libsss_sudoplugin.so
>  
>      Unfortunately, SUDO doesn't allow to have more than one policy
>      plugin activated, so comment out standard sudoers plugin.
>  
>   3. Enable it in sssd.conf ;-)
>      services += sudo
>  
>   4. Run sssd
>  
>   5. Run sudo
>  
>  I left there some debug messages, so don't worry when you see:
>    CMD Return code: 0
>    errnop: 0
>    Command exited with status 0
>  
>  in the output. But tell me if there is anything else than 0 :-)
>   
In overall - good work! Pavel also has follow-up patches, so he wasn't
idling in the meantime between him sending out this patchset and us
getting it reviewed. Sorry for the delay, Pavel.

As sudo does not seem to have a bug tracker upstream and the Red Hat
sudo maintainer is working with us, I'm going to open one generic "SSSD
Sudo Feature tracker" bug in bugzilla.redhat.com for Fedora rawhide sudo
and then one bug per feature we'd like to see in sudo blocking this
tracker bug.

>  [PATCH 1/5]
>  Adds a client function that allows client to send data to responder.
>  And set sudo responder protocol version.
>   
Ack but please consider squashing together with the responder part. Does
this patch make sense standalone?

>  [PATCH 2/5]
>  SUDO plugin.
>   
Please move private declarations (such as struct plugin_state) out of
the header.

The patch includes a fallback to sudoers plugin in case this a user is
not found. Currently this is handled inside the sudoers plugin based on
the order of "sudo" parameter in nsswitch.conf. The plugin order should
be made more generic in sudo upstream, but short-term we may want to
support some kind of fallback ourselves.

There's a TODO in the code we discussed off-list:
"TODO: Do we need to check major and minor version or major is enough?"
We need to ask sudo upstream on the specifics of versioning.

We couldn't agree on #defining macros such as APPEND_ELEMENT or
APPEND_ZERO inside a function and #undefining them later. I know that it
would be nice to use a nested function for the task if C allowed them,
but I don't like neither defining macros inside a function nor macros
silently changing local variables. Other opinions welcome.

Casting pointers such as "result = *((int*)response);" should be changed
to use our safe memory alignment macros to avoid reintroducing #390.

get_qualified_command_path() should be enhanced in order to be
compatible with sudo's find_path(). Long-term sudo should export a
function to perform the path-lookup so plugins don't have to define
their own function for the task.

Nitpicks - sudo_printf_t in policy_open() should be renamed, we use
underscore for "output" variables.
- the harcoded fields[4] should be changed to use a constant. Same
applies for looping over the fields.

>  [PATCH 3/5]
>  SUDO responder.
>   
Nested macros again. Same comment as for patch #2.

If you want to keep functions that will be implemented in a future patch
in this one (as opposed to simply adding them when they're available),
please define a simple skeleton that would just return EOK.

>  [PATCH 4/5]
>  Configure and Makefile updates so we can build plugin and responder.
>   
Ack, but this patch might be squashed together into the plugin and
responder patches respectively, but I don't really have a strong opinion.

>  [PATCH 5/5]
>  Just a little update that adds sudo responder to known services of monitor.
>  
>   
Ack, but this could be squashed into the responder itself.

[Pavel Březina]

Dne 31.10.2011 18:35, Jakub Hrozek napsal(a):
>  On 10/10/2011 03:20 PM, Pavel Březina wrote:
> > https://fedorahosted.org/sssd/ticket/623
> >
> > Helpful links:
> >   - SUDO plugin API
> >     http://www.gratisoft.us/sudo/man/1.8.1/sudo_plugin.man.html
> >
> > What does it do currently?
> >   1. sudo plugin sends input data to responder
> >   2. responder sends back to plugin that user is allowed to run the
> >      command
> >
> > What does it not do?
> >   - Doesn't require any PAM authentication
> >   - Doesn't read anything from LDAP
> >   - Doesn't set any environment variables (and reset current environment)
> >     This actually can cause some troubles to applications, so far I've
> >     encountered an error message in VIM (no $HOME specified)
> >
> > How to test it?
> >   1. Install SUDO version 1.8 or greater
> >      I am running 1.8.2 built from source:
> >        ftp://ftp.sudo.ws/pub/sudo/sudo-1.8.2.tar.gz
> >
> >   2. Enable SUDO plugin in /etc/sudo.conf
> >      Plugin sss_sudo_policy /usr/lib/sudo/libsss_sudoplugin.so
> >
> >      Unfortunately, SUDO doesn't allow to have more than one policy
> >      plugin activated, so comment out standard sudoers plugin.
> >
> >   3. Enable it in sssd.conf ;-)
> >      services += sudo
> >
> >   4. Run sssd
> >
> >   5. Run sudo
> >
> > I left there some debug messages, so don't worry when you see:
> >    CMD Return code: 0
> >    errnop: 0
> >    Command exited with status 0
> >
> > in the output. But tell me if there is anything else than 0 :-)
> >
>
>  In overall - good work! Pavel also has follow-up patches, so he wasn't
>  idling in the meantime between him sending out this patchset and us
>  getting it reviewed. Sorry for the delay, Pavel. 
Thank you for your review.

>  As sudo does not seem to have a bug tracker upstream and the Red Hat
>  sudo maintainer is working with us, I'm going to open one generic "SSSD
>  Sudo Feature tracker" bug in bugzilla.redhat.com for Fedora rawhide sudo
>  and then one bug per feature we'd like to see in sudo blocking this
>  tracker bug.
>
> > [PATCH 1/5]
> > Adds a client function that allows client to send data to responder.
> > And set sudo responder protocol version.
> >
>
>  Ack but please consider squashing together with the responder part. Does
>  this patch make sense standalone? 
Most of the patches doesn't make sense to be standalone :)
It is actually used by plugin not the responder and I've put it in a 
separate patch so the review could be easier.

> > [PATCH 2/5]
> > SUDO plugin.
> >
>
>  Please move private declarations (such as struct plugin_state) out of
>  the header. 
Done.

>  The patch includes a fallback to sudoers plugin in case this a user is
>  not found. Currently this is handled inside the sudoers plugin based on
>  the order of "sudo" parameter in nsswitch.conf. The plugin order should
>  be made more generic in sudo upstream, but short-term we may want to
>  support some kind of fallback ourselves.
>
>  There's a TODO in the code we discussed off-list:
>  "TODO: Do we need to check major and minor version or major is enough?"
>  We need to ask sudo upstream on the specifics of versioning.
>
>  We couldn't agree on #defining macros such as APPEND_ELEMENT or
>  APPEND_ZERO inside a function and #undefining them later. I know that it
>  would be nice to use a nested function for the task if C allowed them,
>  but I don't like neither defining macros inside a function nor macros
>  silently changing local variables. Other opinions welcome.
>
>  Casting pointers such as "result = *((int*)response);" should be changed
>  to use our safe memory alignment macros to avoid reintroducing #390. 
Done.

>  get_qualified_command_path() should be enhanced in order to be
>  compatible with sudo's find_path(). Long-term sudo should export a
>  function to perform the path-lookup so plugins don't have to define
>  their own function for the task. 
Done.

>  Nitpicks - sudo_printf_t in policy_open() should be renamed, we use
>  underscore for "output" variables. 
Done.

>  - the harcoded fields[4] should be changed to use a constant. Same
>  applies for looping over the fields. 
Changed to NULL terminated array.

> > [PATCH 3/5]
> > SUDO responder.
> >
>
>  Nested macros again. Same comment as for patch #2.
>
>  If you want to keep functions that will be implemented in a future patch
>  in this one (as opposed to simply adding them when they're available),
>  please define a simple skeleton that would just return EOK. 
I haven't decided yet if it is necessary (haven't think of it though).

> > [PATCH 4/5]
> > Configure and Makefile updates so we can build plugin and responder.
> >
>
>  Ack, but this patch might be squashed together into the plugin and
>  responder patches respectively, but I don't really have a strong opinion.
>
> > [PATCH 5/5]
> > Just a little update that adds sudo responder to known services of monitor.
> >
> >
>
>  Ack, but this could be squashed into the responder itself.
>  
Done.

I've attach some more patches currently indexed 5-8. It is a skeleton of 
data provider part. Their names should be self-explanatory.
The last one (SUDO responder - refresh SUDOers) is just a DP call for 
testing purpose, it will be changed of course :)

[PATCHES] SUDO integration

[Pavel Březina]

Hi all,
sending some new patches for review.

It includes working data provider, refactored responder, a few bug fixes 
and Jakub's sysdb API.

[PATCHES] SUDO integration

[Jakub Hrozek]

On Tue, Nov 15, 2011 at 02:38:39PM +0100, Pavel Březina wrote:
>  Hi all,
>  sending some new patches for review.
>  
>  It includes working data provider, refactored responder, a few bug
>  fixes and Jakub's sysdb API.
>   
Please note that the sysdb API will change (I'm working on that now).
The most obvious change is that it really does not belong into sysdb, I
think it should be in a separate module instead to keep only
name-service-switch data in sysdb.

[Jakub Hrozek]

On Mon, Oct 31, 2011 at 06:35:50PM +0100, Jakub Hrozek wrote:
>  On 10/10/2011 03:20 PM, Pavel Březina wrote:
>  > https://fedorahosted.org/sssd/ticket/623
>  > 
>  > Helpful links:
>  >  - SUDO plugin API
>  >    http://www.gratisoft.us/sudo/man/1.8.1/sudo_plugin.man.html
>  > 
>  > What does it do currently?
>  >  1. sudo plugin sends input data to responder
>  >  2. responder sends back to plugin that user is allowed to run the
>  >     command
>  > 
>  > What does it not do?
>  >  - Doesn't require any PAM authentication
>  >  - Doesn't read anything from LDAP
>  >  - Doesn't set any environment variables (and reset current environment)
>  >    This actually can cause some troubles to applications, so far I've
>  >    encountered an error message in VIM (no $HOME specified)
>  > 
>  > How to test it?
>  >  1. Install SUDO version 1.8 or greater
>  >     I am running 1.8.2 built from source:
>  >       ftp://ftp.sudo.ws/pub/sudo/sudo-1.8.2.tar.gz
>  > 
>  >  2. Enable SUDO plugin in /etc/sudo.conf
>  >     Plugin sss_sudo_policy /usr/lib/sudo/libsss_sudoplugin.so
>  > 
>  >     Unfortunately, SUDO doesn't allow to have more than one policy
>  >     plugin activated, so comment out standard sudoers plugin.
>  > 
>  >  3. Enable it in sssd.conf ;-)
>  >     services += sudo
>  > 
>  >  4. Run sssd
>  > 
>  >  5. Run sudo
>  > 
>  > I left there some debug messages, so don't worry when you see:
>  >   CMD Return code: 0
>  >   errnop: 0
>  >   Command exited with status 0
>  > 
>  > in the output. But tell me if there is anything else than 0 :-)
>  > 
>  
>  In overall - good work! Pavel also has follow-up patches, so he wasn't
>  idling in the meantime between him sending out this patchset and us
>  getting it reviewed. Sorry for the delay, Pavel.
>  
>  As sudo does not seem to have a bug tracker upstream and the Red Hat
>  sudo maintainer is working with us, I'm going to open one generic "SSSD
>  Sudo Feature tracker" bug in bugzilla.redhat.com for Fedora rawhide sudo
>  and then one bug per feature we'd like to see in sudo blocking this
>  tracker bug. 
Turns out sudo upstream has a bug tracker.  We're still going to use RHBZ,
though. The work is going to be done by the Red Hat sudo maintainer and
by using Red Hat Bugzilla, we have the freedom of creating dependencies
etc. I think Daniel can create RFEs upstream if he needs to.

The tracking bug is https://bugzilla.redhat.com/show_bug.cgi?id=750879
The individual subtasks block this tracker.