Bug 37132 - Digest authentication does not work if the username or URI contain a comma
Summary: Digest authentication does not work if the username or URI contain a comma
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 5
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 5.5.7
Hardware: Other other
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-10-18 01:00 UTC by Robert Wille
Modified: 2005-11-28 12:50 UTC (History)
0 users



Attachments
Patch to fix this defect (1.08 KB, patch)
2005-11-14 21:00 UTC, Robert Wille
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Wille 2005-10-18 01:00:42 UTC
The digest authenticator does a simple tokenization of the authentication
header. If the username or the URI contain a comma, authentication fails because
the tokenization does not take into consideration tokens that have commas in them. 

To duplicate, go to any page that requires digest authentication, and add the
query string ?a=b,c. Authentication will fail. If you've already authenticated,
you may need to close your browser and try again.

Following is modified version of
org.apache.catalina.authenticator.DigestAuthenticator.findPrincipal that fixes
the problem. It tokenizes using a regular expression that ignores commas between
quotes (or rather, it ignores commas that are not followed by an even number of
quotes).

I would have tested and modified 5.5.12, except that I am using a custom build
based on 5.5.7. However, I have looked at DigestAuthenticator.java in 5.5.12 and
noticed that it has the same problem.

    protected static Principal findPrincipal(Request request,
                                             String authorization,
                                             Realm realm) {

        //System.out.println("Authorization token : " + authorization);
        // Validate the authorization credentials format
        if (authorization == null)
            return (null);
        if (!authorization.startsWith("Digest "))
            return (null);
        authorization = authorization.substring(7).trim();

        String[] tokens = authorization.split(",(?=(?:[^\"]*\"[^\"]*\")+$)");
        
        String userName = null;
        String realmName = null;
        String nOnce = null;
        String nc = null;
        String cnonce = null;
        String qop = null;
        String uri = null;
        String response = null;
        String method = request.getMethod();

        for (int i = 0; i < tokens.length; i++) {
            String currentToken = tokens[i];
            if (currentToken.length() == 0)
                continue;
            
            int equalSign = currentToken.indexOf('=');
            if (equalSign < 0)
                return null;
            String currentTokenName =
                currentToken.substring(0, equalSign).trim();
            String currentTokenValue =
                currentToken.substring(equalSign + 1).trim();
            if ("username".equals(currentTokenName))
                userName = removeQuotes(currentTokenValue);
            if ("realm".equals(currentTokenName))
                realmName = removeQuotes(currentTokenValue, true);
            if ("nonce".equals(currentTokenName))
                nOnce = removeQuotes(currentTokenValue);
            if ("nc".equals(currentTokenName))
                nc = removeQuotes(currentTokenValue);
            if ("cnonce".equals(currentTokenName))
                cnonce = removeQuotes(currentTokenValue);
            if ("qop".equals(currentTokenName))
                qop = removeQuotes(currentTokenValue);
            if ("uri".equals(currentTokenName))
                uri = removeQuotes(currentTokenValue);
            if ("response".equals(currentTokenName))
                response = removeQuotes(currentTokenValue);
        }

        if ( (userName == null) || (realmName == null) || (nOnce == null)
             || (uri == null) || (response == null) )
            return null;

        // Second MD5 digest used to calculate the digest :
        // MD5(Method + ":" + uri)
        String a2 = method + ":" + uri;
        //System.out.println("A2:" + a2);

        byte[] buffer = null;
        synchronized (md5Helper) {
            buffer = md5Helper.digest(a2.getBytes());
        }
        String md5a2 = md5Encoder.encode(buffer);

        return (realm.authenticate(userName, response, nOnce, nc, cnonce, qop,
                                   realmName, md5a2));

    }
Comment 1 Yoav Shapira 2005-10-20 03:27:05 UTC
I see the problem: can you please submit a patch in the diff -u format we ask
for on the Get Involved pages
(http://www.apache.org/dev/contributors.html#patches) ?  I'll gladly commit it...
Comment 2 Robert Wille 2005-11-14 21:00:29 UTC
Created attachment 16968 [details]
Patch to fix this defect
Comment 3 Yoav Shapira 2005-11-28 21:50:02 UTC
Fixed, thank you for finding this issue and submitting the patch.