GNOME.org
 

Re: GNOME System Monitor will use libgnomesu

On Sat, 2005-01-08 at 15:06 +0100, Hongli Lai wrote:
> Mark McLoughlin wrote:
> > 	Back in November we had a very long discussion about the proposal to
> > add libgnomesu in 2.10. I don't think we ever came to a consensus that
> > it should be added ...
> 
> Nobody had any objections. libgnomesu is API/ABI stable, the user 
> interface is HIG-compliant, and it works. I've fixed every single 
> libgnomesu bug in Bugzilla. Sure you can discuss until the eternity, but 
> that won't get work done. The discussion is already way too long and 
> there are no objections.

	Okay, let me repeat everything I said in the previous thread. And let
me be clear that these are "objections" to the adoption of
libgnomesu ... although, how you could claim to not think these were
objections first time around is beyond me ...

	I'll also include my original mails for reference - something seems to
have been up with archiving those days.

 High level concerns:

   - libgnomesu is a "launch this program as root" API - I think that's 
     the wrong form for this functionality to take and an install time 
     "this app needs to be run as root" interface is more appropriate
     (e.g. like usermode)

     We discussed use cases for a libgnomesu API, and I felt each one 
     of those use cases were misguided. I also suggested a better (in 
     terms of usability) way for the "root requiring" features in
     gnome-system-monitor to be presented, such that a libgnomesu like
     API wouldn't be the appropriate way to implement it.

   - I worry that the addition libgnomesu-like API to the desktop will 
     lead to the UI being littered (in unpredictable places) with root 
     password dialogs e.g.

       - "Open as root" in Nautilus context menus
       - "Run as root" in the run dialog
       - Random features in apps asking for root like the "renice" and
         "kill" features in gnome-system-monitor

     What a horrible thought.

 Specific objections about the API/implementation:

   - APIs should correspond to the gspawn() APIs where possible

   - multi-screen apps need a way to specify which screen to
     launch the program on

   - should have error reporting via GError

   - "async" APIs which block

   - users of async APIs need to call waitpid()

   - No startup-notification

   - BinReloc stuff is silly to just have in one GNOME module - its an 
     issue which cannot be addressed one module at a time

   - Re-entrancy issues

   - Issue about maintainability of an entire copy of "su"

   - licensing issues - some of the code is (c) Red Hat, Inc., but the
     Copyright notice appears to have been removed

   - No checking for EINTR from waitpid()

   - ...

Cheers,
Mark.
--- Begin Message --- 
Hi Beno�

On Fri, 2004-11-26 at 15:16 +0000, Beno�Dejean wrote:
> Le vendredi 26 novembre 2004 �5:52 +0100, Murray Cumming a �it :
> > 
> > Given that it's still being intensively discussed, that would probably be
> > premature.  If the discussion comes to a stop then you might want to
> > summarise the discussion and then say why, on the basis of that, it's
> > still a good idea to go ahead.
> 
> Current gnome-system-monitor askpass is :
> - dirty
> - unsecured
> - not portable (Linux only)
> 
> I'd like to fix this as soon as possible.

	If you feel the feature is insecure and poorly implemented, one
immediate option you have is just to disable it ...

> So libgnomesu may be reworked, extented ..., but right now, it's the
> best solution i have.

	In the discussion with Hongli, I said:

  ... worries me because it leaks some admin-tool functionality into a
  user tool which I think is likely to be frustrating and confusing for
  users who *don't* have root. It also isn't optimal for users who *do*
  have root - having to type the root password for every process you
  want to kill isn't fun.

	I'm not sure how you feel about that because you didn't respond, but I
think it would be worthwhile to step back and think about it for a
moment.

	The two features that require root in procman is "Change Priority" and
"Kill Process", right? If someone who does have root uses
gnome-system-monitor for either of these tasks, they're not going to
enjoy typing the root password every time. If someone who doesn't have
root tries to use either of these features and are then told they're not
allowed too, I think they will be irritated by the unpredictability of
tool - "if I'm not allowed do this, why did you put the button there?".

	Personally, I think it would be a lot more sensible for procman to have
