Re: [Bug-wget] [PATCH v3] bug #48811: netrc password wins over interactive --ask-password

[Tim Ruehsen]

Thanks Piotr,

just one question for my understanding...

In the current code, 'user' and 'password' selection matches:

> -  user = u->user;
> -  passwd = u->passwd;
> -  search_netrc (u->host, (const char **)&user, (const char **)&passwd, 1);
> -  user = user ? user : (opt.ftp_user ? opt.ftp_user : opt.user);
> -  if (!user) user = "anonymous";
> -  passwd = passwd ? passwd : (opt.ftp_passwd ? opt.ftp_passwd :
> opt.passwd); -  if (!passwd) passwd = "-wget@";

In the new code, the expressions for the selection do not match:

> +  cred->user = cred->user ? cred->user : (protocol_specific_user ?
> protocol_specific_user : opt.user);
> +  cred->passwd = opt_passwd ? opt_passwd : (u->passwd ? u->passwd : 
> protocol_specific_passwd);

Is this by purpose and if so, please explain / add a explanatory comment.

Regards, Tim

On Tuesday, November 22, 2016 10:32:18 PM CET Piotr Wajda wrote:
> * utils.h: New struct net_credentials and pick_credentials function
> * utils.c: pick_credentials for selecting FTP/HTTP user/passwd according to
> importance * retr.c: define net_cred structure to hold user/passwd pair
> * ftp.c: call pick_credentials before getftp
> * http.c: call pick_credentials before gethttp
> 
> ---
>  src/ftp.c   | 25 ++++++++++---------------
>  src/http.c  | 26 +++++++++++---------------
>  src/retr.c  |  3 +++
>  src/utils.c | 20 ++++++++++++++++++++
>  src/utils.h | 11 +++++++++++
>  5 files changed, 55 insertions(+), 30 deletions(-)
> 
> diff --git a/src/ftp.c b/src/ftp.c
> index 947d8f2..42c84ad 100644
> --- a/src/ftp.c
> +++ b/src/ftp.c
> @@ -81,6 +81,7 @@ typedef struct
>    struct url *proxy;            /* FTWK-style proxy */
>  } ccon;
> 
> +extern struct net_credentials net_cred;
> 
>  /* Look for regexp "( *[0-9]+ *byte" (literal parenthesis) anywhere in
>     the string S, and return the number converted to wgint, if found, 0
> @@ -321,13 +322,13 @@ static uerr_t
>  getftp (struct url *u, struct url *original_url,
>          wgint passed_expected_bytes, wgint *qtyread,
>          wgint restval, ccon *con, int count, wgint *last_expected_bytes,
> -        FILE *warc_tmp)
> +        FILE *warc_tmp, struct net_credentials *ftp_cred)
>  {
>    int csock, dtsock, local_sock, res;
>    uerr_t err = RETROK;          /* appease the compiler */
>    FILE *fp = NULL;
>    char *respline, *tms;
> -  const char *user, *passwd, *tmrate;
> +  const char *tmrate;
>    int cmd = con->cmd;
>    wgint expected_bytes = 0;
>    bool got_expected_bytes = false;
> @@ -359,14 +360,6 @@ getftp (struct url *u, struct url *original_url,
> 
>    *qtyread = restval;
> 
> -  user = u->user;
> -  passwd = u->passwd;
> -  search_netrc (u->host, (const char **)&user, (const char **)&passwd, 1);
> -  user = user ? user : (opt.ftp_user ? opt.ftp_user : opt.user);
> -  if (!user) user = "anonymous";
> -  passwd = passwd ? passwd : (opt.ftp_passwd ? opt.ftp_passwd :
> opt.passwd); -  if (!passwd) passwd = "-wget@";
> -
>    dtsock = -1;
>    local_sock = -1;
>    con->dltime = 0;
> @@ -461,18 +454,18 @@ getftp (struct url *u, struct url *original_url,
> 
>        /* Second: Login with proper USER/PASS sequence.  */
>        logprintf (LOG_VERBOSE, _("Logging in as %s ... "),
> -                 quotearg_style (escape_quoting_style, user));
> +                 quotearg_style (escape_quoting_style, ftp_cred->user));
>        if (opt.server_response)
>          logputs (LOG_ALWAYS, "\n");
>        if (con->proxy)
>          {
>            /* If proxy is in use, log in as address@hidden */
> -          char *logname = concat_strings (user, "@", u->host, (char *) 0);
> -          err = ftp_login (csock, logname, passwd);
> +          char *logname = concat_strings (ftp_cred->user, "@", u->host,
> (char *) 0); +          err = ftp_login (csock, logname, ftp_cred->passwd);
>            xfree (logname);
>          }
>        else
> -        err = ftp_login (csock, user, passwd);
> +        err = ftp_login (csock, ftp_cred->user, ftp_cred->passwd);
> 
>        /* FTPRERR, FTPSRVERR, WRITEFAILED, FTPLOGREFUSED, FTPLOGINC */
>        switch (err)
> @@ -1937,10 +1930,12 @@ ftp_loop_internal (struct url *u, struct url
> *original_url, struct fileinfo *f, else
>          len = 0;
> 
> +      pick_credentials (u, opt.ftp_user, opt.ftp_passwd, opt.user,
> opt.passwd, 1, &net_cred); +
>        /* If we are working on a WARC record, getftp should also write
>           to the warc_tmp file. */
>        err = getftp (u, original_url, len, &qtyread, restval, con, count,
> -                    &last_expected_bytes, warc_tmp);
> +                    &last_expected_bytes, warc_tmp, &net_cred);
> 
>        if (con->csock == -1)
>          con->st &= ~DONE_CWD;
> diff --git a/src/http.c b/src/http.c
> index 67b3686..fbd10dd 100644
> --- a/src/http.c
> +++ b/src/http.c
> @@ -79,6 +79,7 @@ as that of the covered work.  */
>  # include "vms.h"
>  #endif /* def __VMS */
> 
> +extern struct net_credentials net_cred;
> 
>  /* Forward decls. */
>  struct http_stat;
> @@ -1813,7 +1814,7 @@ time_to_rfc1123 (time_t time, char *buf, size_t
> bufsize) static struct request *
>  initialize_request (const struct url *u, struct http_stat *hs, int *dt,
> struct url *proxy, bool inhibit_keep_alive, bool *basic_auth_finished, -   
>                 wgint *body_data_size, char **user, char **passwd, uerr_t
> *ret) +                    wgint *body_data_size, struct net_credentials
> **http_cred, uerr_t *ret) {
>    bool head_only = !!(*dt & HEAD_ONLY);
>    struct request *req;
> @@ -1875,21 +1876,14 @@ initialize_request (const struct url *u, struct
> http_stat *hs, int *dt, struct u request_set_header (req, "Accept", "*/*",
> rel_none);
>    request_set_header (req, "Accept-Encoding", "identity", rel_none);
> 
> -  /* Find the username and password for authentication. */
> -  *user = u->user;
> -  *passwd = u->passwd;
> -  search_netrc (u->host, (const char **)user, (const char **)passwd, 0);
> -  *user = *user ? *user : (opt.http_user ? opt.http_user : opt.user);
> -  *passwd = *passwd ? *passwd : (opt.http_passwd ? opt.http_passwd :
> opt.passwd); -
>    /* We only do "site-wide" authentication with "global" user/password
>     * values unless --auth-no-challange has been requested; URL
> user/password * info overrides. */
> -  if (*user && *passwd && (!u->user || opt.auth_without_challenge))
> +  if ((*http_cred)->user && (*http_cred)->passwd && (!u->user ||
> opt.auth_without_challenge)) {
>        /* If this is a host for which we've already received a Basic
>         * challenge, we'll go ahead and send Basic authentication creds. */
> -      *basic_auth_finished = maybe_send_basic_creds (u->host, *user,
> *passwd, req); +      *basic_auth_finished = maybe_send_basic_creds
> (u->host, (*http_cred)->user, (*http_cred)->passwd, req); }
> 
>    /* Generate the Host header, HOST:PORT.  Take into account that:
> @@ -3061,12 +3055,11 @@ fail:
>     server, and u->url will be requested.  */
>  static uerr_t
>  gethttp (const struct url *u, struct url *original_url, struct http_stat
> *hs, -         int *dt, struct url *proxy, struct iri *iri, int count)
> +         int *dt, struct url *proxy, struct iri *iri, int count, struct
> net_credentials *http_cred) {
>    struct request *req = NULL;
> 
>    char *type = NULL;
> -  char *user, *passwd;
>    char *proxyauth;
>    int statcode;
>    int write_error;
> @@ -3175,7 +3168,7 @@ gethttp (const struct url *u, struct url
> *original_url, struct http_stat *hs, uerr_t ret;
>      req = initialize_request (u, hs, dt, proxy, inhibit_keep_alive,
>                                &basic_auth_finished, &body_data_size,
> -                              &user, &passwd, &ret);
> +                              &http_cred, &ret);
>      if (req == NULL)
>        {
>          retval = ret;
> @@ -3509,7 +3502,7 @@ gethttp (const struct url *u, struct url
> *original_url, struct http_stat *hs, pconn.authorized = false;
> 
>        {
> -        auth_err = check_auth (u, user, passwd, resp, req,
> +        auth_err = check_auth (u, http_cred->user, http_cred->passwd, resp,
> req, &ntlm_seen, &retry,
>                                 &basic_auth_finished,
>                                 &auth_finished);
> @@ -4152,8 +4145,11 @@ http_loop (const struct url *u, struct url
> *original_url, char **newloc, else
>          *dt &= ~SEND_NOCACHE;
> 
> +      /* Find the username and password for authentication. */
> +      pick_credentials (u, opt.http_user, opt.http_passwd, opt.user,
> opt.passwd, 0, &net_cred); +
>        /* Try fetching the document, or at least its head.  */
> -      err = gethttp (u, original_url, &hstat, dt, proxy, iri, count);
> +      err = gethttp (u, original_url, &hstat, dt, proxy, iri, count,
> &net_cred);
> 
>        /* Time?  */
>        tms = datetime_str (time (NULL));
> diff --git a/src/retr.c b/src/retr.c
> index 7b32b35..1e77e9e 100644
> --- a/src/retr.c
> +++ b/src/retr.c
> @@ -71,6 +71,9 @@ FILE *output_stream;
>  /* Whether output_document is a regular file we can manipulate,
>     i.e. not `-' or a device file. */
>  bool output_stream_regular;
> +
> +/* Structure holding FTP or HTTP credentials  */
> +struct net_credentials net_cred;
> 
>  static struct {
>    wgint chunk_bytes;
> diff --git a/src/utils.c b/src/utils.c
> index dcf90a6..a15e5e3 100644
> --- a/src/utils.c
> +++ b/src/utils.c
> @@ -30,6 +30,7 @@ shall include the source code for the parts of OpenSSL
> used as well as that of the covered work.  */
> 
>  #include "wget.h"
> +#include "netrc.h"
> 
>  #include "sha256.h"
>  #include <stdio.h>
> @@ -440,6 +441,25 @@ fmttime (time_t t, const char *fmt)
>    return output;
>  }
> 
> +/*
> + Pick all credentials provided using --user, --password, --http-user,
> --http-password, --ftp-user, --ftp-password, --ask-password and from .netrc
> file. Select most important ones. + */
> +void
> +pick_credentials (const struct url *u, char *protocol_specific_user, char
> *protocol_specific_passwd, char *opt_user, char *opt_passwd, int
> slack_default, struct net_credentials *cred) { +  cred->passwd = NULL;
> +  cred->user = u->user;
> +  cred->user = cred->user ? cred->user : (protocol_specific_user ?
> protocol_specific_user : opt.user); +  cred->passwd = opt_passwd ?
> opt_passwd : (u->passwd ? u->passwd : protocol_specific_passwd); +  if
> (!cred->user && !cred->passwd)
> +    search_netrc (u->host, (const char **)&cred->user, (const char
> **)&cred->passwd, slack_default); +
> +  if (slack_default)
> +    {
> +      if (!cred->user) cred->user = "anonymous";
> +      if (!cred->passwd) cred->passwd = "-wget@";
> +    }
> +}
> +
>  /* Return pointer to a static char[] buffer in which zero-terminated
>     string-representation of TM (in form hh:mm:ss) is printed.
> 
> diff --git a/src/utils.h b/src/utils.h
> index f224b73..c7450fe 100644
> --- a/src/utils.h
> +++ b/src/utils.h
> @@ -29,6 +29,9 @@ Corresponding Source for a non-source form of such a
> combination shall include the source code for the parts of OpenSSL used as
> well as that of the covered work.  */
> 
> +// Include this to handle 'const struct url *' in pick_credentials()
> +#include "url.h"
> +
>  #ifndef UTILS_H
>  #define UTILS_H
> 
> @@ -63,6 +66,14 @@ struct file_memory {
>    int mmap_p;
>  };
> 
> +/* Struct and function for handling HTTP and FTP credentials */
> +struct net_credentials {
> +    char *user;
> +    char *passwd;
> +};
> +
> +void pick_credentials (const struct url *, char *, char *, char *, char *,
> int, struct net_credentials *); +
>  #define HYPHENP(x) (*(x) == '-' && !*((x) + 1))
> 
>  char *time_str (time_t);

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