Reject BASIC authorization header with null password

RFC 7617 mandates that a ':' must be present.
As there does not seem to be any adverse effects, I will leave it as
Tomcat 12 only.

Author: rmaucher

java/org/apache/catalina/authenticator/BasicAuthenticator.java

@@ -188,8 +188,7 @@ public String getUsername() {
         /**
          * Trivial accessor.
          *
-         * @return the decoded password token as a String, or <code>null</code> if no password was found in the
-         *             credentials.
+         * @return the decoded password token as a String, which is never <code>null</code>, but can be empty.
          */
         public String getPassword() {
             return password;
@@ -228,7 +227,7 @@ private byte[] parseBase64() throws IllegalArgumentException {
         }
 
         /*
-         * Extract the mandatory username token and separate it from the optional password token. Tolerate surplus
+         * Extract the mandatory username and password tokens separated by a colon. Tolerate surplus
          * surrounding white space.
          */
         private void parseCredentials(byte[] decoded) throws IllegalArgumentException {
@@ -241,10 +240,9 @@ private void parseCredentials(byte[] decoded) throws IllegalArgumentException {
                 }
             }
 
-            // Tomcat allows a null password
+            // Null password is not allowed according to RFC 7617
             if (colon < 0) {
-                username = new String(decoded, charset);
-                // password will remain null!
+                throw new IllegalArgumentException(sm.getString("basicAuthenticator.noColon"));
             } else {
                 username = new String(decoded, 0, colon, charset);
                 password = new String(decoded, colon + 1, decoded.length - colon - 1, charset);

java/org/apache/catalina/authenticator/LocalStrings.properties

@@ -41,6 +41,7 @@ authenticator.userPermissionFail=User [{0}] does not have authorization to acces
 
 basicAuthenticator.invalidAuthorization=Invalid Authorization header
 basicAuthenticator.invalidCharset=The only permitted values are null, the empty string or UTF-8
+basicAuthenticator.noColon=Basic Authorization credentials do not contain a colon
 basicAuthenticator.notBase64=Basic Authorization credentials are not Base64
 basicAuthenticator.notBasic=Authorization header method is not ''Basic''
 

test/org/apache/catalina/authenticator/TestBasicAuthParser.java

@@ -46,13 +46,12 @@ public void testGoodCredentials() throws Exception {
         Assert.assertEquals(PASSWORD, credentials.getPassword());
     }
 
-    @Test
-    public void testGoodCredentialsNoPassword() throws Exception {
+    @Test(expected = IllegalArgumentException.class)
+    public void testBadCredentialsNoPassword() throws Exception {
         final BasicAuthHeader AUTH_HEADER = new BasicAuthHeader(NICE_METHOD, USER_NAME, null);
+        @SuppressWarnings("unused")
         BasicAuthenticator.BasicCredentials credentials =
                 new BasicAuthenticator.BasicCredentials(AUTH_HEADER.getHeader(), StandardCharsets.UTF_8);
-        Assert.assertEquals(USER_NAME, credentials.getUsername());
-        Assert.assertNull(credentials.getPassword());
     }
 
     @Test
@@ -65,14 +64,13 @@ public void testGoodCrib() throws Exception {
         Assert.assertEquals(PASSWORD, credentials.getPassword());
     }
 
-    @Test
-    public void testGoodCribUserOnly() throws Exception {
+    @Test(expected = IllegalArgumentException.class)
+    public void testBadCribUserOnly() throws Exception {
         final String BASE64_CRIB = "dXNlcmlk";
         final BasicAuthHeader AUTH_HEADER = new BasicAuthHeader(NICE_METHOD, BASE64_CRIB);
+        @SuppressWarnings("unused")
         BasicAuthenticator.BasicCredentials credentials =
                 new BasicAuthenticator.BasicCredentials(AUTH_HEADER.getHeader(), StandardCharsets.UTF_8);
-        Assert.assertEquals(USER_NAME, credentials.getUsername());
-        Assert.assertNull(credentials.getPassword());
     }
 
     @Test
@@ -108,8 +106,8 @@ public void testGoodCribBase64Big() throws Exception {
         // Our decoder accepts a long token without complaint.
         // 80 characters
         final String USER_LONG = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/AAAABBBBCCCCDDDD";
-        final String BASE64_CRIB = "QUJDREVGR0hJSktMTU5PUFFSU1RVVldYWVphYmNkZWZnaGlqa2xtbm9wcXJzdHV2d3h5ejAxMjM0" +
-                "NTY3ODkrL0FBQUFCQkJCQ0NDQ0REREQ="; // no new line
+        final String BASE64_CRIB = "QUJDREVGR0hJSktMTU5PUFFSU1RVVldYWVphYmNkZWZnaGlqa2xtbm9wcXJzdHV2d3h5e" +
+                "jAxMjM0NTY3ODkrL0FBQUFCQkJCQ0NDQ0REREQ6"; // no new line
         final BasicAuthHeader AUTH_HEADER = new BasicAuthHeader(NICE_METHOD, BASE64_CRIB);
         BasicAuthenticator.BasicCredentials credentials =
                 new BasicAuthenticator.BasicCredentials(AUTH_HEADER.getHeader(), StandardCharsets.UTF_8);

webapps/docs/changelog.xml

@@ -217,6 +217,10 @@
         implementations), along with version compatibility warnings and
         third-party library version information. (csutherl)
       </add>
+      <fix>
+        Reject BASIC authorization with no password, to comply with RFC 7617
+        strictly. (remm)
+      </fix>
       <!-- Entries for backport and removal before 12.0.0-M1 below this line -->
       <fix>
         Avoid a race condition with concurrent lookups for a singleton JNDI