| Message ID | 1421078294-26234-1-git-send-email-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
Ping On Mon, Jan 12, 2015 at 03:58:14PM +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 | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > qga/commands-win32.c | 6 ++++ > qga/qapi-schema.json | 19 +++++++++++ > 3 files changed, 115 insertions(+) > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index f6f3e3c..4887889 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -1875,6 +1875,90 @@ 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); > + size_t acctpwlen = strlen(acctpw); > + > + 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 (qemu_write_full(datafd[1], acctpw, acctpwlen) != acctpwlen) { > + error_setg_errno(errp, errno, "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 set admin password"); > + 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 +1994,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..25118e2 100644 > --- a/qga/qapi-schema.json > +++ b/qga/qapi-schema.json > @@ -738,3 +738,22 @@ > ## > { '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 > +# > +# If the @crypted flag is true, it is the callers responsibility > +# to ensure the correct crypt() encryption scheme is used. This > +# command does not attempt to interpret or report on the encryption > +# scheme. Refer to the documentation of the guest operating system > +# in question to determine what is supported. > +# > +# Returns: Nothing on success. > +# > +# Since 2.3 > +## > +{ 'command': 'guest-set-admin-password', > + 'data': { 'crypted': 'bool', 'password': 'str' } } > -- > 2.1.0 > Regards, Daniel
On 01/12/2015 08:58 AM, 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 | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > qga/commands-win32.c | 6 ++++ > qga/qapi-schema.json | 19 +++++++++++ > 3 files changed, 115 insertions(+) > > +++ b/qga/qapi-schema.json > @@ -738,3 +738,22 @@ > ## > { '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 > +# > +# If the @crypted flag is true, it is the callers responsibility s/callers/caller's/ > +# to ensure the correct crypt() encryption scheme is used. This > +# command does not attempt to interpret or report on the encryption > +# scheme. Refer to the documentation of the guest operating system > +# in question to determine what is supported. > +# > +# Returns: Nothing on success. > +# > +# Since 2.3 > +## > +{ 'command': 'guest-set-admin-password', > + 'data': { 'crypted': 'bool', 'password': 'str' } } > Normally, 'password':'str' means we are passing UTF8 JSON. But what if the desired password is NOT valid UTF8, but still valid to the end user (for example, a user that intentionally wants a Latin1 encoded password that uses 8-bit characters)? In other interfaces, we've allowed an enum that specifies whether a raw data string is 'utf8' or 'base64' encoded; should we have such a parameter here?
On Tue, Feb 03, 2015 at 03:16:08PM -0700, Eric Blake wrote: > On 01/12/2015 08:58 AM, 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 | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > qga/commands-win32.c | 6 ++++ > > qga/qapi-schema.json | 19 +++++++++++ > > 3 files changed, 115 insertions(+) > > > > > +++ b/qga/qapi-schema.json > > @@ -738,3 +738,22 @@ > > ## > > { '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 > > +# > > +# If the @crypted flag is true, it is the callers responsibility > > s/callers/caller's/ > > > +# to ensure the correct crypt() encryption scheme is used. This > > +# command does not attempt to interpret or report on the encryption > > +# scheme. Refer to the documentation of the guest operating system > > +# in question to determine what is supported. > > +# > > +# Returns: Nothing on success. > > +# > > +# Since 2.3 > > +## > > +{ 'command': 'guest-set-admin-password', > > + 'data': { 'crypted': 'bool', 'password': 'str' } } > > > > Normally, 'password':'str' means we are passing UTF8 JSON. But what if > the desired password is NOT valid UTF8, but still valid to the end user > (for example, a user that intentionally wants a Latin1 encoded password > that uses 8-bit characters)? In other interfaces, we've allowed an enum > that specifies whether a raw data string is 'utf8' or 'base64' encoded; > should we have such a parameter here? IMHO, QEMU itself needs to avoid interpreting the password at all and just pass the raw bytes through to the operating system specific command as-is. ie we should be 8-bit pure in what we accept. This probably argues for unconditionally using base64 encoded data for the parameter. Regards, Daniel
On Mon, Jan 12, 2015 at 03:58:14PM +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." } }' Does it have to be a QMP command? Wouldn't the recently (re-)submitted guest-exec allow to do the same, by running "chpasswd" in the guest and piping the username:password into its stdin? Besides I think it makes sense to (optionally) pass the username, to allow to change the password for arbitrary users. This would make the functionality useful for systems where root password plays no role as root logins are disallowed, and the only access to root shell is via sudo from a user belonging to a particular group (IIRC Ubuntu is usually set up like that). > NB windows support is desirable, but not implemented in this > patch. Yes Windows may have an issue with username here too, because the admin user can be any user (and even "Administrator" can be localized). Roman.
On Wed, Feb 04, 2015 at 01:48:40PM +0300, Roman Kagan wrote: > On Mon, Jan 12, 2015 at 03:58:14PM +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." } }' > > Does it have to be a QMP command? Wouldn't the recently (re-)submitted > guest-exec allow to do the same, by running "chpasswd" in the guest and > piping the username:password into its stdin? guest-exec puts the burden on the calling application to figure out which command to invoke and what syntax it has. This is really sucky for any kind of cross-OS portability. ie windows is going to be completely different from Linux, and even different UNIX variants are different to some extent. I don't consider guest-exec to be something that managment applications should *ever* use to build features around. It is just a useful mechanism for human administrators to do ad-hoc interactions with guests. > Besides I think it makes sense to (optionally) pass the username, to > allow to change the password for arbitrary users. This would make the > functionality useful for systems where root password plays no role as > root logins are disallowed, and the only access to root shell is via > sudo from a user belonging to a particular group (IIRC Ubuntu is usually > set up like that). Yep, extending it to any username is a possibility if it is thought to be useful > > > NB windows support is desirable, but not implemented in this > > patch. > > Yes Windows may have an issue with username here too, because the admin > user can be any user (and even "Administrator" can be localized). Regards, Daniel
On 12/01/15 18:58, 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 | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > qga/commands-win32.c | 6 ++++ > qga/qapi-schema.json | 19 +++++++++++ > 3 files changed, 115 insertions(+) > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index f6f3e3c..4887889 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -1875,6 +1875,90 @@ 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); > + size_t acctpwlen = strlen(acctpw); > + > + 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 (qemu_write_full(datafd[1], acctpw, acctpwlen) != acctpwlen) { > + error_setg_errno(errp, errno, "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 set admin password"); > + 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 +1994,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..25118e2 100644 > --- a/qga/qapi-schema.json > +++ b/qga/qapi-schema.json > @@ -738,3 +738,22 @@ > ## > { '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 > +# > +# If the @crypted flag is true, it is the callers responsibility > +# to ensure the correct crypt() encryption scheme is used. This > +# command does not attempt to interpret or report on the encryption > +# scheme. Refer to the documentation of the guest operating system > +# in question to determine what is supported. > +# > +# Returns: Nothing on success. > +# > +# Since 2.3 > +## > +{ 'command': 'guest-set-admin-password', > + 'data': { 'crypted': 'bool', 'password': 'str' } } While implementing such functionality for Windows NT we can suffer from particular problem: -The password must be passed to WinApi function as a plain text, so we would need entire the encryption mechanism if we used ctypted: true
On Wed, Feb 04, 2015 at 04:25:47PM +0300, Olga Krishtal wrote: > On 12/01/15 18:58, 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 | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > qga/commands-win32.c | 6 ++++ > > qga/qapi-schema.json | 19 +++++++++++ > > 3 files changed, 115 insertions(+) > > > >diff --git a/qga/commands-posix.c b/qga/commands-posix.c > >index f6f3e3c..4887889 100644 > >--- a/qga/commands-posix.c > >+++ b/qga/commands-posix.c > >@@ -1875,6 +1875,90 @@ 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); > >+ size_t acctpwlen = strlen(acctpw); > >+ > >+ 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 (qemu_write_full(datafd[1], acctpw, acctpwlen) != acctpwlen) { > >+ error_setg_errno(errp, errno, "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 set admin password"); > >+ 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 +1994,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..25118e2 100644 > >--- a/qga/qapi-schema.json > >+++ b/qga/qapi-schema.json > >@@ -738,3 +738,22 @@ > > ## > > { '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 > >+# > >+# If the @crypted flag is true, it is the callers responsibility > >+# to ensure the correct crypt() encryption scheme is used. This > >+# command does not attempt to interpret or report on the encryption > >+# scheme. Refer to the documentation of the guest operating system > >+# in question to determine what is supported. > >+# > >+# Returns: Nothing on success. > >+# > >+# Since 2.3 > >+## > >+{ 'command': 'guest-set-admin-password', > >+ 'data': { 'crypted': 'bool', 'password': 'str' } } > While implementing such functionality for Windows NT we can suffer from > particular problem: > -The password must be passed to WinApi function as a plain text, so we would > need entire the encryption mechanism if we used ctypted: true In that scenario, I'd suggest the windows impl of the command simply report an error if the flag was crypted==true. QEMU shouldn't try to interpret the data in any way IMHO, so that'd rule out decryption of any kind. In any case, 'crypt' as a concept is all about doing a one-way hash so you can't decrypt regardless. Regards, Daniel
diff --git a/qga/commands-posix.c b/qga/commands-posix.c index f6f3e3c..4887889 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1875,6 +1875,90 @@ 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); + size_t acctpwlen = strlen(acctpw); + + 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 (qemu_write_full(datafd[1], acctpw, acctpwlen) != acctpwlen) { + error_setg_errno(errp, errno, "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 set admin password"); + 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 +1994,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..25118e2 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -738,3 +738,22 @@ ## { '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 +# +# If the @crypted flag is true, it is the callers responsibility +# to ensure the correct crypt() encryption scheme is used. This +# command does not attempt to interpret or report on the encryption +# scheme. Refer to the documentation of the guest operating system +# in question to determine what is supported. +# +# Returns: Nothing on success. +# +# Since 2.3 +## +{ 'command': 'guest-set-admin-password', + 'data': { 'crypted': 'bool', 'password': 'str' } }
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 | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++ qga/commands-win32.c | 6 ++++ qga/qapi-schema.json | 19 +++++++++++ 3 files changed, 115 insertions(+)