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

[Ray Strode <halfline src gnome org>]

commit 9644be082bb676c84064b5b91b3ee1444f9b9031
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        | 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
@@ -21,12 +21,14 @@
 
 #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 */
@@ -176,7 +178,6 @@ main (int argc, char *argv[])
        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;
@@ -188,6 +189,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,
@@ -196,12 +198,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,
@@ -294,16 +292,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);
 
@@ -329,20 +331,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