an "admin mode" and a "user mode". If run in "admin mode", e.g. as root,
then "Change Priority", "Kill Process" would be in the menu and in admin
mode it wouldn't.

> > But that's just my opinion. You are the maintainer.
> 
> if this means that i'm free to use it or not, i'm going to commit
> Hongli's patch tonight. Thanks

	The way things work currently is that maintainers are free to add
dependancies and new modules like this. But I think that's under the
assumption that the maintainer does so in co-ordination with the rest of
the project - i.e. that there's a general consensus that is a good idea,
doesn't conflict with anything else etc. I don't think its valid for a
maintainer to add a module/dependancy while ignoring ongoing
discussion/disagreement about whether its a good idea.

Cheers,
Mark.

--- End Message ---
--- Begin Message --- 
On Thu, 2004-11-25 at 20:20 +0100, Hongli Lai wrote:
> Mark McLoughlin wrote:
> > 	(1) worries me because it leaks some admin-tool functionality into a
> > user tool which I think is likely to be frustrating and confusing for
> > users who *don't* have root. It also isn't optimal for users who *do*
> > have root - having to type the root password for every process you want
> > to kill isn't fun.
> 
> I don't think there is any better way.

	You could have separate admin tool and user tool rather than trying to
make a hybrid work.

> And you don't have to enter the root password every time, if you're on 
> RedHat. libgnomesu uses the pam_timestamp extension when available.

	That would be true if renice used consolehelper. It doesn't, but it
could. Or procman could come with a helper that used consolehelper. But
I think a separate admin tool makes more sense.

> > 	There's nothing (apart from the lack of shadow password support)
> > distribution specific about usermode/consolehelper really.
> 
> Except that it's only included in some distributions?

	There are some very simple and surmountable reasons why usermode isn't
shipped widely yet, IMHO. We don't know yet how libgnomesu will fare
with distributors either, do we?

> >>>  - No startup-notification integration
> >>
> >>Last time I checked, libstartupnotify is *still* marked as "unstable API 
> >>which can change any time".
> > 
> > 
> > 	Yes, but if libgnomesu was part of the Desktop Release, it would be
> > perfectly valid for it to use the API.
> 
> A bit offtopic: but is the latest libstartupnotification binary 
> compatible with 0.4? I'm on FC1 and I'm stuck with GNOME 2.4. Garnome 
> GNOME just breaks too many things (menus show up duplicate items, etc.).

	It should be, I think - there have been API additions but no
incompatible ABI changes.

> > 	But, yeah, you do need multiple backends - you need to support shadow
> > passwords. Could do that in the same backend binary, though, right?
> 
> No. The PAM backend is linked to libpam.so, which isn't available on 
> systems without PAM.

	That's something you can check at buildtime and use #ifdefs for. The
binary doesn't have to detect at runtime whether libpam is installed.

> >>>  - The thought of a Nautilus "Open as Superuser" component gives me
> >>>    the heebie-jeebies. I'm not sure exactly why. Its irrelevant now
> >>>    with Alex's recent changes to Nautilus, anyway :-)
> >>
> >>Did I miss anything? What changes?
> > 
> > 	No bonobo components for Nautilus anymore.
> 
> How's that related to privilege raising?

	Its related to the fact your "Open as superuser" component doesn't work
anymore.

Cheers,
Mark.

--- End Message ---
--- Begin Message --- 
Hi Carlos,

On Wed, 2004-11-24 at 18:08 +0100, Carlos Garnacho wrote:

