CVE-2011-5063

@@ -23,13 +23,16 @@
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.security.Principal;
+import java.util.LinkedHashMap;
+import java.util.Map;
 import java.util.StringTokenizer;
 
 import javax.servlet.http.HttpServletResponse;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.apache.catalina.LifecycleException;
 import org.apache.catalina.Realm;
 import org.apache.catalina.connector.Request;
 import org.apache.catalina.connector.Response;
@@ -47,8 +50,8 @@
  * @version $Id$
  */
 
-public class DigestAuthenticator
-    extends AuthenticatorBase {
+public class DigestAuthenticator extends AuthenticatorBase {
+
     private static Log log = LogFactory.getLog(DigestAuthenticator.class);
 
 
@@ -67,6 +70,11 @@ public class DigestAuthenticator
         "org.apache.catalina.authenticator.DigestAuthenticator/1.0";
 
 
+    /**
+     * Tomcat's DIGEST implementation only supports auth quality of protection.
+     */
+    protected static final String QOP = "auth";
+
     // ----------------------------------------------------------- Constructors
 
 
@@ -91,15 +99,46 @@ public DigestAuthenticator() {
     protected static MessageDigest md5Helper;
 
 
+    /**
+     * List of client nonce values currently being tracked
+     */
+    protected Map cnonces;
+
+
+    /**
+     * Maximum number of client nonces to keep in the cache. If not specified,
+     * the default value of 1000 is used.
+     */
+    protected int cnonceCacheSize = 1000;
+
+
     /**
      * Private key.
      */
-    protected String key = "Catalina";
+    protected String key = null;
 
 
-    // ------------------------------------------------------------- Properties
+    /**
+     * How long server nonces are valid for in milliseconds. Defaults to 5
+     * minutes.
+     */
+    protected long nonceValidity = 5 * 60 * 1000;
+
+
+    /**
+     * Opaque string.
+     */
+    protected String opaque;
 
 
+    /**
+     * Should the URI be validated as required by RFC2617? Can be disabled in
+     * reverse proxies where the proxy has modified the URI.
+     */
+    protected boolean validateUri = true;
+
+    // ------------------------------------------------------------- Properties
+
     /**
      * Return descriptive information about this Valve implementation.
      */
@@ -110,9 +149,58 @@ public String getInfo() {
     }
 
 
-    // --------------------------------------------------------- Public Methods
+    public int getCnonceCacheSize() {
+        return cnonceCacheSize;
+    }
 
 
+    public void setCnonceCacheSize(int cnonceCacheSize) {
+        this.cnonceCacheSize = cnonceCacheSize;
+    }
+
+
+    public String getKey() {
+        return key;
+    }
+
+
+    public void setKey(String key) {
+        this.key = key;
+    }
+
+
+    public long getNonceValidity() {
+        return nonceValidity;
+    }
+
+
+    public void setNonceValidity(long nonceValidity) {
+        this.nonceValidity = nonceValidity;
+    }
+
+
+    public String getOpaque() {
+        return opaque;
+    }
+
+
+    public void setOpaque(String opaque) {
+        this.opaque = opaque;
+    }
+
+
+    public boolean isValidateUri() {
+        return validateUri;
+    }
+
+
+    public void setValidateUri(boolean validateUri) {
+        this.validateUri = validateUri;
+    }
+
+
+    // --------------------------------------------------------- Public Methods
+
     /**
      * Authenticate the user making this request, based on the specified
      * login configuration.  Return <code>true</code> if any specified
@@ -172,8 +260,13 @@ public boolean authenticate(Request request,
 
         // Validate any credentials already included with this request
         String authorization = request.getHeader("authorization");
+        DigestInfo digestInfo = new DigestInfo(getOpaque(), getNonceValidity(),
+                getKey(), cnonces, isValidateUri());
         if (authorization != null) {
-            principal = findPrincipal(request, authorization, context.getRealm());
+            if (digestInfo.validate(request, authorization, config)) {
+                principal = digestInfo.authenticate(context.getRealm());
+            }
+            
             if (principal != null) {
                 String username = parseUsername(authorization);
                 register(request, response, principal,
@@ -185,11 +278,12 @@ public boolean authenticate(Request request,
 
         // Send an "unauthorized" response and an appropriate challenge
 
-        // Next, generate a nOnce token (that is a token which is supposed
+        // Next, generate a nonce token (that is a token which is supposed
         // to be unique).
-        String nOnce = generateNOnce(request);
+        String nonce = generateNonce(request);
 
-        setAuthenticateHeader(request, response, config, nOnce);
+        setAuthenticateHeader(request, response, config, nonce,
+                digestInfo.isNonceStale());
         response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
         //      hres.flushBuffer();
         return (false);
@@ -200,92 +294,6 @@ public boolean authenticate(Request request,
     // ------------------------------------------------------ Protected Methods
 
 
-    /**
-     * Parse the specified authorization credentials, and return the
-     * associated Principal that these credentials authenticate (if any)
-     * from the specified Realm.  If there is no such Principal, return
-     * <code>null</code>.
-     *
-     * @param request HTTP servlet request
-     * @param authorization Authorization credentials from this request
-     * @param realm Realm used to authenticate Principals
-     */
-    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();
-
-        // Bugzilla 37132: http://issues.apache.org/bugzilla/show_bug.cgi?id=37132
-        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));
-
-    }
-
-
     /**
      * Parse the username from the specified authorization string.  If none
      * can be identified, return <code>null</code>
@@ -294,7 +302,6 @@ protected static Principal findPrincipal(Request request,
      */
     protected String parseUsername(String authorization) {
 
-        //System.out.println("Authorization token : " + authorization);
         // Validate the authorization credentials format
         if (authorization == null)
             return (null);
@@ -354,20 +361,20 @@ protected static String removeQuotes(String quotedString) {
      *
      * @param request HTTP Servlet request
      */
-    protected String generateNOnce(Request request) {
+    protected String generateNonce(Request request) {
 
         long currentTime = System.currentTimeMillis();
 
-        String nOnceValue = request.getRemoteAddr() + ":" +
-            currentTime + ":" + key;
+        
+        String ipTimeKey =
+            request.getRemoteAddr() + ":" + currentTime + ":" + getKey();
 
-        byte[] buffer = null;
+        byte[] buffer;
         synchronized (md5Helper) {
-            buffer = md5Helper.digest(nOnceValue.getBytes());
+            buffer = md5Helper.digest(ipTimeKey.getBytes());
         }
-        nOnceValue = md5Encoder.encode(buffer);
 
-        return nOnceValue;
+        return currentTime + ":" + md5Encoder.encode(buffer);
     }
 
 
@@ -379,7 +386,7 @@ protected String generateNOnce(Request request) {
      *      WWW-Authenticate    = "WWW-Authenticate" ":" "Digest"
      *                            digest-challenge
      *
-     *      digest-challenge    = 1#( realm | [ domain ] | nOnce |
+     *      digest-challenge    = 1#( realm | [ domain ] | nonce |
      *                  [ digest-opaque ] |[ stale ] | [ algorithm ] )
      *
      *      realm               = "realm" "=" realm-value
@@ -396,29 +403,300 @@ protected String generateNOnce(Request request) {
      * @param response HTTP Servlet response
      * @param config    Login configuration describing how authentication
      *              should be performed
-     * @param nOnce nonce token
+     * @param nonce nonce token
      */
     protected void setAuthenticateHeader(Request request,
                                          Response response,
                                          LoginConfig config,
-                                         String nOnce) {
+                                         String nonce,
+                                         boolean isNonceStale) {
 
         // Get the realm name
         String realmName = config.getRealmName();
         if (realmName == null)
             realmName = REALM_NAME;
 
-        byte[] buffer = null;
-        synchronized (md5Helper) {
-            buffer = md5Helper.digest(nOnce.getBytes());
+        String authenticateHeader;
+        if (isNonceStale) {
+            authenticateHeader = "Digest realm=\"" + realmName + "\", " +
+            "qop=\"" + QOP + "\", nonce=\"" + nonce + "\", " + "opaque=\"" +
+            getOpaque() + "\", stale=true";
+        } else {
+            authenticateHeader = "Digest realm=\"" + realmName + "\", " +
+            "qop=\"" + QOP + "\", nonce=\"" + nonce + "\", " + "opaque=\"" +
+            getOpaque() + "\"";
         }
 
-        String authenticateHeader = "Digest realm=\"" + realmName + "\", "
-            +  "qop=\"auth\", nonce=\"" + nOnce + "\", " + "opaque=\""
-            + md5Encoder.encode(buffer) + "\"";
         response.setHeader("WWW-Authenticate", authenticateHeader);
 
     }
 
 
+    // ------------------------------------------------------- Lifecycle Methods
+    
+    public void start() throws LifecycleException {
+        super.start();
+        
+        // Generate a random secret key
+        if (getKey() == null) {
+            setKey(generateSessionId());
+        }
+        
+        // Generate the opaque string the same way
+        if (getOpaque() == null) {
+            setOpaque(generateSessionId());
+        }
+        
+        cnonces = new LinkedHashMap() {
+
+            private static final long serialVersionUID = 1L;
+            private static final long LOG_SUPPRESS_TIME = 5 * 60 * 1000;
+
+            private long lastLog = 0;
+
+            protected boolean removeEldestEntry(Map.Entry eldest) {
+                // This is called from a sync so keep it simple
+                long currentTime = System.currentTimeMillis();
+                if (size() > getCnonceCacheSize()) {
+                    if (lastLog < currentTime && (currentTime -
+                            ((NonceInfo) eldest.getValue()).getTimestamp()) <
+                            getNonceValidity()) {
+                        // Replay attack is possible
+                        log.warn(sm.getString(
+                                "digestAuthenticator.cacheRemove"));
+                        lastLog = currentTime + LOG_SUPPRESS_TIME;
+                    }
+                    return true;
+                }
+                return false;
+            }
+        };
+    }
+ 
+    private static class DigestInfo {
+
+        private String opaque;
+        private long nonceValidity;
+        private String key;
+        private Map cnonces;
+        private boolean validateUri = true;
+
+        private String userName = null;
+        private String method = null;
+        private String uri = null;
+        private String response = null;
+        private String nonce = null;
+        private String nc = null;
+        private String cnonce = null;
+        private String realmName = null;
+        private String qop = null;
+
+        private boolean nonceStale = false;
+
+
+        public DigestInfo(String opaque, long nonceValidity, String key,
+                Map cnonces, boolean validateUri) {
+            this.opaque = opaque;
+            this.nonceValidity = nonceValidity;
+            this.key = key;
+            this.cnonces = cnonces;
+            this.validateUri = validateUri;
+        }
+
+        public boolean validate(Request request, String authorization,
+                LoginConfig config) {
+            // Validate the authorization credentials format
+            if (authorization == null) {
+                return false;
+            }
+            if (!authorization.startsWith("Digest ")) {
+                return false;
+            }
+            authorization = authorization.substring(7).trim();
+
+            // Bugzilla 37132: http://issues.apache.org/bugzilla/show_bug.cgi?id=37132
+            String[] tokens = authorization.split(",(?=(?:[^\"]*\"[^\"]*\")+$)");
+
+            method = request.getMethod();
+            String opaque = null;
+
+            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 false;
+                }
+                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 ("opaque".equals(currentTokenName))
+                    opaque = removeQuotes(currentTokenValue);
+            }
+
+            if ( (userName == null) || (realmName == null) || (nonce == null)
+                 || (uri == null) || (response == null) ) {
+                return false;
+            }
+
+            // Validate the URI - should match the request line sent by client
+            if (validateUri) {
+                String uriQuery;
+                String query = request.getQueryString();
+                if (query == null) {
+                    uriQuery = request.getRequestURI();
+                } else {
+                    uriQuery = request.getRequestURI() + "?" + query;
+                }
+                if (!uri.equals(uriQuery)) {
+                    return false;
+                }
+            }
+
+            // Validate the Realm name
+            String lcRealm = config.getRealmName();
+            if (lcRealm == null) {
+                lcRealm = REALM_NAME;
+            }
+            if (!lcRealm.equals(realmName)) {
+                return false;
+            }
+            
+            // Validate the opaque string
+            if (!this.opaque.equals(opaque)) {
+                return false;
+            }
+
+            // Validate nonce
+            int i = nonce.indexOf(":");
+            if (i < 0 || (i + 1) == nonce.length()) {
+                return false;
+            }
+            long nonceTime;
+            try {
+                nonceTime = Long.parseLong(nonce.substring(0, i));
+            } catch (NumberFormatException nfe) {
+                return false;
+            }
+            String md5clientIpTimeKey = nonce.substring(i + 1);
+            long currentTime = System.currentTimeMillis();
+            if ((currentTime - nonceTime) > nonceValidity) {
+                nonceStale = true;
+                return false;
+            }
+            String serverIpTimeKey =
+                request.getRemoteAddr() + ":" + nonceTime + ":" + key;
+            byte[] buffer = null;
+            synchronized (md5Helper) {
+                buffer = md5Helper.digest(serverIpTimeKey.getBytes());
+            }
+            String md5ServerIpTimeKey = md5Encoder.encode(buffer);
+            if (!md5ServerIpTimeKey.equals(md5clientIpTimeKey)) {
+                return false;
+            }
+
+            // Validate qop
+            if (qop != null && !QOP.equals(qop)) {
+                return false;
+            }
+
+            // Validate cnonce and nc
+            // Check if presence of nc and nonce is consistent with presence of qop
+            if (qop == null) {
+                if (cnonce != null || nc != null) {
+                    return false;
+                }
+            } else {
+                if (cnonce == null || nc == null) {
+                    return false;
+                }
+                if (nc.length() != 8) {
+                    return false;
+                }
+                long count;
+                try {
+                    count = Long.parseLong(nc, 16);
+                } catch (NumberFormatException nfe) {
+                    return false;
+                }
+                NonceInfo info;
+                synchronized (cnonces) {
+                    info = (NonceInfo) cnonces.get(cnonce);
+                }
+                if (info == null) {
+                    info = new NonceInfo();
+                } else {
+                    if (count <= info.getCount()) {
+                        return false;
+                    }
+                }
+                info.setCount(count);
+                info.setTimestamp(currentTime);
+                synchronized (cnonces) {
+                    cnonces.put(cnonce, info);
+                }
+            }
+            return true;
+        }
+
+        public boolean isNonceStale() {
+            return nonceStale;
+        }
+
+        public Principal authenticate(Realm realm) {
+            // Second MD5 digest used to calculate the digest :
+            // MD5(Method + ":" + uri)
+            String a2 = method + ":" + uri;
+
+            byte[] buffer;
+            synchronized (md5Helper) {
+                buffer = md5Helper.digest(a2.getBytes());
+            }
+            String md5a2 = md5Encoder.encode(buffer);
+
+            return realm.authenticate(userName, response, nonce, nc, cnonce,
+                    qop, realmName, md5a2);
+        }
+
+    }
+
+    private static class NonceInfo {
+        private volatile long count;
+        private volatile long timestamp;
+        
+        public void setCount(long l) {
+            count = l;
+        }
+        
+        public long getCount() {
+            return count;
+        }
+        
+        public void setTimestamp(long l) {
+            timestamp = l;
+        }
+        
+        public long getTimestamp() {
+            return timestamp;
+        }
+    }
 }

@@ -28,5 +28,7 @@ authenticator.sessionExpired=The time allowed for the login process has been exc
 authenticator.unauthorized=Cannot authenticate with the provided credentials
 authenticator.userDataConstraint=This request violates a User Data constraint for this application
 
+digestAuthenticator.cacheRemove=A valid entry has been removed from client nonce cache to make room for new entries. A replay attack is now possible. To prevent the possibility of replay attacks, reduce nonceValidity or increase cnonceCacheSize. Further warnings of this type will be suppressed for 5 minutes.
+ 
 formAuthenticator.forwardErrorFail=Unexpected error forwarding to error page
 formAuthenticator.forwardLoginFail=Unexpected error forwarding to login page

@@ -60,10 +60,30 @@
                description="Fully qualified class name of the managed object"
                type="java.lang.String"
                writeable="false"/>
-      
+
+    <attribute name="cnonceCacheSize"
+               description="The size of the cnonce cache used to prevent replay attacks"
+               type="int"/>
+
     <attribute   name="entropy"
                description="A String initialization parameter used to increase the  entropy of the initialization of our random number generator"
                type="java.lang.String"/>
+
+    <attribute name="key"
+               description="The secret key used by digest authentication"
+               type="java.lang.String"/>
+      
+    <attribute name="nonceValidity"
+               description="The time, in milliseconds, for which a server issued nonce will be valid"
+               type="long"/>
+
+    <attribute name="opaque"
+               description="The opaque server string used by digest authentication"
+               type="java.lang.String"/>
+
+    <attribute name="validateUri"
+               description="Should the uri be validated as required by RFC2617?"
+               type="boolean"/>
   </mbean>
   
   <mbean name="FormAuthenticator"

@@ -352,22 +352,27 @@ public Principal authenticate(String username, byte[] credentials) {
      *
      * @param username Username of the Principal to look up
      * @param clientDigest Digest which has been submitted by the client
-     * @param nOnce Unique (or supposedly unique) token which has been used
+     * @param nonce Unique (or supposedly unique) token which has been used
      * for this request
      * @param realm Realm name
      * @param md5a2 Second MD5 digest used to calculate the digest :
      * MD5(Method + ":" + uri)
      */
     public Principal authenticate(String username, String clientDigest,
-                                  String nOnce, String nc, String cnonce,
+                                  String nonce, String nc, String cnonce,
                                   String qop, String realm,
                                   String md5a2) {
 
         String md5a1 = getDigest(username, realm);
         if (md5a1 == null)
             return null;
-        String serverDigestValue = md5a1 + ":" + nOnce + ":" + nc + ":"
-            + cnonce + ":" + qop + ":" + md5a2;
+        String serverDigestValue;
+        if (qop == null) {
+            serverDigestValue = md5a1 + ":" + nonce + ":" + md5a2;
+        } else {
+            serverDigestValue = md5a1 + ":" + nonce + ":" + nc + ":" +
+                    cnonce + ":" + qop + ":" + md5a2;
+        }
 
         byte[] valueBytes = null;
         if(getDigestEncoding() == null) {
@@ -389,7 +394,7 @@ public Principal authenticate(String username, String clientDigest,
 
         if (log.isDebugEnabled()) {
             log.debug("Digest : " + clientDigest + " Username:" + username 
-                    + " ClientSigest:" + clientDigest + " nOnce:" + nOnce 
+                    + " ClientSigest:" + clientDigest + " nonce:" + nonce 
                     + " nc:" + nc + " cnonce:" + cnonce + " qop:" + qop 
                     + " realm:" + realm + "md5a2:" + md5a2 
                     + " Server digest:" + serverDigest);

@@ -78,6 +78,10 @@
         SecurityConfig.setSecurityProperty() when the value provided by JRE is
         null. (kkolinko)
       </fix>
+      <add>
+        Add additional configuration options to the DIGEST authenticator.
+        (markt)
+      </add>
     </changelog>
   </subsection>
   <subsection name="Coyote">

@@ -504,6 +504,12 @@
         used.</p>
       </attribute>
 
+      <attribute name="cnonceCacheSize" required="false">
+        <p>To protect against replay attacks, the DIGEST authenticator tracks
+        client nonce and nonce count values. This attribute controls the size
+        of that cache. If not specified, the default value of 1000 is used.</p>
+      </attribute>
+
       <attribute name="disableProxyCaching" required="false">
         <p>Controls the caching of pages that are protected by security
         constraints. Setting this to <code>false</code> may help work around
@@ -514,6 +520,26 @@
         <code>true</code> will be used.</p>
       </attribute>
 
+      <attribute name="key" required="false">
+        <p>The secret key used by digest authentication. If not set, a secure
+        random value is generated. This should normally only be set when it is
+        necessary to keep key values constant either across server restarts
+        and/or across a cluster.</p>
+      </attribute>
+
+      <attribute name="nonceValidity" required="false">
+        <p>The time, in milliseconds, that a server generated nonce will be
+        considered valid for use in authentication. If not specified, the
+        default value of 300000 (5 minutes) will be used.</p>
+      </attribute>
+
+      <attribute name="opaque" required="false">
+        <p>The opaque server string used by digest authentication. If not set, a
+        random value is generated. This should normally only be set when it is
+        necessary to keep opaque values constant either across server restarts
+        and/or across a cluster.</p>
+      </attribute>
+
       <attribute name="securePagesWithPragma" required="false">
         <p>Controls the caching of pages that are protected by security
         constraints. Setting this to <code>false</code> may help work around
@@ -523,6 +549,14 @@
         If not set, the default value of <code>true</code> will be used.</p>
       </attribute>
 
+      <attribute name="validateUri" required="false">
+        <p>Should the URI be validated as required by RFC2617? If not specified,
+        the default value of <code>true</code> will be used. This should
+        normally only be set when Tomcat is located behind a reverse proxy and
+        the proxy is modifying the URI passed to Tomcat such that DIGEST
+        authentication always fails.</p>
+      </attribute>
+
     </attributes>
 
   </subsection>

@@ -25,12 +25,6 @@ $Id$
 PATCHES PROPOSED TO BACKPORT:
   [ New proposals should be added at the end of the list ]
 
-* Add additional configuration options to the DIGEST authenticator
-  http://people.apache.org/~markt/patches/2011-04-01-digest-tc5.patch
-  +1: markt: jfclere
-  +1: kkolinko: with r1158177 (s/nOnce/nonce)
-  -1:
-
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=47880
   Clarify error messages in *.sh files to mention that if a script is not
   found it might be because execute permission is needed.

@@ -23,11 +23,14 @@
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.security.Principal;
+import java.util.LinkedHashMap;
+import java.util.Map;
 import java.util.StringTokenizer;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.apache.catalina.LifecycleException;
 import org.apache.catalina.Realm;
 import org.apache.catalina.connector.Request;
 import org.apache.catalina.deploy.LoginConfig;
@@ -46,8 +49,8 @@
  * @version $Id$
  */
 
-public class DigestAuthenticator
-    extends AuthenticatorBase {
+public class DigestAuthenticator extends AuthenticatorBase {
+
     private static final Log log = LogFactory.getLog(DigestAuthenticator.class);
 
 
@@ -66,6 +69,11 @@ public class DigestAuthenticator
         "org.apache.catalina.authenticator.DigestAuthenticator/1.0";
 
 
+    /**
+     * Tomcat's DIGEST implementation only supports auth quality of protection.
+     */
+    protected static final String QOP = "auth";
+
     // ----------------------------------------------------------- Constructors
 
 
@@ -90,15 +98,46 @@ public DigestAuthenticator() {
     protected static volatile MessageDigest md5Helper;
 
 
+    /**
+     * List of client nonce values currently being tracked
+     */
+    protected Map<String,NonceInfo> cnonces;
+
+
+    /**
+     * Maximum number of client nonces to keep in the cache. If not specified,
+     * the default value of 1000 is used.
+     */
+    protected int cnonceCacheSize = 1000;
+
+
     /**
      * Private key.
      */
-    protected String key = "Catalina";
+    protected String key = null;
 
 
-    // ------------------------------------------------------------- Properties
+    /**
+     * How long server nonces are valid for in milliseconds. Defaults to 5
+     * minutes.
+     */
+    protected long nonceValidity = 5 * 60 * 1000;
+
+
+    /**
+     * Opaque string.
+     */
+    protected String opaque;
 
 
+    /**
+     * Should the URI be validated as required by RFC2617? Can be disabled in
+     * reverse proxies where the proxy has modified the URI.
+     */
+    protected boolean validateUri = true;
+
+    // ------------------------------------------------------------- Properties
+
     /**
      * Return descriptive information about this Valve implementation.
      */
@@ -110,9 +149,58 @@ public String getInfo() {
     }
 
 
-    // --------------------------------------------------------- Public Methods
+    public int getCnonceCacheSize() {
+        return cnonceCacheSize;
+    }
+
+
+    public void setCnonceCacheSize(int cnonceCacheSize) {
+        this.cnonceCacheSize = cnonceCacheSize;
+    }
+
+
+    public String getKey() {
+        return key;
+    }
+
+
+    public void setKey(String key) {
+        this.key = key;
+    }
+
+
+    public long getNonceValidity() {
+        return nonceValidity;
+    }
+
+
+    public void setNonceValidity(long nonceValidity) {
+        this.nonceValidity = nonceValidity;
+    }
+
+
+    public String getOpaque() {
+        return opaque;
+    }
+
+
+    public void setOpaque(String opaque) {
+        this.opaque = opaque;
+    }
+
+
+    public boolean isValidateUri() {
+        return validateUri;
+    }
+
+
+    public void setValidateUri(boolean validateUri) {
+        this.validateUri = validateUri;
+    }
 
 
+    // --------------------------------------------------------- Public Methods
+
     /**
      * Authenticate the user making this request, based on the specified
      * login configuration.  Return <code>true</code> if any specified
@@ -173,8 +261,13 @@ public boolean authenticate(Request request,
 
         // Validate any credentials already included with this request
         String authorization = request.getHeader("authorization");
+        DigestInfo digestInfo = new DigestInfo(getOpaque(), getNonceValidity(),
+                getKey(), cnonces, isValidateUri());
         if (authorization != null) {
-            principal = findPrincipal(request, authorization, context.getRealm());
+            if (digestInfo.validate(request, authorization, config)) {
+                principal = digestInfo.authenticate(context.getRealm());
+            }
+            
             if (principal != null) {
                 String username = parseUsername(authorization);
                 register(request, response, principal,
@@ -188,9 +281,10 @@ public boolean authenticate(Request request,
 
         // Next, generate a nOnce token (that is a token which is supposed
         // to be unique).
-        String nOnce = generateNOnce(request);
+        String nonce = generateNonce(request);
 
-        setAuthenticateHeader(request, response, config, nOnce);
+        setAuthenticateHeader(request, response, config, nonce,
+                digestInfo.isNonceStale());
         response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
         //      hres.flushBuffer();
         return (false);
@@ -207,92 +301,6 @@ protected String getAuthMethod() {
     // ------------------------------------------------------ Protected Methods
 
 
-    /**
-     * Parse the specified authorization credentials, and return the
-     * associated Principal that these credentials authenticate (if any)
-     * from the specified Realm.  If there is no such Principal, return
-     * <code>null</code>.
-     *
-     * @param request HTTP servlet request
-     * @param authorization Authorization credentials from this request
-     * @param realm Realm used to authenticate Principals
-     */
-    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();
-
-        // Bugzilla 37132: http://issues.apache.org/bugzilla/show_bug.cgi?id=37132
-        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));
-
-    }
-
-
     /**
      * Parse the username from the specified authorization string.  If none
      * can be identified, return <code>null</code>
@@ -301,7 +309,6 @@ protected static Principal findPrincipal(Request request,
      */
     protected String parseUsername(String authorization) {
 
-        //System.out.println("Authorization token : " + authorization);
         // Validate the authorization credentials format
         if (authorization == null)
             return (null);
@@ -361,20 +368,20 @@ protected static String removeQuotes(String quotedString) {
      *
      * @param request HTTP Servlet request
      */
-    protected String generateNOnce(Request request) {
+    protected String generateNonce(Request request) {
 
         long currentTime = System.currentTimeMillis();
 
-        String nOnceValue = request.getRemoteAddr() + ":" +
-            currentTime + ":" + key;
+        
+        String ipTimeKey =
+            request.getRemoteAddr() + ":" + currentTime + ":" + getKey();
 
-        byte[] buffer = null;
+        byte[] buffer;
         synchronized (md5Helper) {
-            buffer = md5Helper.digest(nOnceValue.getBytes());
+            buffer = md5Helper.digest(ipTimeKey.getBytes());
         }
-        nOnceValue = md5Encoder.encode(buffer);
 
-        return nOnceValue;
+        return currentTime + ":" + md5Encoder.encode(buffer);
     }
 
 
@@ -408,24 +415,298 @@ protected String generateNOnce(Request request) {
     protected void setAuthenticateHeader(HttpServletRequest request,
                                          HttpServletResponse response,
                                          LoginConfig config,
-                                         String nOnce) {
+                                         String nOnce,
+                                         boolean isNonceStale) {
 
         // Get the realm name
         String realmName = config.getRealmName();
         if (realmName == null)
             realmName = REALM_NAME;
 
-        byte[] buffer = null;
-        synchronized (md5Helper) {
-            buffer = md5Helper.digest(nOnce.getBytes());
+        String authenticateHeader;
+        if (isNonceStale) {
+            authenticateHeader = "Digest realm=\"" + realmName + "\", " +
+            "qop=\"" + QOP + "\", nonce=\"" + nOnce + "\", " + "opaque=\"" +
+            getOpaque() + "\", stale=true";
+        } else {
+            authenticateHeader = "Digest realm=\"" + realmName + "\", " +
+            "qop=\"" + QOP + "\", nonce=\"" + nOnce + "\", " + "opaque=\"" +
+            getOpaque() + "\"";
         }
 
-        String authenticateHeader = "Digest realm=\"" + realmName + "\", "
-            +  "qop=\"auth\", nonce=\"" + nOnce + "\", " + "opaque=\""
-            + md5Encoder.encode(buffer) + "\"";
         response.setHeader(AUTH_HEADER_NAME, authenticateHeader);
 
     }
 
 
+    // ------------------------------------------------------- Lifecycle Methods
+    
+    @Override
+    protected synchronized void startInternal() throws LifecycleException {
+        super.startInternal();
+        
+        // Generate a random secret key
+        if (getKey() == null) {
+            setKey(sessionIdGenerator.generateSessionId());
+        }
+        
+        // Generate the opaque string the same way
+        if (getOpaque() == null) {
+            setOpaque(sessionIdGenerator.generateSessionId());
+        }
+        
+        cnonces = new LinkedHashMap<String, DigestAuthenticator.NonceInfo>() {
+
+            private static final long serialVersionUID = 1L;
+            private static final long LOG_SUPPRESS_TIME = 5 * 60 * 1000;
+
+            private long lastLog = 0;
+
+            @Override
+            protected boolean removeEldestEntry(
+                    Map.Entry<String,NonceInfo> eldest) {
+                // This is called from a sync so keep it simple
+                long currentTime = System.currentTimeMillis();
+                if (size() > getCnonceCacheSize()) {
+                    if (lastLog < currentTime &&
+                            currentTime - eldest.getValue().getTimestamp() <
+                            getNonceValidity()) {
+                        // Replay attack is possible
+                        log.warn(sm.getString(
+                                "digestAuthenticator.cacheRemove"));
+                        lastLog = currentTime + LOG_SUPPRESS_TIME;
+                    }
+                    return true;
+                }
+                return false;
+            }
+        };
+    }
+ 
+    private static class DigestInfo {
+
+        private String opaque;
+        private long nonceValidity;
+        private String key;
+        private Map<String,NonceInfo> cnonces;
+        private boolean validateUri = true;
+
+        private String userName = null;
+        private String method = null;
+        private String uri = null;
+        private String response = null;
+        private String nonce = null;
+        private String nc = null;
+        private String cnonce = null;
+        private String realmName = null;
+        private String qop = null;
+
+        private boolean nonceStale = false;
+
+
+        public DigestInfo(String opaque, long nonceValidity, String key,
+                Map<String,NonceInfo> cnonces, boolean validateUri) {
+            this.opaque = opaque;
+            this.nonceValidity = nonceValidity;
+            this.key = key;
+            this.cnonces = cnonces;
+            this.validateUri = validateUri;
+        }
+
+        public boolean validate(Request request, String authorization,
+                LoginConfig config) {
+            // Validate the authorization credentials format
+            if (authorization == null) {
+                return false;
+            }
+            if (!authorization.startsWith("Digest ")) {
+                return false;
+            }
+            authorization = authorization.substring(7).trim();
+
+            // Bugzilla 37132: http://issues.apache.org/bugzilla/show_bug.cgi?id=37132
+            String[] tokens = authorization.split(",(?=(?:[^\"]*\"[^\"]*\")+$)");
+
+            method = request.getMethod();
+            String opaque = null;
+
+            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 false;
+                }
+                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 ("opaque".equals(currentTokenName))
+                    opaque = removeQuotes(currentTokenValue);
+            }
+
+            if ( (userName == null) || (realmName == null) || (nonce == null)
+                 || (uri == null) || (response == null) ) {
+                return false;
+            }
+
+            // Validate the URI - should match the request line sent by client
+            if (validateUri) {
+                String uriQuery;
+                String query = request.getQueryString();
+                if (query == null) {
+                    uriQuery = request.getRequestURI();
+                } else {
+                    uriQuery = request.getRequestURI() + "?" + query;
+                }
+                if (!uri.equals(uriQuery)) {
+                    return false;
+                }
+            }
+
+            // Validate the Realm name
+            String lcRealm = config.getRealmName();
+            if (lcRealm == null) {
+                lcRealm = REALM_NAME;
+            }
+            if (!lcRealm.equals(realmName)) {
+                return false;
+            }
+            
+            // Validate the opaque string
+            if (!this.opaque.equals(opaque)) {
+                return false;
+            }
+
+            // Validate nonce
+            int i = nonce.indexOf(":");
+            if (i < 0 || (i + 1) == nonce.length()) {
+                return false;
+            }
+            long nOnceTime;
+            try {
+                nOnceTime = Long.parseLong(nonce.substring(0, i));
+            } catch (NumberFormatException nfe) {
+                return false;
+            }
+            String md5clientIpTimeKey = nonce.substring(i + 1);
+            long currentTime = System.currentTimeMillis();
+            if ((currentTime - nOnceTime) > nonceValidity) {
+                nonceStale = true;
+                return false;
+            }
+            String serverIpTimeKey =
+                request.getRemoteAddr() + ":" + nOnceTime + ":" + key;
+            byte[] buffer = null;
+            synchronized (md5Helper) {
+                buffer = md5Helper.digest(serverIpTimeKey.getBytes());
+            }
+            String md5ServerIpTimeKey = md5Encoder.encode(buffer);
+            if (!md5ServerIpTimeKey.equals(md5clientIpTimeKey)) {
+                return false;
+            }
+
+            // Validate qop
+            if (qop != null && !QOP.equals(qop)) {
+                return false;
+            }
+
+            // Validate cnonce and nc
+            // Check if presence of nc and nonce is consistent with presence of qop
+            if (qop == null) {
+                if (cnonce != null || nc != null) {
+                    return false;
+                }
+            } else {
+                if (cnonce == null || nc == null) {
+                    return false;
+                }
+                if (nc.length() != 8) {
+                    return false;
+                }
+                long count;
+                try {
+                    count = Long.parseLong(nc, 16);
+                } catch (NumberFormatException nfe) {
+                    return false;
+                }
+                NonceInfo info;
+                synchronized (cnonces) {
+                    info = cnonces.get(cnonce);
+                }
+                if (info == null) {
+                    info = new NonceInfo();
+                } else {
+                    if (count <= info.getCount()) {
+                        return false;
+                    }
+                }
+                info.setCount(count);
+                info.setTimestamp(currentTime);
+                synchronized (cnonces) {
+                    cnonces.put(cnonce, info);
+                }
+            }
+            return true;
+        }
+
+        public boolean isNonceStale() {
+            return nonceStale;
+        }
+
+        public Principal authenticate(Realm realm) {
+            // Second MD5 digest used to calculate the digest :
+            // MD5(Method + ":" + uri)
+            String a2 = method + ":" + uri;
+
+            byte[] buffer;
+            synchronized (md5Helper) {
+                buffer = md5Helper.digest(a2.getBytes());
+            }
+            String md5a2 = md5Encoder.encode(buffer);
+
+            return realm.authenticate(userName, response, nonce, nc, cnonce,
+                    qop, realmName, md5a2);
+        }
+
+    }
+
+    private static class NonceInfo {
+        private volatile long count;
+        private volatile long timestamp;
+        
+        public void setCount(long l) {
+            count = l;
+        }
+        
+        public long getCount() {
+            return count;
+        }
+        
+        public void setTimestamp(long l) {
+            timestamp = l;
+        }
+        
+        public long getTimestamp() {
+            return timestamp;
+        }
+    }
 }

@@ -28,6 +28,8 @@ authenticator.sessionExpired=The time allowed for the login process has been exc
 authenticator.unauthorized=Cannot authenticate with the provided credentials
 authenticator.userDataConstraint=This request violates a User Data constraint for this application
 
+digestAuthenticator.cacheRemove=A valid entry has been removed from client nonce cache to make room for new entries. A replay attack is now possible. To prevent the possibility of replay attacks, reduce nonceValidity or increase cnonceCacheSize. Further warnings of this type will be suppressed for 5 minutes.
+ 
 formAuthenticator.forwardErrorFail=Unexpected error forwarding to error page
 formAuthenticator.forwardLoginFail=Unexpected error forwarding to login page
 

@@ -90,10 +90,26 @@
                type="java.lang.String"
                writeable="false"/>
       
+    <attribute name="cnonceCacheSize"
+               description="The size of the cnonce cache used to prevent replay attacks"
+               type="int"/>
+      
     <attribute name="disableProxyCaching"
                description="Controls the caching of pages that are protected by security constraints"
                type="boolean"/>
       
+    <attribute name="key"
+               description="The secret key used by digest authentication"
+               type="java.lang.String"/>
+      
+    <attribute name="nonceValidity"
+               description="The time, in milliseconds, for which a server issued nonce will be valid"
+               type="long"/>
+
+    <attribute name="opaque"
+               description="The opaque server string used by digest authentication"
+               type="java.lang.String"/>
+      
     <attribute name="securePagesWithPragma"
                description="Controls the caching of pages that are protected by security constraints"
                type="boolean"/>
@@ -114,6 +130,10 @@
                description="The name of the LifecycleState that this component is currently in"
                type="java.lang.String"
                writeable="false"/>
+
+    <attribute name="validateUri"
+               description="Should the uri be validated as required by RFC2617?"
+               type="boolean"/>
   </mbean>
   
   <mbean name="FormAuthenticator"

@@ -364,8 +364,13 @@ public Principal authenticate(String username, String clientDigest,
         String md5a1 = getDigest(username, realm);
         if (md5a1 == null)
             return null;
-        String serverDigestValue = md5a1 + ":" + nOnce + ":" + nc + ":"
-            + cnonce + ":" + qop + ":" + md5a2;
+        String serverDigestValue;
+        if (qop == null) {
+            serverDigestValue = md5a1 + ":" + nOnce + ":" + md5a2;
+        } else {
+            serverDigestValue = md5a1 + ":" + nOnce + ":" + nc + ":" +
+                    cnonce + ":" + qop + ":" + md5a2;
+        }
 
         byte[] valueBytes = null;
         if(getDigestEncoding() == null) {

@@ -0,0 +1,349 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.apache.catalina.authenticator;
+
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.deploy.LoginConfig;
+import org.apache.catalina.deploy.SecurityCollection;
+import org.apache.catalina.deploy.SecurityConstraint;
+import org.apache.catalina.startup.TestTomcat.MapRealm;
+import org.apache.catalina.startup.TesterServlet;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.catalina.util.MD5Encoder;
+import org.apache.tomcat.util.buf.ByteChunk;
+
+public class TestDigestAuthenticator extends TomcatBaseTest {
+
+    private static String USER = "user";
+    private static String PWD = "pwd";
+    private static String ROLE = "role";
+    private static String URI = "/protected";
+    private static String QUERY = "?foo=bar";
+    private static String CONTEXT_PATH = "/foo";
+    private static String CLIENT_AUTH_HEADER = "authorization";
+    private static String REALM = "TestRealm";
+    private static String CNONCE = "cnonce";
+    private static String NC1 = "00000001";
+    private static String NC2 = "00000002";
+    private static String QOP = "auth";
+
+    public void testAllValid() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, false, true, REALM, true, true,
+                NC1, NC2, CNONCE, QOP, true, true);
+    }
+
+    public void testValidNoQop() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, false, true, REALM, true, true,
+                null, null, null, null, true, true);
+    }
+
+    public void testValidQuery() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI + QUERY, false, true, REALM, true,
+                true, NC1, NC2, CNONCE, QOP, true, true);
+    }
+
+    public void testInvalidUriFail() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, true, true, REALM, true, true,
+                NC1, NC2, CNONCE, QOP, false, false);
+    }
+
+    public void testInvalidUriPass() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, true, false, REALM, true, true,
+                NC1, NC2, CNONCE, QOP, true, true);
+    }
+
+    public void testInvalidRealm() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, false, true, "null", true, true,
+                NC1, NC2, CNONCE, QOP, false, false);
+    }
+
+    public void testInvalidNonce() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, false, true, REALM, false, true,
+                NC1, NC2, CNONCE, QOP, false, true);
+    }
+
+    public void testInvalidOpaque() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, false, true, REALM, true, false,
+                NC1, NC2, CNONCE, QOP, false, true);
+    }
+
+    public void testInvalidNc1() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, false, true, REALM, true, true,
+                "null", null, CNONCE, QOP, false, false);
+    }
+
+    public void testInvalidQop() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, false, true, REALM, true, true,
+                NC1, NC2, CNONCE, "null", false, false);
+    }
+
+    public void testInvalidQopCombo1() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, false, true, REALM, true, true,
+                NC1, NC2, CNONCE, null, false, false);
+    }
+
+    public void testInvalidQopCombo2() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, false, true, REALM, true, true,
+                NC1, NC2, null, QOP, false, false);
+    }
+
+    public void testInvalidQopCombo3() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, false, true, REALM, true, true,
+                NC1, NC2, null, null, false, false);
+    }
+
+    public void testInvalidQopCombo4() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, false, true, REALM, true, true,
+                null, null, CNONCE, QOP, false, false);
+    }
+
+    public void testInvalidQopCombo5() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, false, true, REALM, true, true,
+                null, null, CNONCE, null, false, false);
+    }
+
+    public void testInvalidQopCombo6() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, false, true, REALM, true, true,
+                null, null, null, QOP, false, false);
+    }
+
+    public void testReplay() throws Exception {
+        doTest(USER, PWD, CONTEXT_PATH + URI, false, true, REALM, true, true,
+                NC1, NC1, CNONCE, QOP, true, false);
+    }
+
+    public void doTest(String user, String pwd, String uri, boolean breakUri,
+            boolean validateUri, String realm, boolean useServerNonce,
+            boolean useServerOpaque, String nc1, String nc2, String cnonce,
+            String qop, boolean req2expect200, boolean req3expect200)
+            throws Exception {
+
+        if (!validateUri) {
+            DigestAuthenticator auth =
+                (DigestAuthenticator) getTomcatInstance().getHost().findChild(
+                        CONTEXT_PATH).getPipeline().getFirst();
+            auth.setValidateUri(false);
+        }
+        getTomcatInstance().start();
+
+        String digestUri;
+        if (breakUri) {
+            digestUri = "/broken" + uri;
+        } else {
+            digestUri = uri;
+        }
+        List<String> auth = new ArrayList<String>();
+        auth.add(buildDigestResponse(user, pwd, digestUri, realm, "null",
+                "null", nc1, cnonce, qop));
+        Map<String,List<String>> reqHeaders = new HashMap<String,List<String>>();
+        reqHeaders.put(CLIENT_AUTH_HEADER, auth);
+
+        Map<String,List<String>> respHeaders =
+            new HashMap<String,List<String>>();
+        
+        // The first request will fail - but we need to extract the nonce 
+        ByteChunk bc = new ByteChunk();
+        int rc = getUrl("http://localhost:" + getPort() + uri, bc, reqHeaders,
+                respHeaders);
+        assertEquals(401, rc);
+        assertNull(bc.toString());
+
+        // Second request should succeed (if we use the server nonce)
+        auth.clear();
+        if (useServerNonce) {
+            if (useServerOpaque) {
+                auth.add(buildDigestResponse(user, pwd, digestUri, realm,
+                        getNonce(respHeaders), getOpaque(respHeaders), nc1,
+                        cnonce, qop));
+            } else {
+                auth.add(buildDigestResponse(user, pwd, digestUri, realm,
+                        getNonce(respHeaders), "null", nc1, cnonce, qop));
+            }
+        } else {
+            auth.add(buildDigestResponse(user, pwd, digestUri, realm,
+                    "null", getOpaque(respHeaders), nc1, cnonce, QOP));
+        }
+        rc = getUrl("http://localhost:" + getPort() + uri, bc, reqHeaders,
+                null);
+        
+        if (req2expect200) {
+            assertEquals(200, rc);
+            assertEquals("OK", bc.toString());
+        } else {
+            assertEquals(401, rc);
+            assertNull(bc.toString());
+        }
+        
+        // Third request should succeed if we increment nc
+        auth.clear();
+        bc.recycle();
+        bc.reset();
+        auth.add(buildDigestResponse(user, pwd, digestUri, realm,
+                getNonce(respHeaders), getOpaque(respHeaders), nc2, cnonce,
+                qop));
+        rc = getUrl("http://localhost:" + getPort() + uri, bc, reqHeaders,
+                null);
+        
+        if (req3expect200) {
+            assertEquals(200, rc);
+            assertEquals("OK", bc.toString());
+        } else {
+            assertEquals(401, rc);
+            assertNull(bc.toString());
+        }
+    }
+
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+
+        // Configure a context with digest auth and a single protected resource
+        Tomcat tomcat = getTomcatInstance();
+        
+        // Must have a real docBase - just use temp
+        Context ctxt = tomcat.addContext(CONTEXT_PATH,
+                System.getProperty("java.io.tmpdir"));
+        
+        // Add protected servlet
+        Tomcat.addServlet(ctxt, "TesterServlet", new TesterServlet());
+        ctxt.addServletMapping(URI, "TesterServlet");
+        SecurityCollection collection = new SecurityCollection();
+        collection.addPattern(URI);
+        SecurityConstraint sc = new SecurityConstraint();
+        sc.addAuthRole(ROLE);
+        sc.addCollection(collection);
+        ctxt.addConstraint(sc);
+        
+        // Configure the Realm
+        MapRealm realm = new MapRealm();
+        realm.addUser(USER, PWD);
+        realm.addUserRole(USER, ROLE);
+        ctxt.setRealm(realm);
+        
+        // Configure the authenticator
+        LoginConfig lc = new LoginConfig();
+        lc.setAuthMethod("DIGEST");
+        lc.setRealmName(REALM);
+        ctxt.setLoginConfig(lc);
+        ctxt.getPipeline().addValve(new DigestAuthenticator());
+    }
+    
+    protected static String getNonce(Map<String,List<String>> respHeaders) {
+        List<String> authHeaders =
+            respHeaders.get(AuthenticatorBase.AUTH_HEADER_NAME);
+        // Assume there is only one
+        String authHeader = authHeaders.iterator().next();
+        
+        int start = authHeader.indexOf("nonce=\"") + 7;
+        int end = authHeader.indexOf("\"", start);
+        return authHeader.substring(start, end);
+    }
+
+    protected static String getOpaque(Map<String,List<String>> respHeaders) {
+        List<String> authHeaders =
+            respHeaders.get(AuthenticatorBase.AUTH_HEADER_NAME);
+        // Assume there is only one
+        String authHeader = authHeaders.iterator().next();
+        
+        int start = authHeader.indexOf("opaque=\"") + 8;
+        int end = authHeader.indexOf("\"", start);
+        return authHeader.substring(start, end);
+    }
+
+    /*
+     * Notes from RFC2617
+     * H(data) = MD5(data)
+     * KD(secret, data) = H(concat(secret, ":", data))
+     * A1 = unq(username-value) ":" unq(realm-value) ":" passwd
+     * A2 = Method ":" digest-uri-value
+     * request-digest  = <"> < KD ( H(A1),     unq(nonce-value)
+                                    ":" nc-value
+                                    ":" unq(cnonce-value)
+                                    ":" unq(qop-value)
+                                    ":" H(A2)
+                                   ) <">
+     */
+    private static String buildDigestResponse(String user, String pwd,
+            String uri, String realm, String nonce, String opaque, String nc,
+            String cnonce, String qop) throws NoSuchAlgorithmException {
+
+        String a1 = user + ":" + realm + ":" + pwd;
+        String a2 = "GET:" + uri;
+        
+        String md5a1 = digest(a1);
+        String md5a2 = digest(a2);
+        
+        String response;
+        if (qop == null) {
+            response = md5a1 + ":" + nonce + ":" + md5a2;
+        } else {
+            response = md5a1 + ":" + nonce + ":" + nc + ":" + cnonce + ":" +
+                    qop + ":" + md5a2;
+        }
+
+        String md5response = digest(response);
+        
+        StringBuilder auth = new StringBuilder();
+        auth.append("Digest username=\"");
+        auth.append(user);
+        auth.append("\", realm=\"");
+        auth.append(realm);
+        auth.append("\", nonce=\"");
+        auth.append(nonce);
+        auth.append("\", uri=\"");
+        auth.append(uri);
+        auth.append("\", opaque=\"");
+        auth.append(opaque);
+        auth.append("\", response=\"");
+        auth.append(md5response);
+        auth.append("\"");
+        if (qop != null) {
+            auth.append(", qop=\"");
+            auth.append(qop);
+            auth.append("\"");
+        }
+        if (nc != null) {
+            auth.append(", nc=\"");
+            auth.append(nc);
+            auth.append("\"");
+        }
+        if (cnonce != null) {
+            auth.append(", cnonce=\"");
+            auth.append(cnonce);
+            auth.append("\"");
+        }
+
+        return auth.toString();
+    }
+    
+    private static String digest(String input) throws NoSuchAlgorithmException {
+        // This is slow but should be OK as this is only a test
+        MessageDigest md5 = MessageDigest.getInstance("MD5");
+        MD5Encoder encoder = new MD5Encoder();
+
+        md5.update(input.getBytes());
+        return encoder.encode(md5.digest());
+    }
+}

