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: [email protected]
For additional commands, e-mail: [email protected]