> > 	One thing that occurred to me when looking at
> > libgnomesu was that
> > usermode is no more Red Hat specific than libgnomesu
> > is e.g. JDS uses
> > usermode without any problems. If we find that GNOME
> > has a need for this
> > kind of functionality, then perhaps it makes as much
> > sense for usermode
> > to be included in GNOME as libgnomesu?
> 
> For me it could be worth asking why isn't it even
> included optionally in some other distros, I'm taking
> the Debian example, a distribution that has thousands
> of packages and that hasn't still packaged this makes
> me think a bit...

	I think there's a number of reasons:

  1) Some of the functionality (e.g. poweroff/halt/reboot and the 
     notification area icon) is predicated on pam_console and 
     pam_timestamp being available. This isn't in upstream PAM, but is 
     shipped as part of our PAM package - so the source for those 
     modules is only available as a patch against upstream PAM.

  2) Its in Red Hat's (public) CVS, releases are only on Red Hat's 
     FTP server - there's just never been any attempt to push it
     as not being Red Hat specific.

	I don't know the "why" for either of these, but I suspect it was just
down to a lack of time. Nalin would know for sure if there's any other
reason.

	There's no reason why it can't be fixed though - I was particularily
determined to make it work on JDS so I packaged the two PAM modules on
their own and usermode itself was just a trivial packaging task.

> Dont take me wrong, I just want to be sure that we
> won't make modules depend on things that distros will
> refuse to package

	Oh, indeed. I agree.

	The "distro" that would probably refuse to ship usermode is Solaris ...
but you'd want some patience (and balls) to get anything like usermode
or libgnomesu past the Solaris ARC :-)

Cheers,
Mark.

--- End Message ---
--- Begin Message --- 
Hi Honglai,

On Wed, 2004-11-24 at 13:50 +0100, Hongli Lai wrote:
> Mark McLoughlin wrote:
> > 	So, what are the GNOME use cases? Hongli, Carlos, Beno�
> 
> 1. GNOME System Tools has a "Renice" menu item, which requires root if 
> you want to give processes a lower nice value.
> 2. The System Log Viewer (gnome-utils) requires root access to read 
> logs. I submitted a patch but nothing happened.
> 3. libgnomesu adds a context menu to Nautilus, which makes it possible 
> to open a folder as root, so you can edit other users' files or system 
> files.

	I think (2) is certainly sensible. It is the typical
graphical-sysadmin-tool just like gnome-system-tools, Fedora system
tools etc.

	(1) worries me because it leaks some admin-tool functionality into a
user tool which I think is likely to be frustrating and confusing for
users who *don't* have root. It also isn't optimal for users who *do*
have root - having to type the root password for every process you want
to kill isn't fun.

	Being able to access other users files in (3) is interesting, but just
starting nautilus as the other user sounds like it would be confusing.
How do you know which window is yours and which window is the other
users? Can you drag and drop between windows? If I was looking to allow
another user to access my files, I'd expect it to work by "Share Folder"
rather than typing in my password for them.

	But anyway, the reason I was asking about use cases was to figure out
whether we need a libgnomesu-like "launch this app as root" API or a
usermode-like "this app needs to be run as root" install time interface.
I'd prefer to err on the side of the latter if we're unsure.

> >   - API problems:
> > 
> >       + It would make sense for the APIs to correspond much more 
> >         closely with the gspawn APIs - effectively, it should be
> >         the same as the gspawn APIs with an added "user" argument.
> >         See gdkspawn, for example.
> 
> I tried it before. It's not possible.
> 
> I've monitored previous attempts of implementing privilege raising in 
> GNOME. None of them succeeded. Basically, it boils down to:
> - PAM is not available on every single OS GNOME runs on.
> - Shadow passwords are, but it's unacceptable because PAM is not used on 
> systems that do support it.

	Okay, I can see that shadow password support is neccessary now.

> - Some more objections about (not) using consolehelper or other 
> distro-specific authentication tools.

	There's nothing (apart from the lack of shadow password support)
distribution specific about usermode/consolehelper really.

