-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add support for SCRAM-SHA-256-PLUS i.e. channel binding #3356
Open
jawj
wants to merge
17
commits into
brianc:master
Choose a base branch
from
jawj:add-scram-sha-256-plus
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
65e1c72
Added support for SCRAM-SHA-256-PLUS i.e. channel binding
jawj a338521
Requested tweaks to channel binding
jawj 8ab2b17
Additional tweaks to channel binding
jawj 37f9285
Fixed lint complaints
jawj ae6ab3f
Update packages/pg/lib/crypto/sasl.js
jawj 152396f
Update packages/pg/lib/crypto/sasl.js
jawj f899f8f
Update packages/pg/lib/client.js
jawj 52e656c
Tweaks to channel binding
jawj 1003bff
Merge branch 'add-scram-sha-256-plus' of https://github.com/jawj/node…
jawj 50ee305
Now using homegrown certificate signature algorithm identification
jawj b3a8757
Update ssl.mdx with channel binding changes
jawj 67a6e9c
Allow for config object being undefined when assigning enableChannelB…
jawj b604068
Fixed a test failing on an updated error message
jawj 88cbe49
Removed - from hash names like SHA-256 for legacy crypto (Node 14 and…
jawj 05690bb
Removed packageManager key from package.json
jawj d9fdccf
Added some SASL/channel binding unit tests
jawj 9a91cb7
Added a unit test for continueSession to check expected SASL session …
jawj File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
function x509Error(msg, cert) { | ||
throw new Error('SASL channel binding: ' + msg + ' when parsing public certificate ' + cert.toString('base64')) | ||
} | ||
|
||
function readASN1Length(data, index) { | ||
let length = data[index++] | ||
if (length < 0x80) return { length, index } | ||
|
||
const lengthBytes = length & 0x7f | ||
if (lengthBytes > 4) x509Error('bad length', data) | ||
|
||
length = 0 | ||
for (let i = 0; i < lengthBytes; i++) { | ||
length = (length << 8) | data[index++] | ||
} | ||
|
||
return { length, index } | ||
} | ||
|
||
function readASN1OID(data, index) { | ||
if (data[index++] !== 0x6) x509Error('non-OID data', data) // 6 = OID | ||
|
||
const { length: OIDLength, index: indexAfterOIDLength } = readASN1Length(data, index) | ||
index = indexAfterOIDLength | ||
lastIndex = index + OIDLength | ||
|
||
const byte1 = data[index++] | ||
let oid = ((byte1 / 40) >> 0) + '.' + (byte1 % 40) | ||
|
||
while (index < lastIndex) { | ||
// loop over numbers in OID | ||
let value = 0 | ||
while (index < lastIndex) { | ||
// loop over bytes in number | ||
const nextByte = data[index++] | ||
value = (value << 7) | (nextByte & 0x7f) | ||
if (nextByte < 0x80) break | ||
} | ||
oid += '.' + value | ||
} | ||
|
||
return { oid, index } | ||
} | ||
|
||
function expectASN1Seq(data, index) { | ||
if (data[index++] !== 0x30) x509Error('non-sequence data', data) // 30 = Sequence | ||
return readASN1Length(data, index) | ||
} | ||
|
||
function signatureAlgorithmHashFromCertificate(data, index) { | ||
// read this thread: https://www.postgresql.org/message-id/17760-b6c61e752ec07060%40postgresql.org | ||
if (index === undefined) index = 0 | ||
index = expectASN1Seq(data, index).index | ||
const { length: certInfoLength, index: indexAfterCertInfoLength } = expectASN1Seq(data, index) | ||
index = indexAfterCertInfoLength + certInfoLength // skip over certificate info | ||
index = expectASN1Seq(data, index).index // skip over signature length field | ||
const { oid, index: indexAfterOID } = readASN1OID(data, index) | ||
switch (oid) { | ||
// RSA | ||
case '1.2.840.113549.1.1.4': | ||
return 'MD5' | ||
case '1.2.840.113549.1.1.5': | ||
return 'SHA-1' | ||
case '1.2.840.113549.1.1.11': | ||
return 'SHA-256' | ||
case '1.2.840.113549.1.1.12': | ||
return 'SHA-384' | ||
case '1.2.840.113549.1.1.13': | ||
return 'SHA-512' | ||
case '1.2.840.113549.1.1.14': | ||
return 'SHA-224' | ||
case '1.2.840.113549.1.1.15': | ||
return 'SHA512-224' | ||
case '1.2.840.113549.1.1.16': | ||
return 'SHA512-256' | ||
// ECDSA | ||
case '1.2.840.10045.4.1': | ||
return 'SHA-1' | ||
case '1.2.840.10045.4.3.1': | ||
return 'SHA-224' | ||
case '1.2.840.10045.4.3.2': | ||
return 'SHA-256' | ||
case '1.2.840.10045.4.3.3': | ||
return 'SHA-384' | ||
case '1.2.840.10045.4.3.4': | ||
return 'SHA-512' | ||
// RSASSA-PSS: hash is indicated separately | ||
case '1.2.840.113549.1.1.10': | ||
index = indexAfterOID | ||
index = expectASN1Seq(data, index).index | ||
if (data[index++] !== 0xa0) x509Error('non-tag data', data) // a0 = constructed tag 0 | ||
index = readASN1Length(data, index).index // skip over tag length field | ||
index = expectASN1Seq(data, index).index // skip over sequence length field | ||
const { oid: hashOID } = readASN1OID(data, index) | ||
switch (hashOID) { | ||
// standalone hash OIDs | ||
case '1.2.840.113549.2.5': | ||
return 'MD5' | ||
case '1.3.14.3.2.26': | ||
return 'SHA-1' | ||
case '2.16.840.1.101.3.4.2.1': | ||
return 'SHA-256' | ||
case '2.16.840.1.101.3.4.2.2': | ||
return 'SHA-384' | ||
case '2.16.840.1.101.3.4.2.3': | ||
return 'SHA-512' | ||
} | ||
x509Error('unknown hash OID ' + hashOID, data) | ||
// Ed25519 -- see https: return//github.com/openssl/openssl/issues/15477 | ||
case '1.3.101.110': | ||
case '1.3.101.112': // ph | ||
return 'SHA-512' | ||
// Ed448 -- still not in pg 17.2 (if supported, digest would be SHAKE256 x 64 bytes) | ||
case '1.3.101.111': | ||
case '1.3.101.113': // ph | ||
x509Error('Ed448 certificate channel binding is not currently supported by Postgres') | ||
} | ||
x509Error('unknown OID ' + oid, data) | ||
} | ||
|
||
module.exports = { signatureAlgorithmHashFromCertificate } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests would work just as well if there were no implementation of channel binding at all, so this PR probably needs a targeted test from someone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in an ideal world we would test using SSL certs with a variety of different signature algorithms. But I'm not sure how easily that fits with the current test setup.
Locally, I have been using:
The
server-*-cert.pem
files from the OpenSSL tests: https://github.com/openssl/openssl/tree/master/test/certsThis Docker file:
Which is run like:
./docker-pg.sh ecdsa
SCRAM_TEST_PGDATABASE=frodo \ SCRAM_TEST_PGUSER=frodo \ SCRAM_TEST_PGPASSWORD=friend \ PGSSLMODE=no-verify \ node --tls-keylog=/path/to/keylog.txt packages/pg/test/integration/client/sasl-scram-tests.js
(The
--tls-keylog
enables the use of Wireshark if needed).