Project

General

Profile

Bug #2974

HTTP digest authentication not compatible with some clients

Added by xvh 27 days ago. Updated 10 days ago.

Status:
Invalid
Priority:
Normal
Assignee:
-
Category:
mod_auth
Target version:
Start date:
2019-08-22
Due date:
% Done:

0%

Estimated time:
Missing in 1.5.x:

Description

Hi,

since version lighttpd-1.4.41 when digest authentication was enhanced the digest authentication is not compatible with .NET HttpClient implementation. The problem is that HttpClient does not include query to the digest uri (when original request is '/resource?param=0' then the uri parameter in authorization header is '/resource' only - I assume that this implementation does not follow the RFC correctly). This causes that requests with queries fail with status 400.

Issue was tested with version 1.4.53. Uri check is implemented in file src/mod_auth.c:1080. 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?

Instead of:

const size_t ulen = strlen(uri);
const size_t rlen = buffer_string_length(con->request.orig_uri);
if (!buffer_is_equal_string(con->request.orig_uri, uri, ulen)
    && !(rlen < ulen && 0 == memcmp(con->request.orig_uri->ptr, uri, rlen) && uri[rlen] == '?')) {

I would expect switched uri variables this way:
if (!buffer_is_equal_string(con->request.orig_uri, uri, ulen)
    && !(ulen < rlen && 0 == memcmp(con->request.orig_uri->ptr, uri, ulen) && con->request.orig_uri->ptr[ulen] == '?')) {

I've attached also patch which solved the compatibility issue for me.

Thank you for the response.


Related issues

Related to Bug #1844: Serious security problem in Digest AuthenticationFixed2008-12-12

Actions

History

#1

Updated by stbuehler 27 days ago

  • Related to Bug #1844: Serious security problem in Digest Authentication added
#2

Updated by stbuehler 27 days ago

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.

#3

Updated by xvh 27 days ago

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.

#4

Updated by gstrauss 10 days ago

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.

#5

Updated by gstrauss 10 days ago

  • Status changed from New to Invalid

https://www.rfc-editor.org/rfc/rfc7616.txt
3.4.6. Various Considerations

   ...
   The "request-target" 
   value is the request-target from the request line as specified in
   Section 3.1.1 of [RFC7230].  This MAY be "*", an "absolute-URI", or
   an "absolute-path" as specified in Section 2.7 of [RFC7230], but it
   MUST agree with the request-target.
   ...

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).

tl;dr: the request URI, including query-string (if present), MUST be used.

.NET HttpClient is not compliant with RFC7616 if .NET HttpClient is not operating on the full URI, including query-string (if present)
https://blogs.msdn.microsoft.com/daniem/2013/02/27/digest-authentication-in-system-net-classes-dont-fully-comply-with-rfc2617/
https://stackoverflow.com/questions/3109507/httpwebrequests-sends-parameterless-uri-in-authorization-header
https://stackoverflow.com/questions/30981376/http-digest-authentication-with-parameters

#6

Updated by xvh 10 days ago

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.

Also available in: Atom