Re: [PATCH v3 2/2] Implement SSH commands in QEMU GA for Windows
On 27/3/24 16:54, Aidan Leuck wrote:
On 22/3/24 18:46, aidan_le...@selinc.com wrote:
From: Aidan Leuck
Signed-off-by: Aidan Leuck
---
qga/commands-windows-ssh.c | 791
+
Huge file, I'm skipping it.
qga/commands-windows-ssh.h | 26 ++
qga/meson.build| 5 +-
qga/qapi-schema.json | 17 +-
4 files changed, 828 insertions(+), 11 deletions(-)
create mode 100644 qga/commands-windows-ssh.c
create mode 100644 qga/commands-windows-ssh.h
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index
9554b566a7..a64a6d91cf 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1562,9 +1562,8 @@
{ 'struct': 'GuestAuthorizedKeys',
'data': {
'keys': ['str']
- },
- 'if': 'CONFIG_POSIX' }
-
For Windows you have to check the CONFIG_WIN32 definition, so you want:
I don't think this is necessary, the QEMU guest agent is compiled for only
POSIX and Windows. I don't see this pattern being used elsewhere in the qapi
schema file. I would be interested in what the maintainers think?
$ git grep -w CONFIG_WIN32 qapi/
qapi/char.json:490:{ 'name': 'console', 'if': 'CONFIG_WIN32' },
qapi/char.json:663: 'if': 'CONFIG_WIN32' },
qapi/misc.json:293:{ 'command': 'get-win32-socket', 'data': {'info':
'str', 'fdname': 'str'}, 'if': 'CONFIG_WIN32' }
'if': { 'any': [ 'CONFIG_POSIX',
'CONFIG_WIN32' ] },
+ }
+}
##
# @guest-ssh-get-authorized-keys:
@@ -1580,8 +1579,8 @@
##
{ 'command': 'guest-ssh-get-authorized-keys',
'data': { 'username': 'str' },
- 'returns': 'GuestAuthorizedKeys',
- 'if': 'CONFIG_POSIX' }
+ 'returns': 'GuestAuthorizedKeys'
+}
##
# @guest-ssh-add-authorized-keys:
@@ -1599,8 +1598,8 @@
# Since: 5.2
##
{ 'command': 'guest-ssh-add-authorized-keys',
- 'data': { 'username': 'str', 'keys': ['str'], '*reset': 'bool' },
- 'if': 'CONFIG_POSIX' }
+ 'data': { 'username': 'str', 'keys': ['str'], '*reset': 'bool' } }
##
# @guest-ssh-remove-authorized-keys:
@@ -1617,8 +1616,8 @@
# Since: 5.2
##
{ 'command': 'guest-ssh-remove-authorized-keys',
- 'data': { 'username': 'str', 'keys': ['str'] },
- 'if': 'CONFIG_POSIX' }
+ 'data': { 'username': 'str', 'keys': ['str'] } }
##
# @GuestDiskStats:
Hi Philippe, thank you for getting back to me so quickly. Looking at the grep
you gave me seems to confirm what I was saying if I am not mistaken? It looks
like CONFIG_WIN32 and CONFIG_POSIX if conditionals are only used when you need
to enable a command on one operating system and not the other. I do believe
that your code snippet is correct it is just overly verbose. The QGA has both
windows and SSH implementations and looking at the guest agent QAPI file when a
command supports both POSIX and Windows the if gate is removed. I am happy to
discuss this further if you have more concerns.
Well, as you said, up to the maintainers.
Regards,
Phil.
RE: [PATCH v3 2/2] Implement SSH commands in QEMU GA for Windows
-Original Message-
From: Philippe Mathieu-Daudé
Sent: Wednesday, March 27, 2024 9:38 AM
To: Aidan Leuck ; qemu-devel@nongnu.org; Markus
Armbruster
Cc: kkost...@redhat.com; berra...@redhat.com
Subject: Re: [PATCH v3 2/2] Implement SSH commands in QEMU GA for Windows
[Caution - External]
On 27/3/24 15:38, Aidan Leuck wrote:
> Hi Philippe,
> Thank you for your feedback I will get these issues addressed. I left one
> small comment on the QAPI schema JSON.
>
> Aidan Leuck
>
> -Original Message-
> From: Philippe Mathieu-Daudé
> Sent: Monday, March 25, 2024 11:51 AM
> To: Aidan Leuck ; qemu-devel@nongnu.org
> Cc: kkost...@redhat.com; berra...@redhat.com
> Subject: Re: [PATCH v3 2/2] Implement SSH commands in QEMU GA for
> Windows
>
> [Caution - External]
>
> On 22/3/24 18:46, aidan_le...@selinc.com wrote:
>> From: Aidan Leuck
>>
>> Signed-off-by: Aidan Leuck
>> ---
>>qga/commands-windows-ssh.c | 791
>> +
>
> Huge file, I'm skipping it.
>
>>qga/commands-windows-ssh.h | 26 ++
>>qga/meson.build| 5 +-
>>qga/qapi-schema.json | 17 +-
>>4 files changed, 828 insertions(+), 11 deletions(-)
>>create mode 100644 qga/commands-windows-ssh.c
>>create mode 100644 qga/commands-windows-ssh.h
>
>
>> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index
>> 9554b566a7..a64a6d91cf 100644
>> --- a/qga/qapi-schema.json
>> +++ b/qga/qapi-schema.json
>> @@ -1562,9 +1562,8 @@
>>{ 'struct': 'GuestAuthorizedKeys',
>> 'data': {
>> 'keys': ['str']
>> - },
>> - 'if': 'CONFIG_POSIX' }
>> -
>
> For Windows you have to check the CONFIG_WIN32 definition, so you want:
>
> I don't think this is necessary, the QEMU guest agent is compiled for only
> POSIX and Windows. I don't see this pattern being used elsewhere in the qapi
> schema file. I would be interested in what the maintainers think?
$ git grep -w CONFIG_WIN32 qapi/
qapi/char.json:490:{ 'name': 'console', 'if': 'CONFIG_WIN32' },
qapi/char.json:663: 'if': 'CONFIG_WIN32' },
qapi/misc.json:293:{ 'command': 'get-win32-socket', 'data': {'info':
'str', 'fdname': 'str'}, 'if': 'CONFIG_WIN32' }
>
> 'if': { 'any': [ 'CONFIG_POSIX',
> 'CONFIG_WIN32' ] },
>
>> + }
>> +}
>>
>>##
>># @guest-ssh-get-authorized-keys:
>> @@ -1580,8 +1579,8 @@
>>##
>>{ 'command': 'guest-ssh-get-authorized-keys',
>> 'data': { 'username': 'str' },
>> - 'returns': 'GuestAuthorizedKeys',
>> - 'if': 'CONFIG_POSIX' }
>> + 'returns': 'GuestAuthorizedKeys'
>> +}
>>
>>##
>># @guest-ssh-add-authorized-keys:
>> @@ -1599,8 +1598,8 @@
>># Since: 5.2
>>##
>>{ 'command': 'guest-ssh-add-authorized-keys',
>> - 'data': { 'username': 'str', 'keys': ['str'], '*reset': 'bool' },
>> - 'if': 'CONFIG_POSIX' }
>> + 'data': { 'username': 'str', 'keys': ['str'], '*reset': 'bool' } }
>>
>>##
>># @guest-ssh-remove-authorized-keys:
>> @@ -1617,8 +1616,8 @@
>># Since: 5.2
>>##
>>{ 'command': 'guest-ssh-remove-authorized-keys',
>> - 'data': { 'username': 'str', 'keys': ['str'] },
>> - 'if': 'CONFIG_POSIX' }
>> + 'data': { 'username': 'str', 'keys': ['str'] } }
>>
>>##
>># @GuestDiskStats:
>
Hi Philippe, thank you for getting back to me so quickly. Looking at the grep
you gave me seems to confirm what I was saying if I am not mistaken? It looks
like CONFIG_WIN32 and CONFIG_POSIX if conditionals are only used when you need
to enable a command on one operating system and not the other. I do believe
that your code snippet is correct it is just overly verbose. The QGA has both
windows and SSH implementations and looking at the guest agent QAPI file when a
command supports both POSIX and Windows the if gate is removed. I am happy to
discuss this further if you have more concerns.
Re: [PATCH v3 2/2] Implement SSH commands in QEMU GA for Windows
On 27/3/24 15:38, Aidan Leuck wrote:
Hi Philippe,
Thank you for your feedback I will get these issues addressed. I left one small
comment on the QAPI schema JSON.
Aidan Leuck
-Original Message-
From: Philippe Mathieu-Daudé
Sent: Monday, March 25, 2024 11:51 AM
To: Aidan Leuck ; qemu-devel@nongnu.org
Cc: kkost...@redhat.com; berra...@redhat.com
Subject: Re: [PATCH v3 2/2] Implement SSH commands in QEMU GA for Windows
[Caution - External]
On 22/3/24 18:46, aidan_le...@selinc.com wrote:
From: Aidan Leuck
Signed-off-by: Aidan Leuck
---
qga/commands-windows-ssh.c | 791
+
Huge file, I'm skipping it.
qga/commands-windows-ssh.h | 26 ++
qga/meson.build| 5 +-
qga/qapi-schema.json | 17 +-
4 files changed, 828 insertions(+), 11 deletions(-)
create mode 100644 qga/commands-windows-ssh.c
create mode 100644 qga/commands-windows-ssh.h
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index
9554b566a7..a64a6d91cf 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1562,9 +1562,8 @@
{ 'struct': 'GuestAuthorizedKeys',
'data': {
'keys': ['str']
- },
- 'if': 'CONFIG_POSIX' }
-
For Windows you have to check the CONFIG_WIN32 definition, so you want:
I don't think this is necessary, the QEMU guest agent is compiled for only
POSIX and Windows. I don't see this pattern being used elsewhere in the qapi
schema file. I would be interested in what the maintainers think?
$ git grep -w CONFIG_WIN32 qapi/
qapi/char.json:490:{ 'name': 'console', 'if': 'CONFIG_WIN32' },
qapi/char.json:663: 'if': 'CONFIG_WIN32' },
qapi/misc.json:293:{ 'command': 'get-win32-socket', 'data': {'info':
'str', 'fdname': 'str'}, 'if': 'CONFIG_WIN32' }
'if': { 'any': [ 'CONFIG_POSIX',
'CONFIG_WIN32' ] },
+ }
+}
##
# @guest-ssh-get-authorized-keys:
@@ -1580,8 +1579,8 @@
##
{ 'command': 'guest-ssh-get-authorized-keys',
'data': { 'username': 'str' },
- 'returns': 'GuestAuthorizedKeys',
- 'if': 'CONFIG_POSIX' }
+ 'returns': 'GuestAuthorizedKeys'
+}
##
# @guest-ssh-add-authorized-keys:
@@ -1599,8 +1598,8 @@
# Since: 5.2
##
{ 'command': 'guest-ssh-add-authorized-keys',
- 'data': { 'username': 'str', 'keys': ['str'], '*reset': 'bool' },
- 'if': 'CONFIG_POSIX' }
+ 'data': { 'username': 'str', 'keys': ['str'], '*reset': 'bool' } }
##
# @guest-ssh-remove-authorized-keys:
@@ -1617,8 +1616,8 @@
# Since: 5.2
##
{ 'command': 'guest-ssh-remove-authorized-keys',
- 'data': { 'username': 'str', 'keys': ['str'] },
- 'if': 'CONFIG_POSIX' }
+ 'data': { 'username': 'str', 'keys': ['str'] } }
##
# @GuestDiskStats:
RE: [PATCH v3 2/2] Implement SSH commands in QEMU GA for Windows
Hi Philippe,
Thank you for your feedback I will get these issues addressed. I left one small
comment on the QAPI schema JSON.
Aidan Leuck
-Original Message-
From: Philippe Mathieu-Daudé
Sent: Monday, March 25, 2024 11:51 AM
To: Aidan Leuck ; qemu-devel@nongnu.org
Cc: kkost...@redhat.com; berra...@redhat.com
Subject: Re: [PATCH v3 2/2] Implement SSH commands in QEMU GA for Windows
[Caution - External]
On 22/3/24 18:46, aidan_le...@selinc.com wrote:
> From: Aidan Leuck
>
> Signed-off-by: Aidan Leuck
> ---
> qga/commands-windows-ssh.c | 791
> +
Huge file, I'm skipping it.
> qga/commands-windows-ssh.h | 26 ++
> qga/meson.build| 5 +-
> qga/qapi-schema.json | 17 +-
> 4 files changed, 828 insertions(+), 11 deletions(-)
> create mode 100644 qga/commands-windows-ssh.c
> create mode 100644 qga/commands-windows-ssh.h
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index
> 9554b566a7..a64a6d91cf 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -1562,9 +1562,8 @@
> { 'struct': 'GuestAuthorizedKeys',
> 'data': {
> 'keys': ['str']
> - },
> - 'if': 'CONFIG_POSIX' }
> -
For Windows you have to check the CONFIG_WIN32 definition, so you want:
I don't think this is necessary, the QEMU guest agent is compiled for only
POSIX and Windows. I don't see this pattern being used elsewhere in the qapi
schema file. I would be interested in what the maintainers think?
'if': { 'any': [ 'CONFIG_POSIX',
'CONFIG_WIN32' ] },
> + }
> +}
>
> ##
> # @guest-ssh-get-authorized-keys:
> @@ -1580,8 +1579,8 @@
> ##
> { 'command': 'guest-ssh-get-authorized-keys',
> 'data': { 'username': 'str' },
> - 'returns': 'GuestAuthorizedKeys',
> - 'if': 'CONFIG_POSIX' }
> + 'returns': 'GuestAuthorizedKeys'
> +}
>
> ##
> # @guest-ssh-add-authorized-keys:
> @@ -1599,8 +1598,8 @@
> # Since: 5.2
> ##
> { 'command': 'guest-ssh-add-authorized-keys',
> - 'data': { 'username': 'str', 'keys': ['str'], '*reset': 'bool' },
> - 'if': 'CONFIG_POSIX' }
> + 'data': { 'username': 'str', 'keys': ['str'], '*reset': 'bool' } }
>
> ##
> # @guest-ssh-remove-authorized-keys:
> @@ -1617,8 +1616,8 @@
> # Since: 5.2
> ##
> { 'command': 'guest-ssh-remove-authorized-keys',
> - 'data': { 'username': 'str', 'keys': ['str'] },
> - 'if': 'CONFIG_POSIX' }
> + 'data': { 'username': 'str', 'keys': ['str'] } }
>
> ##
> # @GuestDiskStats:
Re: [PATCH v3 2/2] Implement SSH commands in QEMU GA for Windows
On 22/3/24 18:46, aidan_le...@selinc.com wrote:
From: Aidan Leuck
Signed-off-by: Aidan Leuck
---
qga/commands-windows-ssh.c | 791 +
Huge file, I'm skipping it.
qga/commands-windows-ssh.h | 26 ++
qga/meson.build| 5 +-
qga/qapi-schema.json | 17 +-
4 files changed, 828 insertions(+), 11 deletions(-)
create mode 100644 qga/commands-windows-ssh.c
create mode 100644 qga/commands-windows-ssh.h
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 9554b566a7..a64a6d91cf 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1562,9 +1562,8 @@
{ 'struct': 'GuestAuthorizedKeys',
'data': {
'keys': ['str']
- },
- 'if': 'CONFIG_POSIX' }
-
For Windows you have to check the CONFIG_WIN32 definition,
so you want:
'if': { 'any': [ 'CONFIG_POSIX',
'CONFIG_WIN32' ] },
+ }
+}
##
# @guest-ssh-get-authorized-keys:
@@ -1580,8 +1579,8 @@
##
{ 'command': 'guest-ssh-get-authorized-keys',
'data': { 'username': 'str' },
- 'returns': 'GuestAuthorizedKeys',
- 'if': 'CONFIG_POSIX' }
+ 'returns': 'GuestAuthorizedKeys'
+}
##
# @guest-ssh-add-authorized-keys:
@@ -1599,8 +1598,8 @@
# Since: 5.2
##
{ 'command': 'guest-ssh-add-authorized-keys',
- 'data': { 'username': 'str', 'keys': ['str'], '*reset': 'bool' },
- 'if': 'CONFIG_POSIX' }
+ 'data': { 'username': 'str', 'keys': ['str'], '*reset': 'bool' }
+}
##
# @guest-ssh-remove-authorized-keys:
@@ -1617,8 +1616,8 @@
# Since: 5.2
##
{ 'command': 'guest-ssh-remove-authorized-keys',
- 'data': { 'username': 'str', 'keys': ['str'] },
- 'if': 'CONFIG_POSIX' }
+ 'data': { 'username': 'str', 'keys': ['str'] }
+}
##
# @GuestDiskStats:
[PATCH v3 2/2] Implement SSH commands in QEMU GA for Windows
From: Aidan Leuck
Signed-off-by: Aidan Leuck
---
qga/commands-windows-ssh.c | 791 +
qga/commands-windows-ssh.h | 26 ++
qga/meson.build| 5 +-
qga/qapi-schema.json | 17 +-
4 files changed, 828 insertions(+), 11 deletions(-)
create mode 100644 qga/commands-windows-ssh.c
create mode 100644 qga/commands-windows-ssh.h
diff --git a/qga/commands-windows-ssh.c b/qga/commands-windows-ssh.c
new file mode 100644
index 00..da402e320c
--- /dev/null
+++ b/qga/commands-windows-ssh.c
@@ -0,0 +1,791 @@
+/*
+ * QEMU Guest Agent win32-specific command implementations for SSH keys.
+ * The implementation is opinionated and expects the SSH implementation to
+ * be OpenSSH.
+ *
+ * Copyright Schweitzer Engineering Laboratories. 2024
+ *
+ * Authors:
+ * Aidan Leuck
+ *
+ * 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
+#include
+
+#include "commands-ssh-core.h"
+#include "commands-windows-ssh.h"
+#include "guest-agent-core.h"
+#include "limits.h"
+#include "lmaccess.h"
+#include "lmapibuf.h"
+#include "lmerr.h"
+#include "qapi/error.h"
+
+#include "qga-qapi-commands.h"
+#include "sddl.h"
+#include "shlobj.h"
+#include "userenv.h"
+
+#define AUTHORIZED_KEY_FILE "authorized_keys"
+#define AUTHORIZED_KEY_FILE_ADMIN "administrators_authorized_keys"
+#define LOCAL_SYSTEM_SID "S-1-5-18"
+#define ADMIN_SID "S-1-5-32-544"
+#define WORLD_SID "S-1-1-0"
+
+/*
+ * Frees userInfo structure. This implements the g_auto cleanup
+ * for the structure.
+ */
+void free_userInfo(PWindowsUserInfo info)
+{
+g_free(info->sshDirectory);
+g_free(info->authorizedKeyFile);
+LocalFree(info->SSID);
+g_free(info->username);
+g_free(info);
+}
+
+/*
+ * Gets the admin SSH folder for OpenSSH. OpenSSH does not store
+ * the authorized_key file in the users home directory for security reasons and
+ * instead stores it at %PROGRAMDATA%/ssh. This function returns the path to
+ * that directory on the users machine
+ *
+ * parameters:
+ * errp -> error structure to set when an error occurs
+ * returns: The path to the ssh folder in %PROGRAMDATA% or NULL if an error
+ * occurred.
+ */
+static char *get_admin_ssh_folder(Error **errp)
+{
+/* Allocate memory for the program data path */
+g_autofree char *programDataPath = NULL;
+char *authkeys_path = NULL;
+PWSTR pgDataW = NULL;
+g_autoptr(GError) gerr = NULL;
+
+/* Get the KnownFolderPath on the machine. */
+HRESULT folderResult =
+SHGetKnownFolderPath(_ProgramData, 0, NULL, );
+if (folderResult != S_OK) {
+error_setg(errp, "Failed to retrieve ProgramData folder");
+return NULL;
+}
+
+/* Convert from a wide string back to a standard character string. */
+programDataPath = g_utf16_to_utf8(pgDataW, -1, NULL, NULL, );
+CoTaskMemFree(pgDataW);
+if (!programDataPath) {
+error_setg(errp,
+ "Failed converting ProgramData folder path to UTF-16 %s",
+ gerr->message);
+return NULL;
+}
+
+/* Build the path to the file. */
+authkeys_path = g_build_filename(programDataPath, "ssh", NULL);
+return authkeys_path;
+}
+
+/*
+ * Gets the path to the SSH folder for the specified user. If the user is an
+ * admin it returns the ssh folder located at %PROGRAMDATA%/ssh. If the user is
+ * not an admin it returns %USERPROFILE%/.ssh
+ *
+ * parameters:
+ * username -> Username to get the SSH folder for
+ * isAdmin -> Whether the user is an admin or not
+ * errp -> Error structure to set any errors that occur.
+ * returns: path to the ssh folder as a string.
+ */
+static char *get_ssh_folder(const char *username, const bool isAdmin,
+Error **errp)
+{
+DWORD maxSize = MAX_PATH;
+g_autofree char *profilesDir = g_new0(char, maxSize);
+
+if (isAdmin) {
+return get_admin_ssh_folder(errp);
+}
+
+/* If not an Admin the SSH key is in the user directory. */
+/* Get the user profile directory on the machine. */
+BOOL ret = GetProfilesDirectory(profilesDir, );
+if (!ret) {
+error_setg_win32(errp, GetLastError(),
+ "failed to retrieve profiles directory");
+return NULL;
+}
+
+/* Builds the filename */
+return g_build_filename(profilesDir, username, ".ssh", NULL);
+}
+
+/*
+ * Creates an entry for the everyone group. This is used when the user is an
+ * Administrator This is consistent with the folder permissions that OpenSSH
+ * creates when it is installed. Anyone can read the file, but only
+ * Administrators and SYSTEM can modify the file.
+ *
+ * parameters:
+ * userInfo -> Information about the current user
+ * pACL -> Pointer to an ACL structure
+ * errp -> Error structure to set any errors that occur
+ * returns: 1 on success, 0 otherwise
+ */
+static bool
