Merge lp:~ycheng-twn/powerd/powerd_allow-non-root-dbus-cmd into lp:powerd

Proposed by Yuan-Chen Cheng on 2014-03-04
Status: Merged
Approved by: Ricardo Salveti on 2014-03-26
Approved revision: 117
Merged at revision: 117
Proposed branch: lp:~ycheng-twn/powerd/powerd_allow-non-root-dbus-cmd
Merge into: lp:powerd
Diff against target: 83 lines (+46/-9)
2 files modified
cli/powerd-cli.c (+1/-2)
debian/powerd.conf (+45/-7)
To merge this branch: bzr merge lp:~ycheng-twn/powerd/powerd_allow-non-root-dbus-cmd
Reviewer Review Type Date Requested Status
Ricardo Salveti 2014-03-04 Approve on 2014-03-26
Review via email: mp+209199@code.launchpad.net

Commit message

Allow non-root to talk to powerd.

Description of the change

allow non-root to talk to powerd.

To post a comment you must log in.
Ricardo Salveti (rsalveti) wrote :

19 --- debian/changelog 2014-01-29 11:24:39 +0000
20 +++ debian/changelog 2014-03-04 09:18:20 +0000

Please don't change the changelog directly, this will be done by the CI train (please make sure the MR commit message contains whatever you wanted in the changelog though).

34 --- debian/powerd.conf 2013-05-20 17:33:44 +0000
35 +++ debian/powerd.conf 2014-03-04 09:18:20 +0000

I'd prefer if we could only open a few dbus properties and methods, instead of everything. We don't want an app to be able to request a suspend blocker, but we can allow the user to use a custom value for brightness.

I changed the powerd.conf file to only export a few interfaces by default, check http://paste.ubuntu.com/7154678/ (already tested).

8 - myeuid = geteuid();
9 - if (myeuid != 0) {
10 - fprintf(stderr,"You must be root to run %s\n",argv[0]);
11 - return -1;
12 - }

As we might just be exporting a few interfaces, I'd just remove the line with 'return -1', and change the error message saying that running as user is not fully supported.

review: Needs Fixing
Ricardo Salveti (rsalveti) wrote :

Looks good, thanks!

review: Approve
Ricardo Salveti (rsalveti) wrote :

10 + fprintf(stderr,"%s: Running as user is not fully supported.", argv[0]);

Minor fix, please add '\n' in the end.

$ powerd-cli list
powerd-cli: Running as user is not fully supported.System State Requests:
...

review: Needs Fixing
116. By Yuan-Chen Cheng on 2014-03-26

add newline

117. By Yuan-Chen Cheng on 2014-03-26

typo fix

Ricardo Salveti (rsalveti) wrote :

Looks good, thanks!

review: Approve
Oliver Grawert (ogra) wrote :

Just a FYI, i had to add the "at_console" policy back since it broke all autopilot image tests (scripts need to unlock the screen via adb shell before running)

https://code.launchpad.net/~ogra/powerd/fix-1298869 has the change

Bug #1298869 has details

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cli/powerd-cli.c'
2--- cli/powerd-cli.c 2013-09-24 14:49:49 +0000
3+++ cli/powerd-cli.c 2014-03-26 04:58:12 +0000
4@@ -1035,8 +1035,7 @@
5
6 myeuid = geteuid();
7 if (myeuid != 0) {
8- fprintf(stderr,"You must be root to run %s\n",argv[0]);
9- return -1;
10+ fprintf(stderr,"%s: Running as user is not fully supported.\n", argv[0]);
11 }
12
13 signal(SIGINT, sigint_quit);
14
15=== modified file 'debian/powerd.conf'
16--- debian/powerd.conf 2013-05-20 17:33:44 +0000
17+++ debian/powerd.conf 2014-03-26 04:58:12 +0000
18@@ -5,20 +5,58 @@
19 "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
20 <busconfig>
21
22- <!-- ../system.conf have denied everything, so we just punch some holes -->
23-
24+ <!-- Only the root user can own the powerd name -->
25 <policy user="root">
26 <allow own="com.canonical.powerd"/>
27+ </policy>
28+
29+ <!-- Permit the root user to invoke all of the methods on powerd,
30+ and to get and set properties. -->
31+ <policy user="root">
32 <allow send_destination="com.canonical.powerd"/>
33 <allow send_interface="com.canonical.powerd"/>
34 </policy>
35
36- <policy at_console="true">
37- <allow send_destination="com.canonical.powerd"/>
38- </policy>
39-
40+ <!-- Allow any user to introspect powerd's interfaces, to obtain the
41+ values of properties and only set some of them (brightness). -->
42 <policy context="default">
43- <deny send_destination="com.canonical.powerd"/>
44+ <allow send_destination="com.canonical.powerd"
45+ send_interface="org.freedesktop.DBus.Introspectable" />
46+ <allow send_destination="com.canonical.powerd"
47+ send_interface="org.freedesktop.DBus.Properties"
48+ send_type="method_call" send_member="Get" />
49+ <allow send_destination="com.canonical.powerd"
50+ send_interface="org.freedesktop.DBus.Properties"
51+ send_type="method_call" send_member="GetAll" />
52+
53+ <allow send_destination="com.canonical.powerd"
54+ send_interface="com.canonical.powerd"
55+ send_type="method_call" send_member="userAutobrightnessEnable" />
56+ <allow send_destination="com.canonical.powerd"
57+ send_interface="com.canonical.powerd"
58+ send_type="method_call" send_member="getBrightnessParams" />
59+ <allow send_destination="com.canonical.powerd"
60+ send_interface="com.canonical.powerd"
61+ send_type="method_call" send_member="setUserBrightness" />
62+
63+ <allow send_destination="com.canonical.powerd"
64+ send_interface="com.canonical.powerd"
65+ send_type="method_call" send_member="listSysRequests" />
66+ <allow send_destination="com.canonical.powerd"
67+ send_interface="com.canonical.powerd"
68+ send_type="method_call" send_member="listDisplayRequests" />
69+
70+ <allow send_destination="com.canonical.powerd"
71+ send_interface="com.canonical.powerd"
72+ send_type="method_call" send_member="getSysRequestStats" />
73+ <allow send_destination="com.canonical.powerd"
74+ send_interface="com.canonical.powerd"
75+ send_type="method_call" send_member="getDispRequestStats" />
76+
77+ <allow send_destination="com.canonical.powerd"
78+ send_interface="com.canonical.powerd"
79+ send_type="method_call" send_member="userAutobrightnessEnable" />
80+
81 </policy>
82
83 </busconfig>

Subscribers

People subscribed via source and target branches