tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1377807 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/authenticator/ java/org/apache/catalina/util/ test/org/apache/catalina/authenticator/ webapps/docs/ webapps/docs/config/
Date Mon, 27 Aug 2012 19:43:06 GMT
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message