Author: markt Date: Tue Sep 4 19:48:27 2012 New Revision: 1380829 URL: http://svn.apache.org/viewvc?rev=1380829&view=rev Log: Various improvements to the DIGEST authenticator including <bug>52954</bug>, the disabling caching of an authenticated user in the session by default, tracking server rather than client nonces and better handling of stale nonce values.
Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/authenticator/DigestAuthenticator.java tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml tomcat/tc6.0.x/trunk/webapps/docs/config/valve.xml Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/authenticator/DigestAuthenticator.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/authenticator/DigestAuthenticator.java?rev=1380829&r1=1380828&r2=1380829&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/catalina/authenticator/DigestAuthenticator.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/authenticator/DigestAuthenticator.java Tue Sep 4 19:48:27 2012 @@ -27,9 +27,9 @@ 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; @@ -80,6 +80,7 @@ public class DigestAuthenticator extends public DigestAuthenticator() { super(); + setCache(false); try { if (md5Helper == null) md5Helper = MessageDigest.getInstance("MD5"); @@ -100,16 +101,16 @@ public class DigestAuthenticator extends /** - * List of client nonce values currently being tracked + * List of server nonce values currently being tracked */ - protected Map<String,NonceInfo> cnonces; + protected Map<String,NonceInfo> nonces; /** - * Maximum number of client nonces to keep in the cache. If not specified, + * Maximum number of server nonces to keep in the cache. If not specified, * the default value of 1000 is used. */ - protected int cnonceCacheSize = 1000; + protected int nonceCacheSize = 1000; /** @@ -150,13 +151,13 @@ public class DigestAuthenticator extends } - public int getCnonceCacheSize() { - return cnonceCacheSize; + public int getNonceCacheSize() { + return nonceCacheSize; } - public void setCnonceCacheSize(int cnonceCacheSize) { - this.cnonceCacheSize = cnonceCacheSize; + public void setNonceCacheSize(int nonceCacheSize) { + this.nonceCacheSize = nonceCacheSize; } @@ -263,18 +264,19 @@ public class DigestAuthenticator extends // Validate any credentials already included with this request String authorization = request.getHeader("authorization"); DigestInfo digestInfo = new DigestInfo(getOpaque(), getNonceValidity(), - getKey(), cnonces, isValidateUri()); + getKey(), nonces, isValidateUri()); if (authorization != null) { - if (digestInfo.validate(request, authorization, config)) { - principal = digestInfo.authenticate(context.getRealm()); - } + if (digestInfo.parse(request, authorization)) { + if (digestInfo.validate(request, config)) { + principal = digestInfo.authenticate(context.getRealm()); + } - if (principal != null) { - String username = parseUsername(authorization); - register(request, response, principal, - Constants.DIGEST_METHOD, - username, null); - return (true); + if (principal != null && !digestInfo.isNonceStale()) { + register(request, response, principal, + HttpServletRequest.DIGEST_AUTH, + digestInfo.getUsername(), null); + return true; + } } } @@ -285,10 +287,9 @@ public class DigestAuthenticator extends String nonce = generateNonce(request); setAuthenticateHeader(request, response, config, nonce, - digestInfo.isNonceStale()); + principal != null && digestInfo.isNonceStale()); response.sendError(HttpServletResponse.SC_UNAUTHORIZED); - // hres.flushBuffer(); - return (false); + return false; } @@ -301,7 +302,10 @@ public class DigestAuthenticator extends * can be identified, return <code>null</code> * * @param authorization Authorization string to be parsed + * + * @deprecated Unused. Will be removed in Tomcat 8.0.x */ + @Deprecated protected String parseUsername(String authorization) { // Validate the authorization credentials format @@ -345,7 +349,7 @@ public class DigestAuthenticator extends } else if (quotedString.length() > 2) { return quotedString.substring(1, quotedString.length() - 1); } else { - return new String(); + return ""; } } @@ -376,7 +380,14 @@ public class DigestAuthenticator extends buffer = md5Helper.digest(ipTimeKey.getBytes()); } - return currentTime + ":" + md5Encoder.encode(buffer); + String nonce = currentTime + ":" + md5Encoder.encode(buffer); + + NonceInfo info = new NonceInfo(currentTime, 100); + synchronized (nonces) { + nonces.put(nonce, info); + } + + return nonce; } @@ -450,7 +461,7 @@ public class DigestAuthenticator extends setOpaque(generateSessionId()); } - cnonces = new LinkedHashMap<String, DigestAuthenticator.NonceInfo>() { + nonces = new LinkedHashMap<String, DigestAuthenticator.NonceInfo>() { private static final long serialVersionUID = 1L; private static final long LOG_SUPPRESS_TIME = 5 * 60 * 1000; @@ -462,7 +473,7 @@ public class DigestAuthenticator extends Map.Entry<String,NonceInfo> eldest) { // This is called from a sync so keep it simple long currentTime = System.currentTimeMillis(); - if (size() > getCnonceCacheSize()) { + if (size() > getNonceCacheSize()) { if (lastLog < currentTime && currentTime - eldest.getValue().getTimestamp() < getNonceValidity()) { @@ -480,10 +491,10 @@ public class DigestAuthenticator extends private static class DigestInfo { - private String opaque; - private long nonceValidity; - private String key; - private Map<String,NonceInfo> cnonces; + private final String opaque; + private final long nonceValidity; + private final String key; + private final Map<String,NonceInfo> nonces; private boolean validateUri = true; private String userName = null; @@ -495,21 +506,27 @@ public class DigestAuthenticator extends private String cnonce = null; private String realmName = null; private String qop = null; + private String opaqueReceived = null; private boolean nonceStale = false; public DigestInfo(String opaque, long nonceValidity, String key, - Map<String,NonceInfo> cnonces, boolean validateUri) { + Map<String,NonceInfo> nonces, boolean validateUri) { this.opaque = opaque; this.nonceValidity = nonceValidity; this.key = key; - this.cnonces = cnonces; + this.nonces = nonces; this.validateUri = validateUri; } - public boolean validate(Request request, String authorization, - LoginConfig config) { + + public String getUsername() { + return userName; + } + + + public boolean parse(Request request, String authorization) { // Validate the authorization credentials format if (authorization == null) { return false; @@ -523,7 +540,6 @@ public class DigestAuthenticator extends String[] tokens = authorization.split(",(?=(?:[^\"]*\"[^\"]*\")+$)"); method = request.getMethod(); - String opaque = null; for (int i = 0; i < tokens.length; i++) { String currentToken = tokens[i]; @@ -555,9 +571,13 @@ public class DigestAuthenticator extends if ("response".equals(currentTokenName)) response = removeQuotes(currentTokenValue); if ("opaque".equals(currentTokenName)) - opaque = removeQuotes(currentTokenValue); + opaqueReceived = removeQuotes(currentTokenValue); } + return true; + } + + public boolean validate(Request request, LoginConfig config) { if ( (userName == null) || (realmName == null) || (nonce == null) || (uri == null) || (response == null) ) { return false; @@ -573,7 +593,23 @@ public class DigestAuthenticator extends uriQuery = request.getRequestURI() + "?" + query; } if (!uri.equals(uriQuery)) { - return false; + // Some clients (older Android) use an absolute URI for + // DIGEST but a relative URI in the request line. + // request. 2.3.5 < fixed Android version <= 4.0.3 + String host = request.getHeader("host"); + String scheme = request.getScheme(); + if (host != null && !uriQuery.startsWith(scheme)) { + StringBuilder absolute = new StringBuilder(); + absolute.append(scheme); + absolute.append("://"); + absolute.append(host); + absolute.append(uriQuery); + if (!uri.equals(absolute.toString())) { + return false; + } + } else { + return false; + } } } @@ -587,7 +623,7 @@ public class DigestAuthenticator extends } // Validate the opaque string - if (!this.opaque.equals(opaque)) { + if (!opaque.equals(opaqueReceived)) { return false; } @@ -606,7 +642,9 @@ public class DigestAuthenticator extends long currentTime = System.currentTimeMillis(); if ((currentTime - nonceTime) > nonceValidity) { nonceStale = true; - return false; + synchronized (nonces) { + nonces.remove(nonce); + } } String serverIpTimeKey = request.getRemoteAddr() + ":" + nonceTime + ":" + key; @@ -625,7 +663,7 @@ public class DigestAuthenticator extends } // Validate cnonce and nc - // Check if presence of nc and nonce is consistent with presence of qop + // Check if presence of nc and Cnonce is consistent with presence of qop if (qop == null) { if (cnonce != null || nc != null) { return false; @@ -634,7 +672,9 @@ public class DigestAuthenticator extends if (cnonce == null || nc == null) { return false; } - if (nc.length() != 8) { + // RFC 2617 says nc must be 8 digits long. Older Android clients + // use 6. 2.3.5 < fixed Android version <= 4.0.3 + if (nc.length() < 6 || nc.length() > 8) { return false; } long count; @@ -644,21 +684,18 @@ public class DigestAuthenticator extends return false; } NonceInfo info; - synchronized (cnonces) { - info = cnonces.get(cnonce); + synchronized (nonces) { + info = nonces.get(nonce); } if (info == null) { - info = new NonceInfo(); + // Nonce is valid but not in cache. It must have dropped out + // of the cache - force a re-authentication + nonceStale = true; } else { - if (count <= info.getCount()) { + if (!info.nonceCountValid(count)) { return false; } } - info.setCount(count); - info.setTimestamp(currentTime); - synchronized (cnonces) { - cnonces.put(cnonce, info); - } } return true; } @@ -685,19 +722,31 @@ public class DigestAuthenticator extends } private static class NonceInfo { - private volatile long count; private volatile long timestamp; - - public void setCount(long l) { - count = l; + private volatile boolean seen[]; + private volatile int offset; + private volatile int count = 0; + + public NonceInfo(long currentTime, int seenWindowSize) { + this.timestamp = currentTime; + seen = new boolean[seenWindowSize]; + offset = seenWindowSize / 2; } - public long getCount() { - return count; - } - - public void setTimestamp(long l) { - timestamp = l; + public synchronized boolean nonceCountValid(long nonceCount) { + if ((count - offset) >= nonceCount || + (nonceCount > count - offset + seen.length)) { + return false; + } + int checkIndex = (int) ((nonceCount + offset) % seen.length); + if (seen[checkIndex]) { + return false; + } else { + seen[checkIndex] = true; + seen[count % seen.length] = false; + count++; + return true; + } } public long getTimestamp() { Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1380829&r1=1380828&r2=1380829&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Tue Sep 4 19:48:27 2012 @@ -192,6 +192,12 @@ when logging in when session IDs are being encoded as path parameters. (markt) </fix> + <fix> + Various improvements to the DIGEST authenticator including + <bug>52954</bug>, the disabling caching of an authenticated user in the + session by default, tracking server rather than client nonces and better + handling of stale nonce values. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> Modified: tomcat/tc6.0.x/trunk/webapps/docs/config/valve.xml URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/config/valve.xml?rev=1380829&r1=1380828&r2=1380829&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/webapps/docs/config/valve.xml (original) +++ tomcat/tc6.0.x/trunk/webapps/docs/config/valve.xml Tue Sep 4 19:48:27 2012 @@ -574,6 +574,12 @@ <attributes> + <attribute name="cache" required="false"> + <p>Should we cache authenticated Principals if the request is part of an + HTTP session? If not specified, the default value of <code>false</code> + will be used.</p> + </attribute> + <attribute name="className" required="true"> <p>Java class name of the implementation to use. This MUST be set to <strong>org.apache.catalina.authenticator.DigestAuthenticator</strong>.</p> @@ -596,6 +602,32 @@ <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="nonceCacheSize" required="false"> + <p>To protect against replay attacks, the DIGEST authenticator tracks + server 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="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 @@ -605,6 +637,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> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org