Merge lp:~jamesodhunt/upstart/disable-system-bus into lp:upstart

Proposed by James Hunt on 2013-09-19
Status: Superseded
Proposed branch: lp:~jamesodhunt/upstart/disable-system-bus
Merge into: lp:upstart
Diff against target: 200 lines (+108/-12)
5 files modified
ChangeLog (+8/-0)
init/control.c (+1/-1)
init/main.c (+31/-11)
init/man/init.8 (+4/-0)
util/tests/test_initctl.c (+64/-0)
To merge this branch: bzr merge lp:~jamesodhunt/upstart/disable-system-bus
Reviewer Review Type Date Requested Status
James Hunt Resubmit on 2013-09-19
Review via email: mp+186599@code.launchpad.net

This proposal has been superseded by a proposal from 2013-11-04.

Description of the change

* init/control.c: Typo.
* init/main.c: Add '--no-dbus' command-line option.
  init/man/init.8: Added '--no-dbus' option.

This option has the effect of stopping Session Inits having access to system-level events for those systems which require such behaviour; in such environments, even if the upstart-event-bridge is running, no events will (can) be proxied from the system level.

A side-effect of booting with '--no-dbus' is that a non-priv user will be unable to query system jobs using initctl (since such users will not have access to the private socket). However, for those making use of '--no-dbus', such behaviour would be deemed a security advantage rather than a limitation.

To post a comment you must log in.
St├ęphane Graber (stgraber) wrote :

The code looks good to me, we should just add one test to confirm that upstart with --disable-dbus won't do anything on SIGUSR1.

1535. By James Hunt on 2013-09-19

* util/tests/test_initctl.c: test_no_dbus(): New test to test
  '--no-dbus' option.

1536. By James Hunt on 2013-09-19

* util/tests/test_initctl.c: test_no_dbus(): Added explicit test for
  SIGUSR1 handling.

James Hunt (jamesodhunt) wrote :

Hi St├ęphane - new tests added.

