Various DIGEST improvements ported from Tomcat 7
git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc5.5.x/trunk@1392248 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/STATUS.txt b/STATUS.txt
index e69b89b..7d71d7a 100644
--- a/STATUS.txt
+++ b/STATUS.txt
@@ -28,11 +28,6 @@
PATCHES PROPOSED TO BACKPORT:
[ New proposals should be added at the end of the list ]
-* Various DIGEST improvements ported from Tomcat 7
- http://people.apache.org/~markt/patches/2012-08-28-digest-tc5.patch
- +1: markt, kkolinko, kfujino
- -1:
-
* Remove unneeded handling of FORM authentication in RealmBase
http://svn.apache.org/viewvc?rev=1377887&view=rev
(r1377892 in 7.0)
diff --git a/container/catalina/src/share/org/apache/catalina/authenticator/DigestAuthenticator.java b/container/catalina/src/share/org/apache/catalina/authenticator/DigestAuthenticator.java
index 68468fb..66217d1 100644
--- a/container/catalina/src/share/org/apache/catalina/authenticator/DigestAuthenticator.java
+++ b/container/catalina/src/share/org/apache/catalina/authenticator/DigestAuthenticator.java
@@ -27,6 +27,7 @@
import java.util.Map;
import java.util.StringTokenizer;
+import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.logging.Log;
@@ -80,6 +81,7 @@
public DigestAuthenticator() {
super();
+ setCache(false);
try {
if (md5Helper == null)
md5Helper = MessageDigest.getInstance("MD5");
@@ -100,16 +102,16 @@
/**
- * List of client nonce values currently being tracked
+ * List of server nonce values currently being tracked
*/
- protected Map cnonces;
+ protected Map 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;
/**
@@ -149,13 +151,13 @@
}
- 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;
}
@@ -261,18 +263,19 @@
// 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;
+ }
}
}
@@ -283,10 +286,9 @@
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;
}
@@ -299,6 +301,8 @@
* can be identified, return <code>null</code>
*
* @param authorization Authorization string to be parsed
+ *
+ * @deprecated Unused. Will be removed in Tomcat 8.0.x
*/
protected String parseUsername(String authorization) {
@@ -343,7 +347,7 @@
} else if (quotedString.length() > 2) {
return quotedString.substring(1, quotedString.length() - 1);
} else {
- return new String();
+ return "";
}
}
@@ -374,7 +378,14 @@
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;
}
@@ -447,7 +458,7 @@
setOpaque(generateSessionId());
}
- cnonces = new LinkedHashMap() {
+ nonces = new LinkedHashMap() {
private static final long serialVersionUID = 1L;
private static final long LOG_SUPPRESS_TIME = 5 * 60 * 1000;
@@ -457,7 +468,7 @@
protected boolean removeEldestEntry(Map.Entry 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 -
((NonceInfo) eldest.getValue()).getTimestamp()) <
getNonceValidity()) {
@@ -475,10 +486,10 @@
private static class DigestInfo {
- private String opaque;
- private long nonceValidity;
- private String key;
- private Map cnonces;
+ private final String opaque;
+ private final long nonceValidity;
+ private final String key;
+ private final Map nonces;
private boolean validateUri = true;
private String userName = null;
@@ -490,21 +501,27 @@
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 cnonces, boolean validateUri) {
+ Map 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;
@@ -518,7 +535,6 @@
String[] tokens = authorization.split(",(?=(?:[^\"]*\"[^\"]*\")+$)");
method = request.getMethod();
- String opaque = null;
for (int i = 0; i < tokens.length; i++) {
String currentToken = tokens[i];
@@ -550,9 +566,13 @@
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;
@@ -568,7 +588,23 @@
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)) {
+ StringBuffer absolute = new StringBuffer();
+ absolute.append(scheme);
+ absolute.append("://");
+ absolute.append(host);
+ absolute.append(uriQuery);
+ if (!uri.equals(absolute.toString())) {
+ return false;
+ }
+ } else {
+ return false;
+ }
}
}
@@ -582,7 +618,7 @@
}
// Validate the opaque string
- if (!this.opaque.equals(opaque)) {
+ if (!opaque.equals(opaqueReceived)) {
return false;
}
@@ -601,7 +637,9 @@
long currentTime = System.currentTimeMillis();
if ((currentTime - nonceTime) > nonceValidity) {
nonceStale = true;
- return false;
+ synchronized (nonces) {
+ nonces.remove(nonce);
+ }
}
String serverIpTimeKey =
request.getRemoteAddr() + ":" + nonceTime + ":" + key;
@@ -620,7 +658,7 @@
}
// 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;
@@ -629,7 +667,9 @@
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;
@@ -639,21 +679,18 @@
return false;
}
NonceInfo info;
- synchronized (cnonces) {
- info = (NonceInfo) cnonces.get(cnonce);
+ synchronized (nonces) {
+ info = (NonceInfo) 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;
}
@@ -680,19 +717,31 @@
}
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() {
diff --git a/container/webapps/docs/changelog.xml b/container/webapps/docs/changelog.xml
index 979722e..de1b727 100644
--- a/container/webapps/docs/changelog.xml
+++ b/container/webapps/docs/changelog.xml
@@ -45,12 +45,6 @@
<update>
Update to Apache Commons Daemon 1.0.9. (markt)
</update>
- <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="Catalina">
@@ -68,6 +62,12 @@
<bug>53531</bug>: Better checking and improved error messages for
directory creation during automatic deployment. (schultz/kkolinko)
</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">
diff --git a/container/webapps/docs/config/valve.xml b/container/webapps/docs/config/valve.xml
index 6ef365d..9eef3ba 100644
--- a/container/webapps/docs/config/valve.xml
+++ b/container/webapps/docs/config/valve.xml
@@ -492,6 +492,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>
