[MIR] colord

Runs a system daemon, so assigning to kees.

I remember raising the disk space for this with Richard at UDS. His reckoning was roughly that "the daemon is about 50 kB; the rest is documentation". Looking at the package here; most of it is .mo files installed under '/usr/share/locale/*', which should be stripped and turned into langpacks when it's added to main.

- why does this daemon need to run as root?
- org.freedesktop.color-manager.modify-profile appears to read any file on the filesystem. It reads the entire file (e.g. DoS with /dev/zero), and might do something via lcms parsing, but I haven't examined what sort of issues are in lcms for reading arbitrary files.
- by default, SearchVolumes is true in the /etc conf file, which means every inserted volume will be searched for color profiles, meaning that the above issue is true for arbitrary volume mounts too (attacker wouldn't need dbus access even).

Richard, can you answer these questions? Thanks.

> why does this daemon need to run as root?

It's a system activated daemon, just the same as upowerd and packagekitd. Architecturally, it's root as it needs to be accessed from different user sessions, and even in environments like a login manager and other system daemons like CUPS.

If you're asking why it couldn't be made to run in a more restricted group set, that's probably a good question. In Fedora and OpenSUSE there's no requirement to choose a private group and user for the daemon, and I'm also not sure what other groups colord would have to be made a member of. If that's really what you want, I can add some options to configure like --with-daemon-username and --with-daemon-group although I would much prefer a patch as I'm not really familiar with all the security and functional implications of running not as root.

>- org.freedesktop.color-manager.modify-profile appears to read any file on the filesystem.

org.freedesktop.color-manager.modify-profile is just the PolicyKit authorization action-id, I'm not sure what you mean there. The only time that authorization is used when the user does SetProperty on the org.freedesktop.ColorManager.Profile interface, and that's not actally reading or writing any files.

>It reads the entire file (e.g. DoS with /dev/zero), and might do something via lcms parsing

Well, in the usual case where the session is passing files to colord, the session just opens the file and passes a file descriptor over dbus to colord. If the session isn't capable of doing that, then the filename is indeed sent, although this breaks security frameworks like selinux pretty hard as then you've got a daemon reading the users files.

I'm open for ideas about whether I need futher checks for the /dev/null case. By handcrafting a malicious CreateProfile with a filename of /dev/null and not passing the FD, I get the following output in colord:

(colord:14986): Cd-WARNING **: LCMS error 1: Read error. Got 0 bytes, block should be of 128 bytes

By using /dev/urandom I basically DoS the daemon (which is expected) and there's not an awful lot that can be done about that as the results would be the same if I just fed a huge ICC profile (a multi-megabyte valid blob) for the daemon to parse. Of course, if there was a parsing bug in the lcms code it's theoretically possible to crash the daemon. Once you've got malicious code running in the session you've got bigger problems than crashing a color daemon, especially if it's going to get auto-restarted on the next use.

> by default, SearchVolumes is true in the /etc conf file

Right, and that's the kind of thing we could add a configure switch for if required. We actually are pretty strict about checking the volume to read profiles, although you could easily crash the daemon if you found a weakness in lcms2, and then created a HFS+ volume with a Library/ColorSync/Profiles/Displays/ path and an hand-crafted color profile.

At the end of the day, autoloading the OSX and Windows profiles is a cute trick, but if you think that's at the detriment to security then we can easily turn it off.

Richard.

I've applied this to master:

commit 3a693ef7d5a949796274f42c7c5e67b933d6ed8f
Author: Richard Hughes <email address hidden>
Date: Sat Aug 13 19:08:54 2011 +0200

    Add a configure argument --enable-volume-search

    This allows distros to disable the external-volume probing, which may be
    slightly more secure at the expense of some 'just-workingness'.

:100644 100644 933b7e7... 8fbfe55... M configure.ac
:100644 100644 460f08d... b3ec91f... M etc/Makefile.am
:100644 000000 6d60d29... 0000000... D etc/colord.conf
:000000 100644 0000000... 1864f8a... A etc/colord.conf.in

commit 7f3514b4e6057f678f50fe67455eecbdf6eb89d3
Author: Richard Hughes <email address hidden>
Date: Sat Aug 13 18:41:22 2011 +0200

    Add a configure argument of --enable-fd-fallback

    This defaults to true, and allows distros to disable opening of the user
    profiles from the daemon.

    This in theory is a little safer, and does not upset frameworks like SELinux.

    The downside is that testing is made much harder and all clients have to support
    FD-passing in DBus messages.

:100644 100644 257a03c... c4a7bf4... M Makefile.am
:100644 100644 fede541... 933b7e7... M configure.ac
:100644 100644 af4e86b... d747c25... M src/cd-profile.c

By the way, gnome-settings-daemon now has a hard dependency on libcolord.

Currently gnome-settings-daemon and gnome-control-center cannot be built (component mismatches). Imo it would be better to disabled the colord support in these two packages for now, so that these can be built and tested. Or start working on the issues that Kees mentioned above.

Thanks for the reply!

The way I look at this is from the perspective of risk. There are two scenarios for crossing privilege boundaries:
 - the local user attacking colord via malicious DBus calls (to gain colord privs)
 - an external user attacking colord via inserted media (to gain colord privs)

If we don't scan media by default, then the entire external attack goes away.

If colord doesn't run as root, then a local attacker gains very little from attacking colord.

The reason I'm so nervous about this is because colord is effectively a direct path to lcms parsing. Any user can trigger the lcms parser _as the root user_, and lcms has a non-zero history of security flaws:

        low(2): CVE-2009-0581 CVE-2009-0793
        medium(3): CVE-2008-5317 CVE-2009-0723 CVE-2009-0733

I would much prefer this daemon run as some "colord" system user instead of as root. The default for any daemon should be to run with least privilege.

Richard, for the inserted-media problem you have already a patch, but is there a chance to make the daemon work as a system user (like "colord") and not as root?

Kees, what about applying the two patches suggested by Richard?

They seem like good ideas, but I haven't reviewed them yet. Does the --enable-fd-fallback patch mean that files are not opened by colord (i.e. they must be opened by the user first)? That would be an improvement for sure, though DoSing the daemon is still possible, but that's a much lower risk, IMO.

>Imo it would be better to disabled the colord support in these two packages for now, so that these can be built and tested.

Just not possible. The g-s-d and g-c-c maintainers are not willing to make the colord support conditional, although I suppose you could manually hack out the building from configure.ac and a couple of Makefile.am's.

>Kees, what about applying the two patches suggested by Richard?

I can just release a new tarball upstream when Kees is happy with the changes.

>Does the --enable-fd-fallback patch mean that files are not opened by colord (i.e. they must be opened by the user first)?

Well, they are still opened by colord (the fd) but the session will have to have parsed the file before the fd is passed. You can never prevent DoSing the daemon, as there's no effective rate or capacity limiting of the messages by the DBus daemon.

I'll work on a user-group patch, although it'll need to be fixed/audited by someone who knows what they are doing, .i.e. not me.

>I'll work on a user-group patch

I think we want something like this: http://people.freedesktop.org/~hughsient/temp/0001-Allow-the-daemon-to-run-with-a-different-user-and-gr.patch

I can't get this to work (the daemon disconnects for me as soon as I change group) and I've run out of time for today, and so I would appreciate someone picking this patch up and fixing/reviewing it for me. In Fedora and RHEL daemons like this run as root.

Thanks.

Richard.

Thanks for that Richard. I'll see if I can get that patch to work, then hand it off to Kees for security review.

I believe the colord 0.1.11-1ubuntu1 package now in the archive resolves the security concerns.

It runs as the colord user, which has access to /var/lib/colord, scanners, and sensor devices, and is otherwise unpriviledged. It also no longer automatically scans removable devices for colour profiles by default.

If that's ok, it'd be nice to get this MIR processed so the rest of g-c-c and g-c-m can build before beta :).

As the latest version addresses Kees' concerns, and this is holding up builds of other packages, I promoted the package now. Keeping the bug open for official signoff/more review.

Thanks, looks good!