| Message ID | 1418647666-18771-1-git-send-email-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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' } }
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(+)