[jbossweb-commits] JBossWeb SVN: r1871 - in branches/2.1.x: java/org/apache/catalina/realm and 1 other directories.

Author: jfrederic.clere(a)jboss.com Date: 2011-11-24 03:23:37 -0500 (Thu, 24 Nov 2011) New Revision: 1871 Modified: branches/2.1.x/java/org/apache/catalina/authenticator/AuthenticatorBase.java branches/2.1.x/java/org/apache/catalina/authenticator/DigestAuthenticator.java branches/2.1.x/java/org/apache/catalina/authenticator/LocalStrings.properties branches/2.1.x/java/org/apache/catalina/authenticator/mbeans-descriptors.xml branches/2.1.x/java/org/apache/catalina/realm/RealmBase.java branches/2.1.x/webapps/docs/changelog.xml Log: Port DIGEST improvements. (r1848) Modified: branches/2.1.x/java/org/apache/catalina/authenticator/AuthenticatorBase.java =================================================================== --- branches/2.1.x/java/org/apache/catalina/authenticator/AuthenticatorBase.java 2011-11-23 09:52:37 UTC (rev 1870) +++ branches/2.1.x/java/org/apache/catalina/authenticator/AuthenticatorBase.java 2011-11-24 08:23:37 UTC (rev 1871) @@ -89,8 +89,17 @@ */ protected static final String DEFAULT_ALGORITHM = "MD5"; + /** + * Authentication header + */ + protected static final String AUTH_HEADER_NAME = "WWW-Authenticate"; /** + * Default authentication realm name. + */ + protected static final String REALM_NAME = "Authentication required"; + + /** * The number of random bytes to include when generating a * session identifier. */ Modified: branches/2.1.x/java/org/apache/catalina/authenticator/DigestAuthenticator.java =================================================================== --- branches/2.1.x/java/org/apache/catalina/authenticator/DigestAuthenticator.java 2011-11-23 09:52:37 UTC (rev 1870) +++ branches/2.1.x/java/org/apache/catalina/authenticator/DigestAuthenticator.java 2011-11-24 08:23:37 UTC (rev 1871) @@ -22,12 +22,18 @@ import java.io.IOException; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +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.HttpServletResponse; - +import org.apache.catalina.Container; +import org.apache.catalina.Engine; +import org.apache.catalina.LifecycleException; import org.apache.catalina.Realm; import org.apache.catalina.connector.Request; import org.apache.catalina.connector.Response; @@ -67,6 +73,11 @@ "org.apache.catalina.authenticator.DigestAuthenticator/1.0"; + /** + * Tomcat's DIGEST implementation only supports auth quality of protection. + */ + protected static final String QOP = "auth"; + // ----------------------------------------------------------- Constructors @@ -92,17 +103,49 @@ /** + * List of client nonce values currently being tracked + */ + protected Map<String,NonceInfo> cnonces; + + + /** + * Maximum number of client nonces to keep in the cache. If not specified, + * the default value of 1000 is used. + */ + protected int cnonceCacheSize = 1000; + + + /** * Private key. */ - protected String key = "Catalina"; + protected String key = null; - // ------------------------------------------------------------- Properties + /** + * How long server nonces are valid for in milliseconds. Defaults to 5 + * minutes. + */ + protected long nonceValidity = 5 * 60 * 1000; /** + * Opaque string. + */ + protected String opaque; + + + /** + * Should the URI be validated as required by RFC2617? Can be disabled in + * reverse proxies where the proxy has modified the URI. + */ + protected boolean validateUri = true; + + // ------------------------------------------------------------- Properties + + /** * Return descriptive information about this Valve implementation. */ + @Override public String getInfo() { return (info); @@ -110,9 +153,58 @@ } + public int getCnonceCacheSize() { + return cnonceCacheSize; + } + + + public void setCnonceCacheSize(int cnonceCacheSize) { + this.cnonceCacheSize = cnonceCacheSize; + } + + + public String getKey() { + return key; + } + + + public void setKey(String key) { + this.key = key; + } + + + public long getNonceValidity() { + return nonceValidity; + } + + + public void setNonceValidity(long nonceValidity) { + this.nonceValidity = nonceValidity; + } + + + public String getOpaque() { + return opaque; + } + + + public void setOpaque(String opaque) { + this.opaque = opaque; + } + + + public boolean isValidateUri() { + return validateUri; + } + + + public void setValidateUri(boolean validateUri) { + this.validateUri = validateUri; + } + + // --------------------------------------------------------- Public Methods - /** * Authenticate the user making this request, based on the specified * login configuration. Return <code>true</code> if any specified @@ -126,6 +218,7 @@ * * @exception IOException if an input/output error occurs */ + @Override public boolean authenticate(Request request, Response response, LoginConfig config) @@ -172,8 +265,13 @@ // Validate any credentials already included with this request String authorization = request.getHeader("authorization"); + DigestInfo digestInfo = new DigestInfo(getOpaque(), getNonceValidity(), + getKey(), cnonces, isValidateUri()); if (authorization != null) { - principal = findPrincipal(request, authorization, context.getRealm()); + if (digestInfo.validate(request, authorization, config)) { + principal = digestInfo.authenticate(context.getRealm()); + } + if (principal != null) { String username = parseUsername(authorization); register(request, response, principal, @@ -185,11 +283,12 @@ // Send an "unauthorized" response and an appropriate challenge - // Next, generate a nOnce token (that is a token which is supposed + // Next, generate a nonce token (that is a token which is supposed // to be unique). - String nOnce = generateNOnce(request); + String nonce = generateNonce(request); - setAuthenticateHeader(request, response, config, nOnce); + setAuthenticateHeader(request, response, config, nonce, + digestInfo.isNonceStale()); response.sendError(HttpServletResponse.SC_UNAUTHORIZED); // hres.flushBuffer(); return (false); @@ -201,92 +300,6 @@ /** - * Parse the specified authorization credentials, and return the - * associated Principal that these credentials authenticate (if any) - * from the specified Realm. If there is no such Principal, return - * <code>null</code>. - * - * @param request HTTP servlet request - * @param authorization Authorization credentials from this request - * @param realm Realm used to authenticate Principals - */ - protected static Principal findPrincipal(Request request, - String authorization, - Realm realm) { - - //System.out.println("Authorization token : " + authorization); - // Validate the authorization credentials format - if (authorization == null) - return (null); - if (!authorization.startsWith("Digest ")) - return (null); - authorization = authorization.substring(7).trim(); - - // Bugzilla 37132: http://issues.apache.org/bugzilla/show_bug.cgi?id=37132 - String[] tokens = authorization.split(",(?=(?:[^\"]*\"[^\"]*\")+$)"); - - String userName = null; - String realmName = null; - String nOnce = null; - String nc = null; - String cnonce = null; - String qop = null; - String uri = null; - String response = null; - String method = request.getMethod(); - - for (int i = 0; i < tokens.length; i++) { - String currentToken = tokens[i]; - if (currentToken.length() == 0) - continue; - - 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)) - userName = removeQuotes(currentTokenValue); - if ("realm".equals(currentTokenName)) - realmName = removeQuotes(currentTokenValue, true); - if ("nonce".equals(currentTokenName)) - nOnce = removeQuotes(currentTokenValue); - if ("nc".equals(currentTokenName)) - nc = removeQuotes(currentTokenValue); - if ("cnonce".equals(currentTokenName)) - cnonce = removeQuotes(currentTokenValue); - if ("qop".equals(currentTokenName)) - qop = removeQuotes(currentTokenValue); - if ("uri".equals(currentTokenName)) - uri = removeQuotes(currentTokenValue); - if ("response".equals(currentTokenName)) - response = removeQuotes(currentTokenValue); - } - - if ( (userName == null) || (realmName == null) || (nOnce == null) - || (uri == null) || (response == null) ) - return null; - - // Second MD5 digest used to calculate the digest : - // MD5(Method + ":" + uri) - String a2 = method + ":" + uri; - //System.out.println("A2:" + a2); - - byte[] buffer = null; - synchronized (md5Helper) { - buffer = md5Helper.digest(a2.getBytes()); - } - String md5a2 = md5Encoder.encode(buffer); - - return (realm.authenticate(userName, response, nOnce, nc, cnonce, qop, - realmName, md5a2)); - - } - - - /** * Parse the username from the specified authorization string. If none * can be identified, return <code>null</code> * @@ -294,7 +307,6 @@ */ protected String parseUsername(String authorization) { - //System.out.println("Authorization token : " + authorization); // Validate the authorization credentials format if (authorization == null) return (null); @@ -354,20 +366,20 @@ * * @param request HTTP Servlet request */ - protected String generateNOnce(Request request) { + protected String generateNonce(Request request) { long currentTime = System.currentTimeMillis(); - String nOnceValue = request.getRemoteAddr() + ":" + - currentTime + ":" + key; + + String ipTimeKey = + request.getRemoteAddr() + ":" + currentTime + ":" + getKey(); - byte[] buffer = null; + byte[] buffer; synchronized (md5Helper) { - buffer = md5Helper.digest(nOnceValue.getBytes()); + buffer = md5Helper.digest(ipTimeKey.getBytes()); } - nOnceValue = md5Encoder.encode(buffer); - return nOnceValue; + return currentTime + ":" + md5Encoder.encode(buffer); } @@ -379,7 +391,7 @@ * WWW-Authenticate = "WWW-Authenticate" ":" "Digest" * digest-challenge * - * digest-challenge = 1#( realm | [ domain ] | nOnce | + * digest-challenge = 1#( realm | [ domain ] | nonce | * [ digest-opaque ] |[ stale ] | [ algorithm ] ) * * realm = "realm" "=" realm-value @@ -396,29 +408,303 @@ * @param response HTTP Servlet response * @param config Login configuration describing how authentication * should be performed - * @param nOnce nonce token + * @param nonce nonce token */ protected void setAuthenticateHeader(Request request, Response response, LoginConfig config, - String nOnce) { + String nonce, + boolean isNonceStale) { // Get the realm name String realmName = config.getRealmName(); if (realmName == null) - realmName = "Realm"; + realmName = REALM_NAME; - byte[] buffer = null; - synchronized (md5Helper) { - buffer = md5Helper.digest(nOnce.getBytes()); - } + String authenticateHeader; + if (isNonceStale) { + authenticateHeader = "Digest realm=\"" + realmName + "\", " + + "qop=\"" + QOP + "\", nonce=\"" + nonce + "\", " + "opaque=\"" + + getOpaque() + "\", stale=true"; + } else { + authenticateHeader = "Digest realm=\"" + realmName + "\", " + + "qop=\"" + QOP + "\", nonce=\"" + nonce + "\", " + "opaque=\"" + + getOpaque() + "\""; + } - String authenticateHeader = "Digest realm=\"" + realmName + "\", " - + "qop=\"auth\", nonce=\"" + nOnce + "\", " + "opaque=\"" - + md5Encoder.encode(buffer) + "\""; response.setHeader("WWW-Authenticate", authenticateHeader); } + // ------------------------------------------------------- Lifecycle Methods + + @Override + public void start() throws LifecycleException { + super.start(); + + // Generate a random secret key + if (getKey() == null) { + setKey(generateSessionId()); + } + + // Generate the opaque string the same way + if (getOpaque() == null) { + setOpaque(generateSessionId()); + } + + cnonces = new LinkedHashMap<String, DigestAuthenticator.NonceInfo>() { + + private static final long serialVersionUID = 1L; + private static final long LOG_SUPPRESS_TIME = 5 * 60 * 1000; + + private long lastLog = 0; + + @Override + protected boolean removeEldestEntry( + Map.Entry<String,NonceInfo> eldest) { + // This is called from a sync so keep it simple + long currentTime = System.currentTimeMillis(); + if (size() > getCnonceCacheSize()) { + if (lastLog < currentTime && + currentTime - eldest.getValue().getTimestamp() < + getNonceValidity()) { + // Replay attack is possible + log.warn(sm.getString( + "digestAuthenticator.cacheRemove")); + lastLog = currentTime + LOG_SUPPRESS_TIME; + } + return true; + } + return false; + } + }; + } + + private static class DigestInfo { + + private String opaque; + private long nonceValidity; + private String key; + private Map<String,NonceInfo> cnonces; + private boolean validateUri = true; + + private String userName = null; + private String method = null; + private String uri = null; + private String response = null; + private String nonce = null; + private String nc = null; + private String cnonce = null; + private String realmName = null; + private String qop = null; + + private boolean nonceStale = false; + + + public DigestInfo(String opaque, long nonceValidity, String key, + Map<String,NonceInfo> cnonces, boolean validateUri) { + this.opaque = opaque; + this.nonceValidity = nonceValidity; + this.key = key; + this.cnonces = cnonces; + this.validateUri = validateUri; + } + + public boolean validate(Request request, String authorization, + LoginConfig config) { + // Validate the authorization credentials format + if (authorization == null) { + return false; + } + if (!authorization.startsWith("Digest ")) { + return false; + } + authorization = authorization.substring(7).trim(); + + // Bugzilla 37132: http://issues.apache.org/bugzilla/show_bug.cgi?id=37132 + 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) + continue; + + int equalSign = currentToken.indexOf('='); + if (equalSign < 0) { + return false; + } + String currentTokenName = + currentToken.substring(0, equalSign).trim(); + String currentTokenValue = + currentToken.substring(equalSign + 1).trim(); + if ("username".equals(currentTokenName)) + userName = removeQuotes(currentTokenValue); + if ("realm".equals(currentTokenName)) + realmName = removeQuotes(currentTokenValue, true); + if ("nonce".equals(currentTokenName)) + nonce = removeQuotes(currentTokenValue); + if ("nc".equals(currentTokenName)) + nc = removeQuotes(currentTokenValue); + if ("cnonce".equals(currentTokenName)) + cnonce = removeQuotes(currentTokenValue); + if ("qop".equals(currentTokenName)) + qop = removeQuotes(currentTokenValue); + if ("uri".equals(currentTokenName)) + uri = removeQuotes(currentTokenValue); + if ("response".equals(currentTokenName)) + response = removeQuotes(currentTokenValue); + if ("opaque".equals(currentTokenName)) + opaque = removeQuotes(currentTokenValue); + } + + if ( (userName == null) || (realmName == null) || (nonce == null) + || (uri == null) || (response == null) ) { + return false; + } + + // Validate the URI - should match the request line sent by client + if (validateUri) { + String uriQuery; + String query = request.getQueryString(); + if (query == null) { + uriQuery = request.getRequestURI(); + } else { + uriQuery = request.getRequestURI() + "?" + query; + } + if (!uri.equals(uriQuery)) { + return false; + } + } + + // Validate the Realm name + String lcRealm = config.getRealmName(); + if (lcRealm == null) { + lcRealm = REALM_NAME; + } + if (!lcRealm.equals(realmName)) { + return false; + } + + // Validate the opaque string + if (!this.opaque.equals(opaque)) { + return false; + } + + // Validate nonce + int i = nonce.indexOf(":"); + if (i < 0 || (i + 1) == nonce.length()) { + return false; + } + long nonceTime; + try { + nonceTime = Long.parseLong(nonce.substring(0, i)); + } catch (NumberFormatException nfe) { + return false; + } + String md5clientIpTimeKey = nonce.substring(i + 1); + long currentTime = System.currentTimeMillis(); + if ((currentTime - nonceTime) > nonceValidity) { + nonceStale = true; + return false; + } + String serverIpTimeKey = + request.getRemoteAddr() + ":" + nonceTime + ":" + key; + byte[] buffer = null; + synchronized (md5Helper) { + buffer = md5Helper.digest(serverIpTimeKey.getBytes()); + } + String md5ServerIpTimeKey = md5Encoder.encode(buffer); + if (!md5ServerIpTimeKey.equals(md5clientIpTimeKey)) { + return false; + } + + // Validate qop + if (qop != null && !QOP.equals(qop)) { + return false; + } + + // Validate cnonce and nc + // Check if presence of nc and nonce is consistent with presence of qop + if (qop == null) { + if (cnonce != null || nc != null) { + return false; + } + } else { + if (cnonce == null || nc == null) { + return false; + } + if (nc.length() != 8) { + return false; + } + long count; + try { + count = Long.parseLong(nc, 16); + } catch (NumberFormatException nfe) { + return false; + } + NonceInfo info; + synchronized (cnonces) { + info = cnonces.get(cnonce); + } + if (info == null) { + info = new NonceInfo(); + } else { + if (count <= info.getCount()) { + return false; + } + } + info.setCount(count); + info.setTimestamp(currentTime); + synchronized (cnonces) { + cnonces.put(cnonce, info); + } + } + return true; + } + + public boolean isNonceStale() { + return nonceStale; + } + + public Principal authenticate(Realm realm) { + // Second MD5 digest used to calculate the digest : + // MD5(Method + ":" + uri) + String a2 = method + ":" + uri; + + byte[] buffer; + synchronized (md5Helper) { + buffer = md5Helper.digest(a2.getBytes()); + } + String md5a2 = md5Encoder.encode(buffer); + + return realm.authenticate(userName, response, nonce, nc, cnonce, + qop, realmName, md5a2); + } + + } + + private static class NonceInfo { + private volatile long count; + private volatile long timestamp; + + public void setCount(long l) { + count = l; + } + + public long getCount() { + return count; + } + + public void setTimestamp(long l) { + timestamp = l; + } + + public long getTimestamp() { + return timestamp; + } + } } Modified: branches/2.1.x/java/org/apache/catalina/authenticator/LocalStrings.properties =================================================================== --- branches/2.1.x/java/org/apache/catalina/authenticator/LocalStrings.properties 2011-11-23 09:52:37 UTC (rev 1870) +++ branches/2.1.x/java/org/apache/catalina/authenticator/LocalStrings.properties 2011-11-24 08:23:37 UTC (rev 1871) @@ -12,3 +12,5 @@ authenticator.sessionExpired=The time allowed for the login process has been exceeded. If you wish to continue you must either click back twice and re-click the link you requested or close and re-open your browser authenticator.unauthorized=Cannot authenticate with the provided credentials authenticator.userDataConstraint=This request violates a User Data constraint for this application + +DigestAuthenticator.cacheRemove=A valid entry has been removed from client nonce cache to make room for new entries. A replay attack is now possible. To prevent the possibility of replay attacks, reduce nonceValidity or increase cnonceCacheSize. Further warnings of this type will be suppressed for 5 minutes Modified: branches/2.1.x/java/org/apache/catalina/authenticator/mbeans-descriptors.xml =================================================================== --- branches/2.1.x/java/org/apache/catalina/authenticator/mbeans-descriptors.xml 2011-11-23 09:52:37 UTC (rev 1870) +++ branches/2.1.x/java/org/apache/catalina/authenticator/mbeans-descriptors.xml 2011-11-24 08:23:37 UTC (rev 1871) @@ -44,10 +44,30 @@ description="Fully qualified class name of the managed object" type="java.lang.String" writeable="false"/> - + + <attribute name="cnonceCacheSize" + description="The size of the cnonce cache used to prevent replay attacks" + type="int"/> + <attribute name="entropy" description="A String initialization parameter used to increase the entropy of the initialization of our random number generator" type="java.lang.String"/> + + <attribute name="key" + description="The secret key used by digest authentication" + type="java.lang.String"/> + + <attribute name="nonceValidity" + description="The time, in milliseconds, for which a server issued nonce will be valid" + type="long"/> + + <attribute name="opaque" + description="The opaque server string used by digest authentication" + type="java.lang.String"/> + + <attribute name="validateUri" + description="Should the uri be validated as required by RFC2617?" + type="boolean"/> </mbean> <mbean name="FormAuthenticator" Modified: branches/2.1.x/java/org/apache/catalina/realm/RealmBase.java =================================================================== --- branches/2.1.x/java/org/apache/catalina/realm/RealmBase.java 2011-11-23 09:52:37 UTC (rev 1870) +++ branches/2.1.x/java/org/apache/catalina/realm/RealmBase.java 2011-11-24 08:23:37 UTC (rev 1871) @@ -367,8 +367,13 @@ String md5a1 = getDigest(username, realm); if (md5a1 == null) return null; - String serverDigestValue = md5a1 + ":" + nOnce + ":" + nc + ":" - + cnonce + ":" + qop + ":" + md5a2; + String serverDigestValue; + if (qop == null) { + serverDigestValue = md5a1 + ":" + nOnce + ":" + md5a2; + } else { + serverDigestValue = md5a1 + ":" + nOnce + ":" + nc + ":" + + cnonce + ":" + qop + ":" + md5a2; + } byte[] valueBytes = null; if(getDigestEncoding() == null) { Modified: branches/2.1.x/webapps/docs/changelog.xml =================================================================== --- branches/2.1.x/webapps/docs/changelog.xml 2011-11-23 09:52:37 UTC (rev 1870) +++ branches/2.1.x/webapps/docs/changelog.xml 2011-11-24 08:23:37 UTC (rev 1871) @@ -17,6 +17,13 @@ <body> <section name="JBoss Web 2.1.13.GA (jfclere)"> <subsection name="Coyote"> + <changelog> + <fix> + Improve DIGEST authentication security. (remm) + </fix> + </changelog> + </subsection> + <subsection name="Coyote"> <changelog> <fix> <bug>51698</bug>: Fix CVE-2011-3190. Prevent AJP message injection. (markt)