Lighttpd

Hi,

xvh wrote:

[...]
I've found out that there is already part of the condition on line 1081 that checks the '?' character in URI and it accepts the digest with query. However it checks the digest uri instead of original request uri. What is the exact purpose of the second part of the condition?

I'm confused by this too, yes. Not quite sure what the motivation for this was, neither the original issue nor the commit fixing it indicate any reason afaics.

And neither RFC 2617 nor RFC 7616 (nor their errata pages) seem to indicate the query string is optional or whether the URI should be checked in the first place (or how to compare it).

I guess you are right and the check for optional query should be the other way round, but I'm gonna leave this to gstrauss.

stbuehler wrote:

And neither RFC 2617 nor RFC 7616 (nor their errata pages) seem to indicate the query string is optional or whether the URI should be checked in the first place (or how to compare it).

From the security point of view the query should be definitely included in the digest and there is sentence "The authenticating server MUST assure that the resource designated by the "uri" parameter is the same as the resource specified in the Request-Line" in RFC 7616 which can be interpreted this way. However some widely used clients implement it without query for some reason.

The first part of the comment above the code is relevant to describe my intent.

detect if attacker is attempting to reuse valid digest for one uri on a different request uri.

        /* detect if attacker is attempting to reuse valid digest for one uri
         * on a different request uri.  Might also happen if intermediate proxy
         * altered client request line.  (Altered request would not result in
         * the same digest as that calculated by the client.)
         * Internal redirects such as with mod_rewrite will modify request uri.
         * Reauthentication is done to detect crossing auth realms, but this
         * uri validation step is bypassed.  con->request.orig_uri is original
         * uri sent in client request. */

The change was introduced in commit 00cc4d7c0 for (#1844)

    [mod_auth] fix Digest auth to be better than Basic (fixes #1844)

    Make Digest authentication more compliant with RFC.

    Excerpt from https://www.rfc-editor.org/rfc/rfc7616.txt Section 5.13:
        The bottom line is that any compliant implementation will be
        relatively weak by cryptographic standards, but any compliant
        implementation will be far superior to Basic Authentication.

    x-ref:
      "Serious security problem in Digest Authentication" 
      https://redmine.lighttpd.net/issues/1844

and updated in commit cb24958c0 for (#2745) to use request.orig_uri instead of request.uri

    [mod_auth] Digest auth fails after rewrite (fixes #2745)

    (affects lighttpd 1.4.41)

    x-ref:
      "HTTP digest + rewrite fails with: digest: auth failed: uri mismatch (1.4.41)" 
      https://redmine.lighttpd.net/issues/2745

Now then, the code was originally written to operate on con->request.uri, which could be modified by mod_rewrite, so if I remember correctly, the second condition allowed the case where the uri= param in the Authorization header contains a query string, but con->request.uri no longer does, i.e. it was removed. I am not sure if that is the reason I did that, but it seems plausible, and so I will consider removing that condition.

I need to go re-read the RFCs before suggesting further changes in this issue.

xvh wrote:

However some widely used clients implement it without query for some reason.

Given that lighttpd 1.4.42 was released almost 3 years ago, and you're the first to report this issue, you're going to having to do a better job backing up that sentence. Are you referring to anything other than .NET HttpClient? Please provide additional details to support your vague statement. I can tell you from personal experience at my $dayjob, that .NET HttpClient is a PoS which is both unreliable and has terrible performance. It is unsurprising to me that it appears to misimplement HTTP Digest authentication.

gstrauss wrote:

This is crystal-clear: the request-target on the request-line sent by the client MUST match the request-target used in H(A2) in the Authenticate header response=..., and therefore the uri= parameter specified in the Authenticate header must be this same value so that the server can reproduce and validate the keyed digest KD containing H(A2).

Thank you for clarification. I completely agree with your conclusion. To be more specific I've found this behaviour only in .NET HttpClient.

I think problem was not reported because preconditions for this issue are unlikely. Sadly our product relies on legacy client implementation which is using this .NET library. I consider this issue as closed.