From b73800da7f384eea66b6eb67f5f40129f3dfc372 Mon Sep 17 00:00:00 2001

From: Ray Strode <rstrode@redhat.com>

Date: Tue, 25 Aug 2020 16:20:42 -0400

Subject: [PATCH 08/15] subman: Don't send secrets through command line

The command line is introspectable with "ps", and it even gets logged

to syslog, so it's not suitable for passing secrets.

Unfortunately, the user's password is currently passed.

This commit addresses that problem by passing the password through

stdin, instead.

---

 plugins/subman/gsd-subman-helper.c        | 32 ++++++++------

 plugins/subman/gsd-subscription-manager.c | 52 ++++++++++++++++++++---

 plugins/subman/meson.build                |  2 +-

 3 files changed, 66 insertions(+), 20 deletions(-)

diff --git a/plugins/subman/gsd-subman-helper.c b/plugins/subman/gsd-subman-helper.c

index 3931ef2e..edf1e41f 100644

--- a/plugins/subman/gsd-subman-helper.c

+++ b/plugins/subman/gsd-subman-helper.c

@@ -1,59 +1,61 @@

 /* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*-

  *

  * Copyright (C) 2019 Richard Hughes <rhughes@redhat.com>

  *

  * Licensed under the GNU General Public License Version 2

  *

  * This program is free software; you can redistribute it and/or modify

  * it under the terms of the GNU General Public License as published by

  * the Free Software Foundation; either version 2 of the License, or

  * (at your option) any later version.

  *

  * This program is distributed in the hope that it will be useful,

  * but WITHOUT ANY WARRANTY; without even the implied warranty of

  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

  * GNU General Public License for more details.

  *

  * You should have received a copy of the GNU General Public License

  * along with this program; if not, write to the Free Software

  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.

  */

 #include "config.h"

+

 #include <sys/types.h>

 #include <unistd.h>

 #include <stdlib.h>

 #include <locale.h>

 #include <gio/gio.h>

