Parse passwords containing colon correctly (#15109)

apache · Oct 9, 2023 · 90a1458 · 90a1458
1 parent b0edbc3
commit 90a1458
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 50 deletions.
diff --git a/.../src/main/java/org/apache/druid/security/basic/authentication/BasicHTTPAuthenticator.java b/.../src/main/java/org/apache/druid/security/basic/authentication/BasicHTTPAuthenticator.java
@@ -182,15 +182,27 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
         return;
       }
 
-      String[] splits = decodedUserSecret.split(":");
-      if (splits.length != 2) {
+      /* From https://www.rfc-editor.org/rfc/rfc7617.html, we can assume that userid won't include a colon but password
+      can.
+
+       The user-id and password MUST NOT contain any control characters (see
+        "CTL" in Appendix B.1 of [RFC5234]).
+
+       Furthermore, a user-id containing a colon character is invalid, as
+       the first colon in a user-pass string separates user-id and password
+       from one another; text after the first colon is part of the password.
+       User-ids containing colons cannot be encoded in user-pass strings.
+
+       */
+      int split = decodedUserSecret.indexOf(':');
+      if (split < 0) {
         // The decoded user secret is not of the right format
         httpResp.sendError(HttpServletResponse.SC_UNAUTHORIZED);
         return;
       }
 
-      String user = splits[0];
-      char[] password = splits[1].toCharArray();
+      String user = decodedUserSecret.substring(0, split);
+      char[] password = decodedUserSecret.substring(split + 1).toCharArray();
 
       // If any authentication error occurs we send a 401 response immediately and do not proceed further down the filter chain.
       // If the authentication result is null and skipOnFailure property is false, we send a 401 response and do not proceed

diff --git a/...ty/src/test/java/org/apache/druid/security/authentication/BasicHTTPAuthenticatorTest.java b/...ty/src/test/java/org/apache/druid/security/authentication/BasicHTTPAuthenticatorTest.java
@@ -112,55 +112,21 @@ public void testGoodPassword() throws IOException, ServletException
   }
 
   @Test
-  public void testGoodPasswordWithValidator() throws IOException, ServletException
+  public void testGoodNonEmptyPasswordWithValidator() throws IOException, ServletException
   {
-    CredentialsValidator validator = EasyMock.createMock(CredentialsValidator.class);
-    BasicHTTPAuthenticator authenticatorWithValidator = new BasicHTTPAuthenticator(
-        CACHE_MANAGER_PROVIDER,
-        "basic",
-        "basic",
-        null,
-        null,
-        false,
-        null, null,
-        false,
-        validator
-    );
-
-    String header = StringUtils.utf8Base64("userA:helloworld");
-    header = StringUtils.format("Basic %s", header);
-
-    EasyMock
-        .expect(
-            validator.validateCredentials(EasyMock.eq("basic"), EasyMock.eq("basic"), EasyMock.eq("userA"), EasyMock.aryEq("helloworld".toCharArray()))
-        )
-        .andReturn(
-            new AuthenticationResult("userA", "basic", "basic", null)
-        )
-        .times(1);
-    EasyMock.replay(validator);
-
-    HttpServletRequest req = EasyMock.createMock(HttpServletRequest.class);
-    EasyMock.expect(req.getHeader("Authorization")).andReturn(header);
-    req.setAttribute(
-        AuthConfig.DRUID_AUTHENTICATION_RESULT,
-        new AuthenticationResult("userA", "basic", "basic", null)
-    );
-    EasyMock.expectLastCall().times(1);
-    EasyMock.replay(req);
-
-    HttpServletResponse resp = EasyMock.createMock(HttpServletResponse.class);
-    EasyMock.replay(resp);
-
-    FilterChain filterChain = EasyMock.createMock(FilterChain.class);
-    filterChain.doFilter(req, resp);
-    EasyMock.expectLastCall().times(1);
-    EasyMock.replay(filterChain);
+    testGoodPasswordWithValidator("userA", "helloworld");
+  }
 
-    Filter authenticatorFilter = authenticatorWithValidator.getFilter();
-    authenticatorFilter.doFilter(req, resp, filterChain);
+  @Test
+  public void testGoodEmptyPasswordWithValidator() throws IOException, ServletException
+  {
+    testGoodPasswordWithValidator("userA", "");
+  }
 
-    EasyMock.verify(req, resp, validator, filterChain);
+  @Test
+  public void testGoodColonInPasswordWithValidator() throws IOException, ServletException
+  {
+    testGoodPasswordWithValidator("userA", "hello:hello");
   }
 
   @Test
@@ -396,4 +362,55 @@ public void testMissingHeader() throws IOException, ServletException
 
     EasyMock.verify(req, resp, filterChain);
   }
+
+  private void testGoodPasswordWithValidator(String username, String password) throws IOException, ServletException
+  {
+    CredentialsValidator validator = EasyMock.createMock(CredentialsValidator.class);
+    BasicHTTPAuthenticator authenticatorWithValidator = new BasicHTTPAuthenticator(
+            CACHE_MANAGER_PROVIDER,
+            "basic",
+            "basic",
+            null,
+            null,
+            false,
+            null, null,
+            false,
+            validator
+    );
+
+    String header = StringUtils.utf8Base64(username + ":" + password);
+    header = StringUtils.format("Basic %s", header);
+
+    EasyMock
+            .expect(
+                    validator.validateCredentials(EasyMock.eq("basic"), EasyMock.eq("basic"), EasyMock.eq(username), EasyMock.aryEq(password.toCharArray()))
+            )
+            .andReturn(
+                    new AuthenticationResult(username, "basic", "basic", null)
+            )
+            .times(1);
+    EasyMock.replay(validator);
+
+    HttpServletRequest req = EasyMock.createMock(HttpServletRequest.class);
+    EasyMock.expect(req.getHeader("Authorization")).andReturn(header);
+    req.setAttribute(
+            AuthConfig.DRUID_AUTHENTICATION_RESULT,
+            new AuthenticationResult(username, "basic", "basic", null)
+    );
+    EasyMock.expectLastCall().times(1);
+    EasyMock.replay(req);
+
+    HttpServletResponse resp = EasyMock.createMock(HttpServletResponse.class);
+    EasyMock.replay(resp);
+
+    FilterChain filterChain = EasyMock.createMock(FilterChain.class);
+    filterChain.doFilter(req, resp);
+    EasyMock.expectLastCall().times(1);
+    EasyMock.replay(filterChain);
+
+    Filter authenticatorFilter = authenticatorWithValidator.getFilter();
+    authenticatorFilter.doFilter(req, resp, filterChain);
+
+    EasyMock.verify(req, resp, validator, filterChain);
+  }
 }