All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] qga: add guest-set-admin-password command
@ 2014-12-15 12:47 Daniel P. Berrange
  2015-01-05 17:06 ` Daniel P. Berrange
  2015-01-09  0:21 ` Michael Roth
  0 siblings, 2 replies; 4+ messages in thread
From: Daniel P. Berrange @ 2014-12-15 12:47 UTC (permalink / raw)
  To: qemu-devel; +Cc: Michael Roth

Add a new 'guest-set-admin-password' command for changing the
root/administrator password. This command is needed to allow
OpenStack to support its API for changing the admin password
on a running guest.

Accepts either the raw password string:

$ virsh -c qemu:///system  qemu-agent-command f21x86_64 \
   '{ "execute": "guest-set-admin-password", "arguments":
     { "crypted": false, "password": "12345678" } }'
  {"return":{}}

Or a pre-encrypted string (recommended)

$ virsh -c qemu:///system  qemu-agent-command f21x86_64 \
   '{ "execute": "guest-set-admin-password", "arguments":
     { "crypted": true, "password":
        "$6$T9O/j/aGPrE...snip....rQoRN4F0.GG0MPjNUNyml." } }'

NB windows support is desirable, but not implemented in this
patch.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 qga/commands-posix.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 qga/commands-win32.c |  6 ++++
 qga/qapi-schema.json | 13 ++++++++
 3 files changed, 108 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index f6f3e3c..3c3998a 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1875,6 +1875,89 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
     return processed;
 }
 
+void qmp_guest_set_admin_password(bool crypted, const char *password,
+                                  Error **errp)
+{
+    Error *local_err = NULL;
+    char *passwd_path = NULL;
+    pid_t pid;
+    int status;
+    int datafd[2] = { -1, -1 };
+    char *acctpw = g_strdup_printf("root:%s\n", password);
+
+    if (strchr(password, '\n')) {
+        error_setg(errp, "forbidden characters in new password");
+        goto out;
+    }
+
+    passwd_path = g_find_program_in_path("chpasswd");
+
+    if (!passwd_path) {
+        error_setg(errp, "cannot find 'passwd' program in PATH");
+        goto out;
+    }
+
+    if (pipe(datafd) < 0) {
+        error_setg(errp, "cannot create pipe FDs");
+        goto out;
+    }
+
+    pid = fork();
+    if (pid == 0) {
+        close(datafd[1]);
+        /* child */
+        setsid();
+        dup2(datafd[0], 0);
+        reopen_fd_to_null(1);
+        reopen_fd_to_null(2);
+
+        if (crypted) {
+            execle(passwd_path, "chpasswd", "-e", NULL, environ);
+        } else {
+            execle(passwd_path, "chpasswd", NULL, environ);
+        }
+        _exit(EXIT_FAILURE);
+    } else if (pid < 0) {
+        error_setg_errno(errp, errno, "failed to create child process");
+        goto out;
+    }
+    close(datafd[0]);
+    datafd[0] = -1;
+
+    if (write(datafd[1], acctpw, strlen(acctpw)) != strlen(acctpw)) {
+        error_setg(errp, "cannot write new account password");
+        goto out;
+    }
+    close(datafd[1]);
+    datafd[1] = -1;
+
+    ga_wait_child(pid, &status, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto out;
+    }
+
+    if (!WIFEXITED(status)) {
+        error_setg(errp, "child process has terminated abnormally");
+        goto out;
+    }
+
+    if (WEXITSTATUS(status)) {
+        error_setg(errp, "child process has failed to suspend");
+        goto out;
+    }
+
+out:
+    g_free(acctpw);
+    g_free(passwd_path);
+    if (datafd[0] != -1) {
+        close(datafd[0]);
+    }
+    if (datafd[1] != -1) {
+        close(datafd[1]);
+    }
+}
+
 #else /* defined(__linux__) */
 
 void qmp_guest_suspend_disk(Error **errp)
@@ -1910,6 +1993,12 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
     return -1;
 }
 
