[Rpm-maint] [PATCH v9a 13/13] Add support for prompting the user for the password using pinentry
Lubos Kardos
lkardos at redhat.com
Tue Aug 11 15:12:57 UTC 2015
Hi,
sorry for late response but I was busy with solving problems with the new
release rpm-4.13.0-alpha. The patches look good. I have comments only about
the last one.
The first one there is a race in pinentry patch. If I enter "password" in
pinentry window then get_pinentry_fskpass() sometimes returns correctly
"password" but it sometimes returns "password\nOK" or it returns NULL. I am
not expert in pinetry protocol but recv() looks too easy to handle a protocol
over a pipe. What if the pipe doesn't contain result in time of calling read(),
you should read from pipe until you get what you expect and if you don't get it
in some time then you should return error. Or what if after calling read()
the returned buffer contains more messages, ...
The second comment. The pinentry should work in terminal without graphical
enviroment but it doesn't work. I don't know where the problem is. If it
worked without graphical environment then you shouldn't need a fallback for
prompting the user directly. And I think it is not correct behavior when you
click "Cancel" in pinentry window and then you are asked for password on
terminal.
But I'm OK with pushing patches upstream without this last pinentry patch with
just support for getting password directly from user without pinentry. Let me
know if you want to fix this patch or should I push patches upstream
without it.
Lubos
----- Original Message -----
> From: "Mimi Zohar" <zohar at linux.vnet.ibm.com>
> To: "Fionnuala Gunter" <fionnuala.gunter at gmail.com>
> Cc: rpm-maint at lists.rpm.org, lkardos at redhat.com, "fin gunter" <fin.gunter at hypori.com>, "Mimi Zohar"
> <zohar at us.ibm.com>
> Sent: Thursday, July 30, 2015 6:00:16 PM
> Subject: Re: [PATCH v9a 13/13] Add support for prompting the user for the password using pinentry
>
> Call pinentry to prompt for the password. On failure revert back
> to prompting the user directly.
>
> Changelog:
> - merged recv() and recv_password()
> ---
> lib/rpmsignfiles.c | 117
> ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> lib/rpmsignfiles.h | 1 +
> rpmsign.c | 4 +-
> 3 files changed, 120 insertions(+), 2 deletions(-)
>
> diff --git a/lib/rpmsignfiles.c b/lib/rpmsignfiles.c
> index 4336f8c..cbb9067 100644
> --- a/lib/rpmsignfiles.c
> +++ b/lib/rpmsignfiles.c
> @@ -7,7 +7,10 @@
> #include "system.h"
> #include "imaevm.h"
>
> +#include <unistd.h>
> #include <termios.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
>
> #include <rpm/rpmlog.h> /* rpmlog */
> #include <rpm/rpmstring.h> /* rnibble */
> @@ -32,6 +35,23 @@ static const char *hash_algo_name[] = {
> [PGPHASHALGO_SHA224] = "sha224",
> };
>
> +#define debug 0
> +
> +#define PARENT_WRITE_PIPE 0
> +#define PARENT_READ_PIPE 1
> +
> +#define NUM_PIPES 2
> +int pipes[NUM_PIPES][2];
> +
> +#define READ_FD 0
> +#define WRITE_FD 1
> +
> +#define PARENT_READ_FD ( pipes[PARENT_READ_PIPE][READ_FD] )
> +#define PARENT_WRITE_FD ( pipes[PARENT_WRITE_PIPE][WRITE_FD] )
> +
> +#define CHILD_READ_FD ( pipes[PARENT_WRITE_PIPE][READ_FD] )
> +#define CHILD_WRITE_FD ( pipes[PARENT_READ_PIPE][WRITE_FD] )
> +
> char *get_fskpass(void)
> {
> struct termios flags, tmp_flags;
> @@ -54,7 +74,7 @@ char *get_fskpass(void)
> return NULL;
> }
>
> - printf("PEM password: ");
> + printf("Enter fskpass: ");
> pwd = fgets(password, passlen, stdin);
> pwd[strlen(pwd) - 1] = '\0'; /* remove newline */
>
> @@ -65,6 +85,101 @@ char *get_fskpass(void)
> return pwd;
> }
>
> +static void send(int pipe, char *command)
> +{
> + if (debug)
> + printf("send: %s", command);
> + write(PARENT_WRITE_FD, command, strlen(command));
> +}
> +
> +static char *recv(int pipe)
> +{
> + char buffer[100];
> + char *password = NULL;
> + int count;
> +
> + memset(buffer, '\0', sizeof buffer);
> + do {
> + count = read(PARENT_READ_FD, buffer, sizeof buffer - 1);
> +
> + if (count < 0)
> + break;
> +
> + if (buffer[0] == 'D') {
> + int len = strlen(buffer);
> +
> + buffer[len - 1] = '\0'; /* remove newline */
> + password = strdup(buffer + 2);
> + memset(buffer, '\0', sizeof buffer);
> + break;
> + } else if (strncmp(buffer, "ERR", 3) == 0) {
> + if (debug)
> + printf("count %d: %s\n", count, buffer);
> + break;
> + } else if (strncmp(buffer, "OK", 2) == 0) {
> + if (debug)
> + printf("count %d: %s\n", count, buffer);
> + break;
> + }
> + } while (0);
> +
> + return password;
> +}
> +
> +char *get_pinentry_fskpass()
> +{
> + char *command, *password = NULL;
> + pid_t child;
> +
> + if (pipe(pipes[PARENT_READ_PIPE]) || pipe(pipes[PARENT_WRITE_PIPE])) {
> + fprintf(stderr, "Pipe failed\n");
> + return NULL;
> + }
> +
> + child = fork();
> + if (child == (pid_t) 0) {
> + dup2(CHILD_READ_FD, STDIN_FILENO);
> + dup2(CHILD_WRITE_FD, STDOUT_FILENO);
> +
> + close(CHILD_READ_FD);
> + close(CHILD_WRITE_FD);
> + close(PARENT_READ_FD);
> + close(PARENT_WRITE_FD);
> +
> + execlp("pinentry", "pinentry", NULL);
> + } else if (child < (pid_t) 0) {
> + fprintf(stderr, "Fork failed\n");
> + return NULL;
> + } else {
> + /* close fds not required by parent */
> + close(CHILD_READ_FD);
> + close(CHILD_WRITE_FD);
> +
> + command = "SETDESC Enter fskpass:\n";
> + send(PARENT_WRITE_FD, command);
> + recv(PARENT_READ_FD);
> +
> + command = "OPTION display PEM\n";
> + send(PARENT_WRITE_FD, command);
> + recv(PARENT_READ_FD);
> +
> + command = "OPTION ttyname ttyname(0)\n";
> + send(PARENT_WRITE_FD, command);
> + recv(PARENT_READ_FD);
> +
> + command = "GETPIN\n";
> + send(PARENT_WRITE_FD, command);
> + password = recv(PARENT_READ_FD);
> +
> + command = "BYE\n";
> + send(PARENT_WRITE_FD, command);
> + recv(PARENT_READ_FD);
> +
> + waitpid(child, NULL, 0);
> + }
> + return password;
> +}
> +
> static char *signFile(const char *algo, const char *fdigest, int diglen,
> const char *key, char *keypass)
> {
> diff --git a/lib/rpmsignfiles.h b/lib/rpmsignfiles.h
> index 52e2482..b00703d 100644
> --- a/lib/rpmsignfiles.h
> +++ b/lib/rpmsignfiles.h
> @@ -15,6 +15,7 @@ extern "C" {
> rpmRC rpmSignFiles(Header h, const char *key, char *keypass);
>
> char *get_fskpass(void); /* get file signing key password */
> +char *get_pinentry_fskpass(void); /* call pinentry to get password */
>
> #ifdef _cplusplus
> }
> diff --git a/rpmsign.c b/rpmsign.c
> index 7f48d98..0cfbc92 100644
> --- a/rpmsign.c
> +++ b/rpmsign.c
> @@ -81,7 +81,9 @@ static int doSign(poptContext optCon)
> #ifndef WITH_IMAEVM
> argerror(_("--fskpass may only be specified when signing files"));
> #else
> - fileSigningKeyPassword = get_fskpass();
> + fileSigningKeyPassword = get_pinentry_fskpass();
> + if (!fileSigningKeyPassword)
> + fileSigningKeyPassword = get_fskpass();
> #endif
> }
>
> --
> 2.1.0
>
>
>
>
More information about the Rpm-maint
mailing list