> So obviously, writing a completely new authentication system is not the 
> solution. It has been tried many times, and people never agree on it.
> 
> libgnomesu on the other hand is a wrapper around whatever whatever is 
> available on the current system. It'll use consolehelper, PAM or shadow 
> passwords, depending on which one is available. Because of the wrapper 
> nature, I cannot provide a complex/powerful API like gspawn.
> 
> Some specific examples of problems:
> - child_setup cannot be implemented. The child setup function will have 
> to be called after authentication has been confirmed. This is simply not 
>   possible.
> - g_spawn_*_with_pipes() cannot be implemented. I can implement it using 
> the modified setuid binaries in libgnomesu, but then I won't be able to 
> implement ssh or sudo support in the future. Both of those programs use 
> stdin (or the tty) to read the password, and to print any status 
> information.
> 
> The only way to solve all this, would be to turn libgnomesu from a 
> wrapper library into a full authentication system. But people won't 
> agree on that, as pointed out earlier.

	Right, I'm sorry. You obviously couldn't implement full g_spawn()
functionality - the point I was trying to make is that the API should
*look* like a g_spawn() API for consistency. Nothing more.

> >       + There's no way for multi-screen apps to specify which screen
> >         the dialogs and the app itself should be displayed.
> 
> Is there a way to do that at all? If you click on an icon on your panel, 
> does GNOME ask you on which screen you want to display the app?

	If you have multiple screens the panel will start the app on the screen
which you click the launcher using the gdk_spawn() API. Essentially, the
libgnomesu() API should be able to take a GdkScreen argument.

> >       + The "async" APIs are a bit of a misnomer since they block
> >         until authentication has completed - why not make them fully
> >         asynchronous by putting an IO watch on the pipe?
> 
> Not possible. The authentication dialog is part of libgnomesu. Depending 
> on all kinds of stuff, the authentication dialog may not even have to 
> show up! For example, if you're already root, or if you're trying to 
> switch to the same user, or you're trying to switch to a user which 
> doesn't have a password. In, in case of RedHat's consolehelper, if 
> you've already entered your password within 5 minutes ago. There is NO 
> way to tell whether an authentication dialog will show up. It all 
> depends on the underlying authentication system.

	I'm not sure I understand. Why does the caller need to know whether an
authentication dialog was displayed?

	As an example, the PAM service could do the following:

  1) g_spawn_async_with_pipes() the PAM backend
  2) Put a watch on the stdout pipe and return to the caller
  3) When there's input on the stdout pipe, process it
  4) If the child asks for a password, display a dialog
  5) When you get a response (asynchronously) from the dialog,
     return write that on the backend helper's stdin pipe

> >   - No startup-notification integration
> 
> Last time I checked, libstartupnotify is *still* marked as "unstable API 
> which can change any time".

	Yes, but if libgnomesu was part of the Desktop Release, it would be
perfectly valid for it to use the API.

