Some privilege reduction patches

[Martin Pitt]

Hi David!

David Zeuthen [2006-02-18 13:27 -0500]:
> Sorry for the delay,

Likewise, I was not online over the weekend. :)

> > E. g. the ACPI helper doesn't need root privileges since
> > it can happily read from acpid. This even helps to prevent some race
> > conditions between acpid and hal which apparently crash acpid in
> > Debian in some cases.  
> 
> As discussed some distributions don't ship acpid and I don't want hal to
> depend on it. 

Right, me neither; *if* acpid is running (i. e. the socket is
present), then hal should stick to it and can drop privs, otherwise it
should connect to the kernel socket and drop privs afterwards.

> I also don't want hald to be restarted on package upgrades, but, eh,
> that's an old discussion...

This was discussed some months ago on the utopia list, and I finally
got persuaed (or, rather 'voted down' :) ), so we don't do that any
more in Ubuntu. However, acpid is still restarted on package upgrades,
and during this window, hal grabs the kernel socket and locks out
acpid.

> Btw, with my GNOME hat on, I remark that some day g-p-m will be able to
> completely replace acpid

Hm, I'm not sure, that rather sounds like a job for hal, but I know
too little about the guts of PM to comment about that.

> Until then I guess that one may configure hald to always connect to
> acpid.

Would work for me, although I still think that runtime detection is
nicer.

> > Likewise, I added privilege dropping to addon-storage.c; for polling
> > CD-ROMs, USB devices, and the like, the respective device groups
> > should be sufficient, i. e. the same groups that hald itself ran in in
> > earlier versions. 
> 
> Right. It does require that all device files can be accessed by the one
> of the groups hal is in (assume "disk" for now).

Right, although it does not make sense at all to add hal to group
disk, since disk is equivalent to root.

>  1) Linux distros needs configure udev to make sure group 'disk' owns
>     the device file and it's at least readable. Questions..

In Debian and Ubuntu we assign different groups to different types of
devices, like 'disk' for fixed hard disks, 'plugdev' for removable
volumes, and 'cdrom' for - guess what.

>  2) Need to make sure that the haldaemon user (or 'hal' on Debian) is
>     member of the appropriate groups.. Not the case right now on Fedora,
>     definitely fixable in the SPEC file, but something we need to make
>     very clear to distributors.

Hmm, I wonder if we can entirely circumvent the group shuffle. I
didn't check the addon-storage code thoroughly yet, but I would assume
that it can just open read-only fds to the particular devices at the
start, then drop privs to the hal user (without any additional groups)
and keep the open fds around to do ioctls on them. This would be more
robust and provide the same level of privilege reduction.

I'll check whether that works; if it does, then I think that would be
much better, do you agree?

> > The latter two modifications are contained in
> > 08_privileges-addons.patch. However, I don't consider them ready for
> > upstream inclusion so far. Currently it's just blunt code duplication
> > from the original drop_privileges() function in hald.c, I would rather
> > like to move that code into a library. If you are interested in the
> > general approach, I would like to generalize that function and put it
> > into hald/linux2/probing/shared.h, and also modify the other addons as
> > appropriate.
> 
> Yes, moving things to shared.h sounds good - for the time being it is
> our 'library' though we may move it to a real library at some point :-)

Ok, thanks.

> > Last, a question: do you have any prefered strategy how to implement
> > sanity checking in the hal-system-storage-* scripts? We don't
> > currently ship them since they do not do any checking on their own
> > (they just use the hal properties, which are unreliable in the current
> > trust model). So, if the privilege separation code should have any
> > sense, all callouts have to do input sanity checking on their own. I
> > would like to work on this if you want, I just want to make sure that
> > nobody else does ATM.
> 
> Yes, let me guess.. Your prime concern is that in a compromised hald,
> for example the property volume.fstype contains "`/some/evil/binary`"
> and a shell script callout expands $HAL_PROP_VOLUME_FSTYPE and then the
> attacker successfully runs /some/evil/binary?

The thing is that the entire idea of privilege separation between hald
and the addons/probes that run as root only makes sense in a trust
model where hald can be compromised, but the callouts can't. I. e. we
cannot trust the values of the hal properties, thus the policy checks
that restrict actions to certain subjects must be moved from hald to
the callouts.

> One idea; would it be sufficient to do e.g.
> 
>  FSTYPE=${HAL_PROP_VOLUME_FSTYPE//[^a-zA-Z0-9_=]/_}
> 
> and just use e.g. $FSTYPE instead of $HAL_PROP_VOLUME_FSTYPE in our
> scripts? I think so.

If I manage to break hal, I could change the properties in a way to
allow me (through the mount helper) to mount /dev/hda1 instead of
/dev/sda1 into /media/, which would allow me to compromise the system
partition. So it's not just a matter of code injection, but also one
of circumventing device type specific policy.

Thanks, and have a nice day!

Martin

-- 
Martin Pitt        http://www.piware.de
Ubuntu Developer   http://www.ubuntu.com
Debian Developer   http://www.debian.org

In a world without walls and fences, who needs Windows and Gates?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: Digital signature
Url : http://lists.freedesktop.org/archives/hal/attachments/20060220/93e3c3c9/attachment-0001.pgp