Commit 7bd5dd4

committed

Handle SASL SCRAM server error responses

Add proper error handling for SCRAM-SERVER-FINAL-MESSAGE error attribute. The SCRAM specification allows servers to return error messages via the 'e' attribute in the server final message. Currently, these errors are ignored and authentication fails later during signature verification. Postgres typically doesn't return this error (see [here](https://github.com/postgres/postgres/blob/2047ad068139f0b8c6da73d0b845ca9ba30fb33d/src/backend/libpq/auth-scram.c#L423) on why), but poolers, or other applications using the postgres protocol might, and it's part of the SCRAM spec, so it probably makes sense for node-postgres to handle it. Aligns behaviour with psql, postgrex, and somewhat with pgJDBC (pgJDBC in particular is stricter with scram errors). For reference: - libpq handling it: https://github.com/postgres/postgres/blob/2047ad068139f0b8c6da73d0b845ca9ba30fb33d/src/interfaces/libpq/fe-auth-scram.c#L708

1 parent 27a2754 commit 7bd5dd4Copy full SHA for 7bd5dd4

File tree

+23

-0

lines changed

+23

-0

lines changed

Lines changed: 6 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -178,7 +178,13 @@ function parseServerFirstMessage(data) {`
`178`	`178`
`179`	`179`	`function parseServerFinalMessage(serverData) {`
`180`	`180`	`const attrPairs = parseAttributePairs(serverData)`
	`181`	`+ const error = attrPairs.get('e')`
`181`	`182`	`const serverSignature = attrPairs.get('v')`
	`183`	`+`
	`184`	`+ if (error) {`
	`185`	+ throw new Error(`SASL: SCRAM-SERVER-FINAL-MESSAGE: server returned error: "${error}"`)
	`186`	`+ }`
	`187`	`+`
`182`	`188`	`if (!serverSignature) {`
`183`	`189`	`throw new Error('SASL: SCRAM-SERVER-FINAL-MESSAGE: server signature is missing')`
`184`	`190`	`} else if (!isBase64(serverSignature)) {`

Lines changed: 17 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -284,6 +284,23 @@ suite.test('sasl/scram', function () {`
`284`	`284`	`)`
`285`	`285`	`})`
`286`	`286`
	`287`	`+ suite.test('fails when server returns an error', function () {`
	`288`	`+ assert.throws(`
	`289`	`+ function () {`
	`290`	`+ sasl.finalizeSession(`
	`291`	`+ {`
	`292`	`+ message: 'SASLResponse',`
	`293`	`+ serverSignature: 'abcd',`
	`294`	`+ },`
	`295`	`+ 'e=no-resources'`
	`296`	`+ )`
	`297`	`+ },`
	`298`	`+ {`
	`299`	`+ message: 'SASL: SCRAM-SERVER-FINAL-MESSAGE: server returned error: "no-resources"',`
	`300`	`+ }`
	`301`	`+ )`
	`302`	`+ })`
	`303`	`+`
`287`	`304`	`suite.test('fails when server signature does not match', function () {`
`288`	`305`	`assert.throws(`
`289`	`306`	`function () {`

Comments

(0)