Author: remy.maucherat(a)jboss.com Date: 2012-10-10 13:50:36 -0400 (Wed, 10 Oct 2012) New Revision: 2107 Added: branches/2.1.x/java/org/apache/catalina/util/ConcurrentMessageDigest.java branches/7.0.x/java/org/apache/catalina/util/ConcurrentMessageDigest.java branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/util/ConcurrentMessageDigest.java Modified: branches/2.1.x/java/org/apache/catalina/authenticator/DigestAuthenticator.java branches/2.1.x/java/org/apache/catalina/realm/RealmBase.java branches/2.1.x/java/org/apache/catalina/util/MD5Encoder.java branches/2.1.x/java/org/apache/coyote/ajp/AjpAprProcessor.java branches/2.1.x/java/org/apache/coyote/ajp/AjpMessage.java branches/2.1.x/java/org/apache/coyote/ajp/AjpProcessor.java branches/2.1.x/java/org/apache/coyote/http11/Http11AprProcessor.java branches/2.1.x/java/org/apache/coyote/http11/Http11Processor.java branches/2.1.x/java/org/apache/coyote/http11/InternalAprOutputBuffer.java branches/2.1.x/java/org/apache/coyote/http11/InternalOutputBuffer.java branches/2.1.x/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java branches/7.0.x/.classpath branches/7.0.x/java/org/apache/catalina/authenticator/DigestAuthenticator.java branches/7.0.x/java/org/apache/catalina/connector/Response.java branches/7.0.x/java/org/apache/catalina/filters/CsrfPreventionFilter.java branches/7.0.x/java/org/apache/catalina/realm/RealmBase.java branches/7.0.x/java/org/apache/catalina/util/MD5Encoder.java branches/7.0.x/java/org/apache/coyote/ajp/AjpAprProcessor.java branches/7.0.x/java/org/apache/coyote/ajp/AjpMessage.java branches/7.0.x/java/org/apache/coyote/ajp/AjpProcessor.java branches/7.0.x/java/org/apache/coyote/http11/Http11AprProcessor.java branches/7.0.x/java/org/apache/coyote/http11/Http11Processor.java branches/7.0.x/java/org/apache/coyote/http11/InternalAprOutputBuffer.java branches/7.0.x/java/org/apache/coyote/http11/InternalOutputBuffer.java branches/7.0.x/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/authenticator/DigestAuthenticator.java branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/realm/RealmBase.java branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/util/MD5Encoder.java branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/ajp/AjpAprProcessor.java branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/ajp/AjpMessage.java branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/ajp/AjpProcessor.java branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/Http11AprProcessor.java branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/Http11NioProcessor.java branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/Http11Processor.java branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/InternalAprOutputBuffer.java branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/InternalNioOutputBuffer.java branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/InternalOutputBuffer.java branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/filters/ChunkedInputFilter.java Log: Port patches to active branches. Modified: branches/2.1.x/java/org/apache/catalina/authenticator/DigestAuthenticator.java =================================================================== --- branches/2.1.x/java/org/apache/catalina/authenticator/DigestAuthenticator.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/2.1.x/java/org/apache/catalina/authenticator/DigestAuthenticator.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -5,30 +5,27 @@ * The ASF licenses this file to You under the Apache License, Version 2.0 * (the "License"); you may not use this file except in compliance with * the License. You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */ - - package org.apache.catalina.authenticator; - import java.io.IOException; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +import java.security.Principal; import java.security.SecureRandom; import java.util.LinkedHashMap; import java.util.Map; import java.util.Random; -import java.security.Principal; -import java.util.StringTokenizer; +import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.catalina.Container; @@ -38,42 +35,26 @@ import org.apache.catalina.connector.Request; import org.apache.catalina.connector.Response; import org.apache.catalina.deploy.LoginConfig; +import org.apache.catalina.util.ConcurrentMessageDigest; import org.apache.catalina.util.MD5Encoder; import org.jboss.logging.Logger; -import org.jboss.logging.Logger; - /** * An <b>Authenticator</b> and <b>Valve</b> implementation of HTTP DIGEST * Authentication (see RFC 2069). * * @author Craig R. McClanahan * @author Remy Maucherat - * @version $Revision$ $Date$ + * @version $Id$ */ - -public class DigestAuthenticator - extends AuthenticatorBase { +public class DigestAuthenticator extends AuthenticatorBase { private static Logger log = Logger.getLogger(DigestAuthenticator.class); // -------------------------------------------------------------- Constants /** - * The MD5 helper object for this class. - */ - protected static final MD5Encoder md5Encoder = new MD5Encoder(); - - - /** - * Descriptive information about this implementation. - */ - protected static final String info = - "org.apache.catalina.authenticator.DigestAuthenticator/1.0"; - - - /** * Tomcat's DIGEST implementation only supports auth quality of protection. */ protected static final String QOP = "auth"; @@ -83,12 +64,13 @@ public DigestAuthenticator() { super(); + setCache(false); try { - if (md5Helper == null) + if (md5Helper == null) { md5Helper = MessageDigest.getInstance("MD5"); + } } catch (NoSuchAlgorithmException e) { - e.printStackTrace(); - throw new IllegalStateException(); + throw new IllegalStateException(e); } } @@ -98,21 +80,22 @@ /** * MD5 message digest provider. + * @deprecated Unused - will be removed in Tomcat 8.0.x onwards */ protected static MessageDigest md5Helper; /** - * 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; /** @@ -142,27 +125,16 @@ // ------------------------------------------------------------- Properties - /** - * Return descriptive information about this Valve implementation. - */ - @Override - public String getInfo() { - - return (info); - + public int getNonceCacheSize() { + return nonceCacheSize; } - public int getCnonceCacheSize() { - return cnonceCacheSize; + public void setNonceCacheSize(int nonceCacheSize) { + this.nonceCacheSize = nonceCacheSize; } - public void setCnonceCacheSize(int cnonceCacheSize) { - this.cnonceCacheSize = cnonceCacheSize; - } - - public String getKey() { return key; } @@ -213,8 +185,6 @@ * * @param request Request we are processing * @param response Response we are creating - * @param config Login configuration describing how authentication - * should be performed * * @exception IOException if an input/output error occurs */ @@ -222,19 +192,18 @@ public boolean authenticate(Request request, Response response, LoginConfig config) - throws IOException { + throws IOException { // Have we already authenticated someone? Principal principal = request.getUserPrincipal(); //String ssoId = (String) request.getNote(Constants.REQ_SSOID_NOTE); if (principal != null) { - if (log.isDebugEnabled()) - log.debug("Already authenticated '" + principal.getName() + "'"); // Associate the session with any existing SSO session in order // to get coordinated session invalidation at logout String ssoId = (String) request.getNote(Constants.REQ_SSOID_NOTE); - if (ssoId != null) + if (ssoId != null) { associate(ssoId, request.getSessionInternal(true)); + } return (true); } @@ -266,19 +235,20 @@ // 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 && !digestInfo.isNonceStale()) { + register(request, response, principal, + HttpServletRequest.DIGEST_AUTH, + digestInfo.getUsername(), null); + return true; + } } - - if (principal != null) { - String username = parseUsername(authorization); - register(request, response, principal, - Constants.DIGEST_METHOD, - username, null); - return (true); - } } // Send an "unauthorized" response and an appropriate challenge @@ -288,11 +258,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; } @@ -300,42 +268,6 @@ /** - * Parse the username from the specified authorization string. If none - * can be identified, return <code>null</code> - * - * @param authorization Authorization string to be parsed - */ - protected String parseUsername(String authorization) { - - // Validate the authorization credentials format - if (authorization == null) - return (null); - if (!authorization.startsWith("Digest ")) - return (null); - authorization = authorization.substring(7).trim(); - - StringTokenizer commaTokenizer = - new StringTokenizer(authorization, ","); - - while (commaTokenizer.hasMoreTokens()) { - String currentToken = commaTokenizer.nextToken(); - int equalSign = currentToken.indexOf('='); - if (equalSign < 0) - return null; - String currentTokenName = - currentToken.substring(0, equalSign).trim(); - String currentTokenValue = - currentToken.substring(equalSign + 1).trim(); - if ("username".equals(currentTokenName)) - return (removeQuotes(currentTokenValue)); - } - - return (null); - - } - - - /** * Removes the quotes on a string. RFC2617 states quotes are optional for * all parameters except realm. */ @@ -348,7 +280,7 @@ } else if (quotedString.length() > 2) { return quotedString.substring(1, quotedString.length() - 1); } else { - return new String(); + return ""; } } @@ -370,16 +302,19 @@ long currentTime = System.currentTimeMillis(); - + String ipTimeKey = request.getRemoteAddr() + ":" + currentTime + ":" + getKey(); - byte[] buffer; - synchronized (md5Helper) { - buffer = md5Helper.digest(ipTimeKey.getBytes()); + byte[] buffer = ConcurrentMessageDigest.digestMD5(ipTimeKey.getBytes()); + 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; } @@ -406,12 +341,10 @@ * * @param request HTTP Servlet request * @param response HTTP Servlet response - * @param config Login configuration describing how authentication - * should be performed * @param nonce nonce token */ - protected void setAuthenticateHeader(Request request, - Response response, + protected void setAuthenticateHeader(HttpServletRequest request, + HttpServletResponse response, LoginConfig config, String nonce, boolean isNonceStale) { @@ -430,15 +363,15 @@ authenticateHeader = "Digest realm=\"" + realmName + "\", " + "qop=\"" + QOP + "\", nonce=\"" + nonce + "\", " + "opaque=\"" + getOpaque() + "\""; - } + } - response.setHeader("WWW-Authenticate", authenticateHeader); + response.setHeader(AUTH_HEADER_NAME, authenticateHeader); } // ------------------------------------------------------- Lifecycle Methods - + @Override public void start() throws LifecycleException { super.start(); @@ -447,14 +380,14 @@ if (getKey() == null) { setKey(generateSessionId()); } - + // Generate the opaque string the same way if (getOpaque() == null) { 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; @@ -465,7 +398,7 @@ 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,13 +413,13 @@ } }; } - + 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; @@ -498,21 +431,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<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; @@ -526,12 +465,12 @@ String[] tokens = authorization.split(",(?=(?:[^\"]*\"[^\"]*\")+$)"); method = request.getMethod(); - String opaque = null; for (int i = 0; i < tokens.length; i++) { String currentToken = tokens[i]; - if (currentToken.length() == 0) + if (currentToken.length() == 0) { continue; + } int equalSign = currentToken.indexOf('='); if (equalSign < 0) { @@ -541,26 +480,39 @@ currentToken.substring(0, equalSign).trim(); String currentTokenValue = currentToken.substring(equalSign + 1).trim(); - if ("username".equals(currentTokenName)) + if ("username".equals(currentTokenName)) { userName = removeQuotes(currentTokenValue); - if ("realm".equals(currentTokenName)) + } + if ("realm".equals(currentTokenName)) { realmName = removeQuotes(currentTokenValue, true); - if ("nonce".equals(currentTokenName)) + } + if ("nonce".equals(currentTokenName)) { nonce = removeQuotes(currentTokenValue); - if ("nc".equals(currentTokenName)) + } + if ("nc".equals(currentTokenName)) { nc = removeQuotes(currentTokenValue); - if ("cnonce".equals(currentTokenName)) + } + if ("cnonce".equals(currentTokenName)) { cnonce = removeQuotes(currentTokenValue); - if ("qop".equals(currentTokenName)) + } + if ("qop".equals(currentTokenName)) { qop = removeQuotes(currentTokenValue); - if ("uri".equals(currentTokenName)) + } + if ("uri".equals(currentTokenName)) { uri = removeQuotes(currentTokenValue); - if ("response".equals(currentTokenName)) + } + if ("response".equals(currentTokenName)) { response = removeQuotes(currentTokenValue); - if ("opaque".equals(currentTokenName)) - opaque = removeQuotes(currentTokenValue); + } + if ("opaque".equals(currentTokenName)) { + 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; @@ -576,7 +528,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)) { + 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; + } } } @@ -588,9 +556,9 @@ if (!lcRealm.equals(realmName)) { return false; } - + // Validate the opaque string - if (!this.opaque.equals(opaque)) { + if (!opaque.equals(opaqueReceived)) { return false; } @@ -609,15 +577,15 @@ 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()); - } - String md5ServerIpTimeKey = md5Encoder.encode(buffer); + byte[] buffer = ConcurrentMessageDigest.digestMD5( + serverIpTimeKey.getBytes()); + String md5ServerIpTimeKey = MD5Encoder.encode(buffer); if (!md5ServerIpTimeKey.equals(md5clientIpTimeKey)) { return false; } @@ -628,7 +596,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; @@ -637,7 +605,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; @@ -647,21 +617,18 @@ 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; } @@ -675,11 +642,9 @@ // MD5(Method + ":" + uri) String a2 = method + ":" + uri; - byte[] buffer; - synchronized (md5Helper) { - buffer = md5Helper.digest(a2.getBytes()); - } - String md5a2 = md5Encoder.encode(buffer); + byte[] buffer = ConcurrentMessageDigest.digestMD5( + a2.getBytes()); + String md5a2 = MD5Encoder.encode(buffer); return realm.authenticate(userName, response, nonce, nc, cnonce, qop, realmName, md5a2); @@ -688,21 +653,33 @@ } 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 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 void setTimestamp(long l) { - timestamp = l; - } - + public long getTimestamp() { return timestamp; } Modified: branches/2.1.x/java/org/apache/catalina/realm/RealmBase.java =================================================================== --- branches/2.1.x/java/org/apache/catalina/realm/RealmBase.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/2.1.x/java/org/apache/catalina/realm/RealmBase.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -728,31 +728,6 @@ if (constraints == null || constraints.length == 0) return (true); - // Specifically allow access to the form login and form error pages - // and the "j_security_check" action - LoginConfig config = context.getLoginConfig(); - if ((config != null) && - (Constants.FORM_METHOD.equals(config.getAuthMethod()))) { - String requestURI = request.getRequestPathMB().toString(); - String loginPage = config.getLoginPage(); - if (loginPage.equals(requestURI)) { - if (log.isDebugEnabled()) - log.debug(" Allow access to login page " + loginPage); - return (true); - } - String errorPage = config.getErrorPage(); - if (errorPage.equals(requestURI)) { - if (log.isDebugEnabled()) - log.debug(" Allow access to error page " + errorPage); - return (true); - } - if (requestURI.endsWith(Constants.FORM_ACTION)) { - if (log.isDebugEnabled()) - log.debug(" Allow access to username/password submission"); - return (true); - } - } - // Which user principal have we already authenticated? Principal principal = request.getPrincipal(); boolean status = false; Added: branches/2.1.x/java/org/apache/catalina/util/ConcurrentMessageDigest.java =================================================================== --- branches/2.1.x/java/org/apache/catalina/util/ConcurrentMessageDigest.java (rev 0) +++ branches/2.1.x/java/org/apache/catalina/util/ConcurrentMessageDigest.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.util; + +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.HashMap; +import java.util.Map; +import java.util.Queue; +import java.util.concurrent.ConcurrentLinkedQueue; + +/** + * A thread safe wrapper around {@link MessageDigest} that does not make use + * of ThreadLocal and - broadly - only creates enough MessageDigest objects + * to satisfy the concurrency requirements. + */ +public class ConcurrentMessageDigest { + + private static final String MD5 = "MD5"; + + private static final Map<String,Queue<MessageDigest>> queues = + new HashMap<String,Queue<MessageDigest>>(); + + + private ConcurrentMessageDigest() { + // Hide default constructor for this utility class + } + + static { + try { + // Init commonly used algorithms + init(MD5); + } catch (NoSuchAlgorithmException e) { + throw new IllegalArgumentException(e); + } + } + + public static byte[] digestMD5(byte[] input) { + return digest(MD5, input); + } + + public static byte[] digest(String algorithm, byte[] input) { + + Queue<MessageDigest> queue = queues.get(algorithm); + if (queue == null) { + throw new IllegalStateException("Must call init() first"); + } + + MessageDigest md = queue.poll(); + if (md == null) { + try { + md = MessageDigest.getInstance(algorithm); + } catch (NoSuchAlgorithmException e) { + // Ignore. Impossible if init() has been successfully called + // first. + throw new IllegalStateException("Must call init() first"); + } + } + + byte[] result = md.digest(input); + + queue.add(md); + + return result; + } + + + /** + * Ensures that {@link #digest(String, byte[])} and + * {@link #digestAsHex(String, byte[])} will support the specified + * algorithm. This method <b>must</b> be called and return successfully + * before using {@link #digest(String, byte[])} or + * {@link #digestAsHex(String, byte[])}. + * + * @param algorithm The message digest algorithm to be supported + * + * @throws NoSuchAlgorithmException If the algorithm is not supported by the + * JVM + */ + public static void init(String algorithm) throws NoSuchAlgorithmException { + synchronized (queues) { + if (!queues.containsKey(algorithm)) { + MessageDigest md = MessageDigest.getInstance(algorithm); + Queue<MessageDigest> queue = new ConcurrentLinkedQueue<MessageDigest>(); + queue.add(md); + queues.put(algorithm, queue); + } + } + } +} Modified: branches/2.1.x/java/org/apache/catalina/util/MD5Encoder.java =================================================================== --- branches/2.1.x/java/org/apache/catalina/util/MD5Encoder.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/2.1.x/java/org/apache/catalina/util/MD5Encoder.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -50,7 +50,7 @@ * @param binaryData Array containing the digest * @return Encoded MD5, or null if encoding failed */ - public String encode( byte[] binaryData ) { + public static String encode( byte[] binaryData ) { if (binaryData.length != 16) return null; Modified: branches/2.1.x/java/org/apache/coyote/ajp/AjpAprProcessor.java =================================================================== --- branches/2.1.x/java/org/apache/coyote/ajp/AjpAprProcessor.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/2.1.x/java/org/apache/coyote/ajp/AjpAprProcessor.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -897,7 +897,7 @@ int port = 0; int mult = 1; for (int i = valueL - 1; i > colonPos; i--) { - int charValue = HexUtils.DEC[(int) valueB[i + valueS]]; + int charValue = HexUtils.DEC[valueB[i + valueS] & 0xff]; if (charValue == -1) { // Invalid character error = true; Modified: branches/2.1.x/java/org/apache/coyote/ajp/AjpMessage.java =================================================================== --- branches/2.1.x/java/org/apache/coyote/ajp/AjpMessage.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/2.1.x/java/org/apache/coyote/ajp/AjpMessage.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -251,10 +251,8 @@ // but is the only consistent approach within the current // servlet framework. It must suffice until servlet output // streams properly encode their output. - if ((c <= 31) && (c != 9)) { + if (((c <= 31) && (c != 9)) || c == 127 || c > 255) { c = ' '; - } else if (c == 127) { - c = ' '; } appendByte(c); } Modified: branches/2.1.x/java/org/apache/coyote/ajp/AjpProcessor.java =================================================================== --- branches/2.1.x/java/org/apache/coyote/ajp/AjpProcessor.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/2.1.x/java/org/apache/coyote/ajp/AjpProcessor.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -902,7 +902,7 @@ int port = 0; int mult = 1; for (int i = valueL - 1; i > colonPos; i--) { - int charValue = HexUtils.DEC[(int) valueB[i + valueS]]; + int charValue = HexUtils.DEC[valueB[i + valueS] & 0xff]; if (charValue == -1) { // Invalid character error = true; Modified: branches/2.1.x/java/org/apache/coyote/http11/Http11AprProcessor.java =================================================================== --- branches/2.1.x/java/org/apache/coyote/http11/Http11AprProcessor.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/2.1.x/java/org/apache/coyote/http11/Http11AprProcessor.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -1558,7 +1558,7 @@ int port = 0; int mult = 1; for (int i = valueL - 1; i > colonPos; i--) { - int charValue = HexUtils.DEC[valueB[i + valueS]]; + int charValue = HexUtils.DEC[valueB[i + valueS] & 0xff]; if (charValue == -1) { // Invalid character error = true; Modified: branches/2.1.x/java/org/apache/coyote/http11/Http11Processor.java =================================================================== --- branches/2.1.x/java/org/apache/coyote/http11/Http11Processor.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/2.1.x/java/org/apache/coyote/http11/Http11Processor.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -1363,7 +1363,7 @@ int port = 0; int mult = 1; for (int i = valueL - 1; i > colonPos; i--) { - int charValue = HexUtils.DEC[valueB[i + valueS]]; + int charValue = HexUtils.DEC[valueB[i + valueS] & 0xff]; if (charValue == -1) { // Invalid character error = true; Modified: branches/2.1.x/java/org/apache/coyote/http11/InternalAprOutputBuffer.java =================================================================== --- branches/2.1.x/java/org/apache/coyote/http11/InternalAprOutputBuffer.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/2.1.x/java/org/apache/coyote/http11/InternalAprOutputBuffer.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -673,10 +673,8 @@ // but is the only consistent approach within the current // servlet framework. It must suffice until servlet output // streams properly encode their output. - if ((c <= 31) && (c != 9)) { + if (((c <= 31) && (c != 9)) || c == 127 || c > 255) { c = ' '; - } else if (c == 127) { - c = ' '; } buf[pos++] = (byte) c; } Modified: branches/2.1.x/java/org/apache/coyote/http11/InternalOutputBuffer.java =================================================================== --- branches/2.1.x/java/org/apache/coyote/http11/InternalOutputBuffer.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/2.1.x/java/org/apache/coyote/http11/InternalOutputBuffer.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -688,10 +688,8 @@ // but is the only consistent approach within the current // servlet framework. It must suffice until servlet output // streams properly encode their output. - if ((c <= 31) && (c != 9)) { + if (((c <= 31) && (c != 9)) || c == 127 || c > 255) { c = ' '; - } else if (c == 127) { - c = ' '; } buf[pos++] = (byte) c; } Modified: branches/2.1.x/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java =================================================================== --- branches/2.1.x/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/2.1.x/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -266,6 +266,7 @@ int result = 0; boolean eol = false; + boolean crfound = false; boolean readDigit = false; boolean trailer = false; @@ -282,13 +283,18 @@ } if (buf[pos] == Constants.CR) { + if (crfound) throw new IOException("Invalid CRLF, two CR characters encountered."); + crfound = true; } else if (buf[pos] == Constants.LF) { + if (!crfound) throw new IOException("Invalid CRLF, no CR character encountered."); eol = true; } else if (buf[pos] == Constants.SEMI_COLON) { trailer = true; + } else if (buf[pos] < 0) { + throw new IOException("Invalid chunk header"); } else if (!trailer) { //don't read data after the trailer - if (HexUtils.DEC[buf[pos]] != -1) { + if (HexUtils.DEC[buf[pos] & 0xff] != -1) { readDigit = true; result *= 16; result += HexUtils.DEC[buf[pos]]; Modified: branches/7.0.x/.classpath =================================================================== --- branches/7.0.x/.classpath 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/7.0.x/.classpath 2012-10-10 17:50:36 UTC (rev 2107) @@ -2,6 +2,5 @@ <classpath> <classpathentry excluding="**/.svn/**" kind="src" path="java"/> <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/> - <classpathentry kind="lib" path="output/jars/jasper-jdt.jar"/> <classpathentry kind="output" path=".settings/output"/> </classpath> Modified: branches/7.0.x/java/org/apache/catalina/authenticator/DigestAuthenticator.java =================================================================== --- branches/7.0.x/java/org/apache/catalina/authenticator/DigestAuthenticator.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/7.0.x/java/org/apache/catalina/authenticator/DigestAuthenticator.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -5,20 +5,17 @@ * The ASF licenses this file to You under the Apache License, Version 2.0 * (the "License"); you may not use this file except in compliance with * the License. You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */ - - package org.apache.catalina.authenticator; - import java.io.IOException; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; @@ -27,7 +24,6 @@ import java.util.LinkedHashMap; import java.util.Map; import java.util.Random; -import java.util.StringTokenizer; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -38,11 +34,12 @@ import org.apache.catalina.Realm; import org.apache.catalina.connector.Request; import org.apache.catalina.deploy.LoginConfig; +import org.apache.catalina.util.ConcurrentMessageDigest; import org.apache.catalina.util.MD5Encoder; +import org.apache.tomcat.util.buf.EncodingToCharset; import org.jboss.logging.Logger; - /** * An <b>Authenticator</b> and <b>Valve</b> implementation of HTTP DIGEST * Authentication (see RFC 2069). @@ -51,28 +48,13 @@ * @author Remy Maucherat * @version $Id$ */ - -public class DigestAuthenticator - extends AuthenticatorBase { +public class DigestAuthenticator extends AuthenticatorBase { private static Logger log = Logger.getLogger(DigestAuthenticator.class); // -------------------------------------------------------------- Constants /** - * The MD5 helper object for this class. - */ - protected static final MD5Encoder md5Encoder = new MD5Encoder(); - - - /** - * Descriptive information about this implementation. - */ - protected static final String info = - "org.apache.catalina.authenticator.DigestAuthenticator/1.0"; - - - /** * Tomcat's DIGEST implementation only supports auth quality of protection. */ protected static final String QOP = "auth"; @@ -82,9 +64,11 @@ public DigestAuthenticator() { super(); + setCache(false); try { - if (md5Helper == null) + if (md5Helper == null) { md5Helper = MessageDigest.getInstance("MD5"); + } } catch (NoSuchAlgorithmException e) { throw new IllegalStateException(e); } @@ -96,21 +80,22 @@ /** * MD5 message digest provider. + * @deprecated Unused - will be removed in Tomcat 8.0.x onwards */ protected static MessageDigest md5Helper; /** - * 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; /** @@ -140,27 +125,16 @@ // ------------------------------------------------------------- Properties - /** - * Return descriptive information about this Valve implementation. - */ - @Override - public String getInfo() { - - return (info); - + public int getNonceCacheSize() { + return nonceCacheSize; } - public int getCnonceCacheSize() { - return cnonceCacheSize; + public void setNonceCacheSize(int nonceCacheSize) { + this.nonceCacheSize = nonceCacheSize; } - public void setCnonceCacheSize(int cnonceCacheSize) { - this.cnonceCacheSize = cnonceCacheSize; - } - - public String getKey() { return key; } @@ -211,8 +185,6 @@ * * @param request Request we are processing * @param response Response we are creating - * @param config Login configuration describing how authentication - * should be performed * * @exception IOException if an input/output error occurs */ @@ -220,19 +192,18 @@ public boolean authenticate(Request request, HttpServletResponse response, LoginConfig config) - throws IOException { + throws IOException { // Have we already authenticated someone? Principal principal = request.getUserPrincipal(); //String ssoId = (String) request.getNote(Constants.REQ_SSOID_NOTE); if (principal != null) { - if (log.isDebugEnabled()) - log.debug("Already authenticated '" + principal.getName() + "'"); // Associate the session with any existing SSO session in order // to get coordinated session invalidation at logout String ssoId = (String) request.getNote(Constants.REQ_SSOID_NOTE); - if (ssoId != null) + if (ssoId != null) { associate(ssoId, request.getSessionInternal(true)); + } return (true); } @@ -264,19 +235,20 @@ // 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 && !digestInfo.isNonceStale()) { + register(request, response, principal, + HttpServletRequest.DIGEST_AUTH, + digestInfo.getUsername(), null); + return true; + } } - - if (principal != null) { - String username = parseUsername(authorization); - register(request, response, principal, - HttpServletRequest.DIGEST_AUTH, - username, null); - return (true); - } } // Send an "unauthorized" response and an appropriate challenge @@ -286,11 +258,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; } @@ -298,42 +268,6 @@ /** - * Parse the username from the specified authorization string. If none - * can be identified, return <code>null</code> - * - * @param authorization Authorization string to be parsed - */ - protected String parseUsername(String authorization) { - - // Validate the authorization credentials format - if (authorization == null) - return (null); - if (!authorization.startsWith("Digest ")) - return (null); - authorization = authorization.substring(7).trim(); - - StringTokenizer commaTokenizer = - new StringTokenizer(authorization, ","); - - while (commaTokenizer.hasMoreTokens()) { - String currentToken = commaTokenizer.nextToken(); - int equalSign = currentToken.indexOf('='); - if (equalSign < 0) - return null; - String currentTokenName = - currentToken.substring(0, equalSign).trim(); - String currentTokenValue = - currentToken.substring(equalSign + 1).trim(); - if ("username".equals(currentTokenName)) - return (removeQuotes(currentTokenValue)); - } - - return (null); - - } - - - /** * Removes the quotes on a string. RFC2617 states quotes are optional for * all parameters except realm. */ @@ -346,7 +280,7 @@ } else if (quotedString.length() > 2) { return quotedString.substring(1, quotedString.length() - 1); } else { - return new String(); + return ""; } } @@ -368,16 +302,20 @@ long currentTime = System.currentTimeMillis(); - + String ipTimeKey = request.getRemoteAddr() + ":" + currentTime + ":" + getKey(); - byte[] buffer; - synchronized (md5Helper) { - buffer = md5Helper.digest(ipTimeKey.getBytes()); + byte[] buffer = ConcurrentMessageDigest.digestMD5( + ipTimeKey.getBytes(EncodingToCharset.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; } @@ -404,11 +342,9 @@ * * @param request HTTP Servlet request * @param response HTTP Servlet response - * @param config Login configuration describing how authentication - * should be performed * @param nonce nonce token */ - protected void setAuthenticateHeader(Request request, + protected void setAuthenticateHeader(HttpServletRequest request, HttpServletResponse response, LoginConfig config, String nonce, @@ -430,13 +366,13 @@ getOpaque() + "\""; } - response.setHeader("WWW-Authenticate", authenticateHeader); + response.setHeader(AUTH_HEADER_NAME, authenticateHeader); } // ------------------------------------------------------- Lifecycle Methods - + @Override public void start() throws LifecycleException { super.start(); @@ -460,14 +396,14 @@ if (getKey() == null) { setKey(generateSessionId(random)); } - + // Generate the opaque string the same way if (getOpaque() == null) { setOpaque(generateSessionId(random)); } - - 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; @@ -478,7 +414,7 @@ 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,13 +429,13 @@ } }; } - + 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; @@ -511,21 +447,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<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; @@ -539,12 +481,12 @@ String[] tokens = authorization.split(",(?=(?:[^\"]*\"[^\"]*\")+$)"); method = request.getMethod(); - String opaque = null; for (int i = 0; i < tokens.length; i++) { String currentToken = tokens[i]; - if (currentToken.length() == 0) + if (currentToken.length() == 0) { continue; + } int equalSign = currentToken.indexOf('='); if (equalSign < 0) { @@ -554,26 +496,39 @@ currentToken.substring(0, equalSign).trim(); String currentTokenValue = currentToken.substring(equalSign + 1).trim(); - if ("username".equals(currentTokenName)) + if ("username".equals(currentTokenName)) { userName = removeQuotes(currentTokenValue); - if ("realm".equals(currentTokenName)) + } + if ("realm".equals(currentTokenName)) { realmName = removeQuotes(currentTokenValue, true); - if ("nonce".equals(currentTokenName)) + } + if ("nonce".equals(currentTokenName)) { nonce = removeQuotes(currentTokenValue); - if ("nc".equals(currentTokenName)) + } + if ("nc".equals(currentTokenName)) { nc = removeQuotes(currentTokenValue); - if ("cnonce".equals(currentTokenName)) + } + if ("cnonce".equals(currentTokenName)) { cnonce = removeQuotes(currentTokenValue); - if ("qop".equals(currentTokenName)) + } + if ("qop".equals(currentTokenName)) { qop = removeQuotes(currentTokenValue); - if ("uri".equals(currentTokenName)) + } + if ("uri".equals(currentTokenName)) { uri = removeQuotes(currentTokenValue); - if ("response".equals(currentTokenName)) + } + if ("response".equals(currentTokenName)) { response = removeQuotes(currentTokenValue); - if ("opaque".equals(currentTokenName)) - opaque = removeQuotes(currentTokenValue); + } + if ("opaque".equals(currentTokenName)) { + 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; @@ -589,7 +544,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)) { + 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; + } } } @@ -601,9 +572,9 @@ if (!lcRealm.equals(realmName)) { return false; } - + // Validate the opaque string - if (!this.opaque.equals(opaque)) { + if (!opaque.equals(opaqueReceived)) { return false; } @@ -622,15 +593,15 @@ 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()); - } - String md5ServerIpTimeKey = md5Encoder.encode(buffer); + byte[] buffer = ConcurrentMessageDigest.digestMD5( + serverIpTimeKey.getBytes(EncodingToCharset.ISO_8859_1)); + String md5ServerIpTimeKey = MD5Encoder.encode(buffer); if (!md5ServerIpTimeKey.equals(md5clientIpTimeKey)) { return false; } @@ -641,7 +612,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; @@ -650,7 +621,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; @@ -660,21 +633,18 @@ 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; } @@ -688,11 +658,9 @@ // MD5(Method + ":" + uri) String a2 = method + ":" + uri; - byte[] buffer; - synchronized (md5Helper) { - buffer = md5Helper.digest(a2.getBytes()); - } - String md5a2 = md5Encoder.encode(buffer); + byte[] buffer = ConcurrentMessageDigest.digestMD5( + a2.getBytes(EncodingToCharset.ISO_8859_1)); + String md5a2 = MD5Encoder.encode(buffer); return realm.authenticate(userName, response, nonce, nc, cnonce, qop, realmName, md5a2); @@ -701,21 +669,33 @@ } 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 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 void setTimestamp(long l) { - timestamp = l; - } - + public long getTimestamp() { return timestamp; } Modified: branches/7.0.x/java/org/apache/catalina/connector/Response.java =================================================================== --- branches/7.0.x/java/org/apache/catalina/connector/Response.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/7.0.x/java/org/apache/catalina/connector/Response.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -37,6 +37,7 @@ import java.util.Vector; import javax.servlet.ServletOutputStream; +import javax.servlet.SessionTrackingMode; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletResponse; @@ -1595,6 +1596,12 @@ // Are we in a valid session that is not using cookies? final Request hreq = request; + + // Is URL encoding permitted + if (!hreq.getServletContext().getEffectiveSessionTrackingModes().contains(SessionTrackingMode.URL)) { + return false; + } + final Session session = hreq.getSessionInternal(false); if (session == null) return (false); Modified: branches/7.0.x/java/org/apache/catalina/filters/CsrfPreventionFilter.java =================================================================== --- branches/7.0.x/java/org/apache/catalina/filters/CsrfPreventionFilter.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/7.0.x/java/org/apache/catalina/filters/CsrfPreventionFilter.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -33,6 +33,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponseWrapper; +import javax.servlet.http.HttpSession; /** * Provides basic CSRF protection for a web application. The filter assumes @@ -142,16 +143,19 @@ } } + HttpSession session = req.getSession(false); + @SuppressWarnings("unchecked") - LruCache<String> nonceCache = - (LruCache<String>) req.getSession(true).getAttribute( + LruCache<String> nonceCache = (session == null) ? null : + (LruCache<String>) session.getAttribute( Constants.CSRF_NONCE_SESSION_ATTR_NAME); if (!skipNonceCheck) { String previousNonce = req.getParameter(Constants.CSRF_NONCE_REQUEST_PARAM); - if (nonceCache != null && !nonceCache.contains(previousNonce)) { + if (nonceCache == null || previousNonce == null + || !nonceCache.contains(previousNonce)) { res.sendError(HttpServletResponse.SC_FORBIDDEN); return; } @@ -159,7 +163,10 @@ if (nonceCache == null) { nonceCache = new LruCache<String>(nonceCacheSize); - req.getSession().setAttribute( + if (session == null) { + session = req.getSession(true); + } + session.setAttribute( Constants.CSRF_NONCE_SESSION_ATTR_NAME, nonceCache); } Modified: branches/7.0.x/java/org/apache/catalina/realm/RealmBase.java =================================================================== --- branches/7.0.x/java/org/apache/catalina/realm/RealmBase.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/7.0.x/java/org/apache/catalina/realm/RealmBase.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -741,31 +741,6 @@ if (constraints == null || constraints.length == 0) return (true); - // Specifically allow access to the form login and form error pages - // and the "j_security_check" action - LoginConfig config = context.getLoginConfig(); - if ((config != null) && - (Constants.FORM_METHOD.equals(config.getAuthMethod()))) { - String requestURI = request.getRequestPathMB().toString(); - String loginPage = config.getLoginPage(); - if (loginPage.equals(requestURI)) { - if (log.isDebugEnabled()) - log.debug(" Allow access to login page " + loginPage); - return (true); - } - String errorPage = config.getErrorPage(); - if (errorPage.equals(requestURI)) { - if (log.isDebugEnabled()) - log.debug(" Allow access to error page " + errorPage); - return (true); - } - if (requestURI.endsWith(Constants.FORM_ACTION)) { - if (log.isDebugEnabled()) - log.debug(" Allow access to username/password submission"); - return (true); - } - } - // Which user principal have we already authenticated? Principal principal = request.getPrincipal(); boolean status = false; Added: branches/7.0.x/java/org/apache/catalina/util/ConcurrentMessageDigest.java =================================================================== --- branches/7.0.x/java/org/apache/catalina/util/ConcurrentMessageDigest.java (rev 0) +++ branches/7.0.x/java/org/apache/catalina/util/ConcurrentMessageDigest.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.util; + +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.HashMap; +import java.util.Map; +import java.util.Queue; +import java.util.concurrent.ConcurrentLinkedQueue; + +/** + * A thread safe wrapper around {@link MessageDigest} that does not make use + * of ThreadLocal and - broadly - only creates enough MessageDigest objects + * to satisfy the concurrency requirements. + */ +public class ConcurrentMessageDigest { + + private static final String MD5 = "MD5"; + + private static final Map<String,Queue<MessageDigest>> queues = + new HashMap<String,Queue<MessageDigest>>(); + + + private ConcurrentMessageDigest() { + // Hide default constructor for this utility class + } + + static { + try { + // Init commonly used algorithms + init(MD5); + } catch (NoSuchAlgorithmException e) { + throw new IllegalArgumentException(e); + } + } + + public static byte[] digestMD5(byte[] input) { + return digest(MD5, input); + } + + public static byte[] digest(String algorithm, byte[] input) { + + Queue<MessageDigest> queue = queues.get(algorithm); + if (queue == null) { + throw new IllegalStateException("Must call init() first"); + } + + MessageDigest md = queue.poll(); + if (md == null) { + try { + md = MessageDigest.getInstance(algorithm); + } catch (NoSuchAlgorithmException e) { + // Ignore. Impossible if init() has been successfully called + // first. + throw new IllegalStateException("Must call init() first"); + } + } + + byte[] result = md.digest(input); + + queue.add(md); + + return result; + } + + + /** + * Ensures that {@link #digest(String, byte[])} and + * {@link #digestAsHex(String, byte[])} will support the specified + * algorithm. This method <b>must</b> be called and return successfully + * before using {@link #digest(String, byte[])} or + * {@link #digestAsHex(String, byte[])}. + * + * @param algorithm The message digest algorithm to be supported + * + * @throws NoSuchAlgorithmException If the algorithm is not supported by the + * JVM + */ + public static void init(String algorithm) throws NoSuchAlgorithmException { + synchronized (queues) { + if (!queues.containsKey(algorithm)) { + MessageDigest md = MessageDigest.getInstance(algorithm); + Queue<MessageDigest> queue = new ConcurrentLinkedQueue<MessageDigest>(); + queue.add(md); + queues.put(algorithm, queue); + } + } + } +} Modified: branches/7.0.x/java/org/apache/catalina/util/MD5Encoder.java =================================================================== --- branches/7.0.x/java/org/apache/catalina/util/MD5Encoder.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/7.0.x/java/org/apache/catalina/util/MD5Encoder.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -50,7 +50,7 @@ * @param binaryData Array containing the digest * @return Encoded MD5, or null if encoding failed */ - public String encode( byte[] binaryData ) { + public static String encode( byte[] binaryData ) { if (binaryData.length != 16) return null; Modified: branches/7.0.x/java/org/apache/coyote/ajp/AjpAprProcessor.java =================================================================== --- branches/7.0.x/java/org/apache/coyote/ajp/AjpAprProcessor.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/7.0.x/java/org/apache/coyote/ajp/AjpAprProcessor.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -980,7 +980,7 @@ int port = 0; int mult = 1; for (int i = valueL - 1; i > colonPos; i--) { - int charValue = HexUtils.DEC[(int) valueB[i + valueS]]; + int charValue = HexUtils.DEC[valueB[i + valueS] & 0xff]; if (charValue == -1) { // Invalid character error = true; Modified: branches/7.0.x/java/org/apache/coyote/ajp/AjpMessage.java =================================================================== --- branches/7.0.x/java/org/apache/coyote/ajp/AjpMessage.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/7.0.x/java/org/apache/coyote/ajp/AjpMessage.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -251,10 +251,8 @@ // but is the only consistent approach within the current // servlet framework. It must suffice until servlet output // streams properly encode their output. - if ((c <= 31) && (c != 9)) { + if (((c <= 31) && (c != 9)) || c == 127 || c > 255) { c = ' '; - } else if (c == 127) { - c = ' '; } appendByte(c); } Modified: branches/7.0.x/java/org/apache/coyote/ajp/AjpProcessor.java =================================================================== --- branches/7.0.x/java/org/apache/coyote/ajp/AjpProcessor.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/7.0.x/java/org/apache/coyote/ajp/AjpProcessor.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -994,7 +994,7 @@ int port = 0; int mult = 1; for (int i = valueL - 1; i > colonPos; i--) { - int charValue = HexUtils.DEC[(int) valueB[i + valueS]]; + int charValue = HexUtils.DEC[valueB[i + valueS] & 0xff]; if (charValue == -1) { // Invalid character error = true; Modified: branches/7.0.x/java/org/apache/coyote/http11/Http11AprProcessor.java =================================================================== --- branches/7.0.x/java/org/apache/coyote/http11/Http11AprProcessor.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/7.0.x/java/org/apache/coyote/http11/Http11AprProcessor.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -1559,7 +1559,7 @@ int port = 0; int mult = 1; for (int i = valueL - 1; i > colonPos; i--) { - int charValue = HexUtils.DEC[valueB[i + valueS]]; + int charValue = HexUtils.DEC[valueB[i + valueS] & 0xff]; if (charValue == -1) { // Invalid character error = true; Modified: branches/7.0.x/java/org/apache/coyote/http11/Http11Processor.java =================================================================== --- branches/7.0.x/java/org/apache/coyote/http11/Http11Processor.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/7.0.x/java/org/apache/coyote/http11/Http11Processor.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -1447,7 +1447,7 @@ int port = 0; int mult = 1; for (int i = valueL - 1; i > colonPos; i--) { - int charValue = HexUtils.DEC[valueB[i + valueS]]; + int charValue = HexUtils.DEC[valueB[i + valueS] & 0xff]; if (charValue == -1) { // Invalid character error = true; Modified: branches/7.0.x/java/org/apache/coyote/http11/InternalAprOutputBuffer.java =================================================================== --- branches/7.0.x/java/org/apache/coyote/http11/InternalAprOutputBuffer.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/7.0.x/java/org/apache/coyote/http11/InternalAprOutputBuffer.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -684,10 +684,8 @@ // but is the only consistent approach within the current // servlet framework. It must suffice until servlet output // streams properly encode their output. - if ((c <= 31) && (c != 9)) { + if (((c <= 31) && (c != 9)) || c == 127 || c > 255) { c = ' '; - } else if (c == 127) { - c = ' '; } buf[pos++] = (byte) c; } Modified: branches/7.0.x/java/org/apache/coyote/http11/InternalOutputBuffer.java =================================================================== --- branches/7.0.x/java/org/apache/coyote/http11/InternalOutputBuffer.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/7.0.x/java/org/apache/coyote/http11/InternalOutputBuffer.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -690,10 +690,8 @@ // but is the only consistent approach within the current // servlet framework. It must suffice until servlet output // streams properly encode their output. - if ((c <= 31) && (c != 9)) { + if (((c <= 31) && (c != 9)) || c == 127 || c > 255) { c = ' '; - } else if (c == 127) { - c = ' '; } buf[pos++] = (byte) c; } Modified: branches/7.0.x/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java =================================================================== --- branches/7.0.x/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/7.0.x/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -266,6 +266,7 @@ int result = 0; boolean eol = false; + boolean crfound = false; boolean readDigit = false; boolean trailer = false; @@ -282,7 +283,10 @@ } if (buf[pos] == Constants.CR) { + if (crfound) throw new IOException("Invalid CRLF, two CR characters encountered."); + crfound = true; } else if (buf[pos] == Constants.LF) { + if (!crfound) throw new IOException("Invalid CRLF, no CR character encountered."); eol = true; } else if (buf[pos] == Constants.SEMI_COLON) { trailer = true; @@ -290,7 +294,7 @@ throw new IOException("Invalid chunk header"); } else if (!trailer) { //don't read data after the trailer - if (HexUtils.DEC[buf[pos]] != -1) { + if (HexUtils.DEC[buf[pos] & 0xff] != -1) { readDigit = true; result *= 16; result += HexUtils.DEC[buf[pos]]; Modified: branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/authenticator/DigestAuthenticator.java =================================================================== --- branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/authenticator/DigestAuthenticator.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/authenticator/DigestAuthenticator.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -5,30 +5,27 @@ * The ASF licenses this file to You under the Apache License, Version 2.0 * (the "License"); you may not use this file except in compliance with * the License. You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */ - - package org.apache.catalina.authenticator; - import java.io.IOException; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +import java.security.Principal; import java.security.SecureRandom; import java.util.LinkedHashMap; import java.util.Map; import java.util.Random; -import java.security.Principal; -import java.util.StringTokenizer; +import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.catalina.Container; @@ -38,19 +35,19 @@ import org.apache.catalina.connector.Request; import org.apache.catalina.connector.Response; import org.apache.catalina.deploy.LoginConfig; +import org.apache.catalina.util.ConcurrentMessageDigest; import org.apache.catalina.util.MD5Encoder; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - /** * An <b>Authenticator</b> and <b>Valve</b> implementation of HTTP DIGEST * Authentication (see RFC 2069). * * @author Craig R. McClanahan * @author Remy Maucherat - * @version $Revision: 467222 $ $Date: 2006-10-24 05:17:11 +0200 (mar., 24 oct. 2006) $ + * @version $Id: DigestAuthenticator.java 1377853 2012-08-27 20:53:56Z markt $ */ public class DigestAuthenticator @@ -61,19 +58,6 @@ // -------------------------------------------------------------- Constants /** - * The MD5 helper object for this class. - */ - protected static final MD5Encoder md5Encoder = new MD5Encoder(); - - - /** - * Descriptive information about this implementation. - */ - protected static final String info = - "org.apache.catalina.authenticator.DigestAuthenticator/1.0"; - - - /** * Tomcat's DIGEST implementation only supports auth quality of protection. */ protected static final String QOP = "auth"; @@ -83,12 +67,13 @@ public DigestAuthenticator() { super(); + setCache(false); try { - if (md5Helper == null) + if (md5Helper == null) { md5Helper = MessageDigest.getInstance("MD5"); + } } catch (NoSuchAlgorithmException e) { - e.printStackTrace(); - throw new IllegalStateException(); + throw new IllegalStateException(e); } } @@ -98,21 +83,22 @@ /** * MD5 message digest provider. + * @deprecated Unused - will be removed in Tomcat 8.0.x onwards */ protected static MessageDigest md5Helper; /** - * 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; /** @@ -142,27 +128,16 @@ // ------------------------------------------------------------- Properties - /** - * Return descriptive information about this Valve implementation. - */ - @Override - public String getInfo() { - - return (info); - + public int getNonceCacheSize() { + return nonceCacheSize; } - public int getCnonceCacheSize() { - return cnonceCacheSize; + public void setNonceCacheSize(int nonceCacheSize) { + this.nonceCacheSize = nonceCacheSize; } - public void setCnonceCacheSize(int cnonceCacheSize) { - this.cnonceCacheSize = cnonceCacheSize; - } - - public String getKey() { return key; } @@ -213,8 +188,6 @@ * * @param request Request we are processing * @param response Response we are creating - * @param config Login configuration describing how authentication - * should be performed * * @exception IOException if an input/output error occurs */ @@ -222,19 +195,18 @@ public boolean authenticate(Request request, Response response, LoginConfig config) - throws IOException { + throws IOException { // Have we already authenticated someone? Principal principal = request.getUserPrincipal(); //String ssoId = (String) request.getNote(Constants.REQ_SSOID_NOTE); if (principal != null) { - if (log.isDebugEnabled()) - log.debug("Already authenticated '" + principal.getName() + "'"); // Associate the session with any existing SSO session in order // to get coordinated session invalidation at logout String ssoId = (String) request.getNote(Constants.REQ_SSOID_NOTE); - if (ssoId != null) + if (ssoId != null) { associate(ssoId, request.getSessionInternal(true)); + } return (true); } @@ -266,19 +238,20 @@ // 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 && !digestInfo.isNonceStale()) { + register(request, response, principal, + HttpServletRequest.DIGEST_AUTH, + digestInfo.getUsername(), null); + return true; + } } - - if (principal != null) { - String username = parseUsername(authorization); - register(request, response, principal, - Constants.DIGEST_METHOD, - username, null); - return (true); - } } // Send an "unauthorized" response and an appropriate challenge @@ -288,11 +261,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; } @@ -300,42 +271,6 @@ /** - * Parse the username from the specified authorization string. If none - * can be identified, return <code>null</code> - * - * @param authorization Authorization string to be parsed - */ - protected String parseUsername(String authorization) { - - // Validate the authorization credentials format - if (authorization == null) - return (null); - if (!authorization.startsWith("Digest ")) - return (null); - authorization = authorization.substring(7).trim(); - - StringTokenizer commaTokenizer = - new StringTokenizer(authorization, ","); - - while (commaTokenizer.hasMoreTokens()) { - String currentToken = commaTokenizer.nextToken(); - int equalSign = currentToken.indexOf('='); - if (equalSign < 0) - return null; - String currentTokenName = - currentToken.substring(0, equalSign).trim(); - String currentTokenValue = - currentToken.substring(equalSign + 1).trim(); - if ("username".equals(currentTokenName)) - return (removeQuotes(currentTokenValue)); - } - - return (null); - - } - - - /** * Removes the quotes on a string. RFC2617 states quotes are optional for * all parameters except realm. */ @@ -348,7 +283,7 @@ } else if (quotedString.length() > 2) { return quotedString.substring(1, quotedString.length() - 1); } else { - return new String(); + return ""; } } @@ -370,16 +305,19 @@ long currentTime = System.currentTimeMillis(); - + String ipTimeKey = request.getRemoteAddr() + ":" + currentTime + ":" + getKey(); - byte[] buffer; - synchronized (md5Helper) { - buffer = md5Helper.digest(ipTimeKey.getBytes()); + byte[] buffer = ConcurrentMessageDigest.digestMD5(ipTimeKey.getBytes()); + 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; } @@ -406,12 +344,10 @@ * * @param request HTTP Servlet request * @param response HTTP Servlet response - * @param config Login configuration describing how authentication - * should be performed * @param nonce nonce token */ - protected void setAuthenticateHeader(Request request, - Response response, + protected void setAuthenticateHeader(HttpServletRequest request, + HttpServletResponse response, LoginConfig config, String nonce, boolean isNonceStale) { @@ -430,15 +366,15 @@ authenticateHeader = "Digest realm=\"" + realmName + "\", " + "qop=\"" + QOP + "\", nonce=\"" + nonce + "\", " + "opaque=\"" + getOpaque() + "\""; - } + } - response.setHeader("WWW-Authenticate", authenticateHeader); + response.setHeader(AUTH_HEADER_NAME, authenticateHeader); } // ------------------------------------------------------- Lifecycle Methods - + @Override public void start() throws LifecycleException { super.start(); @@ -447,14 +383,14 @@ if (getKey() == null) { setKey(generateSessionId()); } - + // Generate the opaque string the same way if (getOpaque() == null) { 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; @@ -465,7 +401,7 @@ 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,13 +416,13 @@ } }; } - + 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; @@ -498,21 +434,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<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; @@ -526,12 +468,12 @@ String[] tokens = authorization.split(",(?=(?:[^\"]*\"[^\"]*\")+$)"); method = request.getMethod(); - String opaque = null; for (int i = 0; i < tokens.length; i++) { String currentToken = tokens[i]; - if (currentToken.length() == 0) + if (currentToken.length() == 0) { continue; + } int equalSign = currentToken.indexOf('='); if (equalSign < 0) { @@ -541,26 +483,39 @@ currentToken.substring(0, equalSign).trim(); String currentTokenValue = currentToken.substring(equalSign + 1).trim(); - if ("username".equals(currentTokenName)) + if ("username".equals(currentTokenName)) { userName = removeQuotes(currentTokenValue); - if ("realm".equals(currentTokenName)) + } + if ("realm".equals(currentTokenName)) { realmName = removeQuotes(currentTokenValue, true); - if ("nonce".equals(currentTokenName)) + } + if ("nonce".equals(currentTokenName)) { nonce = removeQuotes(currentTokenValue); - if ("nc".equals(currentTokenName)) + } + if ("nc".equals(currentTokenName)) { nc = removeQuotes(currentTokenValue); - if ("cnonce".equals(currentTokenName)) + } + if ("cnonce".equals(currentTokenName)) { cnonce = removeQuotes(currentTokenValue); - if ("qop".equals(currentTokenName)) + } + if ("qop".equals(currentTokenName)) { qop = removeQuotes(currentTokenValue); - if ("uri".equals(currentTokenName)) + } + if ("uri".equals(currentTokenName)) { uri = removeQuotes(currentTokenValue); - if ("response".equals(currentTokenName)) + } + if ("response".equals(currentTokenName)) { response = removeQuotes(currentTokenValue); - if ("opaque".equals(currentTokenName)) - opaque = removeQuotes(currentTokenValue); + } + if ("opaque".equals(currentTokenName)) { + 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; @@ -576,7 +531,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)) { + 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; + } } } @@ -588,9 +559,9 @@ if (!lcRealm.equals(realmName)) { return false; } - + // Validate the opaque string - if (!this.opaque.equals(opaque)) { + if (!opaque.equals(opaqueReceived)) { return false; } @@ -609,15 +580,15 @@ 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()); - } - String md5ServerIpTimeKey = md5Encoder.encode(buffer); + byte[] buffer = ConcurrentMessageDigest.digestMD5( + serverIpTimeKey.getBytes()); + String md5ServerIpTimeKey = MD5Encoder.encode(buffer); if (!md5ServerIpTimeKey.equals(md5clientIpTimeKey)) { return false; } @@ -628,7 +599,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; @@ -637,7 +608,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; @@ -647,21 +620,18 @@ 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; } @@ -675,11 +645,9 @@ // MD5(Method + ":" + uri) String a2 = method + ":" + uri; - byte[] buffer; - synchronized (md5Helper) { - buffer = md5Helper.digest(a2.getBytes()); - } - String md5a2 = md5Encoder.encode(buffer); + byte[] buffer = ConcurrentMessageDigest.digestMD5( + a2.getBytes()); + String md5a2 = MD5Encoder.encode(buffer); return realm.authenticate(userName, response, nonce, nc, cnonce, qop, realmName, md5a2); @@ -688,21 +656,33 @@ } 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 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 void setTimestamp(long l) { - timestamp = l; - } - + public long getTimestamp() { return timestamp; } Modified: branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/realm/RealmBase.java =================================================================== --- branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/realm/RealmBase.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/realm/RealmBase.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -728,31 +728,6 @@ if (constraints == null || constraints.length == 0) return (true); - // Specifically allow access to the form login and form error pages - // and the "j_security_check" action - LoginConfig config = context.getLoginConfig(); - if ((config != null) && - (Constants.FORM_METHOD.equals(config.getAuthMethod()))) { - String requestURI = request.getRequestPathMB().toString(); - String loginPage = config.getLoginPage(); - if (loginPage.equals(requestURI)) { - if (log.isDebugEnabled()) - log.debug(" Allow access to login page " + loginPage); - return (true); - } - String errorPage = config.getErrorPage(); - if (errorPage.equals(requestURI)) { - if (log.isDebugEnabled()) - log.debug(" Allow access to error page " + errorPage); - return (true); - } - if (requestURI.endsWith(Constants.FORM_ACTION)) { - if (log.isDebugEnabled()) - log.debug(" Allow access to username/password submission"); - return (true); - } - } - // Which user principal have we already authenticated? Principal principal = request.getPrincipal(); boolean status = false; Added: branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/util/ConcurrentMessageDigest.java =================================================================== --- branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/util/ConcurrentMessageDigest.java (rev 0) +++ branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/util/ConcurrentMessageDigest.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.util; + +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.HashMap; +import java.util.Map; +import java.util.Queue; +import java.util.concurrent.ConcurrentLinkedQueue; + +/** + * A thread safe wrapper around {@link MessageDigest} that does not make use + * of ThreadLocal and - broadly - only creates enough MessageDigest objects + * to satisfy the concurrency requirements. + */ +public class ConcurrentMessageDigest { + + private static final String MD5 = "MD5"; + + private static final Map<String,Queue<MessageDigest>> queues = + new HashMap<String,Queue<MessageDigest>>(); + + + private ConcurrentMessageDigest() { + // Hide default constructor for this utility class + } + + static { + try { + // Init commonly used algorithms + init(MD5); + } catch (NoSuchAlgorithmException e) { + throw new IllegalArgumentException(e); + } + } + + public static byte[] digestMD5(byte[] input) { + return digest(MD5, input); + } + + public static byte[] digest(String algorithm, byte[] input) { + + Queue<MessageDigest> queue = queues.get(algorithm); + if (queue == null) { + throw new IllegalStateException("Must call init() first"); + } + + MessageDigest md = queue.poll(); + if (md == null) { + try { + md = MessageDigest.getInstance(algorithm); + } catch (NoSuchAlgorithmException e) { + // Ignore. Impossible if init() has been successfully called + // first. + throw new IllegalStateException("Must call init() first"); + } + } + + byte[] result = md.digest(input); + + queue.add(md); + + return result; + } + + + /** + * Ensures that {@link #digest(String, byte[])} and + * {@link #digestAsHex(String, byte[])} will support the specified + * algorithm. This method <b>must</b> be called and return successfully + * before using {@link #digest(String, byte[])} or + * {@link #digestAsHex(String, byte[])}. + * + * @param algorithm The message digest algorithm to be supported + * + * @throws NoSuchAlgorithmException If the algorithm is not supported by the + * JVM + */ + public static void init(String algorithm) throws NoSuchAlgorithmException { + synchronized (queues) { + if (!queues.containsKey(algorithm)) { + MessageDigest md = MessageDigest.getInstance(algorithm); + Queue<MessageDigest> queue = new ConcurrentLinkedQueue<MessageDigest>(); + queue.add(md); + queues.put(algorithm, queue); + } + } + } +} Modified: branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/util/MD5Encoder.java =================================================================== --- branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/util/MD5Encoder.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/catalina/util/MD5Encoder.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -50,7 +50,7 @@ * @param binaryData Array containing the digest * @return Encoded MD5, or null if encoding failed */ - public String encode( byte[] binaryData ) { + public static String encode( byte[] binaryData ) { if (binaryData.length != 16) return null; Modified: branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/ajp/AjpAprProcessor.java =================================================================== --- branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/ajp/AjpAprProcessor.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/ajp/AjpAprProcessor.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -884,7 +884,7 @@ int port = 0; int mult = 1; for (int i = valueL - 1; i > colonPos; i--) { - int charValue = HexUtils.DEC[(int) valueB[i + valueS]]; + int charValue = HexUtils.DEC[valueB[i + valueS] & 0xff]; if (charValue == -1) { // Invalid character error = true; Modified: branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/ajp/AjpMessage.java =================================================================== --- branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/ajp/AjpMessage.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/ajp/AjpMessage.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -251,10 +251,8 @@ // but is the only consistent approach within the current // servlet framework. It must suffice until servlet output // streams properly encode their output. - if ((c <= 31) && (c != 9)) { + if (((c <= 31) && (c != 9)) || c == 127 || c > 255) { c = ' '; - } else if (c == 127) { - c = ' '; } appendByte(c); } Modified: branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/ajp/AjpProcessor.java =================================================================== --- branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/ajp/AjpProcessor.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/ajp/AjpProcessor.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -890,7 +890,7 @@ int port = 0; int mult = 1; for (int i = valueL - 1; i > colonPos; i--) { - int charValue = HexUtils.DEC[(int) valueB[i + valueS]]; + int charValue = HexUtils.DEC[valueB[i + valueS] & 0xff]; if (charValue == -1) { // Invalid character error = true; Modified: branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/Http11AprProcessor.java =================================================================== --- branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/Http11AprProcessor.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/Http11AprProcessor.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -1461,7 +1461,7 @@ int port = 0; int mult = 1; for (int i = valueL - 1; i > colonPos; i--) { - int charValue = HexUtils.DEC[(int) valueB[i + valueS]]; + int charValue = HexUtils.DEC[valueB[i + valueS] & 0xff]; if (charValue == -1) { // Invalid character error = true; Modified: branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/Http11NioProcessor.java =================================================================== --- branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/Http11NioProcessor.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/Http11NioProcessor.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -1495,7 +1495,7 @@ int port = 0; int mult = 1; for (int i = valueL - 1; i > colonPos; i--) { - int charValue = HexUtils.DEC[(int) valueB[i + valueS]]; + int charValue = HexUtils.DEC[valueB[i + valueS] & 0xff]; if (charValue == -1) { // Invalid character error = true; Modified: branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/Http11Processor.java =================================================================== --- branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/Http11Processor.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/Http11Processor.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -1384,7 +1384,7 @@ int port = 0; int mult = 1; for (int i = valueL - 1; i > colonPos; i--) { - int charValue = HexUtils.DEC[(int) valueB[i + valueS]]; + int charValue = HexUtils.DEC[valueB[i + valueS] & 0xff]; if (charValue == -1) { // Invalid character error = true; Modified: branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/InternalAprOutputBuffer.java =================================================================== --- branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/InternalAprOutputBuffer.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/InternalAprOutputBuffer.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -658,10 +658,8 @@ // but is the only consistent approach within the current // servlet framework. It must suffice until servlet output // streams properly encode their output. - if ((c <= 31) && (c != 9)) { + if (((c <= 31) && (c != 9)) || c == 127 || c > 255) { c = ' '; - } else if (c == 127) { - c = ' '; } buf[pos++] = (byte) c; } Modified: branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/InternalNioOutputBuffer.java =================================================================== --- branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/InternalNioOutputBuffer.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/InternalNioOutputBuffer.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -719,10 +719,8 @@ // but is the only consistent approach within the current // servlet framework. It must suffice until servlet output // streams properly encode their output. - if ((c <= 31) && (c != 9)) { + if (((c <= 31) && (c != 9)) || c == 127 || c > 255) { c = ' '; - } else if (c == 127) { - c = ' '; } buf[pos++] = (byte) c; } Modified: branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/InternalOutputBuffer.java =================================================================== --- branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/InternalOutputBuffer.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/InternalOutputBuffer.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -704,10 +704,8 @@ // but is the only consistent approach within the current // servlet framework. It must suffice until servlet output // streams properly encode their output. - if ((c <= 31) && (c != 9)) { + if (((c <= 31) && (c != 9)) || c == 127 || c > 255) { c = ' '; - } else if (c == 127) { - c = ' '; } buf[pos++] = (byte) c; } Modified: branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/filters/ChunkedInputFilter.java =================================================================== --- branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/filters/ChunkedInputFilter.java 2012-10-10 17:48:22 UTC (rev 2106) +++ branches/JBOSSWEB_2_0_0_GA_CP/src/share/classes/org/apache/coyote/http11/filters/ChunkedInputFilter.java 2012-10-10 17:50:36 UTC (rev 2107) @@ -19,13 +19,12 @@ import java.io.IOException; -import org.apache.tomcat.util.buf.ByteChunk; -import org.apache.tomcat.util.buf.HexUtils; - import org.apache.coyote.InputBuffer; import org.apache.coyote.Request; import org.apache.coyote.http11.Constants; import org.apache.coyote.http11.InputFilter; +import org.apache.tomcat.util.buf.ByteChunk; +import org.apache.tomcat.util.buf.HexUtils; /** * Chunked input filter. Parses chunked data according to @@ -123,7 +122,7 @@ if (endChunk) return -1; - if(needCRLFParse) { + if (needCRLFParse) { needCRLFParse = false; parseCRLF(); } @@ -154,7 +153,7 @@ chunk.setBytes(buf, pos, remaining); pos = pos + remaining; remaining = 0; - needCRLFParse = true; + needCRLFParse = true; } return result; @@ -249,6 +248,7 @@ int result = 0; boolean eol = false; + boolean crfound = false; boolean readDigit = false; boolean trailer = false; @@ -260,13 +260,16 @@ } if (buf[pos] == Constants.CR) { + if (crfound) throw new IOException("Invalid CRLF, two CR characters encountered."); + crfound = true; } else if (buf[pos] == Constants.LF) { + if (!crfound) throw new IOException("Invalid CRLF, no CR character encountered."); eol = true; } else if (buf[pos] == Constants.SEMI_COLON) { trailer = true; } else if (!trailer) { //don't read data after the trailer - if (HexUtils.DEC[buf[pos]] != -1) { + if (HexUtils.DEC[buf[pos] & 0xff] != -1) { readDigit = true; result *= 16; result += HexUtils.DEC[buf[pos]]; @@ -303,6 +306,7 @@ throws IOException { boolean eol = false; + boolean crfound = false; while (!eol) { @@ -312,7 +316,10 @@ } if (buf[pos] == Constants.CR) { + if (crfound) throw new IOException("Invalid CRLF, two CR characters encountered."); + crfound = true; } else if (buf[pos] == Constants.LF) { + if (!crfound) throw new IOException("Invalid CRLF, no CR character encountered."); eol = true; } else { throw new IOException("Invalid CRLF");