@@ -0,0 +1,262 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.apache.catalina.authenticator;
+
+import java.io.IOException;
+import java.security.MessageDigest;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.deploy.LoginConfig;
+import org.apache.catalina.deploy.SecurityCollection;
+import org.apache.catalina.deploy.SecurityConstraint;
+import org.apache.catalina.startup.TestTomcat.MapRealm;
+import org.apache.catalina.startup.TesterServlet;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.catalina.util.MD5Encoder;
+import org.apache.tomcat.util.buf.ByteChunk;
+
+public class TesterDigestAuthenticatorPerformance extends TomcatBaseTest {
+
+    private static String USER = "user";
+    private static String PWD = "pwd";
+    private static String ROLE = "role";
+    private static String URI = "/protected";
+    private static String CONTEXT_PATH = "/foo";
+    private static String CLIENT_AUTH_HEADER = "authorization";
+    private static String REALM = "TestRealm";
+    private static String QOP = "auth";
+
+    
+    public void testSimple() throws Exception {
+        doTest(100, 1000);
+    }
+
+    public void doTest(int threadCount, int requestCount) throws Exception {
+        
+        getTomcatInstance().start();
+
+        TesterRunnable runnables[] = new TesterRunnable[threadCount];
+        Thread threads[] = new Thread[threadCount];
+        
+        // Create the runnables & threads
+        for (int i = 0; i < threadCount; i++) {
+            runnables[i] = new TesterRunnable(i, requestCount);
+            threads[i] = new Thread(runnables[i]);
+        }
+
+        long start = System.currentTimeMillis();
+
+        // Start the threads
+        for (int i = 0; i < threadCount; i++) {
+            threads[i].start();
+        }
+        
+        // Wait for the threads to finish
+        for (int i = 0; i < threadCount; i++) {
+            threads[i].join();
+        }
+        double wallTime = System.currentTimeMillis() - start; 
+        
+        // Gather the results...
+        double totalTime = 0;
+        int totalSuccess = 0;
+        for (int i = 0; i < threadCount; i++) {
+            System.out.println("Thread: " + i + " Success: " +
+                    runnables[i].getSuccess());
+            totalSuccess = totalSuccess + runnables[i].getSuccess();
+            totalTime = totalTime + runnables[i].getTime();
+        }
+        
+        System.out.println("Average time per request (user): " +
+                totalTime/(threadCount * requestCount));
+        System.out.println("Average time per request (wall): " +
+                wallTime/(threadCount * requestCount));
+        
+        assertEquals(requestCount * threadCount, totalSuccess);
+    }
+
+    private class TesterRunnable implements Runnable {
+
+        // Number of valid requests required
+        private int requestCount;
+        
+        private String nonce;
+        private String opaque;
+
+        private String cnonce;
+
+        private Map<String,List<String>> reqHeaders;
+        private List<String> authHeader;
+        
+        private MessageDigest digester;
+        private MD5Encoder encoder;
+
+        private String md5a1;
+        private String md5a2;
+
+        private String path;
+
+        private int success = 0;
+        private long time = 0;
+
+        // All init code should be in here. run() needs to be quick
+        public TesterRunnable(int id, int requestCount) throws Exception {
+            this.requestCount = requestCount;
+
+            path = "http://localhost:" + getPort() + CONTEXT_PATH + URI;
+
+            // Make the first request as we need the Digest challenge to obtain
+            // the server nonce
+            Map<String,List<String>> respHeaders =
+                    new HashMap<String,List<String>>();
+            getUrl(path, new ByteChunk(), respHeaders);
+            
+            nonce = TestDigestAuthenticator.getNonce(respHeaders);
+            opaque = TestDigestAuthenticator.getOpaque(respHeaders);
+            
+            cnonce = "cnonce" + id;
+
+            reqHeaders = new  HashMap<String,List<String>>();
+            authHeader = new ArrayList<String>();
+            reqHeaders.put(CLIENT_AUTH_HEADER, authHeader);
+            
+            digester = MessageDigest.getInstance("MD5");
+            encoder = new MD5Encoder();
+
+            String a1 = USER + ":" + REALM + ":" + PWD;
+            String a2 = "GET:" + CONTEXT_PATH + URI;
+            
+            md5a1 = encoder.encode(digester.digest(a1.getBytes()));
+            md5a2 = encoder.encode(digester.digest(a2.getBytes()));
+        }
+
+        @Override
+        public void run() {
+            int rc;
+            int nc = 0;
+            ByteChunk bc = new ByteChunk();
+            long start = System.currentTimeMillis();
+            for (int i = 0; i < requestCount; i++) {
+                nc++;
+                authHeader.clear();
+                authHeader.add(buildDigestResponse(nc));
+                
+                rc = -1;
+                bc.recycle();
+                bc.reset();
+                
+                try {
+                    rc = getUrl(path, bc, reqHeaders, null);
+                } catch (IOException ioe) {
+                    // Ignore
+                }
+             
+                if (rc == 200 && "OK".equals(bc.toString())) {
+                    success++;
+                }
+            }
+            time = System.currentTimeMillis() - start;
+        }
+        
+        public int getSuccess() {
+            return success;
+        }
+
+        public long getTime() {
+            return time;
+        }
+
+        private String buildDigestResponse(int nc) {
+            
+            String ncString = String.format("%1$08x", Integer.valueOf(nc));
+
+            String response = md5a1 + ":" + nonce + ":" + ncString + ":" +
+                    cnonce + ":" + QOP + ":" + md5a2;
+
+            String md5response =
+                encoder.encode(digester.digest(response.getBytes()));
+    
+            StringBuilder auth = new StringBuilder();
+            auth.append("Digest username=\"");
+            auth.append(USER);
+            auth.append("\", realm=\"");
+            auth.append(REALM);
+            auth.append("\", nonce=\"");
+            auth.append(nonce);
+            auth.append("\", uri=\"");
+            auth.append(CONTEXT_PATH + URI);
+            auth.append("\", opaque=\"");
+            auth.append(opaque);
+            auth.append("\", response=\"");
+            auth.append(md5response);
+            auth.append("\"");
+            auth.append(", qop=\"");
+            auth.append(QOP);
+            auth.append("\"");
+            auth.append(", nc=\"");
+            auth.append(ncString);
+            auth.append("\"");
+            auth.append(", cnonce=\"");
+            auth.append(cnonce);
+            auth.append("\"");
+
+            return auth.toString();
+        }
+    }
+
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+
+        // Configure a context with digest auth and a single protected resource
+        Tomcat tomcat = getTomcatInstance();
+        
+        // Must have a real docBase - just use temp
+        Context ctxt = tomcat.addContext(CONTEXT_PATH,
+                System.getProperty("java.io.tmpdir"));
+        
+        // Add protected servlet
+        Tomcat.addServlet(ctxt, "TesterServlet", new TesterServlet());
+        ctxt.addServletMapping(URI, "TesterServlet");
+        SecurityCollection collection = new SecurityCollection();
+        collection.addPattern(URI);
+        SecurityConstraint sc = new SecurityConstraint();
+        sc.addAuthRole(ROLE);
+        sc.addCollection(collection);
+        ctxt.addConstraint(sc);
+        
+        // Configure the Realm
+        MapRealm realm = new MapRealm();
+        realm.addUser(USER, PWD);
+        realm.addUserRole(USER, ROLE);
+        ctxt.setRealm(realm);
+        
+        // Configure the authenticator
+        LoginConfig lc = new LoginConfig();
+        lc.setAuthMethod("DIGEST");
+        lc.setRealmName(REALM);
+        ctxt.setLoginConfig(lc);
+        DigestAuthenticator authenticator = new DigestAuthenticator();
+        authenticator.setCnonceCacheSize(100);
+        ctxt.getPipeline().addValve(authenticator);
+    }
+}

