[gnome-settings-daemon/rhel/account-and-subman-plugins: 14/14] subman: Don't send secrets through command line

[Ray Strode <halfline src gnome org>]

commit 221154acc5c6e037ed212af4f97a76b188af7f10
Author: Ray Strode <rstrode redhat com>
Date:   Tue Aug 25 16:20:42 2020 -0400

    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        | 33 +++++++++++---------
 plugins/subman/gsd-subscription-manager.c | 52 +++++++++++++++++++++++++++----
 plugins/subman/meson.build                |  2 +-
 3 files changed, 66 insertions(+), 21 deletions(-)
---
diff --git a/plugins/subman/gsd-subman-helper.c b/plugins/subman/gsd-subman-helper.c
index 442041b8..7f8d93e2 100644
--- a/plugins/subman/gsd-subman-helper.c
+++ b/plugins/subman/gsd-subman-helper.c
@@ -21,11 +21,13 @@
 
 #include "config.h"
 
+
 #include <sys/types.h>
 #include <unistd.h>
 #include <stdlib.h>
 
 #include <gio/gio.h>
+#include <gio/gunixinputstream.h>
 #include <json-glib/json-glib.h>
 
 #define DBUS_TIMEOUT 300000 /* 5 minutes */
@@ -170,12 +172,10 @@ int
 main (int argc, char *argv[])
 {
        const gchar *userlang = ""; /* as root, so no translations */
-       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;
@@ -187,6 +187,7 @@ main (int argc, char *argv[])
        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,
@@ -195,12 +196,8 @@ main (int argc, char *argv[])
                        &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,
@@ -289,16 +286,20 @@ main (int argc, char *argv[])
                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);
 
@@ -324,20 +325,24 @@ main (int argc, char *argv[])
        } 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);
 
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
@@ -21,6 +21,7 @@
 #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>
@@ -544,26 +545,45 @@ _client_register_with_keys (GsdSubscriptionManager *manager,
 {
        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))
@@ -588,6 +608,9 @@ _client_register (GsdSubscriptionManager *manager,
 {
        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)
@@ -598,21 +621,38 @@ _client_register (GsdSubscriptionManager *manager,
        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))
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
@@ -49,7 +49,7 @@ 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