Re: [monit-dev] "check program" change uid preliminary patch

[Leif Gustafson]

That's a very good point.  The problem now is that the argument to 
"check program" is only allowed to be a single executable file.  That's 
already an annoying limitation, and if it were fixed we'd be able to 
just use su directly as you suggested.


On 03/16/2012 4:14 AM, Igor Homyakov wrote:
> Hi guys,
>
> Is it really necessary to re-implement su/sudo functionality ?  I
> don't think so.  From my point of view we've got over-engineering
> here.  I believe that su developers are smart enough to deal with
> security/race issues.
>
> So "check program su username -c command" is more secure, more
> flexible and more simple way  to do that.
>
> Kind regards
> Igor Homyakov
>
> On Fri, Mar 16, 2012 at 11:11, Jan-Henrik Haukeland<address@hidden>  wrote:
> > Hi
> >
> > Looks like a fine start to me. Thanks for the work. It should be possible to 
> > avoid the global stuff and use the local struct myprogram programset object. 
> > I'll take a look and add the refactored patch.
> >
> >
> > On Mar 16, 2012, at 5:11 AM, Leif Gustafson wrote:
> >
> > > Hello,
> > >   Following up on the conversation in monit-general about adding an option
> > > to "check program" to drop root privileges, I took a look at the source code
> > > and tried to see how difficult it would be to add it.  I'm not extremely
> > > familiar with the source tree nor do I usually use C for my day job so be
> > > gentle, but I was able to mock up a preliminary working patch (against
> > > 5.3.2).  I think this is a pretty important security feature for "check
> > > program" because if the user isn't careful a malicious trojan could be
> > > executed as root.
> > >
> > >   I had to make a couple of workarounds to get this to work.  The "check"
> > > statements appear to be designed to only take one parameter (in this case,
> > > the path to the program), so I had to modify the grammar of "check program"
> > > a bit.  But also, the order in which the statement is being parsed means the
> > > Service_t hasn't been created yet when uid/gid are parsed.  So, I have to
> > > save those in static global variables and then set them in the instance of
> > > Service_t once it has been created.
> > >
> > >   So, testing this with something like "check program test-script with path
> > > '/home/lgustafson/test-uid.sh' as uid 1000 as gid 1000 if status != 0 then
> > > alert" worked for me.  The script was indeed run as my uid/gid.  The caveat
> > > is the supplemental group list still contains root, so the script could
> > > still modify writeable files owned by gid 0.  I'm not certain that there is
> > > a portable solution for that, but it could be resolved on some of the
> > > platforms you support.
> > >
> > >   Like I said, this is just a preliminary patch/brainstorming exercise.
> > > <check-program-set-uid.patch>_______________________________________________
> > > monit-dev mailing list
> > > address@hidden
> > > https://lists.nongnu.org/mailman/listinfo/monit-dev
> >
> > _______________________________________________
> > monit-dev mailing list
> > address@hidden
> > https://lists.nongnu.org/mailman/listinfo/monit-dev
> _______________________________________________
> monit-dev mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/monit-dev