bug-wget
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Bug-wget] [Bug--Wget] Issue with RFC 2067 Digest Headers


From: Tim Rühsen
Subject: Re: [Bug-wget] [Bug--Wget] Issue with RFC 2067 Digest Headers
Date: Sat, 13 Jul 2013 14:31:40 +0200
User-agent: KMail/1.13.7 (Linux/3.9-1-amd64; KDE/4.8.4; x86_64; ; )

Am Freitag, 12. Juli 2013 schrieb Giuseppe Scrivano:
> Tim Rühsen <address@hidden> writes:
> 
> >> +  realm = opaque = nonce = qop = NULL;
> >> +  algorithm = "MD5";
> >
> > Don't do that.
> > 1. 'algorithm' will be xfreed later
> > 2. this forces a 'algorithm="MD5" parameter even if it wasn't given before
> > Instead use:
> >      if (algorithm != NULL && ! strcmp (algorithm, "MD5-sess"))
> >
> > The function does not free values allocated by strdupdelim () when 
returning.
> > That seems to be something that has never been done.
> >
> > I hope, I am not too late ;-)
> 
> ops, sorry, I merged too quickly.  I want to change the behaviour, if
> the algorithm is not specified then assume MD5.  This is different than
> before as we assumed it is always specified, and it is incorrect as
> Darshit pointed out with his example which is compliant with the RFC.

We assumed 'algorithm' to be MD5 before (implicitely), but had the bug to miss 
one check before strcmp().

For me, your changes look ok.

There is just that little issue (more a kind of favour) that I mentioned under 
as 2. : When the server does not mention 'algorithm' in WWW-Authenticate:, 
should we introduce it in the clients Authenticate: Header ? I can't say what 
is better... RFC 2069 and RFC 2617 leave it open.
At least we would introduce an additional (unneeded) xstrdup/free.

So, the decision is yours ;-)

Regards, Tim

> 
> Something against this, I wait for your ACK this time :-)
> 
> diff --git a/src/http.c b/src/http.c
> index 9f274dc..3af7009 100644
> --- a/src/http.c
> +++ b/src/http.c
> @@ -3703,8 +3703,7 @@ digest_authentication_encode (const char *au, const 
char *user,
>    param_token name, value;
>  
>  
> -  realm = opaque = nonce = qop = NULL;
> -  algorithm = "MD5";
> +  realm = opaque = nonce = algorithm = qop = NULL;
>  
>    au += 6;                      /* skip over `Digest' */
>    while (extract_param (&au, &name, &value, ','))
> @@ -3743,6 +3742,9 @@ digest_authentication_encode (const char *au, const 
char *user,
>        return NULL;
>      }
>  
> +  if (algorithm == NULL)
> +    algorithm = xstrdup ("MD5");
> +
>    /* Calculate the digest value.  */
>    {
>      struct md5_ctx ctx;
> @@ -3829,7 +3831,7 @@ digest_authentication_encode (const char *au, const 
char *user,
>               + strlen (path)
>               + 2 * MD5_DIGEST_SIZE /*strlen (response_digest)*/
>               + (opaque ? strlen (opaque) : 0)
> -             + (algorithm ? strlen (algorithm) : 0)
> +             + strlen (algorithm)
>               + (qop ? 128: 0)
>               + strlen (cnonce)
>               + 128;
> @@ -3856,11 +3858,15 @@ digest_authentication_encode (const char *au, const 
char *user,
>          res_len += snprintf(res + res_len, res_size - res_len, ", 
opaque=\"%s\"", opaque);
>        }
>  
> -    if (algorithm)
> -      {
> -        snprintf(res + res_len, res_size - res_len, ", algorithm=\"%s\"", 
algorithm);
> -      }
> +    snprintf(res + res_len, res_size - res_len, ", algorithm=\"%s\"", 
algorithm);
>    }
> +
> +  xfree_null (realm);
> +  xfree_null (opaque);
> +  xfree_null (nonce);
> +  xfree_null (qop);
> +  xfree_null (algorithm);
> +
>    return res;
>  }
>  #endif /* ENABLE_DIGEST */
> 

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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]