From 4be6cb23235b29d6ce450c2dacaef09c52d1aeea Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= <marcandre.lureau@redhat.com>

Date: Thu, 29 Jul 2021 04:55:52 -0400

Subject: [PATCH 02/14] qga: add ssh-{add, remove}-authorized-keys

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

RH-Author: Marc-André Lureau <marcandre.lureau@redhat.com>

Message-id: <20210609100615.2501448-3-marcandre.lureau@redhat.com>

Patchwork-id: 101688

O-Subject: [RHEL-8.5.0 qemu-kvm PATCH 2/4] qga: add ssh-{add, remove}-authorized-keys

Bugzilla: 1967716

RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>

RH-Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>

RH-Acked-by: Michal Privoznik <mprivozn@redhat.com>

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Add new commands to add and remove SSH public keys from

~/.ssh/authorized_keys.

I took a different approach for testing, including the unit tests right

with the code. I wanted to overwrite the function to get the user

details, I couldn't easily do that over QMP. Furthermore, I prefer

having unit tests very close to the code, and unit files that are domain

specific (commands-posix is too crowded already). FWIW, that

coding/testing style is Rust-style (where tests can or should even be

part of the documentation!).

Fixes:

https://bugzilla.redhat.com/show_bug.cgi?id=1885332

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

*squashed in fix-ups for setting file ownership and use of QAPI

 conditionals for CONFIG_POSIX instead of stub definitions

*disable qga-ssh-test for now due to G_TEST_OPTION_ISOLATE_DIRS

 triggering leak detector in build-oss-fuzz

*fix disallowed g_assert* usage reported by checkpatch

Signed-off-by: Michael Roth <michael.roth@amd.com>

(cherry picked from commit 8d769ec777dccbff199711aba43aa6297fe4a0e0)

[ Fixes trivial backport conflicts and use Makefile.objs build-sys ]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

---

 qga/Makefile.objs        |   2 +-

 qga/commands-posix-ssh.c | 407 +++++++++++++++++++++++++++++++++++++++

 qga/qapi-schema.json     |  35 ++++

 3 files changed, 443 insertions(+), 1 deletion(-)

 create mode 100644 qga/commands-posix-ssh.c

diff --git a/qga/Makefile.objs b/qga/Makefile.objs

index 80e6bb3c2e..c8da634db0 100644

--- a/qga/Makefile.objs

+++ b/qga/Makefile.objs

@@ -1,6 +1,6 @@

 commands-posix.o-libs := $(LIBUDEV_LIBS)

 qga-obj-y = commands.o guest-agent-command-state.o main.o

-qga-obj-$(CONFIG_POSIX) += commands-posix.o channel-posix.o

+qga-obj-$(CONFIG_POSIX) += commands-posix.o channel-posix.o commands-posix-ssh.o

 qga-obj-$(CONFIG_WIN32) += commands-win32.o channel-win32.o service-win32.o

 qga-obj-$(CONFIG_WIN32) += vss-win32.o

 qga-obj-y += qapi-generated/qga-qapi-types.o qapi-generated/qga-qapi-visit.o

diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c

new file mode 100644

index 0000000000..f74d89679c

--- /dev/null

+++ b/qga/commands-posix-ssh.c

@@ -0,0 +1,407 @@

+ /*

+  * This work is licensed under the terms of the GNU GPL, version 2 or later.

+  * See the COPYING file in the top-level directory.

+  */

+#include "qemu/osdep.h"

+

+#include <glib-unix.h>

+#include <glib/gstdio.h>

+#include <locale.h>

+#include <pwd.h>

+

+#include "qapi/error.h"

+#include "qga-qapi-commands.h"

+

+#ifdef QGA_BUILD_UNIT_TEST

+static struct passwd *

+test_get_passwd_entry(const gchar *user_name, GError **error)

+{

+    struct passwd *p;

+    int ret;

+

+    if (!user_name || g_strcmp0(user_name, g_get_user_name())) {

+        g_set_error(error, G_UNIX_ERROR, 0, "Invalid user name");

+        return NULL;

+    }

+

+    p = g_new0(struct passwd, 1);

+    p->pw_dir = (char *)g_get_home_dir();

+    p->pw_uid = geteuid();

+    p->pw_gid = getegid();

+

+    ret = g_mkdir_with_parents(p->pw_dir, 0700);

+    g_assert(ret == 0);

+

+    return p;

+}

