This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/dev/pr-12595-7e0ae7d512b1eedfdd21d0e2a1d9b6e59248345d in repository https://gitbox.apache.org/repos/asf/seatunnel.git
commit 0600214baacb1b6da12b14b593f0abf9f9f0f7c2 Author: Sepuri Sai Krishna <[email protected]> AuthorDate: Sun Oct 18 13:59:41 2026 +0000 [Fix][Zeta] Compare basic auth credentials without a timing discrepancy (#12595) --- .../engine/e2e/BasicAuthenticationIT.java | 35 ++++ .../engine/server/rest/filter/BasicAuthFilter.java | 53 ++++- .../server/rest/filter/BasicAuthFilterTest.java | 216 +++++++++++++++++++++ 3 files changed, 301 insertions(+), 3 deletions(-) diff --git a/seatunnel-e2e/seatunnel-engine-e2e/connector-seatunnel-e2e-base/src/test/java/org/apache/seatunnel/engine/e2e/BasicAuthenticationIT.java b/seatunnel-e2e/seatunnel-engine-e2e/connector-seatunnel-e2e-base/src/test/java/org/apache/seatunnel/engine/e2e/BasicAuthenticationIT.java index 4c0402f5cb..e0636492ad 100644 --- a/seatunnel-e2e/seatunnel-engine-e2e/connector-seatunnel-e2e-base/src/test/java/org/apache/seatunnel/engine/e2e/BasicAuthenticationIT.java +++ b/seatunnel-e2e/seatunnel-engine-e2e/connector-seatunnel-e2e-base/src/test/java/org/apache/seatunnel/engine/e2e/BasicAuthenticationIT.java @@ -110,6 +110,41 @@ public class BasicAuthenticationIT extends SeaTunnelEngineContainer { .statusCode(401); } + /** + * Test that a correct username paired with a wrong password returns 401 Unauthorized. The + * existing incorrect-credentials case gets both halves wrong, so it cannot tell whether the + * password is checked at all once the username fails to match. + */ + @Test + public void testAccessWithCorrectUsernameAndWrongPassword() { + String credentials = USERNAME + COLON + "wrongpassword"; + String encodedCredentials = Base64.getEncoder().encodeToString(credentials.getBytes()); + + given().header(BASIC_AUTH_HEADER, BASIC_AUTH_PREFIX + encodedCredentials) + .get(HTTP + server.getHost() + COLON + server.getMappedPort(8080) + "/") + .then() + .statusCode(401); + } + + /** + * Test that credentials which are a proper prefix of the configured ones return 401 + * Unauthorized. A prefix is the shape a credential-guessing probe walks through one character + * at a time, so it is worth pinning that the matching leading characters buy nothing. + */ + @Test + public void testAccessWithPrefixOfCorrectCredentials() { + String credentials = + USERNAME.substring(0, USERNAME.length() - 1) + + COLON + + PASSWORD.substring(0, PASSWORD.length() - 1); + String encodedCredentials = Base64.getEncoder().encodeToString(credentials.getBytes()); + + given().header(BASIC_AUTH_HEADER, BASIC_AUTH_PREFIX + encodedCredentials) + .get(HTTP + server.getHost() + COLON + server.getMappedPort(8080) + "/") + .then() + .statusCode(401); + } + /** Test that accessing the web UI with correct credentials returns 200 OK. */ @Test public void testAccessWithCorrectCredentials() { diff --git a/seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/rest/filter/BasicAuthFilter.java b/seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/rest/filter/BasicAuthFilter.java index 373b41babb..824305b984 100644 --- a/seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/rest/filter/BasicAuthFilter.java +++ b/seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/rest/filter/BasicAuthFilter.java @@ -34,6 +34,7 @@ import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; /** Basic authentication filter for the web UI. */ @Slf4j @@ -83,9 +84,16 @@ public class BasicAuthFilter implements Filter { String username = values[0]; String password = values[1]; - // Check if the username and password match the configured values - if (username.equals(httpConfig.getBasicAuthUsername()) - && password.equals(httpConfig.getBasicAuthPassword())) { + // Both comparisons are evaluated before the branch, on purpose. String.equals + // returns at the first differing character, and && skips the password + // comparison entirely when the username is wrong, so response time separates a + // wrong username from a correct username with a wrong password, and grows with + // the number of leading characters guessed correctly (CWE-208). + boolean usernameMatches = + credentialMatches(username, httpConfig.getBasicAuthUsername()); + boolean passwordMatches = + credentialMatches(password, httpConfig.getBasicAuthPassword()); + if (usernameMatches && passwordMatches) { // Authentication successful, proceed with the request chain.doFilter(request, response); return; @@ -98,6 +106,45 @@ public class BasicAuthFilter implements Filter { httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized"); } + /** + * Compares a credential taken from the request against the configured one without leaking, + * through timing, how many leading bytes matched. + * + * <p>MessageDigest.isEqual accumulates the difference over every byte instead of returning at + * the first mismatch, so the work it does is independent of how many leading bytes agree. Where + * it enters that loop it reads exactly as many bytes as its first argument holds, so the + * request's own value is passed first: the loop length is then something the caller already + * knows. + * + * <p>How much the length itself is hidden depends on the JDK, so this claims no more than was + * measured. On the builds disassembled with {@code javap -c}, 11.0.32, 17.0.20 and 21.0.12, the + * length difference is folded into the accumulator and the loop runs for the first argument's + * length, with no early return when the lengths differ. On the Java 8 builds checked, 8u71 and + * 8u171, {@code isEqual} instead compares the two array lengths with {@code if_icmpeq} and + * returns false immediately when they differ, so there the equality of the byte lengths can + * still be inferred, though the leading bytes cannot. SeaTunnel ships for Java 8, so do not + * rely on the length being hidden. No boundary update is named on purpose: only the builds + * listed were disassembled, and which 8u or 11.x update first changed shape was not + * established. + * + * <p>One further early exit, seen in all three builds disassembled here: when the configured + * value is empty, {@code isEqual} returns whether the provided value is empty too without + * entering the loop. The 8u builds were not checked for this case. It only arises from an empty + * configured credential. + * + * @param provided the value supplied by the request, never null at the call sites below + * @param expected the configured value, which is null when the credential is not set + * @return true only when the credential is configured and the two values are equal + */ + private static boolean credentialMatches(String provided, String expected) { + if (expected == null) { + return false; + } + return MessageDigest.isEqual( + provided.getBytes(StandardCharsets.UTF_8), + expected.getBytes(StandardCharsets.UTF_8)); + } + @Override public void destroy() { // No resources to release diff --git a/seatunnel-engine/seatunnel-engine-server/src/test/java/org/apache/seatunnel/engine/server/rest/filter/BasicAuthFilterTest.java b/seatunnel-engine/seatunnel-engine-server/src/test/java/org/apache/seatunnel/engine/server/rest/filter/BasicAuthFilterTest.java new file mode 100644 index 0000000000..567d357dbc --- /dev/null +++ b/seatunnel-engine/seatunnel-engine-server/src/test/java/org/apache/seatunnel/engine/server/rest/filter/BasicAuthFilterTest.java @@ -0,0 +1,216 @@ +/* + * 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.seatunnel.engine.server.rest.filter; + +import org.apache.seatunnel.engine.common.config.server.HttpConfig; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import javax.servlet.FilterChain; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Covers which credentials the basic auth filter accepts, and that a credential it cannot compare + * is refused rather than throwing. + */ +public class BasicAuthFilterTest { + + private static final String USER = "admin"; + private static final String PASSWORD = "s3cret"; + + @Test + void testCorrectCredentialsPassThroughTheChain() throws Exception { + AtomicInteger chainCalls = new AtomicInteger(); + HttpServletResponse response = mock(HttpServletResponse.class); + + doFilterWith(config(USER, PASSWORD), USER, PASSWORD, response, chainCalls); + + Assertions.assertEquals(1, chainCalls.get()); + verify(response, never()).sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized"); + } + + @Test + void testWrongPasswordIsRejected() throws Exception { + AtomicInteger chainCalls = new AtomicInteger(); + HttpServletResponse response = mock(HttpServletResponse.class); + + doFilterWith(config(USER, PASSWORD), USER, "wrong", response, chainCalls); + + Assertions.assertEquals(0, chainCalls.get()); + verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized"); + } + + @Test + void testWrongUsernameIsRejected() throws Exception { + AtomicInteger chainCalls = new AtomicInteger(); + HttpServletResponse response = mock(HttpServletResponse.class); + + doFilterWith(config(USER, PASSWORD), "root", PASSWORD, response, chainCalls); + + Assertions.assertEquals(0, chainCalls.get()); + verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized"); + } + + @Test + void testCredentialThatIsAPrefixOfTheConfiguredOneIsRejected() throws Exception { + // A prefix is the shape a timing probe walks through one character at a time, so pin + // that it is refused rather than accepted on the characters that do match. + AtomicInteger chainCalls = new AtomicInteger(); + HttpServletResponse response = mock(HttpServletResponse.class); + + doFilterWith(config(USER, PASSWORD), USER, "s3cre", response, chainCalls); + + Assertions.assertEquals(0, chainCalls.get()); + verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized"); + } + + @Test + void testUnsetCredentialIsRejectedAndDoesNotThrow() throws Exception { + // The configured values are compared as bytes now, so an unset credential must be + // refused explicitly. Reading bytes off a null would turn a 401 into a 500. + for (HttpConfig config : + new HttpConfig[] {config(null, PASSWORD), config(USER, null), config(null, null)}) { + AtomicInteger chainCalls = new AtomicInteger(); + HttpServletResponse response = mock(HttpServletResponse.class); + + Assertions.assertDoesNotThrow( + () -> doFilterWith(config, USER, PASSWORD, response, chainCalls)); + + Assertions.assertEquals(0, chainCalls.get()); + verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized"); + } + } + + @Test + void testAuthenticationDisabledSkipsTheCredentialCheckEntirely() throws Exception { + HttpConfig config = new HttpConfig(); + config.setEnableBasicAuth(false); + AtomicInteger chainCalls = new AtomicInteger(); + HttpServletResponse response = mock(HttpServletResponse.class); + + doFilterWith(config, "anyone", "anything", response, chainCalls); + + Assertions.assertEquals(1, chainCalls.get()); + verify(response, never()).sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized"); + } + + @Test + void testEmptyConfiguredCredentialMatchesOnlyAnEmptyProvidedValue() throws Exception { + // An unset credential is refused by an explicit null guard, but an empty one is not the + // same thing: two zero-length byte arrays compare equal, so a deployment that configures + // an empty password accepts an empty one. That is unchanged from String.equals and is + // pinned here because it is surprising enough to be worth stating rather than + // rediscovering. + AtomicInteger emptyAccepted = new AtomicInteger(); + HttpServletResponse acceptResponse = mock(HttpServletResponse.class); + + doFilterWith(config("", ""), "", "", acceptResponse, emptyAccepted); + + Assertions.assertEquals( + 1, emptyAccepted.get(), "an empty credential pair must still match"); + verify(acceptResponse, never()) + .sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized"); + + // Empty on one side only must be refused, in both directions. + AtomicInteger emptyAgainstConfigured = new AtomicInteger(); + HttpServletResponse emptyResponse = mock(HttpServletResponse.class); + + doFilterWith(config(USER, PASSWORD), "", "", emptyResponse, emptyAgainstConfigured); + + Assertions.assertEquals( + 0, emptyAgainstConfigured.get(), "an empty credential must not match a set one"); + verify(emptyResponse).sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized"); + + AtomicInteger configuredAgainstEmpty = new AtomicInteger(); + HttpServletResponse setResponse = mock(HttpServletResponse.class); + + doFilterWith(config("", ""), USER, PASSWORD, setResponse, configuredAgainstEmpty); + + Assertions.assertEquals( + 0, configuredAgainstEmpty.get(), "a set credential must not match an empty one"); + verify(setResponse).sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized"); + } + + @Test + void testMultiByteCredentialIsComparedByItsUtf8Bytes() throws Exception { + // The comparison moved from String.equals to a byte comparison, so the encoding now + // matters. The filter decodes the header with StandardCharsets.UTF_8, and these pin that + // a multi-byte credential both matches itself and is still distinguished from a near + // miss, whether or not the near miss has the same byte length. + String multiByte = "pässwörd"; + AtomicInteger accepted = new AtomicInteger(); + HttpServletResponse acceptResponse = mock(HttpServletResponse.class); + + doFilterWith(config(USER, multiByte), USER, multiByte, acceptResponse, accepted); + + Assertions.assertEquals(1, accepted.get(), "a multi-byte credential must match itself"); + verify(acceptResponse, never()) + .sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized"); + + // "pässwürd" differs by one multi-byte character and so has the same UTF-8 length as the + // configured value; "passwörd" substitutes ASCII and is a byte shorter. Both must fail. + for (String nearMiss : new String[] {"pässwürd", "passwörd"}) { + AtomicInteger chainCalls = new AtomicInteger(); + HttpServletResponse response = mock(HttpServletResponse.class); + + doFilterWith(config(USER, multiByte), USER, nearMiss, response, chainCalls); + + Assertions.assertEquals(0, chainCalls.get(), nearMiss + " must be rejected"); + verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized"); + } + } + + private static HttpConfig config(String username, String password) { + HttpConfig config = new HttpConfig(); + config.setEnableBasicAuth(true); + config.setBasicAuthUsername(username); + config.setBasicAuthPassword(password); + return config; + } + + private static void doFilterWith( + HttpConfig config, + String username, + String password, + HttpServletResponse response, + AtomicInteger chainCalls) + throws Exception { + String header = + "Basic " + + Base64.getEncoder() + .encodeToString( + (username + ":" + password) + .getBytes(StandardCharsets.UTF_8)); + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getHeader("Authorization")).thenReturn(header); + + FilterChain chain = (req, resp) -> chainCalls.incrementAndGet(); + new BasicAuthFilter(config).doFilter(request, response, chain); + } +}