+void qmp_guest_set_admin_password(bool crypted, const char *password,
+                                  Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+}
+
 #endif
 
 #if !defined(CONFIG_FSFREEZE)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 3bcbeae..56854d5 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -446,6 +446,12 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
     return -1;
 }
 
+void qmp_guest_set_admin_password(bool crypted, const char *password,
+                                  Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+}
+
 /* add unsupported commands to the blacklist */
 GList *ga_command_blacklist_init(GList *blacklist)
 {
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 376e79f..202d3be 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -738,3 +738,16 @@
 ##
 { 'command': 'guest-get-fsinfo',
   'returns': ['GuestFilesystemInfo'] }
+
+##
+# @guest-set-admin-password
+#
+# @crypted: true if password is already crypt()d, false if raw
+# @password: the new password entry
+#
+# Returns: Nothing on success.
+#
+# Since 2.3
+##
+{ 'command': 'guest-set-admin-password',
+  'data': { 'crypted': 'bool', 'password': 'str' } }
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] qga: add guest-set-admin-password command
  2014-12-15 12:47 [Qemu-devel] [PATCH] qga: add guest-set-admin-password command Daniel P. Berrange
@ 2015-01-05 17:06 ` Daniel P. Berrange
  2015-01-09  0:21 ` Michael Roth
  1 sibling, 0 replies; 4+ messages in thread
From: Daniel P. Berrange @ 2015-01-05 17:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Michael Roth

icmp(echo-request)

Daniel