+#include <gio/gunixinputstream.h>

 #include <json-glib/json-glib.h>

 #define DBUS_TIMEOUT 300000 /* 5 minutes */

 static const char *locale;

 static void

 _helper_convert_error (const gchar *json_txt, GError **error)

 {

 	JsonNode *json_root;

 	JsonObject *json_obj;

 	const gchar *message;

 	g_autoptr(JsonParser) json_parser = json_parser_new ();

 	/* this may be plain text or JSON :| */

 	if (!json_parser_load_from_data (json_parser, json_txt, -1, NULL)) {

 		g_set_error_literal (error,

 				     G_IO_ERROR,

 				     G_IO_ERROR_NOT_SUPPORTED,

 				     json_txt);

 		return;

 	}

 	json_root = json_parser_get_root (json_parser);

 	json_obj = json_node_get_object (json_root);

 	if (!json_object_has_member (json_obj, "message")) {

 		g_set_error (error,

 			     G_IO_ERROR,

 			     G_IO_ERROR_INVALID_DATA,

 			     "no message' in %s", json_txt);

 		return;

 	}

@@ -149,86 +151,82 @@ _helper_save_config (const gchar *key, const gchar *value, GError **error)

 					       G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES |

 					       G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS,

 					       NULL,

 					       "com.redhat.RHSM1",

 					       "/com/redhat/RHSM1/Config",

 					       "com.redhat.RHSM1.Config",

 					       NULL, error);

 	if (proxy == NULL) {

 		g_prefix_error (error, "Failed to get proxy: ");

 		return FALSE;

 	}

 	res = g_dbus_proxy_call_sync (proxy, "Set",

 				      g_variant_new ("(svs)",

 						     key,

 						     g_variant_new_string (value),

 						     locale),

 				      G_DBUS_CALL_FLAGS_NONE,

 				      DBUS_TIMEOUT,

 				      NULL, error);

 	return res != NULL;

 }

 int

 main (int argc, char *argv[])

 {

 	g_autofree gchar *activation_key = NULL;

 	g_autofree gchar *address = NULL;

 	g_autofree gchar *hostname = NULL;

 	g_autofree gchar *kind = NULL;

 	g_autofree gchar *organisation = NULL;

-	g_autofree gchar *password = NULL;

 	g_autofree gchar *port = NULL;

 	g_autofree gchar *prefix = NULL;

 	g_autofree gchar *proxy_server = NULL;

 	g_autofree gchar *username = NULL;

 	g_autoptr(GDBusConnection) conn_private = NULL;

 	g_autoptr(GDBusProxy) proxy = NULL;

 	g_autoptr(GError) error = NULL;

 	g_autoptr(GOptionContext) context = g_option_context_new (NULL);

 	g_autoptr(GVariantBuilder) proxy_options = NULL;

 	g_autoptr(GVariantBuilder) subman_conopts = NULL;

 	g_autoptr(GVariantBuilder) subman_options = NULL;

+	g_autoptr(GInputStream) standard_input_stream = g_unix_input_stream_new (STDIN_FILENO, FALSE);

 	const GOptionEntry options[] = {

 		{ "kind", '\0', G_OPTION_FLAG_NONE, G_OPTION_ARG_STRING,

 			&kind, "Kind, e.g. 'username' or 'key'", NULL },

 		{ "address", '\0', G_OPTION_FLAG_NONE, G_OPTION_ARG_STRING,

 			&address, "UNIX address", NULL },

 		{ "username", '\0', G_OPTION_FLAG_NONE, G_OPTION_ARG_STRING,

 			&username, "Username", NULL },

-		{ "password", '\0', G_OPTION_FLAG_NONE, G_OPTION_ARG_STRING,

-			&password, "Password", NULL },

 		{ "organisation", '\0', G_OPTION_FLAG_NONE, G_OPTION_ARG_STRING,

 			&organisation, "Organisation", NULL },

-		{ "activation-key", '\0', G_OPTION_FLAG_NONE, G_OPTION_ARG_STRING,

-			&activation_key, "Activation keys", NULL },

 		{ "hostname", '\0', G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_STRING,

 			&hostname, "Registration server hostname", NULL },

 		{ "prefix", '\0', G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_STRING,

 			&prefix, "Registration server prefix", NULL },

 		{ "port", '\0', G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_STRING,

 			&port, "Registration server port", NULL },

 		{ "proxy", '\0', G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_STRING,

 			&proxy_server, "Proxy settings", NULL },

 		{ NULL}

 	};

 	/* check calling UID */

 	if (getuid () != 0 || geteuid () != 0) {

 		g_printerr ("This program can only be used by the root user\n");

 		return G_IO_ERROR_NOT_SUPPORTED;

 	}

 	setlocale (LC_ALL, "");

 	locale = setlocale (LC_MESSAGES, NULL);

 	g_option_context_add_main_entries (context, options, NULL);

 	if (!g_option_context_parse (context, &argc, &argv, &error)) {

 		g_printerr ("Failed to parse arguments: %s\n", error->message);

 		return G_IO_ERROR_NOT_SUPPORTED;

 	}

 	/* uncommon actions */

 	if (kind == NULL) {

 		g_printerr ("No --kind specified\n");

 		return G_IO_ERROR_INVALID_DATA;

@@ -267,109 +265,117 @@ main (int argc, char *argv[])

 				       NULL, /* GDBusInterfaceInfo */

 				       NULL, /* name */

 				       "/com/redhat/RHSM1/Register",

 				       "com.redhat.RHSM1.Register",

 				       NULL, &error);

 	if (proxy == NULL) {

 		g_printerr ("Count not contact RHSM: %s\n", error->message);

 		return G_IO_ERROR_NOT_FOUND;

 	}

 	/* no options */

 	subman_options = g_variant_builder_new (G_VARIANT_TYPE("a{ss}"));

 	/* set registration server */

 	if (hostname == NULL || hostname[0] == '\0')

 		hostname = g_strdup ("subscription.rhsm.redhat.com");

 	if (prefix == NULL || prefix[0] == '\0')

 		prefix = g_strdup ("/subscription");

 	if (port == NULL || port[0] == '\0')

 		port = g_strdup ("443");

 	subman_conopts = g_variant_builder_new (G_VARIANT_TYPE("a{ss}"));

 	g_variant_builder_add (subman_conopts, "{ss}", "host", hostname);

 	g_variant_builder_add (subman_conopts, "{ss}", "handler", prefix);

 	g_variant_builder_add (subman_conopts, "{ss}", "port", port);

 	/* call into RHSM */

 	if (g_strcmp0 (kind, "register-with-key") == 0) {

 		g_auto(GStrv) activation_keys = NULL;

 		g_autoptr(GError) error_local = NULL;

 		g_autoptr(GVariant) res = NULL;

+		gchar activation_key[PIPE_BUF + 1] = "";

-		if (activation_key == NULL) {

-			g_printerr ("Required --activation-key\n");

-			return G_IO_ERROR_INVALID_DATA;

-		}

 		if (organisation == NULL) {

 			g_printerr ("Required --organisation\n");

 			return G_IO_ERROR_INVALID_DATA;

 		}

+		g_input_stream_read (standard_input_stream, activation_key, sizeof (activation_key) - 1, NULL, &error_local);

+

+		if (error_local != NULL) {

+			g_printerr ("Could not read activation key: %s\n", error_local->message);

+			return G_IO_ERROR_INVALID_DATA;

+		}

+

 		g_debug ("trying to unregister in case machine is already registered");

 		_helper_unregister (NULL);

 		g_debug ("registering using activation key");

 		activation_keys = g_strsplit (activation_key, ",", -1);

 		res = g_dbus_proxy_call_sync (proxy,

 					      "RegisterWithActivationKeys",

 					      g_variant_new ("(s^asa{ss}a{ss}s)",

 							     organisation,

 							     activation_keys,

 							     subman_options,

 							     subman_conopts,

 							     locale),

 					      G_DBUS_CALL_FLAGS_NO_AUTO_START,

 					      DBUS_TIMEOUT,

 					      NULL, &error_local);

 		if (res == NULL) {

 			g_dbus_error_strip_remote_error (error_local);

 			_helper_convert_error (error_local->message, &error);

 			g_printerr ("Failed to RegisterWithActivationKeys: %s\n", error->message);

 			return error->code;

 		}

 	} else if (g_strcmp0 (kind, "register-with-username") == 0) {

 		g_autoptr(GError) error_local = NULL;

 		g_autoptr(GVariant) res = NULL;

+		gchar password[PIPE_BUF + 1] = "";

 		if (username == NULL) {

 			g_printerr ("Required --username\n");

 			return G_IO_ERROR_INVALID_DATA;

 		}

-		if (password == NULL) {

-			g_printerr ("Required --password\n");

-			return G_IO_ERROR_INVALID_DATA;

-		}

 		if (organisation == NULL) {

 			g_printerr ("Required --organisation\n");

 			return G_IO_ERROR_INVALID_DATA;

 		}

+		g_input_stream_read (standard_input_stream, password, sizeof (password) - 1, NULL, &error_local);

+

+		if (error_local != NULL) {

+			g_printerr ("Could not read password: %s\n", error_local->message);

+			return G_IO_ERROR_INVALID_DATA;

+		}

+

 		g_debug ("trying to unregister in case machine is already registered");

 		_helper_unregister (NULL);

 		g_debug ("registering using username and password");

 		res = g_dbus_proxy_call_sync (proxy,

 					      "Register",

 					      g_variant_new ("(sssa{ss}a{ss}s)",

 							     organisation,

 							     username,

 							     password,

 							     subman_options,

 							     subman_conopts,

 							     locale),

 					      G_DBUS_CALL_FLAGS_NO_AUTO_START,

 					      DBUS_TIMEOUT,

 					      NULL, &error_local);

 		if (res == NULL) {

 			g_dbus_error_strip_remote_error (error_local);

 			_helper_convert_error (error_local->message, &error);

 			g_printerr ("Failed to Register: %s\n", error->message);

 			return error->code;

 		}

 	} else {

 		g_printerr ("Invalid --kind specified: %s\n", kind);

 		return G_IO_ERROR_INVALID_DATA;

 	}

 	/* set the new hostname */

 	if (!_helper_save_config ("server.hostname", hostname, &error)) {

 		g_printerr ("Failed to save hostname: %s\n", error->message);

diff --git a/plugins/subman/gsd-subscription-manager.c b/plugins/subman/gsd-subscription-manager.c

index e2c16056..0838d490 100644

--- a/plugins/subman/gsd-subscription-manager.c

+++ b/plugins/subman/gsd-subscription-manager.c

@@ -1,53 +1,54 @@

 /* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 8 -*-

  *

  * Copyright (C) 2019 Richard Hughes <richard@hughsie.com>

  * Copyright (C) 2019 Kalev Lember <klember@redhat.com>

  *

  * This program is free software; you can redistribute it and/or modify

  * it under the terms of the GNU General Public License as published by

  * the Free Software Foundation; either version 2 of the License, or

  * (at your option) any later version.

  *

  * This program is distributed in the hope that it will be useful,

  * but WITHOUT ANY WARRANTY; without even the implied warranty of

  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

  * GNU General Public License for more details.

  *

  * You should have received a copy of the GNU General Public License

  * along with this program; if not, see <http://www.gnu.org/licenses/>.

  *

  */

 #include "config.h"

 #include <glib/gi18n.h>

+#include <gio/gunixinputstream.h>

 #include <gdk/gdk.h>

 #include <gtk/gtk.h>

 #include <json-glib/json-glib.h>

 #include <libnotify/notify.h>

 #include "gnome-settings-profile.h"

 #include "gsd-subman-common.h"

 #include "gsd-subscription-manager.h"

 #define GSD_DBUS_NAME "org.gnome.SettingsDaemon"

 #define GSD_DBUS_PATH "/org/gnome/SettingsDaemon"

 #define GSD_DBUS_BASE_INTERFACE "org.gnome.SettingsDaemon"

 #define GSD_SUBSCRIPTION_DBUS_NAME		GSD_DBUS_NAME ".Subscription"

 #define GSD_SUBSCRIPTION_DBUS_PATH		GSD_DBUS_PATH "/Subscription"

 #define GSD_SUBSCRIPTION_DBUS_INTERFACE		GSD_DBUS_BASE_INTERFACE ".Subscription"

 static const gchar introspection_xml[] =

 "<node>"

 "  <interface name='org.gnome.SettingsDaemon.Subscription'>"

 "    <method name='Register'>"

 "      <arg type='a{sv}' name='options' direction='in'/>"

 "    </method>"

 "    <method name='Unregister'/>"

 "    <property name='InstalledProducts' type='aa{sv}' access='read'/>"

 "    <property name='SubscriptionStatus' type='u' access='read'/>"

 "  </interface>"

 "</node>";

 #define GSD_SUBSCRIPTION_MANAGER_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), GSD_TYPE_SUBSCRIPTION_MANAGER, GsdSubscriptionManagerPrivate))

@@ -517,129 +518,168 @@ _client_maybe__show_notification (GsdSubscriptionManager *manager)

 		}

 	}

 	/* nag again */

 	if (priv->subscription_status == GSD_SUBMAN_SUBSCRIPTION_STATUS_UNKNOWN &&

 	    g_timer_elapsed (priv->timer_last_notified, NULL) > 60 * 60 * 24) {

 		_show_notification (manager, _NOTIFY_REGISTRATION_REQUIRED);

 		return;

 	}

 	if (priv->subscription_status == GSD_SUBMAN_SUBSCRIPTION_STATUS_INVALID &&

 	    g_timer_elapsed (priv->timer_last_notified, NULL) > 60 * 60 * 24) {

 		_show_notification (manager, _NOTIFY_EXPIRED);

 		return;

 	}

 	if (priv->subscription_status == GSD_SUBMAN_SUBSCRIPTION_STATUS_PARTIALLY_VALID &&

 	    g_timer_elapsed (priv->timer_last_notified, NULL) > 60 * 60 * 24) {

 		_show_notification (manager, _NOTIFY_EXPIRED);

 		return;

 	}

 }

 static gboolean

 _client_register_with_keys (GsdSubscriptionManager *manager,

 				  const gchar *hostname,

 				  const gchar *organisation,

 				  const gchar *activation_key,

 				  GError **error)

 {

 	GsdSubscriptionManagerPrivate *priv = manager->priv;

 	g_autoptr(GSubprocess) subprocess = NULL;

+	g_autoptr(GBytes) stdin_buf = g_bytes_new (activation_key, strlen (activation_key) + 1);

+	g_autoptr(GBytes) stderr_buf = NULL;

+	gint rc;

 	/* apparently: "we can't send registration credentials over the regular

 	 * system or session bus since those aren't really locked down..." */

 	if (!_client_register_start (manager, error))

 		return FALSE;

 	g_debug ("spawning %s", LIBEXECDIR "/gsd-subman-helper");

-	subprocess = g_subprocess_new (G_SUBPROCESS_FLAGS_STDERR_PIPE, error,

+	subprocess = g_subprocess_new (G_SUBPROCESS_FLAGS_STDIN_PIPE | G_SUBPROCESS_FLAGS_STDERR_PIPE, error,

 				       "pkexec", LIBEXECDIR "/gsd-subman-helper",

 				       "--kind", "register-with-key",

 				       "--address", priv->address,

 				       "--hostname", hostname,

 				       "--organisation", organisation,

-				       "--activation-key", activation_key,

 				       NULL);

 	if (subprocess == NULL) {

 		g_prefix_error (error, "failed to find pkexec: ");

 		return FALSE;

 	}

-	if (!_client_subprocess_wait_check (subprocess, error))

+

+	if (!g_subprocess_communicate (subprocess, stdin_buf, NULL, NULL, &stderr_buf, error)) {

+		g_prefix_error (error, "failed to run pkexec: ");

 		return FALSE;

+	}

+

+	rc = g_subprocess_get_exit_status (subprocess);

+	if (rc != 0) {

+		if (g_bytes_get_size (stderr_buf) == 0) {

+			g_set_error_literal (error, G_IO_ERROR, rc,

+			                     "Failed to run helper without stderr");

+			return FALSE;

+		}

+

+		g_set_error (error, G_IO_ERROR, rc,

+			     "%.*s",

+			     g_bytes_get_size (stderr_buf),

+			     g_bytes_get_data (stderr_buf, NULL));

+	}

 	/* FIXME: also do on error? */

 	if (!_client_register_stop (manager, error))

 		return FALSE;

 	if (!_client_subscription_status_update (manager, error))

 		return FALSE;

 	if (!_client_installed_products_update (manager, error))

 		return FALSE;

 	_client_maybe__show_notification (manager);

 	/* success */

 	return TRUE;

 }

 static gboolean

 _client_register (GsdSubscriptionManager *manager,

 			 const gchar *hostname,

 			 const gchar *organisation,

 			 const gchar *username,

 			 const gchar *password,

 			 GError **error)

 {

 	GsdSubscriptionManagerPrivate *priv = manager->priv;

 	g_autoptr(GSubprocess) subprocess = NULL;

+	g_autoptr(GBytes) stdin_buf = g_bytes_new (password, strlen (password) + 1);

+	g_autoptr(GBytes) stderr_buf = NULL;

+	gint rc;

 	/* fallback */

 	if (organisation == NULL)

 		organisation = "";

 	/* apparently: "we can't send registration credentials over the regular

 	 * system or session bus since those aren't really locked down..." */

 	if (!_client_register_start (manager, error))

 		return FALSE;

 	g_debug ("spawning %s", LIBEXECDIR "/gsd-subman-helper");

-	subprocess = g_subprocess_new (G_SUBPROCESS_FLAGS_STDERR_PIPE, error,

+	subprocess = g_subprocess_new (G_SUBPROCESS_FLAGS_STDIN_PIPE | G_SUBPROCESS_FLAGS_STDERR_PIPE,

+				       error,

 				       "pkexec", LIBEXECDIR "/gsd-subman-helper",

 				       "--kind", "register-with-username",

 				       "--address", priv->address,

 				       "--hostname", hostname,

 				       "--organisation", organisation,

 				       "--username", username,

-				       "--password", password,

 				       NULL);

 	if (subprocess == NULL) {

 		g_prefix_error (error, "failed to find pkexec: ");

 		return FALSE;

 	}

-	if (!_client_subprocess_wait_check (subprocess, error))

+

+	if (!g_subprocess_communicate (subprocess, stdin_buf, NULL, NULL, &stderr_buf, error)) {

+		g_prefix_error (error, "failed to run pkexec: ");

 		return FALSE;

+	}

+

+	rc = g_subprocess_get_exit_status (subprocess);

+	if (rc != 0) {

+		if (g_bytes_get_size (stderr_buf) == 0) {

+			g_set_error_literal (error, G_IO_ERROR, rc,

+			                     "Failed to run helper without stderr");

+			return FALSE;

+		}

+

+		g_set_error (error, G_IO_ERROR, rc,

+			     "%.*s",

+			     g_bytes_get_size (stderr_buf),

+			     g_bytes_get_data (stderr_buf, NULL));

+	}

 	/* FIXME: also do on error? */

 	if (!_client_register_stop (manager, error))

 		return FALSE;

 	if (!_client_subscription_status_update (manager, error))

 		return FALSE;

 	if (!_client_installed_products_update (manager, error))

 		return FALSE;

 	_client_maybe__show_notification (manager);

 	return TRUE;

 }

 static gboolean

 _client_unregister (GsdSubscriptionManager *manager, GError **error)

 {

 	g_autoptr(GSubprocess) subprocess = NULL;

 	/* apparently: "we can't send registration credentials over the regular

 	 * system or session bus since those aren't really locked down..." */

 	if (!_client_register_start (manager, error))

 		return FALSE;

 	g_debug ("spawning %s", LIBEXECDIR "/gsd-subman-helper");

 	subprocess = g_subprocess_new (G_SUBPROCESS_FLAGS_STDERR_PIPE, error,

 				       "pkexec", LIBEXECDIR "/gsd-subman-helper",

 				       "--kind", "unregister",

 				       NULL);

 	if (subprocess == NULL) {

 		g_prefix_error (error, "failed to find pkexec: ");

 		return FALSE;

 	}

diff --git a/plugins/subman/meson.build b/plugins/subman/meson.build

index bfd073b6..e4b4589d 100644

--- a/plugins/subman/meson.build

+++ b/plugins/subman/meson.build

@@ -22,35 +22,35 @@ executable(

   c_args: cflags,

   install: true,

   install_rpath: gsd_pkglibdir,

   install_dir: gsd_libexecdir

 )

 # .Register needs to be called from root as subman can't do PolicyKit...

 policy = 'org.gnome.settings-daemon.plugins.subman.policy'

 policy_in = configure_file(

   input: policy + '.in.in',

   output: policy + '.in',

   configuration: plugins_conf

 )

 i18n.merge_file(

   policy,

   input: policy_in,

   output: policy,

   po_dir: po_dir,

   install: true,

   install_dir: join_paths(gsd_datadir, 'polkit-1', 'actions')

 )

 install_data('org.gnome.settings-daemon.plugins.subman.rules',

              install_dir : join_paths(gsd_datadir, 'polkit-1', 'rules.d'))

 executable(

   'gsd-subman-helper',

   'gsd-subman-helper.c',

   include_directories: top_inc,

-  dependencies: [gio_dep, jsonglib_dep],

+  dependencies: [gio_dep, gio_unix_dep, jsonglib_dep],

   install: true,

   install_rpath: gsd_pkglibdir,

   install_dir: gsd_libexecdir

 )

-- 

2.30.0