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

[Liam R. Howlett]

* Tim R?hsen <address@hidden> [160724 05:19]:
> 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...

I am not familiar with this call.  I'm happy to look at making this
change.

> * the 'strace' is left over from your tests !?

Yes.  Sorry.  I had pulled out the debug in my other tested versions
(1.15 and 1.16) - I will resubmit with this change and anything else
necessary.

> * could you attach your patch as a file, best generated with 'git 
> format-patch 
> -1' ?

I used git format-patch <previous git id> --cover for the patch I sent
followed by git send-email on both files.  I will attach any future
patches to the generated cover if this is your preference.

> * Did you already signed the FSF copyright assignment ? The patch is 
> considered 'non-trivial'.

I have not, but I don't think this will be an issue.  I'll look into
this.

Thank you,
Liam



> 
> 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 *);
>