On Mon, Dec 15, 2014 at 12:47:46PM +0000, Daniel P. Berrange wrote:
> Add a new 'guest-set-admin-password' command for changing the
> root/administrator password. This command is needed to allow
> OpenStack to support its API for changing the admin password
> on a running guest.
> 
> Accepts either the raw password string:
> 
> $ virsh -c qemu:///system  qemu-agent-command f21x86_64 \
>    '{ "execute": "guest-set-admin-password", "arguments":
>      { "crypted": false, "password": "12345678" } }'
>   {"return":{}}
> 
> Or a pre-encrypted string (recommended)
> 
> $ virsh -c qemu:///system  qemu-agent-command f21x86_64 \
>    '{ "execute": "guest-set-admin-password", "arguments":
>      { "crypted": true, "password":
>         "$6$T9O/j/aGPrE...snip....rQoRN4F0.GG0MPjNUNyml." } }'
> 
> NB windows support is desirable, but not implemented in this
> patch.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  qga/commands-posix.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qga/commands-win32.c |  6 ++++
>  qga/qapi-schema.json | 13 ++++++++
>  3 files changed, 108 insertions(+)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index f6f3e3c..3c3998a 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1875,6 +1875,89 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
>      return processed;
>  }
>  
> +void qmp_guest_set_admin_password(bool crypted, const char *password,
> +                                  Error **errp)
> +{
> +    Error *local_err = NULL;
> +    char *passwd_path = NULL;
> +    pid_t pid;
> +    int status;
> +    int datafd[2] = { -1, -1 };
> +    char *acctpw = g_strdup_printf("root:%s\n", password);
> +
> +    if (strchr(password, '\n')) {
> +        error_setg(errp, "forbidden characters in new password");
> +        goto out;
> +    }
> +
> +    passwd_path = g_find_program_in_path("chpasswd");
> +
> +    if (!passwd_path) {
> +        error_setg(errp, "cannot find 'passwd' program in PATH");
> +        goto out;
> +    }
> +
> +    if (pipe(datafd) < 0) {
> +        error_setg(errp, "cannot create pipe FDs");
> +        goto out;
> +    }
> +
> +    pid = fork();
> +    if (pid == 0) {
> +        close(datafd[1]);
> +        /* child */
> +        setsid();
> +        dup2(datafd[0], 0);
> +        reopen_fd_to_null(1);
> +        reopen_fd_to_null(2);
> +
> +        if (crypted) {
> +            execle(passwd_path, "chpasswd", "-e", NULL, environ);
> +        } else {
> +            execle(passwd_path, "chpasswd", NULL, environ);
> +        }
> +        _exit(EXIT_FAILURE);
> +    } else if (pid < 0) {
> +        error_setg_errno(errp, errno, "failed to create child process");
> +        goto out;
> +    }
> +    close(datafd[0]);
> +    datafd[0] = -1;
> +
> +    if (write(datafd[1], acctpw, strlen(acctpw)) != strlen(acctpw)) {
> +        error_setg(errp, "cannot write new account password");
> +        goto out;
> +    }
> +    close(datafd[1]);
> +    datafd[1] = -1;
> +
> +    ga_wait_child(pid, &status, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        goto out;
> +    }
> +
> +    if (!WIFEXITED(status)) {
> +        error_setg(errp, "child process has terminated abnormally");
> +        goto out;
> +    }
> +
> +    if (WEXITSTATUS(status)) {
> +        error_setg(errp, "child process has failed to suspend");
> +        goto out;
> +    }
> +
> +out:
> +    g_free(acctpw);
> +    g_free(passwd_path);
> +    if (datafd[0] != -1) {
> +        close(datafd[0]);
> +    }
> +    if (datafd[1] != -1) {
> +        close(datafd[1]);
> +    }
> +}
> +
>  #else /* defined(__linux__) */
>  
>  void qmp_guest_suspend_disk(Error **errp)
> @@ -1910,6 +1993,12 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
>      return -1;
>  }
>  
> +void qmp_guest_set_admin_password(bool crypted, const char *password,
> +                                  Error **errp)
> +{
> +    error_set(errp, QERR_UNSUPPORTED);
> +}
> +
>  #endif
>  
>  #if !defined(CONFIG_FSFREEZE)
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 3bcbeae..56854d5 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -446,6 +446,12 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
>      return -1;
>  }
>  
> +void qmp_guest_set_admin_password(bool crypted, const char *password,
> +                                  Error **errp)
> +{
> +    error_set(errp, QERR_UNSUPPORTED);
> +}
> +
>  /* add unsupported commands to the blacklist */
>  GList *ga_command_blacklist_init(GList *blacklist)
>  {
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 376e79f..202d3be 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -738,3 +738,16 @@
>  ##
>  { 'command': 'guest-get-fsinfo',
>    'returns': ['GuestFilesystemInfo'] }
> +
> +##
> +# @guest-set-admin-password
> +#
> +# @crypted: true if password is already crypt()d, false if raw
> +# @password: the new password entry
> +#
> +# Returns: Nothing on success.
> +#
> +# Since 2.3
> +##
> +{ 'command': 'guest-set-admin-password',
> +  'data': { 'crypted': 'bool', 'password': 'str' } }



> -- 
> 2.1.0
> 

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] qga: add guest-set-admin-password command
  2014-12-15 12:47 [Qemu-devel] [PATCH] qga: add guest-set-admin-password command Daniel P. Berrange
  2015-01-05 17:06 ` Daniel P. Berrange
@ 2015-01-09  0:21 ` Michael Roth
  2015-01-12 15:54   ` Daniel P. Berrange
  1 sibling, 1 reply; 4+ messages in thread
From: Michael Roth @ 2015-01-09  0:21 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel

Quoting Daniel P. Berrange (2014-12-15 06:47:46)
> Add a new 'guest-set-admin-password' command for changing the
> root/administrator password. This command is needed to allow
> OpenStack to support its API for changing the admin password
> on a running guest.
> 
> Accepts either the raw password string:
> 
> $ virsh -c qemu:///system  qemu-agent-command f21x86_64 \
>    '{ "execute": "guest-set-admin-password", "arguments":
>      { "crypted": false, "password": "12345678" } }'
>   {"return":{}}
> 
> Or a pre-encrypted string (recommended)
> 
> $ virsh -c qemu:///system  qemu-agent-command f21x86_64 \
>    '{ "execute": "guest-set-admin-password", "arguments":
>      { "crypted": true, "password":
>         "$6$T9O/j/aGPrE...snip....rQoRN4F0.GG0MPjNUNyml." } }'
> 
> NB windows support is desirable, but not implemented in this
> patch.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  qga/commands-posix.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qga/commands-win32.c |  6 ++++
>  qga/qapi-schema.json | 13 ++++++++
>  3 files changed, 108 insertions(+)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index f6f3e3c..3c3998a 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1875,6 +1875,89 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
>      return processed;
>  }
> 
> +void qmp_guest_set_admin_password(bool crypted, const char *password,
> +                                  Error **errp)
> +{
> +    Error *local_err = NULL;
> +    char *passwd_path = NULL;
> +    pid_t pid;
> +    int status;
> +    int datafd[2] = { -1, -1 };
> +    char *acctpw = g_strdup_printf("root:%s\n", password);
> +
> +    if (strchr(password, '\n')) {
> +        error_setg(errp, "forbidden characters in new password");
> +        goto out;
> +    }
> +
> +    passwd_path = g_find_program_in_path("chpasswd");
> +
> +    if (!passwd_path) {
> +        error_setg(errp, "cannot find 'passwd' program in PATH");
> +        goto out;
> +    }
> +
> +    if (pipe(datafd) < 0) {
> +        error_setg(errp, "cannot create pipe FDs");
> +        goto out;
> +    }
> +
> +    pid = fork();
> +    if (pid == 0) {
> +        close(datafd[1]);
> +        /* child */
> +        setsid();
> +        dup2(datafd[0], 0);
> +        reopen_fd_to_null(1);
> +        reopen_fd_to_null(2);
> +
> +        if (crypted) {
> +            execle(passwd_path, "chpasswd", "-e", NULL, environ);
> +        } else {
> +            execle(passwd_path, "chpasswd", NULL, environ);
> +        }
> +        _exit(EXIT_FAILURE);
> +    } else if (pid < 0) {
> +        error_setg_errno(errp, errno, "failed to create child process");
> +        goto out;
> +    }
> +    close(datafd[0]);
> +    datafd[0] = -1;
> +
> +    if (write(datafd[1], acctpw, strlen(acctpw)) != strlen(acctpw)) {
> +        error_setg(errp, "cannot write new account password");
> +        goto out;
> +    }

We should probably retry on EINTR, and for cases where -1 is returned I
think error_setg_errno() would be useful.

> +    close(datafd[1]);
> +    datafd[1] = -1;
> +
> +    ga_wait_child(pid, &status, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        goto out;
> +    }
> +
> +    if (!WIFEXITED(status)) {
> +        error_setg(errp, "child process has terminated abnormally");
> +        goto out;
> +    }
> +
> +    if (WEXITSTATUS(status)) {
> +        error_setg(errp, "child process has failed to suspend");

"... has failed to set admin password" or somesuch

> +        goto out;
> +    }
> +
> +out:
> +    g_free(acctpw);
> +    g_free(passwd_path);
> +    if (datafd[0] != -1) {
> +        close(datafd[0]);
> +    }
> +    if (datafd[1] != -1) {
> +        close(datafd[1]);
> +    }
> +}
> +
>  #else /* defined(__linux__) */
> 
>  void qmp_guest_suspend_disk(Error **errp)
> @@ -1910,6 +1993,12 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
>      return -1;
>  }
> 
> +void qmp_guest_set_admin_password(bool crypted, const char *password,
> +                                  Error **errp)
> +{
> +    error_set(errp, QERR_UNSUPPORTED);
> +}
> +
>  #endif
> 
>  #if !defined(CONFIG_FSFREEZE)
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 3bcbeae..56854d5 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -446,6 +446,12 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
>      return -1;
>  }
> 
> +void qmp_guest_set_admin_password(bool crypted, const char *password,
> +                                  Error **errp)
> +{
> +    error_set(errp, QERR_UNSUPPORTED);
> +}
> +
>  /* add unsupported commands to the blacklist */
>  GList *ga_command_blacklist_init(GList *blacklist)
>  {
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 376e79f..202d3be 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -738,3 +738,16 @@
>  ##
>  { 'command': 'guest-get-fsinfo',
>    'returns': ['GuestFilesystemInfo'] }
> +
> +##
> +# @guest-set-admin-password
> +#
> +# @crypted: true if password is already crypt()d, false if raw

Should we have some sort of note about what sort of encryption
scheme is expected, or a way to query for it?

> +# @password: the new password entry
> +#
> +# Returns: Nothing on success.
> +#
> +# Since 2.3
> +##
> +{ 'command': 'guest-set-admin-password',
> +  'data': { 'crypted': 'bool', 'password': 'str' } }
> -- 
> 2.1.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] qga: add guest-set-admin-password command
  2015-01-09  0:21 ` Michael Roth
