Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitfa2272a

Browse files
authored
Remove node forge dependency (#648)
Replaced `node-forge` with `@peculiar/x509` (more modern, lightweight, and widely adopted). Node.js's built-in `crypto` module was attempted first, but `keyUsage` returns `undefined`.**Testing in Electron**All vitest tests now run in Electron to mirror VS Code's environment. This adds a few seconds overhead vs Node.js. `electron` was added as dev dependency for BoringSSL compatibility (Node.js uses OpenSSL).
1 parentebbbb9a commitfa2272a

File tree

6 files changed

+535
-89
lines changed

6 files changed

+535
-89
lines changed

‎.vscode/settings.json‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,9 @@
1010
},
1111
"[jsonc]": {
1212
"editor.defaultFormatter":"esbenp.prettier-vscode"
13-
}
13+
},
14+
"vitest.nodeEnv": {
15+
"ELECTRON_RUN_AS_NODE":"1"
16+
},
17+
"vitest.nodeExecutable":"node_modules/.bin/electron"
1418
}

‎package.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
"package":"webpack --mode production --devtool hidden-source-map",
2626
"package:prerelease":"npx vsce package --pre-release",
2727
"pretest":"tsc -p . --outDir out && tsc -p test --outDir out && yarn run build && yarn run lint",
28-
"test":"vitest",
28+
"test":"ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs",
2929
"test:ci":"CI=true yarn test",
3030
"test:integration":"vscode-test",
3131
"vscode:prepublish":"yarn package",
@@ -343,12 +343,12 @@
343343
"word-wrap":"1.2.5"
344344
},
345345
"dependencies": {
346+
"@peculiar/x509":"^1.14.0",
346347
"axios":"1.12.2",
347348
"date-fns":"^3.6.0",
348349
"eventsource":"^3.0.6",
349350
"find-process":"https://github.com/coder/find-process#fix/sequoia-compat",
350351
"jsonc-parser":"^3.3.1",
351-
"node-forge":"^1.3.1",
352352
"openpgp":"^6.2.2",
353353
"pretty-bytes":"^7.1.0",
354354
"proxy-agent":"^6.5.0",
@@ -361,7 +361,6 @@
361361
"@types/eventsource":"^3.0.0",
362362
"@types/glob":"^7.1.3",
363363
"@types/node":"^22.14.1",
364-
"@types/node-forge":"^1.3.14",
365364
"@types/semver":"^7.7.1",
366365
"@types/ua-parser-js":"0.7.36",
367366
"@types/vscode":"^1.73.0",
@@ -375,6 +374,7 @@
375374
"bufferutil":"^4.0.9",
376375
"coder":"https://github.com/coder/coder#main",
377376
"dayjs":"^1.11.13",
377+
"electron":"^39.1.2",
378378
"eslint":"^8.57.1",
379379
"eslint-config-prettier":"^10.1.8",
380380
"eslint-import-resolver-typescript":"^4.4.4",

‎src/error.ts‎

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
1+
import{
2+
X509Certificate,
3+
KeyUsagesExtension,
4+
KeyUsageFlags,
5+
}from"@peculiar/x509";
16
import{isAxiosError}from"axios";
27
import{isApiError,isApiErrorResponse}from"coder/site/src/api/errors";
3-
import*asforgefrom"node-forge";
4-
import*astlsfrom"tls";
8+
import*astlsfrom"node:tls";
59
import*asvscodefrom"vscode";
610

711
import{typeLogger}from"./logging/logger";
@@ -23,10 +27,6 @@ export enum X509_ERR {
2327
UNTRUSTED_CHAIN="Your Coder deployment's certificate chain does not appear to be trusted by this system. The root of the certificate chain must be added to this system's trust store. ",
2428
}
2529

26-
interfaceKeyUsage{
27-
keyCertSign:boolean;
28-
}
29-
3030
exportclassCertificateErrorextendsError{
3131
publicstaticActionAllowInsecure="Allow Insecure";
3232
publicstaticActionOK="OK";
@@ -80,7 +80,7 @@ export class CertificateError extends Error {
8080
consturl=newURL(address);
8181
constsocket=tls.connect(
8282
{
83-
port:parseInt(url.port,10)||443,
83+
port:Number.parseInt(url.port,10)||443,
8484
host:url.hostname,
8585
rejectUnauthorized:false,
8686
},
@@ -91,29 +91,27 @@ export class CertificateError extends Error {
9191
thrownewError("no peer certificate");
9292
}
9393

94-
// We use node-forge for two reasons:
95-
// 1. Node/Electron only provide extended key usage.
96-
// 2. Electron's checkIssued() will fail because it suffers from same
97-
// the key usage bug that we are trying to work around here in the
98-
// first place.
99-
constcert=forge.pki.certificateFromPem(x509.toString());
100-
if(!cert.issued(cert)){
94+
// We use "@peculiar/x509" because Node's x509 returns an undefined `keyUsage`.
95+
constcert=newX509Certificate(x509.toString());
96+
constisSelfIssued=cert.subject===cert.issuer;
97+
if(!isSelfIssued){
10198
returnresolve(X509_ERR.PARTIAL_CHAIN);
10299
}
103100

104101
// The key usage needs to exist but not have cert signing to fail.
105-
constkeyUsage=cert.getExtension({name:"keyUsage"})as
106-
|KeyUsage
107-
|undefined;
108-
if(keyUsage&&!keyUsage.keyCertSign){
109-
returnresolve(X509_ERR.NON_SIGNING);
110-
}else{
111-
// This branch is currently untested; it does not appear possible to
112-
// get the error "unable to verify" with a self-signed certificate
113-
// unless the key usage was the issue since it would have errored
114-
// with "self-signed certificate" instead.
115-
returnresolve(X509_ERR.UNTRUSTED_LEAF);
102+
constextension=cert.getExtension(KeyUsagesExtension);
103+
if(extension){
104+
consthasKeyCertSign=
105+
extension.usages&KeyUsageFlags.keyCertSign;
106+
if(!hasKeyCertSign){
107+
returnresolve(X509_ERR.NON_SIGNING);
108+
}
116109
}
110+
// This branch is currently untested; it does not appear possible to
111+
// get the error "unable to verify" with a self-signed certificate
112+
// unless the key usage was the issue since it would have errored
113+
// with "self-signed certificate" instead.
114+
returnresolve(X509_ERR.UNTRUSTED_LEAF);
117115
},
118116
);
119117
socket.on("error",reject);

‎src/remote/remote.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,7 @@ export class Remote {
344344
}catch(error){
345345
subscription.dispose();
346346
reject(error);
347+
return;
347348
}finally{
348349
inProgress=false;
349350
}

‎test/unit/error.test.ts‎

Lines changed: 41 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
1+
import{
2+
KeyUsagesExtension,
3+
X509CertificateasX509CertificatePeculiar,
4+
}from"@peculiar/x509";
15
importaxiosfrom"axios";
2-
import*asfsfrom"fs/promises";
3-
importhttpsfrom"https";
6+
import{X509CertificateasX509CertificateNode}from"node:crypto";
7+
import*asfsfrom"node:fs/promises";
8+
importhttpsfrom"node:https";
49
import{afterAll,beforeAll,describe,expect,it,vi}from"vitest";
510

611
import{CertificateError,X509_ERR,X509_ERR_CODE}from"@/error";
@@ -12,14 +17,11 @@ describe("Certificate errors", () => {
1217
// Before each test we make a request to sanity check that we really get the
1318
// error we are expecting, then we run it through CertificateError.
1419

15-
// TODO: These sanity checks need to be ran in an Electron environment to
16-
// reflect real usage in VS Code. We should either revert back to the standard
17-
// extension testing framework which I believe runs in a headless VS Code
18-
// instead of using vitest or at least run the tests through Electron running as
19-
// Node (for now I do this manually by shimming Node).
20-
constisElectron=
21-
(process.versions.electron||process.env.ELECTRON_RUN_AS_NODE)&&
22-
!process.env.VSCODE_PID;// Running from the test explorer in VS Code
20+
// These tests run in Electron (BoringSSL) for accurate certificate validation testing.
21+
22+
it("should run in Electron environment",()=>{
23+
expect(process.versions.electron).toBeTruthy();
24+
});
2325

2426
beforeAll(()=>{
2527
vi.mock("vscode",()=>{
@@ -114,8 +116,7 @@ describe("Certificate errors", () => {
114116
});
115117

116118
// In Electron a self-issued certificate without the signing capability fails
117-
// (again with the same "unable to verify" error) but in Node self-issued
118-
// certificates are not required to have the signing capability.
119+
// (again with the same "unable to verify" error)
119120
it("detects self-signed certificates without signing capability",async()=>{
120121
constaddress=awaitstartServer("no-signing");
121122
constrequest=axios.get(address,{
@@ -124,26 +125,16 @@ describe("Certificate errors", () => {
124125
servername:"localhost",
125126
}),
126127
});
127-
if(isElectron){
128-
awaitexpect(request).rejects.toHaveProperty(
129-
"code",
130-
X509_ERR_CODE.UNABLE_TO_VERIFY_LEAF_SIGNATURE,
131-
);
132-
try{
133-
awaitrequest;
134-
}catch(error){
135-
constwrapped=awaitCertificateError.maybeWrap(
136-
error,
137-
address,
138-
logger,
139-
);
140-
expect(wrappedinstanceofCertificateError).toBeTruthy();
141-
expect((wrappedasCertificateError).x509Err).toBe(
142-
X509_ERR.NON_SIGNING,
143-
);
144-
}
145-
}else{
146-
awaitexpect(request).resolves.toHaveProperty("data","foobar");
128+
awaitexpect(request).rejects.toHaveProperty(
129+
"code",
130+
X509_ERR_CODE.UNABLE_TO_VERIFY_LEAF_SIGNATURE,
131+
);
132+
try{
133+
awaitrequest;
134+
}catch(error){
135+
constwrapped=awaitCertificateError.maybeWrap(error,address,logger);
136+
expect(wrappedinstanceofCertificateError).toBeTruthy();
137+
expect((wrappedasCertificateError).x509Err).toBe(X509_ERR.NON_SIGNING);
147138
}
148139
});
149140

@@ -157,6 +148,24 @@ describe("Certificate errors", () => {
157148
awaitexpect(request).resolves.toHaveProperty("data","foobar");
158149
});
159150

151+
// Node's X509Certificate.keyUsage is unreliable, so use a third-party parser
152+
it("parses no-signing cert keyUsage with third-party library",async()=>{
153+
constcertPem=awaitfs.readFile(
154+
getFixturePath("tls","no-signing.crt"),
155+
"utf-8",
156+
);
157+
158+
// Node's implementation seems to always return `undefined`
159+
constnodeCert=newX509CertificateNode(certPem);
160+
expect(nodeCert.keyUsage).toBeUndefined();
161+
162+
// Here we can correctly get the KeyUsages
163+
constpeculiarCert=newX509CertificatePeculiar(certPem);
164+
constextension=peculiarCert.getExtension(KeyUsagesExtension);
165+
expect(extension).toBeDefined();
166+
expect(extension?.usages).toBeTruthy();
167+
});
168+
160169
// Both environments give the same error code when a self-issued certificate is
161170
// untrusted.
162171
it("detects self-signed certificates",async()=>{

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp