Skip to content

Commit

Permalink
fix: Use SASLprep normalization for SCRAM authentication (#2052)
Browse files Browse the repository at this point in the history
Using NO_PREPARATION the handling of spaces was incorrect, the driver should use SASL_PREPARATION.

Signed-off-by: Jorge Solórzano <jorsol@gmail.com>
  • Loading branch information
jorsol committed Feb 10, 2021
1 parent 0dbc607 commit b480004
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 6 deletions.
8 changes: 7 additions & 1 deletion .travis/travis_install_head_postgres.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,10 @@ sudo mkdir -p ${PG_DATADIR}
sudo chmod 777 ${PG_DATADIR}
sudo chown -R postgres:postgres ${PG_DATADIR}

sudo su postgres -c "/usr/local/pgsql/bin/pg_ctl -D ${PG_DATADIR} -U postgres initdb"
sudo su postgres -c "/usr/local/pgsql/bin/pg_ctl initdb -D ${PG_DATADIR} -U postgres -o -k"

sudo sh -c "echo 'local all all trust' > ${PG_DATADIR}/pg_hba.conf"
sudo sh -c "echo 'host all testscram all scram-sha-256' >> ${PG_DATADIR}/pg_hba.conf"
sudo sh -c "echo 'host all all all trust' >> ${PG_DATADIR}/pg_hba.conf"
sudo sh -c "echo 'local replication all trust' >> ${PG_DATADIR}/pg_hba.conf"
sudo sh -c "echo -n 'host replication all all trust' >> ${PG_DATADIR}/pg_hba.conf"
7 changes: 5 additions & 2 deletions .travis/travis_install_postgres.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ then
sudo apt-get update
sudo apt-get -o Dpkg::Options::="--force-confdef" -o Dpkg::Options::="--force-confnew" install postgresql-${PG_VERSION} postgresql-contrib-${PG_VERSION} -qq

sudo sh -c "echo 'local all postgres trust' > /etc/postgresql/${PG_VERSION}/main/pg_hba.conf"
sudo sh -c "echo 'local all all trust' >> /etc/postgresql/${PG_VERSION}/main/pg_hba.conf"
sudo sh -c "echo 'local all all trust' > /etc/postgresql/${PG_VERSION}/main/pg_hba.conf"
if [ ${PG_VERSION} -ge '10' ]
then
sudo sh -c "echo 'host all testscram all scram-sha-256' >> /etc/postgresql/${PG_VERSION}/main/pg_hba.conf"
fi
sudo sh -c "echo -n 'host all all 127.0.0.1/32 trust' >> /etc/postgresql/${PG_VERSION}/main/pg_hba.conf"

if [ ${PG_VERSION} = '8.4' ]
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ Notable changes since version 42.0.0, read the complete [History of Changes](htt
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).

## [Unreleased]

**Notable Changes**
- Now the driver use SASLprep normalization for SCRAM authentication fixing some issues with spaces in passwords.

### Changed

### Added
Expand All @@ -13,6 +17,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Fix "Required class information missing" when old org.jboss:jandex parses pgjdbc classes [issue 2008][https://github.com/pgjdbc/pgjdbc/issues/2008]
- Fix PGCopyInputStream returning the last row twice when reading with CopyOut API [issue 2016][https://github.com/pgjdbc/pgjdbc/issues/2016]
- Fix Connnection.isValid() to not wait longer than existing network timeout [PR #2040](https://github.com/pgjdbc/pgjdbc/pull/2040)
- Use SASLprep normalization for SCRAM authentication [PR #2052](https://github.com/pgjdbc/pgjdbc/pull/2052)

## [42.2.18]
### Fixed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void processServerMechanismsAndInit() throws IOException, PSQLException {
try {
scramClient = ScramClient
.channelBinding(ScramClient.ChannelBinding.NO)
.stringPreparation(StringPreparations.NO_PREPARATION)
.stringPreparation(StringPreparations.SASL_PREPARATION)
.selectMechanismBasedOnServerAdvertised(mechanisms.toArray(new String[]{}))
.setup();
} catch (IllegalArgumentException e) {
Expand Down
105 changes: 105 additions & 0 deletions pgjdbc/src/test/java/org/postgresql/jdbc/ScramTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* Copyright (c) 2021, PostgreSQL Global Development Group
* See the LICENSE file in the project root for more information.
*/

package org.postgresql.jdbc;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

import org.postgresql.core.ServerVersion;
import org.postgresql.test.TestUtil;
import org.postgresql.util.PSQLState;

import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.Properties;

class ScramTest {

private static Connection con;
private static final String ROLE_NAME = "testscram";

@BeforeAll
public static void setUp() throws Exception {
con = TestUtil.openPrivilegedDB();
assumeTrue(TestUtil.haveMinimumServerVersion(con, ServerVersion.v10));
}

@AfterAll
public static void tearDown() throws Exception {
try (Statement stmt = con.createStatement()) {
stmt.execute("DROP ROLE IF EXISTS " + ROLE_NAME);
}
TestUtil.closeDB(con);
}

/**
* Test creating a role with passwords WITH spaces and opening a connection using the same
* password, should work because is the "same" password.
*
* <p>https://github.com/pgjdbc/pgjdbc/issues/1970
*/
@ParameterizedTest
@ValueSource(strings = {"My Space", "$ec ret", " rover june spelling ",
"!zj5hs*k5 STj@DaRUy", "q\u00A0w\u2000e\u2003r\u2009t\u3000y"})
void testPasswordWithSpace(String passwd) throws SQLException {
createRole(passwd); // Create role password with spaces.

Properties props = new Properties();
props.setProperty("username", ROLE_NAME);
props.setProperty("password", passwd);

try (Connection c = assertDoesNotThrow(() -> TestUtil.openDB(props));
Statement stmt = c.createStatement();
ResultSet rs = stmt.executeQuery("SELECT current_user")) {
assertTrue(rs.next());
assertEquals(ROLE_NAME, rs.getString(1));
}
}

/**
* Test creating a role with passwords WITHOUT spaces and opening a connection using password with
* spaces should fail since the spaces should not be stripped out.
*
* <p>https://github.com/pgjdbc/pgjdbc/issues/2000
*/
@ParameterizedTest
@ValueSource(strings = {"My Space", "$ec ret", "rover june spelling",
"!zj5hs*k5 STj@DaRUy", "q\u00A0w\u2000e\u2003r\u2009t\u3000y"})
void testPasswordWithoutSpace(String passwd) throws SQLException {
String passwdNoSpaces = passwd.codePoints()
.filter(i -> !Character.isSpaceChar(i))
.collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append)
.toString();

createRole(passwdNoSpaces); // Create role password without spaces.

Properties props = new Properties();
props.setProperty("username", ROLE_NAME);
props.setProperty("password", passwd); // Open connection with spaces

SQLException ex = assertThrows(SQLException.class, () -> TestUtil.openDB(props));
assertEquals(PSQLState.INVALID_PASSWORD.getState(), ex.getSQLState());
}

private void createRole(String passwd) throws SQLException {
try (Statement stmt = con.createStatement()) {
stmt.execute("SET password_encryption='scram-sha-256'");
stmt.execute("DROP ROLE IF EXISTS " + ROLE_NAME);
stmt.execute("CREATE ROLE " + ROLE_NAME + " WITH LOGIN PASSWORD '" + passwd + "'");
}
}

}
7 changes: 5 additions & 2 deletions pgjdbc/src/test/java/org/postgresql/test/TestUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -341,11 +341,14 @@ public static Connection openDB(Properties props) throws SQLException {
"user name is not specified. Please specify 'username' property via -D or build.properties");
}
props.setProperty("user", user);
String password = getPassword();

// Allow properties to override the password.
String password = props.getProperty("password");
if (password == null) {
password = "";
password = getPassword() != null ? getPassword() : "";
}
props.setProperty("password", password);

String sslPassword = getSslPassword();
if (sslPassword != null) {
PGProperty.SSL_PASSWORD.set(props, sslPassword);
Expand Down

0 comments on commit b480004

Please sign in to comment.