> But anyway, even this is not possible. For startup notification to work, 
> you have to set an environment variable for the child, then setup some 
> kind of X property. It has to be setup before exec("authentication 
> executable") is called. But what if the user entered the wrong password, 
> or forgot the password, or pressed Cancel? The startup notification 
> cursor will stay visible until the timeout has elasped. Very ugly.

	It requires some communication between the backend and the launching
process. See how usermode does it, for example. One thing that I notice
usermode doesn't do is handle launchee failures, but that shouldn't be
hard to do.

	Its important, though, that if the support was added we only do it for
apps which we know supports it. So, you'd have a "startup_notify" flag
in the API which the panel would set depending on the StartupNotify flag
in the .desktop file.

> >   - Ignoring the consolehelper backend for a minute, isn't the PAM
> >     backend sufficient?
> 
> There is no make sure that RedHat's authentication tray icon appears, 
> other than to directly use consolehelper. (If anybody knows how, please 
> tell me.)

	Sure, sure - I was ignoring usermode/consolehelper because I don't
think it really makes sense for usermode to exist if we use libgnomesu.

	But, yeah, you do need multiple backends - you need to support shadow
passwords. Could do that in the same backend binary, though, right?

> >   - The BinReloc stuff looks very dodgy to have in a library - what
> >     happens if an app which links to libgnomesu also uses this stuff?
> 
> All BinReloc symbols are namespaced with "__libgnomesu_". It's very 
> unlikely that any application will cause symbol clashes.

	Okay, hadn't noticed that.

> >     Unless we thought that a hack like this was the way to for GNOME
> >     to address the relocatable package problem, then I think this
> >     should be removed.
> 
> If you know a better solution, I'd like to hear it.
> 
> Really, we've extensively researched the relocation problem. There is NO 
> clean and cross-platform solution! Unless someone convinces the POSIX 
> standard group (or whatever they're called) to add an API similar to 
> Win32's GetModuleFileName(), the relocation problem is here to stay.

	Right, we haven't figured out a good way to fix this. But it would
strike me as pointless for one small part of GNOME to try and fix it
independantly of the rest of GNOME - especially when the hack is going
to make security concious people very nervous.

> >   - Since we run the mainloop from some of the services, won't we be
> >     screwed by re-entrancy - i.e. if you click on a button that causes
> >     the an app to be launcher with libgnomesu, and while the password
> >     dialog is up, you click on the button again, what happens?
> 
> The dialog is modal. The user won't be able to click anything else.

	I don't see where the password dialog is being made modal ..

	Anyway, I'm not sure that relying on the dialog being modal is good
enough to protect against re-entrancy. What happens if you're launching
from an idle handler or timeout?

> >   - The thought of a Nautilus "Open as Superuser" component gives me
> >     the heebie-jeebies. I'm not sure exactly why. Its irrelevant now
> >     with Alex's recent changes to Nautilus, anyway :-)
> 
> Did I miss anything? What changes?

	No bonobo components for Nautilus anymore.

> >   - We've a whole copy of "su" in here. Are we going to keep up to date 
> >     with upstream changes to the code? Copying and pasting code like 
> >     this worries me.
> 
> I haven't really taken a look at the 'real' su since I copied & pasted 
> it... It's a pain to get synchronized with upstream changes.

	Right, that's my point ... shouldn't we be worried about that ? :-)

> >   - su-backend/common.c:modify_environment() is a copy of usermode and
> >     su code, both of which are licensed under the GPL, and you've 
> >     removed the copyright notice and re-licensed to LGPL.
> 
> Hm this is a problem indeed. Maybe license the setuid executables as 
> GPL, and the library as LGPL?

	That would work, but don't forget to restore the copyright notices.

> >   - No error checking with the xauth stuff in the PAM and su backends.
> >     But why not just use the same $XAUTHORITY?
> 
> Permission problems. Try switching to another non-root user.

	Ah :-)

Thanks,
Mark.

--- End Message ---
--- Begin Message --- 
Hi,

On Tue, 2004-11-23 at 11:00 +0100, Murray Cumming wrote:

> libgnomesu:
>   Not until a Desktop modules uses it, or says that they want to use it.

	I've taken a look at this and come away feeling fairly queasy at the
thought of including this in GNOME and making widespread use of it. Some
detailed, but not exhaustive, comments below - I think this requires a
closer look even if all the comments are addressed.

	One thing that occurred to me when looking at libgnomesu was that
usermode is no more Red Hat specific than libgnomesu is e.g. JDS uses
usermode without any problems. If we find that GNOME has a need for this
kind of functionality, then perhaps it makes as much sense for usermode
to be included in GNOME as libgnomesu?

	Anyway, that's putting the horse in front of the cart a bit. What we
really need to think about the use cases for this run-as-root
functionality in GNOME and consider whether a libgnomesu-like
run-this-as-root API makes more sense than a usermode-like
allow-this-app-to-be-run-as-root interface.

	So, what are the GNOME use cases? Hongli, Carlos, Beno�

