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

Proposed by Dimitri John Ledkov on 2013-11-04
Status: Merged
Merged at revision: 1555
Proposed branch: lp:~jamesodhunt/upstart/disable-system-bus
Merge into: lp:upstart
Diff against target: 216 lines (+119/-10) (has conflicts)
4 files modified
ChangeLog (+11/-0)
init/main.c (+37/-10)
init/man/init.8 (+4/-0)
util/tests/test_initctl.c (+67/-0)
Text conflict in ChangeLog
Text conflict in init/main.c
Text conflict in util/tests/test_initctl.c
To merge this branch: bzr merge lp:~jamesodhunt/upstart/disable-system-bus
Reviewer Review Type Date Requested Status
James Hunt 2013-11-04 Pending
Review via email: mp+193764@code.launchpad.net

This proposal supersedes a proposal from 2013-09-19.

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 : Posted in a previous version of this proposal

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.

James Hunt (jamesodhunt) wrote : Posted in a previous version of this proposal

Hi St├ęphane - new tests added.

review: Resubmit

Preview Diff

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

Subscribers

People subscribed via source and target branches