Re: [Bug-wget] [PATCH] Let Wget select strongest auth challenge

[Giuseppe Scrivano]

Tim Rühsen <address@hidden> writes:

> Am Samstag, 22. November 2014, 16:24:18 schrieb Darshit Shah:
>> Another reason why I never got around to implementing this feature is that
>> it is required by almost no one. The issue at hand is that when a Server
>> responds with two possible authentication methods, the client is expected
>> to choose the strongest one it knows. Instead Wget chooses the first one it
>> knows. This violates the RFC and hence I marked it up as a bug. I'll
>> probably add all this information into the test file in a while and push
>> it.
>
> Hi Darshit,
>
> I just made up a patch to 
>
> 1. Parse multiple challenges from WWW-Authenticate
> 2. Select the strongest auth scheme
>
> Please have a look at it.
>
> Tim
>
> From a4c9939376cd8327e55111af3b190dd2e91f5746 Mon Sep 17 00:00:00 2001
> From: Tim Ruehsen <address@hidden>
> Date: Sat, 22 Nov 2014 22:00:28 +0100
> Subject: [PATCH] Select most secure auth challenge
>
> ---
>  src/http.c                         | 67 
> ++++++++++++++++++++++++++++++++------
>  testenv/server/http/http_server.py |  2 +-
>  2 files changed, 58 insertions(+), 11 deletions(-)
>
> diff --git a/src/http.c b/src/http.c
> index 87ceffd..832707d 100644
> --- a/src/http.c
> +++ b/src/http.c
> @@ -2380,26 +2380,64 @@ read_header:
>               the value "negotiate", and other(s) with data.  Loop over
>               all the occurrences and pick the one we recognize.  */
>            int wapos;
> +          char *buf;
> +          const char *www_authenticate = NULL;
>            const char *wabeg, *waend;
> -          char *www_authenticate = NULL;
> -          for (wapos = 0;
> -               (wapos = resp_header_locate (resp, "WWW-Authenticate", wapos,
> +          const char *digest = NULL, *basic = NULL, *ntlm = NULL;
> +          for (wapos = 0; !ntlm
> +               && (wapos = resp_header_locate (resp, "WWW-Authenticate", 
> wapos,
>                                              &wabeg, &waend)) != -1;
>                 ++wapos)
> -            if (known_authentication_scheme_p (wabeg, waend))
> -              {
> -                BOUNDED_TO_ALLOCA (wabeg, waend, www_authenticate);
> -                break;
> -              }
> +            {
> +              param_token name, value;
> +
> +              BOUNDED_TO_ALLOCA (wabeg, waend, buf);
> +              www_authenticate = buf;
> +
> +              for (;!ntlm;)
> +                {
> +                  /* extract the auth-scheme */
> +                  while (c_isspace (*www_authenticate)) www_authenticate++;
> +                  name.e = name.b = www_authenticate;
> +                  while (*name.e && !c_isspace (*name.e)) name.e++;
> +
> +                  if (name.b == name.e)
> +                    break;
> +
> +                  DEBUGP (("Auth scheme found '%.*s'\n", (int) (name.e - 
> name.b), name.b));
> +
> +                  if (known_authentication_scheme_p (name.b, name.e))
> +                    {
> +                      if (BEGINS_WITH (name.b, "NTLM"))
> +                        {
> +                          ntlm = name.b;
> +                          break; // most secure
> +                        }

should this be guarded by #ifdef ENABLE_NTLM?  Can we replace the C++
style comment with the C.  I know there are other few places using them,
but I think we should have just one style.

ACK for the rest.

Regards,
Giuseppe