[Freeipa-devel] [PATCH] add requires_root option to Command

[Simo Sorce]

On Wed, 2009-04-22 at 09:40 -0400, Rob Crittenden wrote:
> Simo Sorce wrote:
> > On Tue, 2009-04-21 at 17:16 -0400, Rob Crittenden wrote:
> >> Simo Sorce wrote:
> >>> On Tue, 2009-04-21 at 10:24 -0400, Rob Crittenden wrote:
> >>>> Some commands will require that the local user have root permissions. 
> >>>> I'm not 100% sure this is the right place to put it but it at least 
> >>>> starts the conversation.
> >>> Speaking just in general terms I don't like doings things like:
> >>> if uid == 0 fail;
> >>>
> >>> I think that we should gracefully catch whatever exception is thrown up
> >>> (access denied or whatever) and then return an error.
> >>>
> >>> Some times this is not possible, and I haven't looked at what's around
> >>> that patch, so this may be the right way in this case.
> >>>
> >>> Simo.
> >>>
> >> That is exactly what this does. It raises an exception that Root is 
> >> required and the client catches this and displays it:
> >>
> >> $ ipa join foo.example.com
> >> ipa: ERROR: This command requires root access
> >>
> >> Otherwise we're going to get file permission errors and nasty things 
> >> like that which won't provide a useful error message to the client. If 
> >> we catch this up front then we can prevent doing unnecessary things.
> >>
> >> Note that this is only for client-side stuff. In this case, when joining 
> >> a machine to the IPA domain I want root access so the keytab we retrieve 
> >> will be protected (and since I'll ultimiately update /etc/krb5.keytab 
> >> root will be mandatory).
> > 
> > Yet, but I would rather check if we can write to /etc/krb5.keytab with
> > the current user (even just using access(2)), not just check if geteuid
> > == 0
> 
>  From access(2):
> 
> Warning:  Using access() to check if a user is authorized to, for 
> example, open a file before actually doing so using open(2) creates a 
> security  hole,  because  the  user  might  exploit the short time 
> interval between checking and opening the file to manipulate it.  For 
> this  reason, the use of this system call should be avoided.

access should  be avoided to take security decisions, but we are not
taking a security decision here, we are just trying to make a more
graceful exit if the user does not have privileges.

> But I see what you are saying. I can probably do this but it is going to 
> take considerably more work and in all likelihood end up with the user 
> needing to be root anyway. This affects way more than just /etc/krb5.keytab.

Ok, then let's get on with the getuid check for now, but add a FIXME
comment that states what we should really do.

Simo.