review: Resubmit

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ChangeLog'
2--- ChangeLog 2013-09-13 04:44:55 +0000
3+++ ChangeLog 2013-09-19 20:37:08 +0000
4@@ -1,3 +1,11 @@
5+2013-09-19 James Hunt <james.hunt@ubuntu.com>
6+
7+ * init/control.c: Typo.
8+ * init/main.c: Add '--no-dbus' command-line option.
9+ init/man/init.8: Added '--no-dbus' option.
10+ * util/tests/test_initctl.c: test_no_dbus(): New test
11+ to test '--no-dbus' option.
12+
13 2013-09-12 Steve Langasek <steve.langasek@ubuntu.com>
14
15 * configure.ac:
16
17=== modified file 'init/control.c'
18--- init/control.c 2013-04-22 10:30:09 +0000
19+++ init/control.c 2013-09-19 20:37:08 +0000
20@@ -260,7 +260,7 @@
21
22 control_handle_bus_type ();
23
24- /* Connect to the D-Bus System Bus and hook everything up into
25+ /* Connect to the appropriate D-Bus bus and hook everything up into
26 * our own main loop automatically.
27 */
28 conn = nih_dbus_bus (use_session_bus ? DBUS_BUS_SESSION : DBUS_BUS_SYSTEM,
29
30=== modified file 'init/main.c'
31--- init/main.c 2013-07-31 09:28:48 +0000
32+++ init/main.c 2013-09-19 20:37:08 +0000
33@@ -120,6 +120,14 @@
34 **/
35 static int disable_startup_event = FALSE;
36
37+/**
38+ * disable_dbus:
39+ *
40+ * If TRUE, do not connect to a D-Bus bus
41+ * (only connect to the private socket).
42+ **/
43+static int disable_dbus = FALSE;
44+
45 extern int no_inherit_env;
46 extern int user_mode;
47 extern int disable_sessions;
48@@ -142,6 +150,9 @@
49 { 0, "default-console", N_("default value for console stanza"),
50 NULL, "VALUE", NULL, console_type_setter },
51
52+ { 0, "no-dbus", N_("do not connect to a D-Bus bus"),
53+ NULL, NULL, &disable_dbus, NULL },
54+
55 { 0, "no-inherit-env", N_("jobs will not inherit environment of init"),
56 NULL, NULL, &no_inherit_env ,NULL },
57
58@@ -592,16 +603,21 @@
59 * fail (since dbus-daemon probably isn't running yet) and will try again
60 * later - don't let ENOMEM stop us though.
61 */
62- while (control_bus_open () < 0) {
63- NihError *err;
64- int number;
65-
66- err = nih_error_get ();
67- number = err->number;
68- nih_free (err);
69-
70- if (number != ENOMEM)
71- break;
72+ if (disable_dbus) {
73+ nih_info (_("Not connecting to %s bus"),
74+ use_session_bus ? "session" : "system");
75+ } else {
76+ while (control_bus_open () < 0) {
77+ NihError *err;
78+ int number;
79+
80+ err = nih_error_get ();
81+ number = err->number;
82+ nih_free (err);
83+
84+ if (number != ENOMEM)
85+ break;
86+ }
87 }
88
89 #ifndef DEBUG
90@@ -932,8 +948,12 @@
91 usr1_handler (void *data,
92 NihSignal *signal)
93 {
94+ if (disable_dbus)
95+ return;
96+
97 if (! control_bus) {
98- nih_info (_("Reconnecting to system bus"));
99+ nih_info (_("Reconnecting to %s bus"),
100+ use_session_bus ? "session" : "system");
101
102 if (control_bus_open () < 0) {
103 NihError *err;
104
105=== modified file 'init/man/init.8'
106--- init/man/init.8 2013-04-02 10:19:07 +0000
107+++ init/man/init.8 2013-09-19 20:37:08 +0000
108@@ -84,6 +84,10 @@
109 .BR console "."
110 .\"
111 .TP
112+.B \-\-no\-dbus
113+Do not connect to a D-Bus bus.
114+.\"
115+.TP
116 .B \-\-no\-inherit\-env
117 Stop jobs from inheriting the initial environment. Only meaningful when
118 running in user mode.
119
120=== modified file 'util/tests/test_initctl.c'
121--- util/tests/test_initctl.c 2013-09-05 16:19:06 +0000
122+++ util/tests/test_initctl.c 2013-09-19 20:37:08 +0000
123@@ -11121,6 +11121,69 @@
124 }
125
126 void
127+test_no_dbus (void)
128+{
129+ nih_local char *cmd = NULL;
130+ char **output;
131+ size_t lines;
132+ pid_t upstart_pid = 0;
133+ pid_t dbus_pid = 0;
134+ char *extra[] = { "--no-dbus", NULL };
135+
136+ TEST_GROUP ("Test '--no-dbus'");
137+
138+ TEST_DBUS (dbus_pid);
139+
140+ /*******************************************************************/
141+ /* First perform a sanity check */
142+
143+ TEST_FEATURE ("Ensure version can be queried normally");
144+
145+ start_upstart_common (&upstart_pid, FALSE, NULL, NULL, NULL);
146+
147+ cmd = nih_sprintf (NULL, "%s version 2>/dev/null", get_initctl ());
148+ TEST_NE_P (cmd, NULL);
149+ RUN_COMMAND (NULL, cmd, &output, &lines);
150+ TEST_EQ (lines, 1);
151+ TEST_STR_MATCH (output[0], "init*(upstart [0-9]*");
152+ nih_free (output);
153+
154+ STOP_UPSTART (upstart_pid);
155+
156+ /*******************************************************************/
157+ /* Now, try with dbus disabled */
158+
159+ TEST_FEATURE ("Ensure '--no-dbus' disables D-Bus");
160+
161+ start_upstart_common (&upstart_pid, FALSE, NULL, NULL, extra);
162+
163+ cmd = nih_sprintf (NULL, "%s version 2>/dev/null", get_initctl ());
164+ TEST_NE_P (cmd, NULL);
165+ RUN_COMMAND (NULL, cmd, &output, &lines);
166+
167+ /* No output on stdout expected */
168+ TEST_EQ (lines, 0);
169+
170+ /*******************************************************************/
171+ TEST_FEATURE ("Ensure D-Bus still disabled on SIGUSR1");
172+
173+ assert0 (kill (upstart_pid, SIGUSR1));
174+
175+ cmd = nih_sprintf (NULL, "%s version 2>/dev/null", get_initctl ());
176+ TEST_NE_P (cmd, NULL);
177+ RUN_COMMAND (NULL, cmd, &output, &lines);
178+
179+ /* No output on stdout expected */
180+ TEST_EQ (lines, 0);
181+
182+ STOP_UPSTART (upstart_pid);
183+
184+ /*******************************************************************/
185+
186+ TEST_DBUS_END (dbus_pid);
187+}
188+
189+void
190 test_quiesce (void)
191 {
192 char confdir[PATH_MAX];
193@@ -16684,6 +16747,7 @@
194 test_reexec ();
195 test_list_sessions ();
196 test_quiesce ();
197+ test_no_dbus ();
198
199 if (in_chroot () && !dbus_configured ()) {
200 fprintf(stderr, "\n\n"

Subscribers

People subscribed via source and target branches