diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 97f4013ba..ab5bef47b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,6 +15,7 @@ jobs: POSTGRES_USER: postgres POSTGRES_PASSWORD: postgres POSTGRES_DB: ci_db_test + POSTGRES_HOST_AUTH_METHOD: 'md5' ports: - 5432:5432 options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5 @@ -23,7 +24,19 @@ jobs: node: ['10', '12', '14', '16', '18'] os: [ubuntu-latest, windows-latest, macos-latest] name: Node.js ${{ matrix.node }} (${{ matrix.os }}) + env: + PGUSER: postgres + PGHOST: localhost + PGPASSWORD: postgres + PGDATABASE: ci_db_test + PGTESTNOSSL: 'true' + SCRAM_TEST_PGUSER: scram_test + SCRAM_TEST_PGPASSWORD: test4scram steps: + - run: | + psql \ + -c "SET password_encryption = 'scram-sha-256'" \ + -c "CREATE ROLE scram_test LOGIN PASSWORD 'test4scram'" - uses: actions/checkout@v3 with: persist-credentials: false @@ -34,4 +47,4 @@ jobs: cache: yarn - run: yarn install # TODO(bmc): get ssl tests working in ci - - run: PGTESTNOSSL=true PGUSER=postgres PGPASSWORD=postgres PGDATABASE=ci_db_test yarn test + - run: yarn test diff --git a/packages/pg/lib/client.js b/packages/pg/lib/client.js index 82d571d8a..2090c4b5f 100644 --- a/packages/pg/lib/client.js +++ b/packages/pg/lib/client.js @@ -247,19 +247,31 @@ class Client extends EventEmitter { _handleAuthSASL(msg) { this._checkPgPass(() => { - this.saslSession = sasl.startSession(msg.mechanisms) - this.connection.sendSASLInitialResponseMessage(this.saslSession.mechanism, this.saslSession.response) + try { + this.saslSession = sasl.startSession(msg.mechanisms) + this.connection.sendSASLInitialResponseMessage(this.saslSession.mechanism, this.saslSession.response) + } catch (err) { + this.connection.emit('error', err) + } }) } _handleAuthSASLContinue(msg) { - sasl.continueSession(this.saslSession, this.password, msg.data) - this.connection.sendSCRAMClientFinalMessage(this.saslSession.response) + try { + sasl.continueSession(this.saslSession, this.password, msg.data) + this.connection.sendSCRAMClientFinalMessage(this.saslSession.response) + } catch (err) { + this.connection.emit('error', err) + } } _handleAuthSASLFinal(msg) { - sasl.finalizeSession(this.saslSession, msg.data) - this.saslSession = null + try { + sasl.finalizeSession(this.saslSession, msg.data) + this.saslSession = null + } catch (err) { + this.connection.emit('error', err) + } } _handleBackendKeyData(msg) { diff --git a/packages/pg/lib/sasl.js b/packages/pg/lib/sasl.js index fb703b270..c8d2d2bdc 100644 --- a/packages/pg/lib/sasl.js +++ b/packages/pg/lib/sasl.js @@ -23,6 +23,9 @@ function continueSession(session, password, serverData) { if (typeof password !== 'string') { throw new Error('SASL: SCRAM-SERVER-FIRST-MESSAGE: client password must be a string') } + if (password === '') { + throw new Error('SASL: SCRAM-SERVER-FIRST-MESSAGE: client password must be a non-empty string') + } if (typeof serverData !== 'string') { throw new Error('SASL: SCRAM-SERVER-FIRST-MESSAGE: serverData must be a string') } diff --git a/packages/pg/test/integration/client/sasl-scram-tests.js b/packages/pg/test/integration/client/sasl-scram-tests.js index debc28685..3b3fd4a57 100644 --- a/packages/pg/test/integration/client/sasl-scram-tests.js +++ b/packages/pg/test/integration/client/sasl-scram-tests.js @@ -73,3 +73,24 @@ suite.testAsync('sasl/scram fails when password is wrong', async () => { ) assert.ok(usingSasl, 'Should be using SASL for authentication') }) + +suite.testAsync('sasl/scram fails when password is empty', async () => { + const client = new pg.Client({ + ...config, + // We use a password function here so the connection defaults do not + // override the empty string value with one from process.env.PGPASSWORD + password: () => '', + }) + let usingSasl = false + client.connection.once('authenticationSASL', () => { + usingSasl = true + }) + await assert.rejects( + () => client.connect(), + { + message: 'SASL: SCRAM-SERVER-FIRST-MESSAGE: client password must be a non-empty string', + }, + 'Error code should be for a password error' + ) + assert.ok(usingSasl, 'Should be using SASL for authentication') +}) diff --git a/packages/pg/test/unit/client/sasl-scram-tests.js b/packages/pg/test/unit/client/sasl-scram-tests.js index e53448bdf..36a5556b4 100644 --- a/packages/pg/test/unit/client/sasl-scram-tests.js +++ b/packages/pg/test/unit/client/sasl-scram-tests.js @@ -80,6 +80,44 @@ test('sasl/scram', function () { ) }) + test('fails when client password is not a string', function () { + for(const badPasswordValue of [null, undefined, 123, new Date(), {}]) { + assert.throws( + function () { + sasl.continueSession( + { + message: 'SASLInitialResponse', + clientNonce: 'a', + }, + badPasswordValue, + 'r=1,i=1' + ) + }, + { + message: 'SASL: SCRAM-SERVER-FIRST-MESSAGE: client password must be a string', + } + ) + } + }) + + test('fails when client password is an empty string', function () { + assert.throws( + function () { + sasl.continueSession( + { + message: 'SASLInitialResponse', + clientNonce: 'a', + }, + '', + 'r=1,i=1' + ) + }, + { + message: 'SASL: SCRAM-SERVER-FIRST-MESSAGE: client password must be a non-empty string', + } + ) + }) + test('fails when iteration is missing in server message', function () { assert.throws( function () {