Cheers,
Mark.

Comments:

  - API problems:

      + It would make sense for the APIs to correspond much more 
        closely with the gspawn APIs - effectively, it should be
        the same as the gspawn APIs with an added "user" argument.
        See gdkspawn, for example.

      + There's no way for multi-screen apps to specify which screen
        the dialogs and the app itself should be displayed.

      + Should have error reporting via GError

      + The "async" APIs are a bit of a misnomer since they block
        until authentication has completed - why not make them fully
        asynchronous by putting an IO watch on the pipe?

      + Users of the async APIs need to call waitpid() - that's messy
        and undocumented. Why not use the intermediate-child technique
        gspawn uses to avoid this? Or even better, use gspawn?

  - No startup-notification integration

  - Ignoring the consolehelper backend for a minute, isn't the PAM
    backend sufficient? Why do we need the su backend? For platforms
    that don't use PAM? If so, which platforms are they?

  - sudo service should be removed if its broken

  - The BinReloc stuff looks very dodgy to have in a library - what
    happens if an app which links to libgnomesu also uses this stuff?
    Unless we thought that a hack like this was the way to for GNOME
    to address the relocatable package problem, then I think this
    should be removed.

  - Since we run the mainloop from some of the services, won't we be
    screwed by re-entrancy - i.e. if you click on a button that causes
    the an app to be launcher with libgnomesu, and while the password
    dialog is up, you click on the button again, what happens?

  - The thought of a Nautilus "Open as Superuser" component gives me
    the heebie-jeebies. I'm not sure exactly why. Its irrelevant now
    with Alex's recent changes to Nautilus, anyway :-)

  - We've a whole copy of "su" in here. Are we going to keep up to date 
    with upstream changes to the code? Copying and pasting code like 
    this worries me.

  - su-backend/common.c:modify_environment() is a copy of usermode and
    su code, both of which are licensed under the GPL, and you've 
    removed the copyright notice and re-licensed to LGPL.

  - Use of waitpid() without checking for EINTR will cause zombies

  - Every time you do find_service() each of the services allocate
    a struct for their function pointers. Seems gratuitous and easily
    avoided.

  - 2 leaks in consolehelper.c:detect()

  - Use GChildSource instead of while (waitpid ()) { sleep (); }

  - Before executing the PAM or su backends you should more carefully
    check that this is a "safe" directory - e.g. not writable by all -
    and more checking of the permissions on the backend so that you
    can be sure no-one has installed a trojan backend. Especially
    important with the BinReloc stuff.

  - No error checking with the xauth stuff in the PAM and su backends.
    But why not just use the same $XAUTHORITY?

--- End Message ---
--- Begin Message --- 
Hi Hongli,

On Fri, 2004-11-26 at 19:56 +0100, Hongli Lai wrote:
> Mark McLoughlin wrote:
> > 	It could be as simple as just making the "Kill process" menu item
> > insensitive if you're not running as root and you select a process that
> > isn't owned by you ...
> 
> I think asking password every time is better than either of these:
> - Not be able to do it at all.
> - Having to open a console, then type 'su -c gnome-system-monitor'

	Yes, of course, if it actually figures on the list of user tasks,
yadda, yadda, yadda (and I think it does)

	The point is, that rather littering a UI unpredictably with password
dialogs it may be better to give the tool an "admin mode" and a "user
mode". In admin mode, you can kill anyone's processes and in user mode
you can only kill your own.

	What I'm getting at is that, in this case, it appears to me a "this
program should prompt for the root password at startup" interface works
out better than a "run this program as root" API[1].

Cheers,
Mark.

[1] - I've probably lost a lot of people here, so:

 + libgnomesu has an API like this

    gboolean gnomesu_exec (gchar *commandline);

 + usermode has an interface where an app installs itself such that 
   if a non-root user launches the program, they get asked for the
   root password

	I'm trying to figure out which we actually need.

--- End Message ---
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]