Re: [Bug-wget] [PATCH] wget: Add --ssh-askpass support

[Tim Rühsen]

Thanks for your contribution, Liam.
I think it is a good feature.

A few questions, some have already been mentioned by Eli and Jeremie.

* what about posix_spawn instead of fork ? It is covered by gnulib...
* the 'strace' is left over from your tests !?
* could you attach your patch as a file, best generated with 'git format-patch 
-1' ?
* Did you already signed the FSF copyright assignment ? The patch is 
considered 'non-trivial'.

Regards, Tim

On Freitag, 22. Juli 2016 20:24:05 CEST Liam R. Howlett wrote:
> This adds the --ssh-askpass option which is disabled by default.
> 
> --ssh-askpass will request the username and password for a given URL by
> executing the external program pointed to by the environment variable
> SSH_ASKPASS.  If the environment variable is not set, an error is
> returned.  If an error occurs requesting the username or password, wget
> will exit.
> 
> Signed-off-by: Liam R. Howlett <address@hidden>
> ---
>  src/init.c    |  3 ++
>  src/main.c    | 92
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/options.h |
>  2 ++
>  src/url.c     |  6 ++++
>  src/url.h     |  1 +
>  5 files changed, 104 insertions(+)
> 
> diff --git a/src/init.c b/src/init.c
> index d043d83..886d701 100644
> --- a/src/init.c
> +++ b/src/init.c
> @@ -322,6 +322,7 @@ static const struct {
>    { "user",             &opt.user,              cmd_string },
>    { "useragent",        NULL,                   cmd_spec_useragent },
>    { "useservertimestamps", &opt.useservertimestamps, cmd_boolean },
> +  { "usesshaskpass", &opt.use_ssh_askpass, cmd_boolean},
>    { "verbose",          NULL,                   cmd_spec_verbose },
>    { "wait",             &opt.wait,              cmd_time },
>    { "waitretry",        &opt.waitretry,         cmd_time },
> @@ -392,6 +393,8 @@ defaults (void)
>    tmp = getenv ("no_proxy");
>    if (tmp)
>      opt.no_proxy = sepstring (tmp);
> +  opt.use_ssh_askpass = false;
> +  opt.ssh_askpass = getenv ("SSH_ASKPASS");
>    opt.prefer_family = prefer_none;
>    opt.allow_cache = true;
>    opt.if_modified_since = true;
> diff --git a/src/main.c b/src/main.c
> index e7d5c66..cf82d3c 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -415,6 +415,7 @@ static struct cmdline_option option_data[] =
>      { "unlink", 0, OPT_BOOLEAN, "unlink", -1 },
>      { "trust-server-names", 0, OPT_BOOLEAN, "trustservernames", -1 },
>      { "use-server-timestamps", 0, OPT_BOOLEAN, "useservertimestamps", -1 },
> +    { "ssh-askpass", 0, OPT_BOOLEAN, "usesshaskpass", -1 },
>      { "user", 0, OPT_VALUE, "user", -1 },
>      { "user-agent", 'U', OPT_VALUE, "useragent", -1 },
>      { "verbose", 'v', OPT_BOOLEAN, "verbose", -1 },
> @@ -691,6 +692,8 @@ Download:\n"),
>      N_("\
>         --ask-password              prompt for passwords\n"),
>      N_("\
> +       --ssh-askpass               Use SSH_ASKPASS for credential
> requests\n"), +    N_("\
>         --no-iri                    turn off IRI support\n"),
>      N_("\
>         --local-encoding=ENC        use ENC as the local encoding for
> IRIs\n"), @@ -1019,6 +1022,81 @@ prompt_for_password (void)
>    return getpass("");
>  }
> 
> +
> +/* Execute external application SSH_ASKPASS which is stored in
> opt.ssh_askpass + */
> +void
> +run_ssh_askpass(const char *question, char **answer)
> +{
> +  char tmp[1024];
> +  pid_t pid;
> +  int com[2];
> +
> +  if (pipe(com) == -1)
> +  {
> +    fprintf(stderr, _("Cannot create pipe"));
> +    exit (WGET_EXIT_GENERIC_ERROR);
> +  }
> +
> +  pid = fork();
> +  if (pid == -1)
> +  {
> +    fprintf(stderr, "Error forking SSH_ASKPASS");
> +    exit (WGET_EXIT_GENERIC_ERROR);
> +  }
> +  else if (pid == 0)
> +  {
> +    /* Child */
> +    dup2(com[1], STDOUT_FILENO);
> +    close(com[0]);
> +    close(com[1]);
> +    fprintf(stdout, "test");
> +    execlp("/usr/bin/strace", "-s256", "-otest.out", opt.ssh_askpass,
> question, (char*)NULL); +    assert("Execlp failed!");
> +  }
> +  else
> +  {
> +    close(com[1]);
> +    unsigned int bytes = read(com[0], tmp, sizeof(tmp));
> +    if (!bytes)
> +    {
> +      fprintf(stderr,
> +        _("Error reading response from SSH_ASKPASS %s %s\n"),
> +        opt.ssh_askpass, question);
> +      exit (WGET_EXIT_GENERIC_ERROR);
> +    }
> +    else if (bytes > 1)
> +      *answer = strndup(tmp, bytes-1);
> +  }
> +}
> +
> +/* set the user name and password*/
> +void
> +ssh_askpass (struct url *u)
> +{
> +  static char question[1024];
> +
> +  if (u->user == NULL || u->user[0] == '\0')
> +  {
> +    sprintf(question, "Username for '%s%s': ",
> +        scheme_leading_string(u->scheme), u->host);
> +    /* Prompt for username */
> +    run_ssh_askpass(question, &u->user);
> +    if (opt.recursive)
> +      opt.user = strdup(u->user);
> +  }
> +
> +  if (u->passwd == NULL || u->passwd[0] == '\0')
> +  {
> +    sprintf(question, "Password for 'address@hidden': ",
> +        scheme_leading_string(u->scheme), u->user,
> +        u->host);
> +    /* Prompt for password */
> +    run_ssh_askpass(question, &u->passwd);
> +    if (opt.recursive)
> +      opt.passwd = strdup(u->passwd);
> +  }
> +}
>  /* Function that prints the line argument while limiting it
>     to at most line_length. prefix is printed on the first line
>     and an appropriate number of spaces are added on subsequent
> @@ -1702,6 +1780,16 @@ for details.\n\n"));
>          exit (WGET_EXIT_GENERIC_ERROR);
>      }
> 
> +  if (opt.use_ssh_askpass)
> +  {
> +    /* can't request credentials until the URL is known. */
> +    if (opt.ssh_askpass == NULL || opt.ssh_askpass[0] == '\0')
> +    {
> +    fprintf(stderr, _("--ssh-askpass requires environment variable
> SSH_ASKPASS to be set.\n")); +      exit(WGET_EXIT_GENERIC_ERROR);
> +    }
> +  }
> +
>  #ifdef USE_WATT32
>    if (opt.wdebug)
>       dbug_init();
> @@ -1920,6 +2008,10 @@ only if outputting to a regular file.\n"));
>          }
>        else
>          {
> +          if (opt.use_ssh_askpass)
> +          {
> +            ssh_askpass(url_parsed);
> +          }
>            if ((opt.recursive || opt.page_requisites)
>                && ((url_scheme (*t) != SCHEME_FTP
>  #ifdef HAVE_SSL
> diff --git a/src/options.h b/src/options.h
> index a8c494b..977c150 100644
> --- a/src/options.h
> +++ b/src/options.h
> @@ -130,6 +130,8 @@ struct options
>    char *user;                   /* Generic username */
>    char *passwd;                 /* Generic password */
>    bool ask_passwd;              /* Ask for password? */
> +  bool use_ssh_askpass;         /* Use SSH_ASKPASS infrastructure */
> +  char *ssh_askpass;            /* value of SSH_ASKPASS */
> 
>    bool always_rest;             /* Always use REST. */
>    wgint start_pos;              /* Start position of a download. */
> diff --git a/src/url.c b/src/url.c
> index ec38d6f..c133d91 100644
> --- a/src/url.c
> +++ b/src/url.c
> @@ -512,6 +512,12 @@ scheme_disable (enum url_scheme scheme)
>    supported_schemes[scheme].flags |= scm_disabled;
>  }
> 
> +const char *
> +scheme_leading_string (enum url_scheme scheme)
> +{
> +  return supported_schemes[scheme].leading_string;
> +}
> +
>  /* Skip the username and password, if present in the URL.  The
>     function should *not* be called with the complete URL, but with the
>     portion after the scheme.
> diff --git a/src/url.h b/src/url.h
> index 7c77737..bf2e3f7 100644
> --- a/src/url.h
> +++ b/src/url.h
> @@ -124,6 +124,7 @@ bool url_has_scheme (const char *);
>  bool url_valid_scheme (const char *);
>  int scheme_default_port (enum url_scheme);
>  void scheme_disable (enum url_scheme);
> +const char *scheme_leading_string(enum url_scheme);
> 
>  char *url_string (const struct url *, enum url_auth_mode);
>  char *url_file_name (const struct url *, char *);

signature.asc
Description: This is a digitally signed message part.