+

+#define g_unix_get_passwd_entry_qemu(username, err) \

+   test_get_passwd_entry(username, err)

+#endif

+

+static struct passwd *

+get_passwd_entry(const char *username, Error **errp)

+{

+    g_autoptr(GError) err = NULL;

+    struct passwd *p;

+

+    ERRP_GUARD();

+

+    p = g_unix_get_passwd_entry_qemu(username, &err;;

+    if (p == NULL) {

+        error_setg(errp, "failed to lookup user '%s': %s",

+                   username, err->message);

+        return NULL;

+    }

+

+    return p;

+}

+

+static bool

+mkdir_for_user(const char *path, const struct passwd *p,

+               mode_t mode, Error **errp)

+{

+    ERRP_GUARD();

+

+    if (g_mkdir(path, mode) == -1) {

+        error_setg(errp, "failed to create directory '%s': %s",

+                   path, g_strerror(errno));

+        return false;

+    }

+

+    if (chown(path, p->pw_uid, p->pw_gid) == -1) {

+        error_setg(errp, "failed to set ownership of directory '%s': %s",

+                   path, g_strerror(errno));

+        return false;

+    }

+

+    if (chmod(path, mode) == -1) {

+        error_setg(errp, "failed to set permissions of directory '%s': %s",

+                   path, g_strerror(errno));

+        return false;

+    }

+

+    return true;

+}

+

+static bool

+check_openssh_pub_key(const char *key, Error **errp)

+{

+    ERRP_GUARD();

+

+    /* simple sanity-check, we may want more? */

+    if (!key || key[0] == '#' || strchr(key, '\n')) {

+        error_setg(errp, "invalid OpenSSH public key: '%s'", key);

+        return false;

+    }

+

+    return true;

+}

+

+static bool

+check_openssh_pub_keys(strList *keys, size_t *nkeys, Error **errp)

+{

+    size_t n = 0;

+    strList *k;

+

+    ERRP_GUARD();

+

+    for (k = keys; k != NULL; k = k->next) {

+        if (!check_openssh_pub_key(k->value, errp)) {

+            return false;

+        }

+        n++;

+    }

+

+    if (nkeys) {

+        *nkeys = n;

+    }

+    return true;

+}

+

+static bool

+write_authkeys(const char *path, const GStrv keys,

+               const struct passwd *p, Error **errp)

+{

+    g_autofree char *contents = NULL;

+    g_autoptr(GError) err = NULL;

+

+    ERRP_GUARD();

+

+    contents = g_strjoinv("\n", keys);

+    if (!g_file_set_contents(path, contents, -1, &err)) {

+        error_setg(errp, "failed to write to '%s': %s", path, err->message);

+        return false;

+    }

+

+    if (chown(path, p->pw_uid, p->pw_gid) == -1) {

+        error_setg(errp, "failed to set ownership of directory '%s': %s",

+                   path, g_strerror(errno));

+        return false;

+    }

+

+    if (chmod(path, 0600) == -1) {

+        error_setg(errp, "failed to set permissions of '%s': %s",

+                   path, g_strerror(errno));

+        return false;

+    }

+

+    return true;

+}

+

+static GStrv

+read_authkeys(const char *path, Error **errp)

+{

+    g_autoptr(GError) err = NULL;

+    g_autofree char *contents = NULL;

+

+    ERRP_GUARD();

+

+    if (!g_file_get_contents(path, &contents, NULL, &err)) {

+        error_setg(errp, "failed to read '%s': %s", path, err->message);

+        return NULL;

+    }

+

+    return g_strsplit(contents, "\n", -1);

+

+}

+

+void

+qmp_guest_ssh_add_authorized_keys(const char *username, strList *keys,

+                                  Error **errp)

+{

+    g_autofree struct passwd *p = NULL;

+    g_autofree char *ssh_path = NULL;

+    g_autofree char *authkeys_path = NULL;

+    g_auto(GStrv) authkeys = NULL;

+    strList *k;

+    size_t nkeys, nauthkeys;

+

+    ERRP_GUARD();

+

+    if (!check_openssh_pub_keys(keys, &nkeys, errp)) {

+        return;

+    }

+

+    p = get_passwd_entry(username, errp);

+    if (p == NULL) {

+        return;

+    }

+

+    ssh_path = g_build_filename(p->pw_dir, ".ssh", NULL);

+    authkeys_path = g_build_filename(ssh_path, "authorized_keys", NULL);

+

+    authkeys = read_authkeys(authkeys_path, NULL);

+    if (authkeys == NULL) {

+        if (!g_file_test(ssh_path, G_FILE_TEST_IS_DIR) &&

+            !mkdir_for_user(ssh_path, p, 0700, errp)) {

+            return;

+        }

+    }

+

+    nauthkeys = authkeys ? g_strv_length(authkeys) : 0;

+    authkeys = g_realloc_n(authkeys, nauthkeys + nkeys + 1, sizeof(char *));

+    memset(authkeys + nauthkeys, 0, (nkeys + 1) * sizeof(char *));

+

+    for (k = keys; k != NULL; k = k->next) {

+        if (g_strv_contains((const gchar * const *)authkeys, k->value)) {

+            continue;

+        }

+        authkeys[nauthkeys++] = g_strdup(k->value);

+    }

+

+    write_authkeys(authkeys_path, authkeys, p, errp);

+}

+

+void

+qmp_guest_ssh_remove_authorized_keys(const char *username, strList *keys,

+                                     Error **errp)

+{

+    g_autofree struct passwd *p = NULL;

+    g_autofree char *authkeys_path = NULL;

+    g_autofree GStrv new_keys = NULL; /* do not own the strings */

+    g_auto(GStrv) authkeys = NULL;

+    GStrv a;

+    size_t nkeys = 0;

+

+    ERRP_GUARD();

+

+    if (!check_openssh_pub_keys(keys, NULL, errp)) {

+        return;

+    }

+

+    p = get_passwd_entry(username, errp);

+    if (p == NULL) {

+        return;

+    }

+

+    authkeys_path = g_build_filename(p->pw_dir, ".ssh",

+                                     "authorized_keys", NULL);

+    if (!g_file_test(authkeys_path, G_FILE_TEST_EXISTS)) {

+        return;

+    }

+    authkeys = read_authkeys(authkeys_path, errp);

+    if (authkeys == NULL) {

+        return;

+    }

+

+    new_keys = g_new0(char *, g_strv_length(authkeys) + 1);

+    for (a = authkeys; *a != NULL; a++) {

+        strList *k;

+

+        for (k = keys; k != NULL; k = k->next) {

+            if (g_str_equal(k->value, *a)) {

+                break;

+            }

+        }

+        if (k != NULL) {

+            continue;

+        }

+

+        new_keys[nkeys++] = *a;

+    }

+

+    write_authkeys(authkeys_path, new_keys, p, errp);

+}

+

+

+#ifdef QGA_BUILD_UNIT_TEST

+#if GLIB_CHECK_VERSION(2, 60, 0)

+static const strList test_key2 = {

+    .value = (char *)"algo key2 comments"

+};

+

+static const strList test_key1_2 = {

+    .value = (char *)"algo key1 comments",

+    .next = (strList *)&test_key2,

+};

+

+static char *

+test_get_authorized_keys_path(void)

+{

+    return g_build_filename(g_get_home_dir(), ".ssh", "authorized_keys", NULL);

+}

+

+static void

+test_authorized_keys_set(const char *contents)

+{

+    g_autoptr(GError) err = NULL;

+    g_autofree char *path = NULL;

+    int ret;

+

+    path = g_build_filename(g_get_home_dir(), ".ssh", NULL);

+    ret = g_mkdir_with_parents(path, 0700);

+    g_assert(ret == 0);

+    g_free(path);

+

+    path = test_get_authorized_keys_path();

+    g_file_set_contents(path, contents, -1, &err;;

+    g_assert(err == NULL);

+}

+

+static void

+test_authorized_keys_equal(const char *expected)

+{

+    g_autoptr(GError) err = NULL;

+    g_autofree char *path = NULL;

+    g_autofree char *contents = NULL;

+

+    path = test_get_authorized_keys_path();

+    g_file_get_contents(path, &contents, NULL, &err;;

+    g_assert(err == NULL);

+

+    g_assert(g_strcmp0(contents, expected) == 0);

+}

+

+static void

+test_invalid_user(void)

+{

+    Error *err = NULL;

+

+    qmp_guest_ssh_add_authorized_keys("", NULL, &err;;

+    error_free_or_abort(&err;;

+

+    qmp_guest_ssh_remove_authorized_keys("", NULL, &err;;

+    error_free_or_abort(&err;;

+}

+

+static void

+test_invalid_key(void)

+{

+    strList key = {

+        .value = (char *)"not a valid\nkey"

+    };

+    Error *err = NULL;

+

+    qmp_guest_ssh_add_authorized_keys(g_get_user_name(), &key, &err;;

+    error_free_or_abort(&err;;

+

+    qmp_guest_ssh_remove_authorized_keys(g_get_user_name(), &key, &err;;

+    error_free_or_abort(&err;;

+}

+

+static void

+test_add_keys(void)

+{

+    Error *err = NULL;

+

+    qmp_guest_ssh_add_authorized_keys(g_get_user_name(),

+                                      (strList *)&test_key2, &err;;

+    g_assert(err == NULL);

+

+    test_authorized_keys_equal("algo key2 comments");

+

+    qmp_guest_ssh_add_authorized_keys(g_get_user_name(),

+                                      (strList *)&test_key1_2, &err;;

+    g_assert(err == NULL);

+

+    /*  key2 came first, and should'nt be duplicated */

+    test_authorized_keys_equal("algo key2 comments\n"

+                               "algo key1 comments");

+}

+

+static void

+test_remove_keys(void)

+{

+    Error *err = NULL;

+    static const char *authkeys =

+        "algo key1 comments\n"

+        /* originally duplicated */

+        "algo key1 comments\n"

+        "# a commented line\n"

+        "algo some-key another\n";

+

+    test_authorized_keys_set(authkeys);

+    qmp_guest_ssh_remove_authorized_keys(g_get_user_name(),

+                                         (strList *)&test_key2, &err;;

+    g_assert(err == NULL);

+    test_authorized_keys_equal(authkeys);

+

+    qmp_guest_ssh_remove_authorized_keys(g_get_user_name(),

+                                         (strList *)&test_key1_2, &err;;

+    g_assert(err == NULL);

+    test_authorized_keys_equal("# a commented line\n"

+                               "algo some-key another\n");

+}

+

+int main(int argc, char *argv[])

+{

+    setlocale(LC_ALL, "");

+

+    g_test_init(&argc, &argv, G_TEST_OPTION_ISOLATE_DIRS, NULL);

+

+    g_test_add_func("/qga/ssh/invalid_user", test_invalid_user);

+    g_test_add_func("/qga/ssh/invalid_key", test_invalid_key);

+    g_test_add_func("/qga/ssh/add_keys", test_add_keys);

+    g_test_add_func("/qga/ssh/remove_keys", test_remove_keys);

+

+    return g_test_run();

+}

+#else

+int main(int argc, char *argv[])

+{

+    g_test_message("test skipped, needs glib >= 2.60");

+    return 0;

+}

+#endif /* GLIB_2_60 */

+#endif /* BUILD_UNIT_TEST */

diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json

index 4222cb92d3..3b85f5a03f 100644

--- a/qga/qapi-schema.json

+++ b/qga/qapi-schema.json

@@ -1273,3 +1273,38 @@

 ##

 { 'command': 'guest-get-osinfo',

   'returns': 'GuestOSInfo' }

+

+##

+# @guest-ssh-add-authorized-keys:

+#

+# @username: the user account to add the authorized keys

+# @keys: the public keys to add (in OpenSSH/sshd(8) authorized_keys format)

+#

+# Append public keys to user .ssh/authorized_keys on Unix systems (not

+# implemented for other systems).

+#

+# Returns: Nothing on success.

+#

+# Since: 5.2

+##

+{ 'command': 'guest-ssh-add-authorized-keys',

+  'data': { 'username': 'str', 'keys': ['str'] },

+  'if': 'defined(CONFIG_POSIX)' }

+

+##

+# @guest-ssh-remove-authorized-keys:

+#

+# @username: the user account to remove the authorized keys

+# @keys: the public keys to remove (in OpenSSH/sshd(8) authorized_keys format)

+#

+# Remove public keys from the user .ssh/authorized_keys on Unix systems (not

+# implemented for other systems). It's not an error if the key is already

+# missing.

+#

+# Returns: Nothing on success.

+#

+# Since: 5.2

+##

+{ 'command': 'guest-ssh-remove-authorized-keys',

+  'data': { 'username': 'str', 'keys': ['str'] },

+  'if': 'defined(CONFIG_POSIX)' }

-- 

2.27.0