Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Use SASLprep normalization for SCRAM authentication #2052

Merged
merged 1 commit into from
Feb 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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