Author: markt Date: Mon Aug 27 19:43:06 2012 New Revision: 1377807 URL: http://svn.apache.org/viewvc?rev=1377807&view=rev Log: Digest improvements: - disable caching of authenticated user in session by default - track server rather than client nonces - better handling of stale nonce values
Added: tomcat/tc7.0.x/trunk/java/org/apache/catalina/util/ConcurrentMessageDigest.java - copied, changed from r1377794, tomcat/trunk/java/org/apache/catalina/util/ConcurrentMessageDigest.java Modified: tomcat/tc7.0.x/trunk/ (props changed) tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/DigestAuthenticator.java tomcat/tc7.0.x/trunk/test/org/apache/catalina/authenticator/TesterDigestAuthenticatorPerformance.java tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml tomcat/tc7.0.x/trunk/webapps/docs/config/valve.xml Propchange: tomcat/tc7.0.x/trunk/ ------------------------------------------------------------------------------ Merged /tomcat/trunk:r1377794 Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/DigestAuthenticator.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/DigestAuthenticator.java?rev=1377807&r1=1377806&r2=1377807&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/DigestAuthenticator.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/DigestAuthenticator.java Mon Aug 27 19:43:06 2012 @@ -20,7 +20,6 @@ package org.apache.catalina.authenticato import java.io.IOException; -import java.nio.charset.Charset; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.Principal; @@ -38,6 +37,7 @@ import org.apache.catalina.deploy.LoginC import org.apache.catalina.util.MD5Encoder; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; +import org.apache.tomcat.util.buf.B2CConverter; @@ -83,6 +83,7 @@ public class DigestAuthenticator extends public DigestAuthenticator() { super(); + setCache(false); try { if (md5Helper == null) md5Helper = MessageDigest.getInstance("MD5"); @@ -103,16 +104,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; /** @@ -153,13 +154,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; } @@ -266,18 +267,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 = digestInfo.getUsername(); - register(request, response, principal, - HttpServletRequest.DIGEST_AUTH, - username, null); - return (true); + if (principal != null && !digestInfo.isNonceStale()) { + register(request, response, principal, + HttpServletRequest.DIGEST_AUTH, + digestInfo.getUsername(), null); + return true; + } } } @@ -288,11 +290,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; } @@ -386,10 +386,17 @@ public class DigestAuthenticator extends byte[] buffer; synchronized (md5Helper) { buffer = md5Helper.digest( - ipTimeKey.getBytes(Charset.defaultCharset())); + ipTimeKey.getBytes(B2CConverter.ISO_8859_1)); + } + + String nonce = currentTime + ":" + MD5Encoder.encode(buffer); + + NonceInfo info = new NonceInfo(currentTime, 100); + synchronized (nonces) { + nonces.put(nonce, info); } - return currentTime + ":" + MD5Encoder.encode(buffer); + return nonce; } @@ -463,7 +470,7 @@ public class DigestAuthenticator extends setOpaque(sessionIdGenerator.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; @@ -475,7 +482,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()) { @@ -493,10 +500,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; @@ -508,16 +515,17 @@ 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; } @@ -526,8 +534,8 @@ public class DigestAuthenticator extends return userName; } - public boolean validate(Request request, String authorization, - LoginConfig config) { + + public boolean parse(Request request, String authorization) { // Validate the authorization credentials format if (authorization == null) { return false; @@ -541,7 +549,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]; @@ -573,9 +580,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; @@ -621,7 +632,7 @@ public class DigestAuthenticator extends } // Validate the opaque string - if (!this.opaque.equals(opaque)) { + if (!opaque.equals(opaqueReceived)) { return false; } @@ -640,14 +651,16 @@ 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; byte[] buffer = null; synchronized (md5Helper) { buffer = md5Helper.digest( - serverIpTimeKey.getBytes(Charset.defaultCharset())); + serverIpTimeKey.getBytes(B2CConverter.ISO_8859_1)); } String md5ServerIpTimeKey = MD5Encoder.encode(buffer); if (!md5ServerIpTimeKey.equals(md5clientIpTimeKey)) { @@ -660,7 +673,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; @@ -681,21 +694,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; } @@ -711,7 +721,7 @@ public class DigestAuthenticator extends byte[] buffer; synchronized (md5Helper) { - buffer = md5Helper.digest(a2.getBytes(Charset.defaultCharset())); + buffer = md5Helper.digest(a2.getBytes(B2CConverter.ISO_8859_1)); } String md5a2 = MD5Encoder.encode(buffer); @@ -722,19 +732,31 @@ public class DigestAuthenticator extends } private static class NonceInfo { - private volatile long count; private volatile long timestamp; - - public void setCount(long l) { - count = l; - } - - public long getCount() { - return count; + 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 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() { Copied: tomcat/tc7.0.x/trunk/java/org/apache/catalina/util/ConcurrentMessageDigest.java (from r1377794, tomcat/trunk/java/org/apache/catalina/util/ConcurrentMessageDigest.java) URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/util/ConcurrentMessageDigest.java?p2=tomcat/tc7.0.x/trunk/java/org/apache/catalina/util/ConcurrentMessageDigest.java&p1=tomcat/trunk/java/org/apache/catalina/util/ConcurrentMessageDigest.java&r1=1377794&r2=1377807&rev=1377807&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/util/ConcurrentMessageDigest.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/util/ConcurrentMessageDigest.java Mon Aug 27 19:43:06 2012 @@ -31,7 +31,7 @@ import java.util.concurrent.ConcurrentLi public class ConcurrentMessageDigest { private static final Map<String,Queue<MessageDigest>> queues = - new HashMap<>(); + new HashMap<String,Queue<MessageDigest>>(); private ConcurrentMessageDigest() { @@ -81,7 +81,8 @@ public class ConcurrentMessageDigest { synchronized (queues) { if (!queues.containsKey(algorithm)) { MessageDigest md = MessageDigest.getInstance(algorithm); - Queue<MessageDigest> queue = new ConcurrentLinkedQueue<>(); + Queue<MessageDigest> queue = + new ConcurrentLinkedQueue<MessageDigest>(); queue.add(md); queues.put(algorithm, queue); } Modified: tomcat/tc7.0.x/trunk/test/org/apache/catalina/authenticator/TesterDigestAuthenticatorPerformance.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/authenticator/TesterDigestAuthenticatorPerformance.java?rev=1377807&r1=1377806&r2=1377807&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/test/org/apache/catalina/authenticator/TesterDigestAuthenticatorPerformance.java (original) +++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/authenticator/TesterDigestAuthenticatorPerformance.java Mon Aug 27 19:43:06 2012 @@ -17,8 +17,7 @@ package org.apache.catalina.authenticator; import java.io.IOException; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; +import java.util.concurrent.atomic.AtomicInteger; import javax.servlet.http.HttpServletResponse; @@ -33,6 +32,7 @@ import org.apache.catalina.core.Standard import org.apache.catalina.deploy.LoginConfig; import org.apache.catalina.filters.TesterResponse; import org.apache.catalina.startup.TestTomcat.MapRealm; +import org.apache.catalina.util.ConcurrentMessageDigest; import org.apache.catalina.util.MD5Encoder; public class TesterDigestAuthenticatorPerformance { @@ -47,6 +47,8 @@ public class TesterDigestAuthenticatorPe private static String REALM = "TestRealm"; private static String QOP = "auth"; + private static final AtomicInteger nonceCount = new AtomicInteger(0); + private DigestAuthenticator authenticator = new DigestAuthenticator(); @@ -60,9 +62,11 @@ public class TesterDigestAuthenticatorPe TesterRunnable runnables[] = new TesterRunnable[threadCount]; Thread threads[] = new Thread[threadCount]; + String nonce = authenticator.generateNonce(new TesterDigestRequest()); + // Create the runnables & threads for (int i = 0; i < threadCount; i++) { - runnables[i] = new TesterRunnable(requestCount); + runnables[i] = new TesterRunnable(nonce, requestCount); threads[i] = new Thread(runnables[i]); } @@ -100,6 +104,8 @@ public class TesterDigestAuthenticatorPe @Before public void setUp() throws Exception { + ConcurrentMessageDigest.init("MD5"); + // Configure the Realm MapRealm realm = new MapRealm(); realm.addUser(USER, PWD); @@ -113,15 +119,14 @@ public class TesterDigestAuthenticatorPe // Make the Context and Realm visible to the Authenticator authenticator.setContainer(context); - // Prevent caching of cnonces so we can the same one for all requests - authenticator.setCnonceCacheSize(0); authenticator.start(); } private class TesterRunnable implements Runnable { - // Number of valid requests required + + private String nonce; private int requestCount; private int success = 0; @@ -132,12 +137,11 @@ public class TesterDigestAuthenticatorPe private LoginConfig config; // All init code should be in here. run() needs to be quick - public TesterRunnable(int requestCount) throws Exception { + public TesterRunnable(String nonce, int requestCount) throws Exception { + this.nonce = nonce; this.requestCount = requestCount; request = new TesterDigestRequest(); - String nonce = authenticator.generateNonce(request); - request.setAuthHeader(buildDigestResponse(nonce)); response = new TesterResponse(); @@ -150,7 +154,8 @@ public class TesterDigestAuthenticatorPe long start = System.currentTimeMillis(); for (int i = 0; i < requestCount; i++) { try { - if (authenticator.authenticate(request, response, config)) { + request.setAuthHeader(buildDigestResponse(nonce)); + if (authenticator.authenticate(request, response)) { success++; } // Clear out authenticated user ready for next iteration @@ -170,25 +175,25 @@ public class TesterDigestAuthenticatorPe return time; } - private String buildDigestResponse(String nonce) - throws NoSuchAlgorithmException { + private String buildDigestResponse(String nonce) { - String ncString = "00000001"; + String ncString = String.format("%1$08x", + Integer.valueOf(nonceCount.incrementAndGet())); String cnonce = "cnonce"; String a1 = USER + ":" + REALM + ":" + PWD; String a2 = METHOD + ":" + CONTEXT_PATH + URI; - MessageDigest digester = MessageDigest.getInstance("MD5"); - - String md5a1 = MD5Encoder.encode(digester.digest(a1.getBytes())); - String md5a2 = MD5Encoder.encode(digester.digest(a2.getBytes())); + String md5a1 = MD5Encoder.encode( + ConcurrentMessageDigest.digest("MD5", a1.getBytes())); + String md5a2 = MD5Encoder.encode( + ConcurrentMessageDigest.digest("MD5", a2.getBytes())); String response = md5a1 + ":" + nonce + ":" + ncString + ":" + cnonce + ":" + QOP + ":" + md5a2; - String md5response = - MD5Encoder.encode(digester.digest(response.getBytes())); + String md5response = MD5Encoder.encode( + ConcurrentMessageDigest.digest("MD5", response.getBytes())); StringBuilder auth = new StringBuilder(); auth.append("Digest username=\""); @@ -255,6 +260,5 @@ public class TesterDigestAuthenticatorPe public String getRequestURI() { return CONTEXT_PATH + URI; } - } } Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1377807&r1=1377806&r2=1377807&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Mon Aug 27 19:43:06 2012 @@ -197,6 +197,11 @@ <fix> <bug>53725</bug>: Fix possible corruption of GZIP'd output. (markt) </fix> + <fix> + Improvements to DIGEST authenticator including the disabling caching of + authenticated user in session by default, tracking server rather than + client nonces and better handling of stale nonce values. (markt) + </fix> </changelog> </subsection> <subsection name="Jasper"> Modified: tomcat/tc7.0.x/trunk/webapps/docs/config/valve.xml URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/config/valve.xml?rev=1377807&r1=1377806&r2=1377807&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/webapps/docs/config/valve.xml (original) +++ tomcat/tc7.0.x/trunk/webapps/docs/config/valve.xml Mon Aug 27 19:43:06 2012 @@ -847,12 +847,6 @@ <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 @@ -870,6 +864,12 @@ 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 --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org