You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2013/06/20 22:36:08 UTC

svn commit: r1495169 - in /tomcat/trunk: java/org/apache/catalina/authenticator/ test/org/apache/catalina/authenticator/ webapps/docs/

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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org