sway icon indicating copy to clipboard operation
sway copied to clipboard

Allow to disable insecure protocols at compile-time

Open ya-isakov opened this issue 4 years ago • 11 comments

This pull-request is a way to allow build sway without insecure ptotocols, such as text-input (which allow to keylog user input from other windows) or screencopy (which allows any application to capture other apps' windows) This is just a temporary solution, until proper security model is in place. Also, everything is enabled by default (like it was before this PR)

ya-isakov avatar Jul 28 '20 18:07 ya-isakov

I think "privileged" is a better wording for these protocols.

emersion avatar Jul 29 '20 10:07 emersion

wlr-layer-shell, wlr-output-manager, wlr-foreign-toplevel-management, wlr-data-control, wlr-gamma-control, wlr-input-inhibit, wlr-virtual-keyboard, wlr-virtual-pointer and wlr-output-power-management also are privileged.

emersion avatar Jul 29 '20 10:07 emersion

@emersion Thank you for providing a list of privileged extensions, but I think that it should be separated on privacy-breaking ones, and not. My pull request is trying to keep Wayland secure, in the meaning, that no app can listen on other apps' input, or see other apps' windows. Extensions in your list could be used to do some bad things with user's apps, but they cannot get sensitive data, e.g. passwords.

P.S. That's why I called my list of protocols 'insecure', not 'privileged'.

ya-isakov avatar Aug 18 '20 14:08 ya-isakov

NACK to adding an option to disable some privileged protocols but not all of them.

emersion avatar Aug 18 '20 14:08 emersion

As I said, this pull request is not about disabling all privileged protocols, it's about disabling insecure ones. With these protocols, user's privacy could be compromised, but not with other listed protocols. So, I think that this review could be merged, to allow distributions to compile sway and not risk user's private info.

ya-isakov avatar Aug 18 '20 17:08 ya-isakov

Your definition is subjective. wlr-data-control allows reading passwords stored in the clipboard, wlr-input-inhibit allows grabbing all input events, wlr-virtual-keyboard and wlr-virtual-pointer allows injecting arbitrary input events, wlr-foreign-toplevel-management allows tracking apps used by the user, and so on. Even a protocol like wlr-layer-shell is insecure, because it's easy to fake a lock screen and grab the user password.

Which is why I'm NACK'ing your split between privileged and insecure protocol.

emersion avatar Aug 18 '20 19:08 emersion

Good point, but, I think, that at least wlr-layer-shell should be enabled for now, as it is required for e.g. mako and bemenu. I can try to disable other listed extensions, and left wlr-layer-shell enabled, will it work?

P.S. And copying passwords through keyboard was never secure, that's why there are browser extensions for popular password managers...

ya-isakov avatar Aug 18 '20 22:08 ya-isakov

will it work?

I still think it's a bad idea.

emersion avatar Aug 19 '20 10:08 emersion

I agree with @emersion that this PR should encompass all privileged protocols. As you mention, this is a temporary addition before a proper security model is added. The purpose of that security model would be to allow granular control of privileged protocols, so if this PR includes that, it's really just an un-finished security model.

fluix-dev avatar Aug 19 '20 17:08 fluix-dev

I like gamma-control protocol. Can gamma-control protocol steal cryptocurrency seed shown on screen? Which protocols can steal seeds on screen? Anything that can read screen, key input, and clipboard can steal cryptocurrency seeds.

crocket avatar Oct 24 '20 04:10 crocket

rebased onto 1.7-rc2:

index e02cd259..9873f2c9 100644
--- a/meson.build
+++ b/meson.build
@@ -102,6 +102,8 @@ endif
 
 have_tray = sdbus.found()
 
+use_insecure_protocols = get_option('use-insecure-protocols')
+
 conf_data = configuration_data()
 
 conf_data.set10('HAVE_XWAYLAND', have_xwayland)
@@ -110,6 +112,7 @@ conf_data.set10('HAVE_LIBSYSTEMD', sdbus.found() and sdbus.name() == 'libsystemd
 conf_data.set10('HAVE_LIBELOGIND', sdbus.found() and sdbus.name() == 'libelogind')
 conf_data.set10('HAVE_BASU', sdbus.found() and sdbus.name() == 'basu')
 conf_data.set10('HAVE_TRAY', have_tray)
+conf_data.set10('USE_INSECURE_PROTOCOLS', use_insecure_protocols)
 
 scdoc = dependency('scdoc', version: '>=1.9.2', native: true, required: get_option('man-pages'))
 if scdoc.found()
@@ -316,5 +319,6 @@ summary({
 	'gdk-pixbuf': gdk_pixbuf.found(),
 	'tray': have_tray,
 	'man-pages': scdoc.found(),
+	'use-insecure-protocols': use_insecure_protocols
 }, bool_yn: true)
 
diff --git a/meson_options.txt b/meson_options.txt
index 6ba67554..454a1332 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -9,3 +9,4 @@ option('tray', type: 'feature', value: 'auto', description: 'Enable support for
 option('gdk-pixbuf', type: 'feature', value: 'auto', description: 'Enable support for more image formats in swaybg')
 option('man-pages', type: 'feature', value: 'auto', description: 'Generate and install man pages')
 option('sd-bus-provider', type: 'combo', choices: ['auto', 'libsystemd', 'libelogind', 'basu'], value: 'auto', description: 'Provider of the sd-bus library')
+option('use-insecure-protocols', type: 'boolean', value: true, description: 'Enable insecure protocols')
diff --git a/sway/input/seat.c b/sway/input/seat.c
index ce933b66..4926f5e1 100644
--- a/sway/input/seat.c
+++ b/sway/input/seat.c
@@ -77,7 +77,9 @@ void seat_destroy(struct sway_seat *seat) {
 			link) {
 		seat_node_destroy(seat_node);
 	}
+#if USE_INSECURE_PROTOCOLS
 	sway_input_method_relay_finish(&seat->im_relay);
+#endif
 	sway_cursor_destroy(seat->cursor);
 	wl_list_remove(&seat->new_node.link);
 	wl_list_remove(&seat->request_start_drag.link);
@@ -201,7 +203,9 @@ static void seat_send_focus(struct sway_node *node, struct sway_seat *seat) {
 
 		seat_keyboard_notify_enter(seat, view->surface);
 		seat_tablet_pads_notify_enter(seat, view->surface);
+#if USE_INSECURE_PROTOCOLS
 		sway_input_method_relay_set_focus(&seat->im_relay, view->surface);
+#endif
 
 		struct wlr_pointer_constraint_v1 *constraint =
 			wlr_pointer_constraints_v1_constraint_for_surface(
@@ -612,7 +616,9 @@ struct sway_seat *seat_create(const char *seat_name) {
 	wl_list_init(&seat->keyboard_groups);
 	wl_list_init(&seat->keyboard_shortcuts_inhibitors);
 
+#if USE_INSECURE_PROTOCOLS
 	sway_input_method_relay_init(seat, &seat->im_relay);
+#endif
 
 	bool first = wl_list_empty(&server.input->seats);
 	wl_list_insert(&server.input->seats, &seat->link);
@@ -1138,7 +1144,9 @@ void seat_set_focus(struct sway_seat *seat, struct sway_node *node) {
 			view_close_popups(last_focus->sway_container->view);
 		}
 		seat_send_unfocus(last_focus, seat);
+#if USE_INSECURE_PROTOCOLS
 		sway_input_method_relay_set_focus(&seat->im_relay, NULL);
+#endif
 		seat->has_focus = false;
 		return;
 	}
@@ -1281,7 +1289,9 @@ void seat_set_focus_surface(struct sway_seat *seat,
 		wlr_seat_keyboard_notify_clear_focus(seat->wlr_seat);
 	}
 
+#if USE_INSECURE_PROTOCOLS
 	sway_input_method_relay_set_focus(&seat->im_relay, surface);
+#endif
 	seat_tablet_pads_notify_enter(seat, surface);
 }
 
diff --git a/sway/meson.build b/sway/meson.build
index 8eab31a2..d1e9c7bc 100644
--- a/sway/meson.build
+++ b/sway/meson.build
@@ -33,7 +33,6 @@ sway_sources = files(
 	'input/seatop_resize_tiling.c',
 	'input/switch.c',
 	'input/tablet.c',
-	'input/text_input.c',
 
 	'config/bar.c',
 	'config/output.c',
@@ -227,6 +226,10 @@ if have_xwayland
 	sway_deps += xcb
 endif
 
+if use_insecure_protocols
+	sway_sources += 'input/text_input.c'
+endif
+
 executable(
 	'sway',
 	sway_sources,
diff --git a/sway/server.c b/sway/server.c
index f50a0987..021b8949 100644
--- a/sway/server.c
+++ b/sway/server.c
@@ -175,8 +175,10 @@ bool server_init(struct sway_server *server) {
 		handle_output_power_manager_set_mode;
 	wl_signal_add(&server->output_power_manager_v1->events.set_mode,
 		&server->output_power_manager_set_mode);
+#if USE_INSECURE_PROTOCOLS
 	server->input_method = wlr_input_method_manager_v2_create(server->wl_display);
 	server->text_input = wlr_text_input_manager_v3_create(server->wl_display);
+#endif
 	server->foreign_toplevel_manager =
 		wlr_foreign_toplevel_manager_v1_create(server->wl_display);
 
@@ -191,8 +193,10 @@ bool server_init(struct sway_server *server) {
 		sway_log(SWAY_INFO, "VR will not be available");
 	}
 
+#if USE_INSECURE_PROTOCOLS
 	wlr_export_dmabuf_manager_v1_create(server->wl_display);
 	wlr_screencopy_manager_v1_create(server->wl_display);
+#endif
 	wlr_data_control_manager_v1_create(server->wl_display);
 	wlr_primary_selection_v1_device_manager_create(server->wl_display);
 	wlr_viewporter_create(server->wl_display);

bqv avatar Aug 31 '22 03:08 bqv

Closing because this is superseded by the security-context protocol.

emersion avatar Dec 27 '23 14:12 emersion