postgresql-jdbc.git / summary / log / commit / refs

commit    95aa87fb8cb9fd1c8a58b60cfbc0e738b8da17bb
Author:   Dave Cramer <davecramer@gmail.com>
Commit:   Sehrope Sarkuni <sehrope@jackdb.com>
Date:     Wed Sep 30 10:51:48 2026 +0000

    fix: refuse every method when requireAuth excludes them all
    
    When requireAuth excluded every authentication method, for example
    !password,!md5,!gss,!sspi,!scram-sha-256,!none, parseRequireAuth
    returned null. checkAuth allows every method for null, so the driver
    used any method the server asked for, even a cleartext password. This
    affects 42.7.11 through 42.7.13. parseRequireAuth now returns the empty
    set, and checkAuth refuses every method with SQLState 08004.
    
    A value without a method in it, such as "," or ",,", also parsed to
    null, because Java's split gives an empty array for it. It now fails
    with SQLState 22023 as an invalid value. Such a value names neither
    methods to allow nor methods to exclude, so it is a mistake, not a
    request to refuse every method. Commas at the very end of a value are
    still ignored, as in "password," or "password,,".
    
    The Javadoc of parseRequireAuth and checkAuth says that checkAuth
    allows every method for null and that an empty set refuses every
    method.
    
    AuthMethodTest covers the parse and checkAuth for each method in the
    enum. RequireAuthConnectTest connects through a socket factory in the
    test. Its sockets read a fixed server reply from memory and record what
    the driver writes. The test checks that the connection fails with 08004
    when the server asks for a cleartext password, and that the driver then
    writes zero bytes after its startup message. It also checks that trust
    fails with 08004. A third test allows the password and checks that the
    driver sends the PasswordMessage, which shows that the recording works.
    With the unfixed AuthMethod, 5 of the 38 tests fail, including both
    refusal tests in RequireAuthConnectTest.
    
    This is the release/42.7.x commit applied to master. The code and the
    tests are the same on both branches.
    
    GHSA-rhp9-mr79-r74h


