-
-
Notifications
You must be signed in to change notification settings - Fork 32.8k
cli: add NODE_USE_SYSTEM_CA=1 #59276
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,29 @@ | ||||||
'use strict'; | ||||||
// This tests that NODE_USE_SYSTEM_CA environment variable works the same | ||||||
// as --use-system-ca flag by comparing certificate counts. | ||||||
|
||||||
const common = require('../common'); | ||||||
if (!common.hasCrypto) common.skip('missing crypto'); | ||||||
|
||||||
const tls = require('tls'); | ||||||
const { spawnSyncAndExitWithoutError } = require('../common/child_process'); | ||||||
|
||||||
const systemCerts = tls.getCACertificates('system'); | ||||||
if (systemCerts.length === 0) { | ||||||
common.skip('no system certificates available'); | ||||||
} | ||||||
|
||||||
const { child: { stdout: expectedLength } } = spawnSyncAndExitWithoutError(process.execPath, [ | ||||||
'--use-system-ca', | ||||||
'-p', | ||||||
`tls.getCACertificates('default').length`, | ||||||
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. super nit... just have a preference to avoid template literals when unnecessary. but feel free to ignore.
Suggested change
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 have am impression that double quotes in an argument don't work very well on Windows. I could give it a try and see if it works though if the CI comes back happy I would prefer to use single quotes (AFAIK Windows does not do anything special with single quotes). |
||||||
], { | ||||||
env: { ...process.env, NODE_USE_SYSTEM_CA: '0' }, | ||||||
}); | ||||||
|
||||||
spawnSyncAndExitWithoutError(process.execPath, [ | ||||||
'-p', | ||||||
`assert.strictEqual(tls.getCACertificates('default').length, ${expectedLength.toString()})`, | ||||||
], { | ||||||
env: { ...process.env, NODE_USE_SYSTEM_CA: '1' }, | ||||||
}); |
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,56 @@ | ||||||||||||||||
// Env: NODE_USE_SYSTEM_CA=1 | ||||||||||||||||
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. FYI @pmarchini when using this feature (thanks for implementing it by the way) I noticed that it does not yet have integration in Lines 90 to 96 in 0fd1ecd
|
||||||||||||||||
// Same as test-native-root-certs.mjs, just testing the environment variable instead of the flag. | ||||||||||||||||
|
||||||||||||||||
import * as common from '../common/index.mjs'; | ||||||||||||||||
import assert from 'node:assert/strict'; | ||||||||||||||||
import https from 'node:https'; | ||||||||||||||||
import fixtures from '../common/fixtures.js'; | ||||||||||||||||
import { it, beforeEach, afterEach, describe } from 'node:test'; | ||||||||||||||||
import { once } from 'events'; | ||||||||||||||||
|
||||||||||||||||
if (!common.hasCrypto) { | ||||||||||||||||
common.skip('requires crypto'); | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
// To run this test, the system needs to be configured to trust | ||||||||||||||||
// the CA certificate first (which needs an interactive GUI approval, e.g. TouchID): | ||||||||||||||||
// see the README.md in this folder for instructions on how to do this. | ||||||||||||||||
const handleRequest = (req, res) => { | ||||||||||||||||
const path = req.url; | ||||||||||||||||
switch (path) { | ||||||||||||||||
case '/hello-world': | ||||||||||||||||
res.writeHead(200); | ||||||||||||||||
res.end('hello world\n'); | ||||||||||||||||
break; | ||||||||||||||||
default: | ||||||||||||||||
assert(false, `Unexpected path: ${path}`); | ||||||||||||||||
} | ||||||||||||||||
}; | ||||||||||||||||
|
||||||||||||||||
describe('use-system-ca', function() { | ||||||||||||||||
|
||||||||||||||||
async function setupServer(key, cert) { | ||||||||||||||||
const theServer = https.createServer({ | ||||||||||||||||
key: fixtures.readKey(key), | ||||||||||||||||
cert: fixtures.readKey(cert), | ||||||||||||||||
}, handleRequest); | ||||||||||||||||
theServer.listen(0); | ||||||||||||||||
await once(theServer, 'listening'); | ||||||||||||||||
|
||||||||||||||||
return theServer; | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
let server; | ||||||||||||||||
|
||||||||||||||||
beforeEach(async function() { | ||||||||||||||||
server = await setupServer('agent8-key.pem', 'agent8-cert.pem'); | ||||||||||||||||
}); | ||||||||||||||||
|
||||||||||||||||
it('trusts a valid root certificate', async function() { | ||||||||||||||||
await fetch(`https://localhost:${server.address().port}/hello-world`); | ||||||||||||||||
}); | ||||||||||||||||
|
||||||||||||||||
afterEach(async function() { | ||||||||||||||||
server?.close(); | ||||||||||||||||
}); | ||||||||||||||||
}); |
Uh oh!
There was an error while loading. Please reload this page.