Bug 698250

It's worth pointing out that those replace the stuff that used to live in gnome-color-manager: colord is mainly code having been split from gnome-color-manager, and this is valid for the dbus/polkit files too.

FWIW, we'd need to get this fixed relatively soon, as we can't push some important bits of GNOME without the new package in Factory, and we can't get the package in Factory without this bug getting fixed.

I can submit the changes if that'd help.

I've submitted a new polkit-default-privs two weeks ago, anything missing?

Ah, thanks, I'll remove the rpmlintrc and re-submit the package, then.

Is it really necessary to run this as root? It links to a lot
of libararies and contains SQL injection vulnerabilities via
its sqlite3 calls.

Why does it actually need root privs? To write the system
wide settings files?

(In reply to comment #5)
> Is it really necessary to run this as root? It links to a lot
> of libararies and contains SQL injection vulnerabilities via
> its sqlite3 calls.
> 
> Why does it actually need root privs? To write the system
> wide settings files?

A new version of colord (in GNOME:Factory right now) allows the use of a non-root user, see
http://gitorious.org/colord/master/commit/75c7028157e8c8abeea0e510445a86bbcf166ee0

This commit log is also of interest:
http://gitorious.org/colord/master/commit/06c6477831a6d4cb4297b6b9a6bff23f9477aa88
"Ensure uid 0 can create devices and profiles even when not on the active console

When colord is running as the root user then this magically works, even though
daemons like cups are not on an active console.

If you switch colord to running as a private user then PolicyKit no longer
implicitly grants the authorisation from the root user, and this means that
printers cannot be registered with colord. By checking for the uid we can grant
an implicit authorisation ourselves before asking PolicyKit."

(sorry, clicked "commit" too soon)

I can switch the package to use a non-root user, if desired. I don't know if we have a user we could use for this, or if we should create a new one. If a new one, we will likely need to add the user to some special groups.

If somehow possible it should run as dedicated user rather than root.
Maybe some gnome-conf user or whatever. However, the SQL injections have
to be fixed nevertheless.

A shortcoming in policykit should not force other daemons to run as root
IMHO. I guess colord has to use dbus sender id to check by themself,
even though I'd expect that something cool like policykit should be
able to grant permissions for actions based on user id's.

This question is unanswered:

> Why does it actually need root privs? To write the system
> wide settings files?

Gnome asks for root password when I plug an eternal TV via HDMI or external monitor via VGA. It is for creating a colord device?
I find it stranger to need the root password to use an external monitor, TV or projector.

AFAICS colord still doesn't run as unprivileged user in Factory.

Reason for the popups is as usual unpolished handling of privileges AFAICS. The involved programs are obviously only made to work with the privileges granted and behave in an annoying way otherwise.

I'd recommend to remove gnome-color-manager from the gnome pattern to prevent installation of colord by default. Looks like nothing else needs it.

(In reply to comment #12)
> I'd recommend to remove gnome-color-manager from the gnome pattern to prevent
> installation of colord by default. Looks like nothing else needs it.

All of GNOME 3 is using libcolord / colord. GTK+ 3 depends on libcolord, gnome-settings-daemon talks to colord, etc.

None of that hard depends on colord though. The system works just fine without.
Looks like it's useless as long as you don't manually configure color profiles for the devices. So I'm against having the risk of yet another default running root service for no gain at all. Those who actually need color management can install the package manually just fine.

submit request 89695
https://gitorious.org/opensuse/patterns/merge_requests/16

couldn't we run colord as a separate user, like ubuntu is doing ?

I still think we should have colord (and g-c-m) installed by default

(In reply to comment #16)
> couldn't we run colord as a separate user, like ubuntu is doing ?

I've built a test package of colord, where the daemon would run as the colord user. It's in home:vuntz:branches:GNOME:Factory.

I'd appreciate testing :-)

Looks like noone bothered to talk to upstream so I did. The SQL injections are real. Upstream would appreciate patches. The only reason colord runs as root were missing patches in polkit. Ubuntu developed them and they are in git head meanwhile. So my impression is that you guys have no clue about this piece of software. It should not be part of the default install.

This is an autogenerated message for OBS integration:
This bug (698250) was mentioned in
https://build.opensuse.org/request/show/89921 Factory / colord

as it turns out colord actually never was included on dvd or live media as it was accidentally blocked via other packages. So colord was only included in the ftp tree. IOW eliminating colord from the ftp default install just makes it consistent with the dvd.
Colord requires fixes to polkit and you also need to modify colord's xml files for polkit to make it work as user (ie add org.freedesktop.policykit.owner). I've submitted the a polkit package with fixes picked from git: sr#89928. Not tested, you need to test that together with the non-root colord.

This is an autogenerated message for OBS integration:
This bug (698250) was mentioned in
https://build.opensuse.org/request/show/89959 Factory / polkit

This is an autogenerated message for OBS integration:
This bug (698250) was mentioned in
https://build.opensuse.org/request/show/90087 Factory / patterns-openSUSE

This is an autogenerated message for OBS integration:
This bug (698250) was mentioned in
https://build.opensuse.org/request/show/90106 Factory / patterns-openSUSE

This is an autogenerated message for OBS integration:
This bug (698250) was mentioned in
https://build.opensuse.org/request/show/90117 Factory / polkit

So what's the status with colord running as non-root?
Who is going to fix the sql injections?

(In reply to comment #25)
> So what's the status with colord running as non-root?

Didn't get any feedback on whether it's working or not :/

> Who is going to fix the sql injections?

I've put a patch in https://bugs.freedesktop.org/show_bug.cgi?id=42904. I think that's enough, but of course, a review would be nice.

The fixes look ok to me (if that are all places of the SQL problems;
I didnt count them...).

The http://www.sqlite.org/c3ref/mprintf.html description is probably
a bit incorrect. The call indeed has to double each \, but it also
has to take care about '. The description is missing that
but the example they give produces the correct result.

Beside the SQL injection fix, we need the non-root fix working.
If we shipped a colord running as root in this or another way,
we probably need to make updates.

"org.freedesktop.color-manager.create-device" is allowed for active sessions
by users which in turn allows to create device entries with arbitrary
device_id which then allows to submit arbitrary SQL statements.

(In reply to comment #25)
> So what's the status with colord running as non-root?

Apparently, doesn't work well. Frédéric tested it and got:

failed to lock sensor: Failed to Lock: GDBus.Error:org.freedesktop.ColorManager.Failed: couldn't check org.freedesktop.color-manager.sensor-lock for auth: Polkit.Error.NotAuthoized: Only trusted callers can use CheckAuthorization for subjects belonging to other identities

That's the reason why I included http://cgit.freedesktop.org/PolicyKit/commit/?id=5cd68a3aa8d5d0fdbbd3baef0601350bd43a0e4d in polkit on 12.1. Ie you need to add org.freedesktop.policykit.owner to the action file.

the security update for 12.1 is now handled in bug 698250

So could anyone please submit a package that a) fully fixes the SQL injections and b) makes colord run as user instead of root?

I built a package in home:vuntz:branches:OBS_Maintained:colord that contains all the SQL fixes, and should run as colord (with a new patch to add annotations to the .policy file). I'll send the patch upstream if it works fine.

Frédéric: could you try it?

(In reply to comment #31)
> the security update for 12.1 is now handled in bug 698250

Probably a mis-paste; that's bug 732996.

feel free to just submit to openSUSE:12.1:Test so we get an update staged already.

Ludwig: I'm playing with the package, and one major issue caused by the default policies is that the color plugin in g-s-d starts early, before a polkit agent is running. Which results in the plugin just failing.

Are the changes to run as colord user enough to change the default policy to not require auth for the current user? If no, we'll likely want to patch g-s-d to delay the color plugin start a bit but that'll result in a horrible user experience (with 4 popups on login).

(In reply to comment #34)
> feel free to just submit to openSUSE:12.1:Test so we get an update staged
> already.

Talking to Richard, he feels an update to 0.0.15 is worth it. Do we want to take the update to fix the security issue? Here's the relevants bits from NEWS; it's really about bug fixes:

===========================
Version 0.1.15
~~~~~~~~~~~~~~
Released: 2011-11-26

Notes:
 - This release fixes an important security bug: CVE-2011-4349.
 - It is recommended all users update to this version, or backport
   these patches:
    * http://gitorious.org/colord/master/commit/1fadd90afcb4bbc47513466ee9bb1e4a8632ac3b
    * http://gitorious.org/colord/master/commit/36549e0ed255e7dfa7852d08a75dd5f00cbd270e

New Features:
 - Add a native driver for the Hughski ColorHug hardware (Richard Hughes)
 - Export cd-math as three projects are now using it (Richard Hughes)

Bugfixes:
 - Documentation fixes and improvements (Laurent Martelli)
 - Do not crash the daemon if adding the device to the db failed (Richard Hughes)
 - Do not match any sensor device with a kernel driver (Richard Hughes)
 - Don't be obscure when the user passes a device-id to colormgr (Richard Hughes)
 - Fix a memory leak when getting properties from a device (Richard Hughes)
 - Fix colormgr device-get-default-profile (Richard Hughes)
 - Fix some conection bugs in colormgr (Florian Höch, Richard Hughes)
 - Fix some potential SQL injections (Ludwig Nussel, Vincent Untz)
 - Make gusb optional (Ludwig Nussel)
 - Only use the udev USB helper if the PID and VID have matches (Richard Hughes)
 - Output the Huey calibration matrices when dumping the sensor (Richard Hughes)

Version 0.1.14
~~~~~~~~~~~~~~
Released: 2011-11-01

Translations:
New Features:
 - Add defines for the i1 Display 3 (Richard Hughes)
 - Add two more DATA_source values to the specification (Richard Hughes)
 - Align the output from colormgr get-devices and get-profiles (Richard Hughes)
 - Allow cd-fix-profile to append and edit new metadata (Richard Hughes)

Bugfixes:
 - Ensure non-native device are added with no driver module (Richard Hughes)
 - Split the sensor and device udev code (Richard Hughes)
===========================

(In reply to comment #35)
> Ludwig: I'm playing with the package, and one major issue caused by the default
> policies is that the color plugin in g-s-d starts early, before a polkit agent
> is running. Which results in the plugin just failing.
> 
> Are the changes to run as colord user enough to change the default policy to
> not require auth for the current user? If no, we'll likely want to patch g-s-d
> to delay the color plugin start a bit but that'll result in a horrible user
> experience (with 4 popups on login).

If you promise to not put colord back in any default installation I'd do that.
I'm still not convinced that the architectore of the user session registering
new devices with colord is sane though. I'd have expected a some kind of X
plugin that does this in the background without involving any user.
Also, why does it take four popups!? Hmm, I guess it should have been
auth_admin_keep and create-device should probably imply the create-profile
privilege.

> (In reply to comment #34)
> > feel free to just submit to openSUSE:12.1:Test so we get an update staged
> > already.
> 
> Talking to Richard, he feels an update to 0.0.15 is worth it. Do we want to
> take the update to fix the security issue? Here's the relevants bits from NEWS;

I'd say it's up to you as package maintainer to decide what's the better option.

(In reply to comment #34)
> feel free to just submit to openSUSE:12.1:Test so we get an update staged
> already.

sr 95603.

(In reply to comment #36)
> (In reply to comment #35)
> > Are the changes to run as colord user enough to change the default policy to
> > not require auth for the current user? If no, we'll likely want to patch g-s-d
> > to delay the color plugin start a bit but that'll result in a horrible user
> > experience (with 4 popups on login).
> 
> If you promise to not put colord back in any default installation I'd do that.

I prefer to not make any such promise, especially as it might get dragged in by accident...

> Also, why does it take four popups!? Hmm, I guess it should have been
> auth_admin_keep and create-device should probably imply the create-profile
> privilege.

So what's happening on each login (assuming a polkit agent is running; if no, it will just fail):
 - create-device -- we create a device for the xrandr screen of this session
 - create-profile -- we create a default profile (I assume)
 - modify-device -- we attach the profile to the device (I assume)
 - modify-device -- no clue

Right now, we don't see any of this anyway since, as I mentioned earlier, the g-s-d plugin does all this before a polkit agent is there. So the issue is hidden.

It might make sense to have create-device imply create-profile and modify-device, if we change the policy to KEEP. I can patch colord for that if you agree it's the right way forward.

*** Bug 706908 has been marked as a duplicate of this bug. ***

(In reply to comment #37)
> It might make sense to have create-device imply create-profile and
> modify-device, if we change the policy to KEEP. I can patch colord for that if
> you agree it's the right way forward.

Actually, this will still be a horrible user experience as you'll still have on popup on each login :/ (the device is re-created on each login)

(In reply to comment #39)
> (In reply to comment #37)
> > It might make sense to have create-device imply create-profile and
> > modify-device, if we change the policy to KEEP. I can patch colord for that if
> > you agree it's the right way forward.

I guess it doesn't make sense to allow creating devices but not to
immediately also create color profiles for them so constructing some
imply chain one way or another looks useful to me at least.

> Actually, this will still be a horrible user experience as you'll still have on
> popup on each login :/ (the device is re-created on each login)

IOW it is impossible to use colord in a more restrictive setup. The
admin can only choose between allowing users to manage color
profiles/devices or annoy them.
So what's wrong here? Some thoughts.
- maybe colord should create permanent entries instead of temporary
  ones. The amount of different devices you use is most likely not
  infinite anyways. That way authentication would be only needed
  once when setting up the device initially.
- colord (or rather the front-end that triggers it) could instead of
  directly throwing an authentication popup at the user just show a
  passive popup or systray icon to indicate that there are
  unconfigured devices available.
- if temporary devices are a must for some reason maybe colord
  should use a different privilege or none at all for temporary
  devices.

I have no real problem with setting the privilege to 'yes' at this
point if colord runs as it's own user. There's not much it can do
besides filling up the hard disk and corrupting it's own database
after all (should be easy to jail via apparmor too). Just keep in
mind that this will paper over design issues IMO.

This is still unfixed in 12.2 beta2. And its pretty annoying. Simply turning off/on an LCD monitor pops up an authorization request for "creating a color managed device" :-/

you use GNOME?

Under KDE, with colord installed, on my laptop I get no popups when adding/disabling an additional TFT.

Of course, you don't get any popup under KDE, since it doesn't try to use colord..

(In reply to comment #42)
> you use GNOME?

Yes

(In reply to comment #43)
> Of course, you don't get any popup under KDE, since it doesn't try to use
> colord..

LOL

very funny. not.


i installed the gnome pattern, used gdm, logged into GNOME.

No popups on tft plug/plugout.

The color management shows an empty list.


So what else does a regular user need to configure to at least come into contact with this dialog?

(In reply to comment #46)
> very funny. not.
> 
> 
> i installed the gnome pattern, used gdm, logged into GNOME.
> 
> No popups on tft plug/plugout.
> 
> The color management shows an empty list.
> 
> 
> So what else does a regular user need to configure to at least come into
> contact with this dialog?

I suspect this is because of what I mentioned in comment #35: the color plugin in g-s-d is started before there's a polkit agent, and so it kind of gives up at the beginning of the session.

Try reloading the plugin:

gsettings set org.gnome.settings-daemon.plugins.color active false
gsettings reset org.gnome.settings-daemon.plugins.color active

(at least, that's the commands I needed to use when I was playing with color management in gnome a few weeks ago)

this helped, i now get the popups.

i am creating a AA profile for usr.lib.colord

I SRed the policykit standard rule change just now.

I also SRed a colord with a /usr/lib/colord aa profile. I should do a colord-sane one too, but it does not seem to trigger on my machine.

(yet again, i am not sure why this needs a global daemon and not just a per-user one :/)

fwiw, the profiles that are set are shared between users somehow via mapping.db, even if i do not select the global method.

(user tux had 1 profile that I set via user marcus, but not the second one... weird)

the sqlite mapping.db does not have user seperation as far as I can see.

This is an autogenerated message for OBS integration:
This bug (698250) was mentioned in
https://build.opensuse.org/request/show/127202 Factory / polkit-default-privs

Marcus: do you want to keep the bug open, or are your sr all that is needed?

i think its good.


you will need to fix the startup issue probably, but this is another bug.

Confirmed. Looks good now.

While a fix was released by altering the polkit privilges, it only reached openSUSE:Factory and thus 12.2.

However, all openSUSE 12.1 GNOME users would still be stuck with a broken color management for displays that are available at startup (or annoyed by many auth_admin dialogs once the session is up) despite the fix looks so "simple".

Any chance this polkit privs change could be released as an online update to openSUSE 12.1?

a fixed colord would need to be released too.