CHANGELOG.md | 4 + .../main/java/org/postgresql/core/AuthMethod.java | 42 ++++- .../java/org/postgresql/core/AuthMethodTest.java | 167 +++++++++++++++++ .../postgresql/core/RequireAuthConnectTest.java | 206 +++++++++++++++++++++ .../org/postgresql/core/RequireAuthValues.java | 37 ++++ 5 files changed, 452 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7c7a5d5..3e3a91ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). * fix: the read buffer returns to its initial size once the message that grew it is drained, so a connection no longer retains 32 MiB after one large error [Issue #4015](https://github.com/pgjdbc/pgjdbc/issues/4015) * fix: exceeding `maxResultBuffer` closes the connection. The row is not read past the limit, so the connection cannot continue, and it is now dropped where the limit is exceeded rather than refused as a protocol violation at the next message. The error reported is the limit alone, with SQLState `08S01` as before [Issue #4015](https://github.com/pgjdbc/pgjdbc/issues/4015) +### Security +* fix: a `requireAuth` value that excludes every authentication method, such as `!password,!md5,!gss,!sspi,!scram-sha-256,!none`, now refuses every method. A connection with such a value now fails with SQLState `08004`. Before, the driver treated the value as if `requireAuth` were not set, and used any method the server asked for, even a cleartext password. This affects 42.7.11 through 42.7.13 [GHSA-rhp9-mr79-r74h](https://github.com/pgjdbc/pgjdbc/security/advisories/GHSA-rhp9-mr79-r74h) +* fix: a `requireAuth` value without a method in it, such as `,` or `,,`, now fails with SQLState `22023`. Before, the driver treated it as if `requireAuth` were not set. Commas at the very end of a value are still ignored, as in `password,`. This affects 42.7.11 through 42.7.13 [GHSA-rhp9-mr79-r74h](https://github.com/pgjdbc/pgjdbc/security/advisories/GHSA-rhp9-mr79-r74h) + ## [42.7.13] (2026-07-06) ### Added diff --git a/pgjdbc/src/main/java/org/postgresql/core/AuthMethod.java b/pgjdbc/src/main/java/org/postgresql/core/AuthMethod.java index 6f5f26dc..66de4f6d 100644 --- a/pgjdbc/src/main/java/org/postgresql/core/AuthMethod.java +++ b/pgjdbc/src/main/java/org/postgresql/core/AuthMethod.java @@ -31,17 +31,40 @@ public enum AuthMethod { } } + /** + * Parses a {@code requireAuth} value into the authentication methods the + * server may request. + * + * @param requireAuth comma-separated method names, either all plain or all + * prefixed with {@code !} + * @return {@code null} when {@code requireAuth} is {@code null}, and + * {@link #checkAuth} allows every method for {@code null}; otherwise the + * allowed methods, empty when the value excludes every method + * @throws PSQLException with {@link PSQLState#INVALID_PARAMETER_VALUE} if + * the value mixes the two forms, names a method twice, names an unknown + * method, or is written without a method + */ public static @Nullable EnumSet<AuthMethod> parseRequireAuth(@Nullable String requireAuth) throws PSQLException { if (requireAuth == null) { return null; } - EnumSet<AuthMethod> allowedMethods; EnumSet<AuthMethod> seenMethods = EnumSet.noneOf(AuthMethod.class); String[] methods = requireAuth.split(","); - boolean isDisallowMode = methods.length > 0 && methods[0].trim().startsWith("!"); + // split drops empty strings at the end, so "," and ",," give an empty + // array. A value without a method in it names neither methods to allow + // nor methods to exclude. It is a mistake, not a request to refuse every + // method, so it fails as an invalid value. + if (methods.length == 0) { + throw new PSQLException( + GT.tr("Invalid authentication method: {0}", requireAuth), + PSQLState.INVALID_PARAMETER_VALUE); + } + boolean isDisallowMode = methods[0].trim().startsWith("!"); - allowedMethods = isDisallowMode ? EnumSet.allOf(AuthMethod.class) : EnumSet.noneOf(AuthMethod.class); + EnumSet<AuthMethod> allowedMethods = isDisallowMode + ? EnumSet.allOf(AuthMethod.class) + : EnumSet.noneOf(AuthMethod.class); for (String method : methods) { method = method.trim(); @@ -62,9 +85,20 @@ public enum AuthMethod { allowedMethods.add(authMethod); } } - return allowedMethods.isEmpty() ? null : allowedMethods; + // Return an empty set, not null. checkAuth allows every method for null, + // so null here would let the server pick any method. + return allowedMethods; } + /** + * Refuses an authentication method that {@code requireAuth} does not allow. + * + * @param allowedMethods the methods to allow, or {@code null} to allow + * every method; an empty set refuses every method + * @param authMethod the method the server asked for + * @throws PSQLException with {@link PSQLState#CONNECTION_REJECTED} if the + * method is not allowed + */ public static void checkAuth(@Nullable EnumSet<AuthMethod> allowedMethods, AuthMethod authMethod) throws PSQLException { if (allowedMethods == null) { return; diff --git a/pgjdbc/src/test/java/org/postgresql/core/AuthMethodTest.java b/pgjdbc/src/test/java/org/postgresql/core/AuthMethodTest.java new file mode 100644 index 00000000..6dfb1eef --- /dev/null +++ b/pgjdbc/src/test/java/org/postgresql/core/AuthMethodTest.java @@ -0,0 +1,167 @@ +/* + * Copyright (c) 2026, PostgreSQL Global Development Group + * See the LICENSE file in the project root for more information. + */ + +package org.postgresql.core; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.postgresql.util.PSQLException; +import org.postgresql.util.PSQLState; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.EnumSet; + +/** + * An unset {@code requireAuth} allows every authentication method, and a + * value that excludes every method refuses all of them. A value cannot mix + * plain and {@code !} names, name a method twice, name an unknown method, or + * be written without a method. Commas at the very end of a value are + * ignored. + */ +class AuthMethodTest { + + @Test + void anUnsetRequireAuthParsesToNull() throws PSQLException { + assertNull(AuthMethod.parseRequireAuth(null), "parseRequireAuth(null)"); + } + + @Test + void aPositiveListAllowsOnlyTheMethodsItNames() throws PSQLException { + assertEquals(EnumSet.of(AuthMethod.SCRAM_SHA_256, AuthMethod.GSS), + AuthMethod.parseRequireAuth("scram-sha-256,gss")); + } + + @Test + void anExclusionListAllowsEveryMethodItDoesNotName() throws PSQLException { + EnumSet<AuthMethod> expected = EnumSet.allOf(AuthMethod.class); + expected.remove(AuthMethod.PASSWORD); + expected.remove(AuthMethod.MD5); + + assertEquals(expected, AuthMethod.parseRequireAuth("!password,!md5")); + } + + /** + * Excluding every method but one leaves exactly that one. Over all runs, + * each method is checked both when it is the only one allowed and when it + * is excluded. A method added to the enum later is checked too. + */ + @ParameterizedTest(name = "every method but {0} excluded") + @EnumSource(AuthMethod.class) + void anExclusionListCanLeaveASingleMethod(AuthMethod kept) + throws PSQLException { + String allButKept = + RequireAuthValues.excluding(EnumSet.complementOf(EnumSet.of(kept))); + EnumSet<AuthMethod> allowed = AuthMethod.parseRequireAuth(allButKept); + + assertEquals(EnumSet.of(kept), allowed, + "parseRequireAuth(\"" + allButKept + "\")"); + assertDoesNotThrow(() -> AuthMethod.checkAuth(allowed, kept)); + for (AuthMethod other : EnumSet.complementOf(EnumSet.of(kept))) { + PSQLException e = assertThrows(PSQLException.class, + () -> AuthMethod.checkAuth(allowed, other), "checkAuth for " + other); + assertEquals(PSQLState.CONNECTION_REJECTED.getState(), e.getSQLState(), + "SQLState of the refusal of " + other); + } + } + + @Test + void surroundingSpaceIsIgnoredInAPositiveList() throws PSQLException { + assertEquals(EnumSet.of(AuthMethod.PASSWORD, AuthMethod.MD5), + AuthMethod.parseRequireAuth(" password , md5 ")); + } + + @Test + void surroundingSpaceIsIgnoredInAnExclusionList() throws PSQLException { + assertEquals( + EnumSet.complementOf(EnumSet.of(AuthMethod.PASSWORD, AuthMethod.MD5)), + AuthMethod.parseRequireAuth(" !password , !md5 ")); + } + + /** + * Excluding every method must parse to an empty set, not to null. + * checkAuth allows every method for null, so with null the driver sends a + * cleartext password to a server that asks for one. + */ + @Test + void excludingEveryMethodParsesToAnEmptySet() throws PSQLException { + EnumSet<AuthMethod> allowed = + AuthMethod.parseRequireAuth(RequireAuthValues.EVERY_METHOD_EXCLUDED); + + assertEquals(EnumSet.noneOf(AuthMethod.class), allowed, + "parseRequireAuth(\"" + RequireAuthValues.EVERY_METHOD_EXCLUDED + + "\")"); + } + + @ParameterizedTest(name = "{0} against an empty allowed set") + @EnumSource(AuthMethod.class) + void anEmptyAllowedSetRefusesEveryMethod(AuthMethod method) { + PSQLException e = assertThrows(PSQLException.class, + () -> AuthMethod.checkAuth(EnumSet.noneOf(AuthMethod.class), method)); + assertEquals(PSQLState.CONNECTION_REJECTED.getState(), e.getSQLState(), + "SQLState of the refusal"); + } + + @ParameterizedTest(name = "{0} against an unset requireAuth") + @EnumSource(AuthMethod.class) + void anUnsetRequireAuthAllowsEveryMethod(AuthMethod method) { + assertDoesNotThrow(() -> AuthMethod.checkAuth(null, method)); + } + + @ParameterizedTest(name = "requireAuth={0}") + @ValueSource(strings = {"password,!md5", "!password,md5"}) + void positiveAndNegativeFormsCannotBeMixed(String requireAuth) { + assertParseError(requireAuth); + } + + @ParameterizedTest(name = "requireAuth={0}") + @ValueSource(strings = {"password,password", "!md5,!md5"}) + void aMethodCannotBeNamedTwice(String requireAuth) { + assertParseError(requireAuth); + } + + @ParameterizedTest(name = "requireAuth={0}") + @ValueSource(strings = {"sha-256", "trust"}) + void anUnknownMethodIsRefused(String requireAuth) { + assertParseError(requireAuth); + } + + /** + * Commas at the very end of the value are ignored, and the methods before + * them are enforced. Users may already have such a value in their + * connection settings, and refusing it would break their connections. + */ + @ParameterizedTest(name = "requireAuth={0}") + @ValueSource(strings = {"password,", "password,,"}) + void commasAtTheEndAreIgnored(String requireAuth) throws PSQLException { + assertEquals(EnumSet.of(AuthMethod.PASSWORD), + AuthMethod.parseRequireAuth(requireAuth), + "parseRequireAuth(\"" + requireAuth + "\")"); + } + + /** + * A value without a method in it is refused as an invalid value. It names + * neither methods to allow nor methods to exclude, so it is a mistake, not + * a request to refuse every method. + */ + @ParameterizedTest(name = "requireAuth={0}") + @ValueSource(strings = {"!", ",", ",,"}) + void aValueWithoutAMethodIsRefused(String requireAuth) { + assertParseError(requireAuth); + } + + private static void assertParseError(String requireAuth) { + PSQLException e = assertThrows(PSQLException.class, + () -> AuthMethod.parseRequireAuth(requireAuth)); + assertEquals(PSQLState.INVALID_PARAMETER_VALUE.getState(), e.getSQLState(), + "SQLState of the parse error"); + } +} diff --git a/pgjdbc/src/test/java/org/postgresql/core/RequireAuthConnectTest.java b/pgjdbc/src/test/java/org/postgresql/core/RequireAuthConnectTest.java new file mode 100644 index 00000000..ee118f54 --- /dev/null +++ b/pgjdbc/src/test/java/org/postgresql/core/RequireAuthConnectTest.java @@ -0,0 +1,206 @@ +/* + * Copyright (c) 2026, PostgreSQL Global Development Group + * See the LICENSE file in the project root for more information. + */ + +package org.postgresql.core; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.postgresql.PGProperty; +import org.postgresql.util.PSQLState; + +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.InputStream; +import java.io.OutputStream; +import java.net.InetAddress; +import java.net.Socket; +import java.nio.ByteBuffer; +import java.sql.DriverManager; +import java.sql.SQLException; +import java.util.Arrays; +import java.util.Map; +import java.util.Properties; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; + +import javax.net.SocketFactory; + +/** + * A requireAuth value that excludes every method must refuse the connection + * with {@link PSQLState#CONNECTION_REJECTED}, whatever the server asks for, + * and the driver must not send the password. + */ +public class RequireAuthConnectTest { + + /** AuthenticationCleartextPassword: the server asks for the password. */ + private static final byte[] CLEARTEXT_PASSWORD_REQUEST = + {'R', 0, 0, 0, 8, 0, 0, 0, 3}; + + /** AuthenticationOk sent before any request: trust authentication. */ + private static final byte[] AUTHENTICATION_OK = + {'R', 0, 0, 0, 8, 0, 0, 0, 0}; + + private static final String PASSWORD = "secret"; + + /** The PasswordMessage that carries {@link #PASSWORD}. */ + private static final byte[] PASSWORD_MESSAGE = + {'p', 0, 0, 0, 11, 's', 'e', 'c', 'r', 'e', 't', 0}; + + @Test + void refusesACleartextPasswordRequest() { + FakeServer server = new FakeServer(CLEARTEXT_PASSWORD_REQUEST); + SQLException e = assertThrows(SQLException.class, + () -> server.connect(RequireAuthValues.EVERY_METHOD_EXCLUDED)); + // The bytes are checked before the SQLState, so if the driver sends the + // password, the test fails on the password and not on the error that + // follows it. + assertArrayEquals(new byte[0], server.writtenAfterStartup(), + "bytes the driver wrote after the startup message"); + assertEquals(PSQLState.CONNECTION_REJECTED.getState(), e.getSQLState(), + "SQLState of the refusal"); + } + + /** + * A server that sends the same request gets a PasswordMessage when + * requireAuth allows the password. Without this test, the empty result in + * {@link #refusesACleartextPasswordRequest} could come from a socket that + * stopped recording. + */ + @Test + void sendsThePasswordWhenRequireAuthAllowsIt() { + FakeServer server = new FakeServer(CLEARTEXT_PASSWORD_REQUEST); + // The reply ends after the request, so the driver fails at end of + // stream once it has sent the password. This test does not check that + // failure. + assertThrows(SQLException.class, () -> server.connect("password")); + assertArrayEquals(PASSWORD_MESSAGE, server.writtenAfterStartup(), + "bytes the driver wrote after the startup message"); + } + + /** + * requireAuth calls trust authentication "none", and + * {@link RequireAuthValues#EVERY_METHOD_EXCLUDED} excludes "none" too. + */ + @Test + void refusesTrust() { + FakeServer server = new FakeServer(AUTHENTICATION_OK); + SQLException e = assertThrows(SQLException.class, + () -> server.connect(RequireAuthValues.EVERY_METHOD_EXCLUDED)); + assertEquals(PSQLState.CONNECTION_REJECTED.getState(), e.getSQLState(), + "SQLState of the refusal"); + } + + /** Replies with a fixed message and records what the driver writes. */ + private static final class FakeServer { + /** + * The driver creates the socket factory from its class name and passes + * it only string properties. So the factory finds the server by the key + * that {@link #connect} passes as socketFactoryArg. + */ + static final Map<String, FakeServer> BY_KEY = new ConcurrentHashMap<>(); + + final byte[] reply; + final ByteArrayOutputStream written = new ByteArrayOutputStream(); + + FakeServer(byte[] reply) { + this.reply = reply; + } + + void connect(String requireAuth) throws SQLException { + String key = UUID.randomUUID().toString(); + Properties props = new Properties(); + PGProperty.USER.set(props, "test"); + PGProperty.PASSWORD.set(props, PASSWORD); + PGProperty.SSL_MODE.set(props, "disable"); + PGProperty.GSS_ENC_MODE.set(props, "disable"); + PGProperty.REQUIRE_AUTH.set(props, requireAuth); + PGProperty.SOCKET_FACTORY.set(props, + FakeServerSocketFactory.class.getName()); + PGProperty.SOCKET_FACTORY_ARG.set(props, key); + BY_KEY.put(key, this); + try { + // The factory ignores the host. The driver records the failure for the + // host in the JVM-wide GlobalHostStatusTracker, which multi-host URLs + // read, so the host is a name that cannot exist. + DriverManager.getConnection( + "jdbc:postgresql://requireauth.invalid/test", props).close(); + } finally { + BY_KEY.remove(key); + } + } + + /** + * Returns what the driver wrote after its startup message, which starts + * with its length. + */ + byte[] writtenAfterStartup() { + byte[] bytes = written.toByteArray(); + int startupLength = ByteBuffer.wrap(bytes).getInt(); + return Arrays.copyOfRange(bytes, startupLength, bytes.length); + } + } + + /** + * Creates sockets that read {@link FakeServer#reply} and write to + * {@link FakeServer#written}. The driver loads this class through the + * socketFactory property and calls only {@link #createSocket()}. + */ + public static class FakeServerSocketFactory extends SocketFactory { + private final FakeServer server; + + public FakeServerSocketFactory(String key) { + this.server = FakeServer.BY_KEY.get(key); + } + + @Override + public Socket createSocket() { + return new Socket() { + private final InputStream in = new ByteArrayInputStream(server.reply); + + /** Reports connected, so the driver does not connect it. */ + @Override + public boolean isConnected() { + return true; + } + + @Override + public InputStream getInputStream() { + return in; + } + + @Override + public OutputStream getOutputStream() { + return server.written; + } + }; + } + + @Override + public Socket createSocket(String host, int port) { + throw new UnsupportedOperationException(); + } + + @Override + public Socket createSocket(String host, int port, InetAddress localHost, + int localPort) { + throw new UnsupportedOperationException(); + } + + @Override + public Socket createSocket(InetAddress host, int port) { + throw new UnsupportedOperationException(); + } + + @Override + public Socket createSocket(InetAddress address, int port, + InetAddress localAddress, int localPort) { + throw new UnsupportedOperationException(); + } + } +} diff --git a/pgjdbc/src/test/java/org/postgresql/core/RequireAuthValues.java b/pgjdbc/src/test/java/org/postgresql/core/RequireAuthValues.java new file mode 100644 index 00000000..b460a380 --- /dev/null +++ b/pgjdbc/src/test/java/org/postgresql/core/RequireAuthValues.java @@ -0,0 +1,37 @@ +/* + * Copyright (c) 2026, PostgreSQL Global Development Group + * See the LICENSE file in the project root for more information. + */ + +package org.postgresql.core; + +import java.util.EnumSet; +import java.util.Locale; +import java.util.stream.Collectors; + +/** + * Builds {@code requireAuth} values from the {@link AuthMethod} enum, so the + * tests pick up a new method without anyone editing a list. If a name built + * here does not match what {@link AuthMethod#fromString} accepts, fromString + * throws and the tests fail. + */ +final class RequireAuthValues { + + /** Excludes every method, so a connection that uses it must be refused. */ + static final String EVERY_METHOD_EXCLUDED = + excluding(EnumSet.allOf(AuthMethod.class)); + + private RequireAuthValues() { + } + + /** + * The requireAuth value that excludes each of {@code methods}, as in + * {@code !password,!md5}. + */ + static String excluding(EnumSet<AuthMethod> methods) { + return methods.stream() + .map(method -> + "!" + method.name().toLowerCase(Locale.ROOT).replace('_', '-')) + .collect(Collectors.joining(",")); + } +} [parent: cc63ab99693c]