Bug 54707 - Buggy Perl http clients cause tomcat digest auth to fail due to quoted nc values (e.g. nc="00000001")
Summary: Buggy Perl http clients cause tomcat digest auth to fail due to quoted nc val...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 7.0.37
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-15 17:41 UTC by bruce weertman
Modified: 2013-03-17 13:48 UTC (History)
1 user (show)



Attachments
Possible change to HttpParser.readLhex and HttpParser.readQuotedLhex (1.77 KB, text/plain)
2013-03-15 22:54 UTC, bruce weertman
Details

Note You need to log in before you can comment on or make changes to this bug.
Description bruce weertman 2013-03-15 17:41:24 UTC
This problem was discovered while trying to make the Perl LWP library work with tomcat where a path was being protected with digest authentication. 

Ultimately, this is a bug with Perl (see https://rt.cpan.org/Public/Bug/Display.html?id=43354), and it has been possibly fixed. 
HOWEVER, this requires users to update their perl libraries etc (a big pain for our customers and for us).

A simple fix to tomcat would solve this problem for us and make life good again.

A typical digest response header from perl looks like:

Authorization: Digest username="joe", realm="ACME", qop="auth", algorithm="MD5", uri="/my/protected/path", nonce="1363130363664:71e75a43d7fdbfff8c54bece373058b8", nc="00000001", cnonce="513fb7fb", response="baeeff0b6b9b7e74e769630160d3725b", message-digest="d41d8cd98f00b204e9800998ecf8427e", opaque="9C2C62C52D30A7D5707F75F5A813F113"

The entry nc="00000001" causes tomcat to reject the request.  It should be nc=00000001 (the perl client's mistake)

The following perl script demonstrates the problem:

#!/usr/bin/perl
use strict;
use LWP;

&doGet("myname", "mypassword", "myrealmname", "myhost", "8080", "/my/protected/path");

sub doGet
{
    my ($username, $password, $realm, $host, $port, $uri) = @_;
    my $url = "http://".$host.":".$port.$uri;

    print "GET: $url\n";

    my $browser = LWP::UserAgent->new;

    $browser->agent('Debug Digest Problem');
    $browser->credentials($host.":".$port,$realm,$username=>$password);

    my $response=$browser->get($url);

    print "HTTP STATUS:".$response->status_line."\n";
    print $response->content;
}

The fix should be in org.apache.tomcat.util.http.parser.HttpParser.java

Looking at build 7.0.37 code:

Around line 147:
                case 3:
                    // FIELD_TYPE_LHEX
                    value = readLhex(input);
                    break;

This switch is hit when the field is 'nc' (due to line 72 fieldTypes.put("nc", FIELD_TYPE_LHEX); ]

The method readLhex() does not tolerant quotes. (see line 434 and below).

A similar tomcat issue was fixed a while back for the quoted qop field. (the qop response field should also not be quoted, but tomcat handles this).

Also see line 375
     * This is not defined in any RFC. It is a special case to handle data from
     * buggy clients (known buggy clients include Microsoft IE 8 & 9, Apple
     * Safari for OSX and iOS) that add quotes to values that should be tokens.

Server software which tolerates this mistake in the perl client includes Apache and Spring's security filter 
(org.springframework.security.web.authentication.www.DigestAuthenticationFilter).
Comment 1 bruce weertman 2013-03-15 22:54:22 UTC
Created attachment 30055 [details]
Possible change to HttpParser.readLhex and HttpParser.readQuotedLhex

To fix the problem, readLhex() must be changed. 

The existing readQuotedLhex() calls readLhex(), therefore it must also be changed. 

This fix places the old readLhex() code inside of readQuotedLhex() so that it 
does not call readLhex(). 

The new readLhex() can tolerate an optional starting quote. 

If there's  a starting quote, but no ending quote, readLhex() returns null.

Easy as Pi.
Comment 2 Mark Thomas 2013-03-15 23:01:15 UTC
Why isn't the fix just:

Index: java/org/apache/tomcat/util/http/parser/HttpParser.java
===================================================================
--- java/org/apache/tomcat/util/http/parser/HttpParser.java	(revision 1456972)
+++ java/org/apache/tomcat/util/http/parser/HttpParser.java	(working copy)
@@ -68,7 +68,7 @@
         fieldTypes.put("cnonce", FIELD_TYPE_QUOTED_STRING);
         fieldTypes.put("opaque", FIELD_TYPE_QUOTED_STRING);
         fieldTypes.put("qop", FIELD_TYPE_QUOTED_TOKEN);
-        fieldTypes.put("nc", FIELD_TYPE_LHEX);
+        fieldTypes.put("nc", FIELD_TYPE_QUOTED_LHEX);
 
         // Setup the flag arrays
         for (int i = 0; i < 128; i++) {
Comment 3 bruce weertman 2013-03-15 23:09:18 UTC
I believe this will make Perl work and everything else fail!. Another alternative would be to make quotes optional all the way round, but I'm not sure that's good idea? Maybe it is, I don't know.
Comment 4 bruce weertman 2013-03-15 23:38:33 UTC
This doesn't really cause a problem since there's a fall through that catches
the mistake... 

But at line 66 you'll see:
fieldTypes.put("digest-uri", FIELD_TYPE_QUOTED_STRING);

I believe that's a typo. Should read:
fieldTypes.put("uri", FIELD_TYPE_QUOTED_STRING);

See: http://tools.ietf.org/html/rfc2617 
3.2.2 The Authorization Request Header

       digest-uri       = "uri" "=" digest-uri-value
       digest-uri-value = request-uri   ; As specified by HTTP/1.1

The RFC's are hard to read!
Comment 5 Mark Thomas 2013-03-16 12:24:46 UTC
(In reply to comment #3)
> I believe this will make Perl work and everything else fail!

You are correct. That is what I get for trying to add something to a bug report when I should be sleeping.

> Another alternative would be to make quotes optional all the way round, but I'm
> not sure that's good idea? Maybe it is, I don't know.

RFC2616 encourages servers to be tolerant of buggy input where they can do so unambiguously. I'll take a more general look at the parser and see what can be done. Either way, this particular issue will be fixed.
Comment 6 Mark Thomas 2013-03-17 13:48:56 UTC
Fixed in trunk and 7.0.x and will be included in 7.0.39 onwards.