@@ -147,6 +147,10 @@
       <fix>
         Don&apos;t register non-singelton DataSource resources with JMX. (markt)
       </fix>
+      <add>
+        Provide additional configuration options for the DIGEST authenticator.
+        (markt)
+      </add>
     </changelog>
   </subsection>
   <subsection name="Coyote">

@@ -549,6 +549,12 @@
         <strong>org.apache.catalina.authenticator.DigestAuthenticator</strong>.</p>
       </attribute>
 
+      <attribute name="cnonceCacheSize" required="false">
+        <p>To protect against replay attacks, the DIGEST authenticator tracks
+        client nonce and nonce count values. This attribute controls the size
+        of that cache. If not specified, the default value of 1000 is used.</p>
+      </attribute>
+
       <attribute name="disableProxyCaching" required="false">
         <p>Controls the caching of pages that are protected by security
         constraints. Setting this to <code>false</code> may help work around
@@ -559,6 +565,26 @@
         <code>true</code> will be used.</p>
       </attribute>
 
+      <attribute name="key" required="false">
+        <p>The secret key used by digest authentication. If not set, a secure
+        random value is generated. This should normally only be set when it is
+        necessary to keep key values constant either across server restarts
+        and/or across a cluster.</p>
+      </attribute>
+
+      <attribute name="nonceValidity" required="false">
+        <p>The time, in milliseconds, that a server generated nonce will be
+        considered valid for use in authentication. If not specified, the
+        default value of 300000 (5 minutes) will be used.</p>
+      </attribute>
+
+      <attribute name="opaque" required="false">
+        <p>The opaque server string used by digest authentication. If not set, a
+        random value is generated. This should normally only be set when it is
+        necessary to keep opaque values constant either across server restarts
+        and/or across a cluster.</p>
+      </attribute>
+
       <attribute name="securePagesWithPragma" required="false">
         <p>Controls the caching of pages that are protected by security
         constraints. Setting this to <code>false</code> may help work around
@@ -595,6 +621,14 @@
         specified, the platform default provider will be used.</p>
       </attribute>
 
+      <attribute name="validateUri" required="false">
+        <p>Should the URI be validated as required by RFC2617? If not specified,
+        the default value of <code>true</code> will be used. This should
+        normally only be set when Tomcat is located behind a reverse proxy and
+        the proxy is modifying the URI passed to Tomcat such that DIGEST
+        authentication always fails.</p>
+      </attribute>
+
     </attributes>
 
   </subsection>