@ 2015-01-12 15:54   ` Daniel P. Berrange
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel P. Berrange @ 2015-01-12 15:54 UTC (permalink / raw)
  To: Michael Roth; +Cc: qemu-devel

On Thu, Jan 08, 2015 at 06:21:19PM -0600, Michael Roth wrote:
> Quoting Daniel P. Berrange (2014-12-15 06:47:46)
> > Add a new 'guest-set-admin-password' command for changing the
> > root/administrator password. This command is needed to allow
> > OpenStack to support its API for changing the admin password
> > on a running guest.
> > 
> > Accepts either the raw password string:
> > 
> > $ virsh -c qemu:///system  qemu-agent-command f21x86_64 \
> >    '{ "execute": "guest-set-admin-password", "arguments":
> >      { "crypted": false, "password": "12345678" } }'
> >   {"return":{}}
> > 
> > Or a pre-encrypted string (recommended)
> > 
> > $ virsh -c qemu:///system  qemu-agent-command f21x86_64 \
> >    '{ "execute": "guest-set-admin-password", "arguments":
> >      { "crypted": true, "password":
> >         "$6$T9O/j/aGPrE...snip....rQoRN4F0.GG0MPjNUNyml." } }'
> > 
> > NB windows support is desirable, but not implemented in this
> > patch.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  qga/commands-posix.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  qga/commands-win32.c |  6 ++++
> >  qga/qapi-schema.json | 13 ++++++++
> >  3 files changed, 108 insertions(+)
> > 

> > +
> > +    if (write(datafd[1], acctpw, strlen(acctpw)) != strlen(acctpw)) {
> > +        error_setg(errp, "cannot write new account password");
> > +        goto out;
> > +    }
> 
> We should probably retry on EINTR, and for cases where -1 is returned I
> think error_setg_errno() would be useful.

I'll make it use  qemu_write_full

> > +    if (!WIFEXITED(status)) {
> > +        error_setg(errp, "child process has terminated abnormally");
> > +        goto out;
> > +    }
> > +
> > +    if (WEXITSTATUS(status)) {
> > +        error_setg(errp, "child process has failed to suspend");
> 
> "... has failed to set admin password" or somesuch

Opps yes, copy+paste mistake.


> > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> > index 376e79f..202d3be 100644
> > --- a/qga/qapi-schema.json
> > +++ b/qga/qapi-schema.json
> > @@ -738,3 +738,16 @@
> >  ##
> >  { 'command': 'guest-get-fsinfo',
> >    'returns': ['GuestFilesystemInfo'] }
> > +
> > +##
> > +# @guest-set-admin-password
> > +#
> > +# @crypted: true if password is already crypt()d, false if raw
> 
> Should we have some sort of note about what sort of encryption
> scheme is expected, or a way to query for it?

I was explicitly not saying anything about the crypt scheme,
because it is not really very easy to determine what is
supported by any given guest OS. In the context in which
this is used, I think the sysadmin will be able to easily
figure out themselves what's needed.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-01-12 15:55 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-15 12:47 [Qemu-devel] [PATCH] qga: add guest-set-admin-password command Daniel P. Berrange
2015-01-05 17:06 ` Daniel P. Berrange
2015-01-09  0:21 ` Michael Roth
2015-01-12 15:54   ` Daniel P. Berrange

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.