Author: markt Date: Thu Jun 20 20:36:08 2013 New Revision: 1495169 URL: http://svn.apache.org/r1495169 Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55101 Make BASIC auth parsing more tolerant of whitespace.
Added: tomcat/trunk/test/org/apache/catalina/authenticator/TestBasicAuthParser.java (with props) Modified: tomcat/trunk/java/org/apache/catalina/authenticator/BasicAuthenticator.java tomcat/trunk/test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/catalina/authenticator/BasicAuthenticator.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/authenticator/BasicAuthenticator.java?rev=1495169&r1=1495168&r2=1495169&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/authenticator/BasicAuthenticator.java (original) +++ tomcat/trunk/java/org/apache/catalina/authenticator/BasicAuthenticator.java Thu Jun 20 20:36:08 2013 @@ -44,8 +44,7 @@ import org.apache.tomcat.util.codec.bina * @version $Id$ */ -public class BasicAuthenticator - extends AuthenticatorBase { +public class BasicAuthenticator extends AuthenticatorBase { private static final Log log = LogFactory.getLog(BasicAuthenticator.class); @@ -98,9 +97,6 @@ public class BasicAuthenticator } // Validate any credentials already included with this request - String username = null; - String password = null; - MessageBytes authorization = request.getCoyoteRequest().getMimeHeaders() .getValue("authorization"); @@ -108,44 +104,27 @@ public class BasicAuthenticator if (authorization != null) { authorization.toBytes(); ByteChunk authorizationBC = authorization.getByteChunk(); - if (authorizationBC.startsWithIgnoreCase("basic ", 0)) { - authorizationBC.setOffset(authorizationBC.getOffset() + 6); - - byte[] decoded = Base64.decodeBase64( - authorizationBC.getBuffer(), - authorizationBC.getOffset(), - authorizationBC.getLength()); - - // Get username and password - int colon = -1; - for (int i = 0; i < decoded.length; i++) { - if (decoded[i] == ':') { - colon = i; - break; - } - } - - if (colon < 0) { - username = new String(decoded, B2CConverter.ISO_8859_1); - } else { - username = new String( - decoded, 0, colon, B2CConverter.ISO_8859_1); - password = new String( - decoded, colon + 1, decoded.length - colon - 1, - B2CConverter.ISO_8859_1); + BasicCredentials credentials = null; + try { + credentials = new BasicCredentials(authorizationBC); + String username = credentials.getUsername(); + String password = credentials.getPassword(); + + principal = context.getRealm().authenticate(username, password); + if (principal != null) { + register(request, response, principal, + HttpServletRequest.BASIC_AUTH, username, password); + return (true); } - - authorizationBC.setOffset(authorizationBC.getOffset() - 6); } - - principal = context.getRealm().authenticate(username, password); - if (principal != null) { - register(request, response, principal, - HttpServletRequest.BASIC_AUTH, username, password); - return (true); + catch (IllegalArgumentException iae) { + if (log.isDebugEnabled()) { + log.debug("Invalid Authorization" + iae.getMessage()); + } } } + // the request could not be authenticated, so reissue the challenge StringBuilder value = new StringBuilder(16); value.append("Basic realm=\""); value.append(getRealmName(context)); @@ -156,9 +135,139 @@ public class BasicAuthenticator } - @Override protected String getAuthMethod() { return HttpServletRequest.BASIC_AUTH; } + + + /** + * Parser for an HTTP Authorization header for BASIC authentication + * as per RFC 2617 section 2, and the Base64 encoded credentials as + * per RFC 2045 section 6.8. + */ + protected static class BasicCredentials { + + // the only authentication method supported by this parser + // note: we include single white space as its delimiter + private static final String METHOD = "basic "; + + private ByteChunk authorization; + private int initialOffset; + private int base64blobOffset; + private int base64blobLength; + + private String username = null; + private String password = null; + + /** + * Parse the HTTP Authorization header for BASIC authentication + * as per RFC 2617 section 2, and the Base64 encoded credentials + * as per RFC 2045 section 6.8. + * + * @param input The header value to parse in-place + * + * @throws IllegalArgumentException If the header does not conform + * to RFC 2617 + */ + public BasicCredentials(ByteChunk input) + throws IllegalArgumentException { + authorization = input; + initialOffset = input.getOffset(); + parseMethod(); + byte[] decoded = parseBase64(); + parseCredentials(decoded); + } + + /** + * Trivial accessor. + * + * @return the decoded username token as a String, which is + * never be <code>null</code>, but can be empty. + */ + public String getUsername() { + return username; + } + + /** + * Trivial accessor. + * + * @return the decoded password token as a String, or <code>null</code> + * if no password was found in the credentials. + */ + public String getPassword() { + return password; + } + + /* + * The authorization method string is case-insensitive and must + * hae at least one space character as a delimiter. + */ + private void parseMethod() throws IllegalArgumentException { + if (authorization.startsWithIgnoreCase(METHOD, 0)) { + // step past the auth method name + base64blobOffset = initialOffset + METHOD.length(); + base64blobLength = authorization.getLength() - METHOD.length(); + } + else { + // is this possible, or permitted? + throw new IllegalArgumentException( + "Authorization header method is not \"Basic\""); + } + } + /* + * Decode the base64-user-pass token, which RFC 2617 states + * can be longer than the 76 characters per line limit defined + * in RFC 2045. The base64 decoder will ignore embedded line + * break characters as well as surplus surrounding white space. + */ + private byte[] parseBase64() throws IllegalArgumentException { + byte[] decoded = Base64.decodeBase64( + authorization.getBuffer(), + base64blobOffset, base64blobLength); + // restore original offset + authorization.setOffset(initialOffset); + if (decoded == null) { + throw new IllegalArgumentException( + "Basic Authorization credentials are not Base64"); + } + return decoded; + } + + /* + * Extract the mandatory username token and separate it from the + * optional password token. Tolerate surplus surrounding white space. + */ + private void parseCredentials(byte[] decoded) + throws IllegalArgumentException { + + int colon = -1; + for (int i = 0; i < decoded.length; i++) { + if (decoded[i] == ':') { + colon = i; + break; + } + } + + if (colon < 0) { + username = new String(decoded, B2CConverter.ISO_8859_1); + // password will remain null! + } + else { + username = new String( + decoded, 0, colon, B2CConverter.ISO_8859_1); + password = new String( + decoded, colon + 1, decoded.length - colon - 1, + B2CConverter.ISO_8859_1); + // tolerate surplus white space around credentials + if (password.length() > 1) { + password = password.trim(); + } + } + // tolerate surplus white space around credentials + if (username.length() > 1) { + username = username.trim(); + } + } + } } Added: tomcat/trunk/test/org/apache/catalina/authenticator/TestBasicAuthParser.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/authenticator/TestBasicAuthParser.java?rev=1495169&view=auto ============================================================================== --- tomcat/trunk/test/org/apache/catalina/authenticator/TestBasicAuthParser.java (added) +++ tomcat/trunk/test/org/apache/catalina/authenticator/TestBasicAuthParser.java Thu Jun 20 20:36:08 2013 @@ -0,0 +1,562 @@ +/* + * 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.authenticator; + +import java.io.IOException; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.tomcat.util.buf.B2CConverter; +import org.apache.tomcat.util.buf.ByteChunk; +import org.apache.tomcat.util.codec.binary.Base64; + +/** + * Test the BasicAuthenticator's BasicCredentials inner class and the + * associated Base64 decoder. + */ +public class TestBasicAuthParser { + + private static final String NICE_METHOD = "Basic"; + private static final String USER_NAME = "userid"; + private static final String PASSWORD = "secret"; + + /* + * test cases with good BASIC Auth credentials - Base64 strings + * can have zero, one or two trailing pad characters + */ + @Test + public void testGoodCredentials() throws Exception { + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, USER_NAME, PASSWORD); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD, credentials.getPassword()); + } + + @Test + public void testGoodCredentialsNoPassword() throws Exception { + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, USER_NAME, null); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertNull(credentials.getPassword()); + } + + @Test + public void testGoodCrib() throws Exception { + final String BASE64_CRIB = "dXNlcmlkOnNlY3JldA=="; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, BASE64_CRIB); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD, credentials.getPassword()); + } + + @Test + public void testGoodCribUserOnly() throws Exception { + final String BASE64_CRIB = "dXNlcmlk"; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, BASE64_CRIB); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertNull(credentials.getPassword()); + } + + @Test + public void testGoodCribOnePad() throws Exception { + final String PASSWORD1 = "secrets"; + final String BASE64_CRIB = "dXNlcmlkOnNlY3JldHM="; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, BASE64_CRIB); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD1, credentials.getPassword()); + } + + /* + * RFC 2045 says the Base64 encoded string should be represented + * as lines of no more than 76 characters. However, RFC 2617 + * says a base64-user-pass token is not limited to 76 char/line. + * It also says all line breaks, including mandatory ones, + * should be ignored during decoding. + * This test case has a line break in the Base64 string. + * (See also testGoodCribBase64Big below). + */ + @Test + public void testGoodCribLineWrap() throws Exception { + final String USER_LONG = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + + "abcdefghijklmnopqrstuvwxyz0123456789+/AAAABBBBCCCC" + + "DDDD"; // 80 characters + final String BASE64_CRIB = "QUJDREVGR0hJSktMTU5PUFFSU1RVVldY" + + "WVphYmNkZWZnaGlqa2xtbm9wcXJzdHV2d3h5ejAxMjM0" + + "\n" + "NTY3ODkrL0FBQUFCQkJCQ0NDQ0REREQ="; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, BASE64_CRIB); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_LONG, credentials.getUsername()); + } + + /* + * RFC 2045 says the Base64 encoded string should be represented + * as lines of no more than 76 characters. However, RFC 2617 + * says a base64-user-pass token is not limited to 76 char/line. + */ + @Test + public void testGoodCribBase64Big() throws Exception { + // Our decoder accepts a long token without complaint. + final String USER_LONG = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + + "abcdefghijklmnopqrstuvwxyz0123456789+/AAAABBBBCCCC" + + "DDDD"; // 80 characters + final String BASE64_CRIB = "QUJDREVGR0hJSktMTU5PUFFSU1RVVldY" + + "WVphYmNkZWZnaGlqa2xtbm9wcXJzdHV2d3h5ejAxMjM0" + + "NTY3ODkrL0FBQUFCQkJCQ0NDQ0REREQ="; // no new line + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, BASE64_CRIB); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_LONG, credentials.getUsername()); + } + + + /* + * verify the parser follows RFC2617 by treating the auth-scheme + * token as case-insensitive. + */ + @Test + public void testAuthMethodCaseBasic() throws Exception { + final String METHOD = "bAsIc"; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(METHOD, USER_NAME, PASSWORD); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD, credentials.getPassword()); + } + + /* + * Confirm the Basic parser rejects an invalid authentication method. + */ + @Test + public void testAuthMethodBadMethod() throws Exception { + final String METHOD = "BadMethod"; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(METHOD, USER_NAME, PASSWORD); + @SuppressWarnings("unused") + BasicAuthenticator.BasicCredentials credentials = null; + try { + credentials = new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.fail("IllegalArgumentException expected"); + } + catch (Exception e) { + Assert.assertTrue(e instanceof IllegalArgumentException); + Assert.assertTrue(e.getMessage().contains("header method")); + } + } + + /* + * Confirm the Basic parser tolerates excess white space after + * the authentication method. + * + * RFC2617 does not define the separation syntax between the auth-scheme + * and basic-credentials tokens. Tomcat tolerates any amount of white + * (within the limits of HTTP header sizes). + */ + @Test + public void testAuthMethodExtraLeadingSpace() throws Exception { + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD + " ", USER_NAME, PASSWORD); + final BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD, credentials.getPassword()); + } + + + /* + * invalid decoded credentials cases + */ + @Test + public void testWrongPassword() throws Exception { + final String PWD_WRONG = "wrong"; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, USER_NAME, PWD_WRONG); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertNotSame(PASSWORD, credentials.getPassword()); + } + + @Test + public void testMissingUsername() throws Exception { + final String EMPTY_USER_NAME = ""; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, EMPTY_USER_NAME, PASSWORD); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(EMPTY_USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD, credentials.getPassword()); + } + + @Test + public void testShortUsername() throws Exception { + final String SHORT_USER_NAME = "a"; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, SHORT_USER_NAME, PASSWORD); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(SHORT_USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD, credentials.getPassword()); + } + + @Test + public void testShortPassword() throws Exception { + final String SHORT_PASSWORD = "a"; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, USER_NAME, SHORT_PASSWORD); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(SHORT_PASSWORD, credentials.getPassword()); + } + + @Test + public void testPasswordHasSpaceEmbedded() throws Exception { + final String PASSWORD_SPACE = "abc def"; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, USER_NAME, PASSWORD_SPACE); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD_SPACE, credentials.getPassword()); + } + + @Test + public void testPasswordHasColonEmbedded() throws Exception { + final String PASSWORD_COLON = "abc:def"; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, USER_NAME, PASSWORD_COLON); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD_COLON, credentials.getPassword()); + } + + @Test + public void testPasswordHasColonLeading() throws Exception { + final String PASSWORD_COLON = ":abcdef"; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, USER_NAME, PASSWORD_COLON); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD_COLON, credentials.getPassword()); + } + + @Test + public void testPasswordHasColonTrailing() throws Exception { + final String PASSWORD_COLON = "abcdef:"; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, USER_NAME, PASSWORD_COLON); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD_COLON, credentials.getPassword()); + } + + /* + * Confirm the Basic parser tolerates excess white space after + * the base64 blob. + * + * RFC2617 does not define this case, but asks servers to be + * tolerant of this kind of client deviation. + */ + @Test + public void testAuthMethodExtraTrailingSpace() throws Exception { + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, USER_NAME, PASSWORD, " "); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD, credentials.getPassword()); + } + + /* + * Confirm the Basic parser tolerates excess white space around + * the username inside the base64 blob. + * + * RFC2617 does not define the separation syntax between the auth-scheme + * and basic-credentials tokens. Tomcat should tolerate any reasonable + * amount of white space. + */ + @Test + public void testUserExtraSpace() throws Exception { + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, " " + USER_NAME + " ", PASSWORD); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD, credentials.getPassword()); + } + + /* + * Confirm the Basic parser tolerates excess white space around + * the username within the base64 blob. + * + * RFC2617 does not define the separation syntax between the auth-scheme + * and basic-credentials tokens. Tomcat should tolerate any reasonable + * amount of white space. + */ + @Test + public void testPasswordExtraSpace() throws Exception { + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, USER_NAME, " " + PASSWORD + " "); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD, credentials.getPassword()); + } + + + /* + * invalid base64 string tests + * + * Refer to RFC2045 section 6.8. + */ + + /* + * non-trailing "=" should trigger premature termination of the + * decoder, returning a truncated string that will eventually + * result in an authentication Assert.failure. + */ + @Test + public void testBadBase64InlineEquals() throws Exception { + final String BASE64_CRIB = "dXNlcmlkOnNlY3J=dAo="; + final String TRUNCATED_PWD = "secr"; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, BASE64_CRIB); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertNotSame(PASSWORD, credentials.getPassword()); + Assert.assertEquals(TRUNCATED_PWD, credentials.getPassword()); + } + + /* + * "-" is not a legal base64 character. The RFC says it must be + * ignored by the decoder. This will scramble the decoded string + * and eventually result in an authentication Assert.failure. + */ + @Test + public void testBadBase64Char() throws Exception { + final String BASE64_CRIB = "dXNlcmlkOnNl-3JldHM="; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, BASE64_CRIB); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertNotSame(PASSWORD, credentials.getPassword()); + } + + /* + * "-" is not a legal base64 character. The RFC says it must be + * ignored by the decoder. This is a very strange case because the + * next character is a pad, which terminates the string normally. + * It is likely (but not certain) the decoded password will be + * damaged and subsequent authentication will fail. + */ + @Test + public void testBadBase64LastChar() throws Exception { + final String BASE64_CRIB = "dXNlcmlkOnNlY3JldA-="; + final String POSSIBLY_DAMAGED_PWD = "secret"; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, BASE64_CRIB); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(POSSIBLY_DAMAGED_PWD, credentials.getPassword()); + } + + /* + * The trailing third "=" is illegal. However, the RFC says the decoder + * must terminate as soon as the first pad is detected, so no error + * will be detected unless the payload has been damaged in some way. + */ + @Test + public void testBadBase64TooManyEquals() throws Exception { + final String BASE64_CRIB = "dXNlcmlkOnNlY3JldA==="; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, BASE64_CRIB); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD, credentials.getPassword()); + } + + /* + * there should be a multiple of 4 encoded characters. However, + * the RFC says the decoder should pad the input string with + * zero bits out to the next boundary. An error will not be detected + * unless the payload has been damaged in some way - this + * particular crib has no damage. + */ + @Test + public void testBadBase64BadLength() throws Exception { + final String BASE64_CRIB = "dXNlcmlkOnNlY3JldA"; + final BasicAuthHeader AUTH_HEADER = + new BasicAuthHeader(NICE_METHOD, BASE64_CRIB); + BasicAuthenticator.BasicCredentials credentials = + new BasicAuthenticator.BasicCredentials( + AUTH_HEADER.getHeader()); + Assert.assertEquals(USER_NAME, credentials.getUsername()); + Assert.assertEquals(PASSWORD, credentials.getPassword()); + } + + + /* + * Encapsulate the logic to generate an HTTP header + * for BASIC Authentication. + * Note: only used internally, so no need to validate arguments. + */ + private final class BasicAuthHeader { + + private final String HTTP_AUTH = "authorization: "; + private final byte[] HEADER = + HTTP_AUTH.getBytes(B2CConverter.ISO_8859_1); + private ByteChunk authHeader; + private int initialOffset = 0; + + /* + * This method creates a valid base64 blob + */ + private BasicAuthHeader(String method, String username, + String password) { + this(method, username, password, null); + } + + /* + * This method creates valid base64 blobs with optional trailing data + */ + private BasicAuthHeader(String method, String username, + String password, String extraBlob) { + prefix(method); + + String userCredentials = + ((password == null) || (password.length() < 1)) + ? username + : username + ":" + password; + byte[] credentialsBytes = + userCredentials.getBytes(B2CConverter.ISO_8859_1); + String base64auth = Base64.encodeBase64String(credentialsBytes); + byte[] base64Bytes = + base64auth.getBytes(B2CConverter.ISO_8859_1); + + byte[] extraBytes = + ((extraBlob == null) || (extraBlob.length() < 1)) + ? null : + extraBlob.getBytes(B2CConverter.ISO_8859_1); + + try { + authHeader.append(base64Bytes, 0, base64Bytes.length); + if (extraBytes != null) { + authHeader.append(extraBytes, 0, extraBytes.length); + } + } + catch (IOException ioe) { + throw new IllegalStateException("unable to extend ByteChunk:" + + ioe.getMessage()); + } + // emulate tomcat server - offset points to method in header + authHeader.setOffset(initialOffset); + } + + /* + * This method allows injection of cribbed base64 blobs, + * without any validation of the contents + */ + private BasicAuthHeader(String method, String fakeBase64) { + prefix(method); + + byte[] fakeBytes = fakeBase64.getBytes(B2CConverter.ISO_8859_1); + + try { + authHeader.append(fakeBytes, 0, fakeBytes.length); + } + catch (IOException ioe) { + throw new IllegalStateException("unable to extend ByteChunk:" + + ioe.getMessage()); + } + // emulate tomcat server - offset points to method in header + authHeader.setOffset(initialOffset); + } + + /* + * construct the common authorization header + */ + private void prefix(String method) { + authHeader = new ByteChunk(); + authHeader.setBytes(HEADER, 0, HEADER.length); + initialOffset = HEADER.length; + + String methodX = method + " "; + byte[] methodBytes = methodX.getBytes(B2CConverter.ISO_8859_1); + + try { + authHeader.append(methodBytes, 0, methodBytes.length); + } + catch (IOException ioe) { + throw new IllegalStateException("unable to extend ByteChunk:" + + ioe.getMessage()); + } + } + + private ByteChunk getHeader() { + return authHeader; + } + } +} Propchange: tomcat/trunk/test/org/apache/catalina/authenticator/TestBasicAuthParser.java ------------------------------------------------------------------------------ svn:eol-style = native Modified: tomcat/trunk/test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java?rev=1495169&r1=1495168&r2=1495169&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java (original) +++ tomcat/trunk/test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java Thu Jun 20 20:36:08 2013 @@ -216,38 +216,28 @@ public class TestNonLoginAndBasicAuthent /* * This is the same as testAcceptProtectedBasic (above), except - * using white space around the username credential. - * - * The request is rejected with 401 SC_UNAUTHORIZED status. - * - * TODO: RFC2617 does not define the separation syntax between the - * auth-scheme and basic-credentials tokens. Tomcat should tolerate - * any reasonable amount of white space and return SC_OK. + * using white space around the username credential. The request + * is accepted. */ @Test public void testUserExtraSpace() throws Exception { doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS, NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED); doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, SPACED_USERNAME, - NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED); + NO_COOKIES, HttpServletResponse.SC_OK); } /* * This is the same as testAcceptProtectedBasic (above), except - * using white space around the password credential. - * - * The request is rejected with 401 SC_UNAUTHORIZED status. - * - * TODO: RFC2617 does not define the separation syntax between the - * auth-scheme and basic-credentials tokens. Tomcat should tolerate - * any reasonable amount of white space and return SC_OK. + * using white space around the password credential. The request + * is accepted. */ @Test public void testPasswordExtraSpace() throws Exception { doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS, NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED); doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, SPACED_PASSWORD, - NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED); + NO_COOKIES, HttpServletResponse.SC_OK); } /* Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1495169&r1=1495168&r2=1495169&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Thu Jun 20 20:36:08 2013 @@ -129,6 +129,10 @@ Change default configuration so that a change to the global web.xml file will trigger a reload of all web applications. (markt) </add> + <fix> + <bug>55101</bug>: Make BASIC authentication more tolerant of whitespace. + Patch provided by Brian Burch. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org