Re: [PATCH 5/7] qga: Add support for user password setting in FreeBSD

[Alexander Ivanov <alexander.ivanov@virtuozzo.com>]

On 29.09.2022 13:22, Marc-André Lureau wrote:
>
> 	
> Caution: This is an external email and has a suspicious subject or 
> content. Please take care when clicking links or opening attachments. 
> When in doubt, contact your IT Department
>
>
> Hi Alexander
>
> On Thu, Sep 29, 2022 at 12:52 PM Alexander Ivanov 
> <alexander.ivanov@virtuozzo.com> wrote:
>
>     Move qmp_guest_set_user_password() from __linux__ condition to
>     (__linux__ || __FreeBSD__) condition. Add command and arguments
>     for password setting in FreeBSD.
>
>     Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
>
>
> Could you explain why you need to move the code?
>
> You could instead have a top declaration?
Now qmp_guest_set_user_password()is under #ifdef __linux__. I want to 
use this function for FreeBSD too, so I moved it under (__linux__ || 
__FreeBSD__) condition. I could create another function for FreeBSD but 
it would lead to code duplication. Unfortunately I don't see another way 
to do it except to add (__linux__ || __FreeBSD__) condition, cut the 
function and paste in this condition. If you could suggest another way I 
would happy.
> If it's really required, please split the patch in move + additions.
Just moving the function without other changes leads to unbuildable code 
in FreeBSD. I thought it's worse than moving and changing at the same 
time. I split the patch if you prefer.
>
>     ---
>      qga/commands-posix.c | 223
>     +++++++++++++++++++++++--------------------
>      1 file changed, 118 insertions(+), 105 deletions(-)
>
>     diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>     index 88e0d0fe24..6ce894ca6e 100644
>     --- a/qga/commands-posix.c
>     +++ b/qga/commands-posix.c
>     @@ -2123,111 +2123,6 @@ int64_t
>     qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
>          return processed;
>      }
>
>     -void qmp_guest_set_user_password(const char *username,
>     -                                 const char *password,
>     -                                 bool crypted,
>     -                                 Error **errp)
>     -{
>     -    Error *local_err = NULL;
>     -    char *passwd_path = NULL;
>     -    pid_t pid;
>     -    int status;
>     -    int datafd[2] = { -1, -1 };
>     -    char *rawpasswddata = NULL;
>     -    size_t rawpasswdlen;
>     -    char *chpasswddata = NULL;
>     -    size_t chpasswdlen;
>     -
>     -    rawpasswddata = (char *)qbase64_decode(password, -1,
>     &rawpasswdlen, errp);
>     -    if (!rawpasswddata) {
>     -        return;
>     -    }
>     -    rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1);
>     -    rawpasswddata[rawpasswdlen] = '\0';
>     -
>     -    if (strchr(rawpasswddata, '\n')) {
>     -        error_setg(errp, "forbidden characters in raw password");
>     -        goto out;
>     -    }
>     -
>     -    if (strchr(username, '\n') ||
>     -        strchr(username, ':')) {
>     -        error_setg(errp, "forbidden characters in username");
>     -        goto out;
>     -    }
>     -
>     -    chpasswddata = g_strdup_printf("%s:%s\n", username,
>     rawpasswddata);
>     -    chpasswdlen = strlen(chpasswddata);
>     -
>     -    passwd_path = g_find_program_in_path("chpasswd");
>     -
>     -    if (!passwd_path) {
>     -        error_setg(errp, "cannot find 'passwd' program in PATH");
>     -        goto out;
>     -    }
>     -
>     -    if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) {
>     -        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) {
>     -            execl(passwd_path, "chpasswd", "-e", NULL);
>     -        } else {
>     -            execl(passwd_path, "chpasswd", NULL);
>     -        }
>     -        _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], chpasswddata, chpasswdlen) !=
>     chpasswdlen) {
>     -        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 user
>     password");
>     -        goto out;
>     -    }
>     -
>     -out:
>     -    g_free(chpasswddata);
>     -    g_free(rawpasswddata);
>     -    g_free(passwd_path);
>     -    if (datafd[0] != -1) {
>     -        close(datafd[0]);
>     -    }
>     -    if (datafd[1] != -1) {
>     -        close(datafd[1]);
>     -    }
>     -}
>     -
>      static void ga_read_sysfs_file(int dirfd, const char *pathname,
>     char *buf,
>                                     int size, Error **errp)
>      {
>     @@ -2793,6 +2688,124 @@ GuestMemoryBlockInfo
>     *qmp_guest_get_memory_block_info(Error **errp)
>
>      #endif
>
>     +#if defined(__linux__) || defined(__FreeBSD__)
>     +void qmp_guest_set_user_password(const char *username,
>     +                                 const char *password,
>     +                                 bool crypted,
>     +                                 Error **errp)
>     +{
>     +    Error *local_err = NULL;
>     +    char *passwd_path = NULL;
>     +    pid_t pid;
>     +    int status;
>     +    int datafd[2] = { -1, -1 };
>     +    char *rawpasswddata = NULL;
>     +    size_t rawpasswdlen;
>     +    char *chpasswddata = NULL;
>     +    size_t chpasswdlen;
>     +
>     +    rawpasswddata = (char *)qbase64_decode(password, -1,
>     &rawpasswdlen, errp);
>     +    if (!rawpasswddata) {
>     +        return;
>     +    }
>     +    rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1);
>     +    rawpasswddata[rawpasswdlen] = '\0';
>     +
>     +    if (strchr(rawpasswddata, '\n')) {
>     +        error_setg(errp, "forbidden characters in raw password");
>     +        goto out;
>     +    }
>     +
>     +    if (strchr(username, '\n') ||
>     +        strchr(username, ':')) {
>     +        error_setg(errp, "forbidden characters in username");
>     +        goto out;
>     +    }
>     +
>     +#ifdef __FreeBSD__
>     +    chpasswddata = g_strdup(rawpasswddata);
>     +    passwd_path = g_find_program_in_path("pw");
>     +#else
>     +    chpasswddata = g_strdup_printf("%s:%s\n", username,
>     rawpasswddata);
>     +    passwd_path = g_find_program_in_path("chpasswd");
>     +#endif
>     +
>     +    chpasswdlen = strlen(chpasswddata);
>     +
>     +    if (!passwd_path) {
>     +        error_setg(errp, "cannot find 'passwd' program in PATH");
>     +        goto out;
>     +    }
>     +
>     +    if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) {
>     +        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);
>     +
>     +#ifdef __FreeBSD__
>     +        const char *h_arg;
>     +        h_arg = (crypted) ? "-H" : "-h";
>     +        execl(passwd_path, "pw", "usermod", "-n", username,
>     h_arg, "0", NULL);
>     +#else
>     +        if (crypted) {
>     +            execl(passwd_path, "chpasswd", "-e", NULL);
>     +        } else {
>     +            execl(passwd_path, "chpasswd", NULL);
>     +        }
>     +#endif
>     +        _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], chpasswddata, chpasswdlen) !=
>     chpasswdlen) {
>     +        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 user
>     password");
>     +        goto out;
>     +    }
>     +
>     +out:
>     +    g_free(chpasswddata);
>     +    g_free(rawpasswddata);
>     +    g_free(passwd_path);
>     +    if (datafd[0] != -1) {
>     +        close(datafd[0]);
>     +    }
>     +    if (datafd[1] != -1) {
>     +        close(datafd[1]);
>     +    }
>     +}
>     +#endif
>     +
>      #ifdef HAVE_GETIFADDRS
>      static GuestNetworkInterface *
>      guest_find_interface(GuestNetworkInterfaceList *head,
>     -- 
>     2.34.1
>
>
>
>
> -- 
> Marc-André Lureau