-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
tls: add allowPartialTrustChain
flag
#54790
Changes from 2 commits
bb69f14
d488340
5f501b4
509156a
433f6b3
3ae1ea0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
addaleax marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
'use strict'; | ||
const common = require('../common'); | ||
|
||
if (!common.hasCrypto) | ||
common.skip('missing crypto'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. non-blocking nit: Since this is using the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think this is okay. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 509156a 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, did this before seeing @RedYetiDev's response – no strong feelings from my side. |
||
|
||
const assert = require('assert'); | ||
const { once } = require('events'); | ||
const tls = require('tls'); | ||
const fixtures = require('../common/fixtures'); | ||
|
||
// agent6-cert.pem is signed by intermediate cert of ca3. | ||
// The server has a cert chain of agent6->ca3->ca1(root) but | ||
|
||
async function test() { | ||
const server = tls.createServer({ | ||
ca: fixtures.readKey('ca3-cert.pem'), | ||
key: fixtures.readKey('agent6-key.pem'), | ||
cert: fixtures.readKey('agent6-cert.pem'), | ||
}, (socket) => socket.resume()); | ||
server.listen(0); | ||
await once(server, 'listening'); | ||
|
||
const opts = { | ||
port: server.address().port, | ||
ca: fixtures.readKey('ca3-cert.pem'), | ||
checkServerIdentity() {} | ||
}; | ||
|
||
// Connecting succeeds with allowPartialTrustChain: true | ||
const client = tls.connect({ ...opts, allowPartialTrustChain: true }); | ||
await once(client, 'secureConnect'); | ||
client.destroy(); | ||
|
||
// Consistency check: Connecting fails without allowPartialTrustChain: true | ||
await assert.rejects(async () => { | ||
const client = tls.connect(opts); | ||
await once(client, 'secureConnect'); | ||
}, { code: 'UNABLE_TO_GET_ISSUER_CERT' }); | ||
|
||
server.close(); | ||
} | ||
|
||
test().catch((err) => process.nextTick(() => { throw err; })); |
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.
non-blocking nit: A short comment here about who owns this pointer and who is responsible for cleaning it up would be good for others coming into the code 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.
Sure, done in 509156a 👍