From 9c31dd41c7e7bc626828c3543bcb7092819a5070 Mon Sep 17 00:00:00 2001 From: ZacharyZcR Date: Thu, 8 Oct 2026 05:37:50 +0800 Subject: [PATCH] fix(ssh): normalize PKCS8 private keys before connecting (#1570) --- package-lock.json | 94 +++++++++++++++++++ package.json | 2 + src/backend/tests/utils/ssh-key-utils.test.ts | 64 +++++++++++++ src/backend/utils/ssh-key-utils.ts | 23 ++++- 4 files changed, 182 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index f09c0cfdd..39e2ea9f8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -38,6 +38,7 @@ "sharp": "^0.35.4", "socks": "^2.8.10", "ssh2": "^1.17.0", + "sshpk": "^1.18.0", "tar": "^7.5.22", "undici": "^8.10.2", "ws": "^8.21.3" @@ -82,6 +83,7 @@ "@types/react": "^19.2.18", "@types/react-dom": "^19.2.4", "@types/ssh2": "^1.15.6", + "@types/sshpk": "^1.17.5", "@types/supertest": "^7.2.1", "@types/ws": "^8.18.1", "@uiw/react-codemirror": "^4.25.11", @@ -6073,6 +6075,16 @@ "dev": true, "license": "MIT" }, + "node_modules/@types/asn1": { + "version": "0.2.4", + "resolved": "https://registry.npmjs.org/@types/asn1/-/asn1-0.2.4.tgz", + "integrity": "sha512-V91DSJ2l0h0gRhVP4oBfBzRBN9lAbPUkGDMCnwedqPKX2d84aAMc9CulOvxdw1f7DfEYx99afab+Rsm3e52jhA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/node": "*" + } + }, "node_modules/@types/better-sqlite3": { "version": "9.6.0", "resolved": "https://registry.npmjs.org/@types/better-sqlite3/-/better-sqlite3-9.6.0.tgz", @@ -6367,6 +6379,17 @@ "dev": true, "license": "MIT" }, + "node_modules/@types/sshpk": { + "version": "1.17.5", + "resolved": "https://registry.npmjs.org/@types/sshpk/-/sshpk-1.17.5.tgz", + "integrity": "sha512-khJbOFAOnyLaqOoGIlDCF2ZghIb0jM1K1N0rRoKL0ObqUMwfkicu5zlJ5Wsp+Xn2EUduVwONXwsgbGk+COaCug==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/asn1": "*", + "@types/node": "*" + } + }, "node_modules/@types/superagent": { "version": "8.1.11", "resolved": "https://registry.npmjs.org/@types/superagent/-/superagent-8.1.11.tgz", @@ -7265,6 +7288,15 @@ "node": ">=12.0.0" } }, + "node_modules/assert-plus": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/assert-plus/-/assert-plus-1.0.0.tgz", + "integrity": "sha512-NfJ4UzBCcQGLDlQq7nHxH+tv3kyZ0hHQqF5BO6J7tNJeP5do1llPr8dZ8zHonfhAu0PHAdMkSo+8o0wxg9lZWw==", + "license": "MIT", + "engines": { + "node": ">=0.8" + } + }, "node_modules/assertion-error": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/assertion-error/-/assertion-error-2.0.1.tgz", @@ -8279,6 +8311,18 @@ "dev": true, "license": "MIT" }, + "node_modules/dashdash": { + "version": "1.14.1", + "resolved": "https://registry.npmjs.org/dashdash/-/dashdash-1.14.1.tgz", + "integrity": "sha512-jRFi8UDGo6j+odZiEpjazZaWqEal3w/basFjQHQEwVtZJGDpxbH1MeYluwCS8Xq5wmLJooDlMgvVarmWfGM44g==", + "license": "MIT", + "dependencies": { + "assert-plus": "^1.0.0" + }, + "engines": { + "node": ">=0.10" + } + }, "node_modules/data-urls": { "version": "7.0.0", "resolved": "https://registry.npmjs.org/data-urls/-/data-urls-7.0.0.tgz", @@ -8687,6 +8731,16 @@ "readable-stream": "^2.0.2" } }, + "node_modules/ecc-jsbn": { + "version": "0.1.2", + "resolved": "https://registry.npmjs.org/ecc-jsbn/-/ecc-jsbn-0.1.2.tgz", + "integrity": "sha512-eh9O+hwRHNbG4BLTjEl3nw044CkGm5X6LoaCf7LPp7UU8Qrt47JYNi6nPX8xjW97TKGKm1ouctg0QSpZe9qrnw==", + "license": "MIT", + "dependencies": { + "jsbn": "~0.1.0", + "safer-buffer": "^2.1.0" + } + }, "node_modules/ecdsa-sig-formatter": { "version": "1.0.11", "resolved": "https://registry.npmjs.org/ecdsa-sig-formatter/-/ecdsa-sig-formatter-1.0.11.tgz", @@ -9971,6 +10025,15 @@ "url": "https://github.com/privatenumber/get-tsconfig?sponsor=1" } }, + "node_modules/getpass": { + "version": "0.1.7", + "resolved": "https://registry.npmjs.org/getpass/-/getpass-0.1.7.tgz", + "integrity": "sha512-0fzj9JxOLfJ+XGLhR8ze3unN0KZCgZwiSSDz168VERjK8Wl8kVSdcu2kspd4s4wtAa1y/qrVRiAA0WclVsu0ng==", + "license": "MIT", + "dependencies": { + "assert-plus": "^1.0.0" + } + }, "node_modules/glob": { "version": "12.0.0", "resolved": "https://registry.npmjs.org/glob/-/glob-12.0.0.tgz", @@ -10656,6 +10719,12 @@ "js-yaml": "bin/js-yaml.js" } }, + "node_modules/jsbn": { + "version": "0.1.1", + "resolved": "https://registry.npmjs.org/jsbn/-/jsbn-0.1.1.tgz", + "integrity": "sha512-UVU9dibq2JcFWxQPA6KCqj5O42VOmAY3zQUfEKxU0KpTGXwNoCjkX1e13eHNvw/xPynt6pU0rZ1htjWTNTSXsg==", + "license": "MIT" + }, "node_modules/jsdom": { "version": "30.1.1", "resolved": "https://registry.npmjs.org/jsdom/-/jsdom-30.1.1.tgz", @@ -13429,6 +13498,31 @@ "nan": "^2.23.0" } }, + "node_modules/sshpk": { + "version": "1.18.0", + "resolved": "https://registry.npmjs.org/sshpk/-/sshpk-1.18.0.tgz", + "integrity": "sha512-2p2KJZTSqQ/I3+HX42EpYOa2l3f8Erv8MWKsy2I9uf4wA7yFIkXRffYdsx86y6z4vHtV8u7g+pPlr8/4ouAxsQ==", + "license": "MIT", + "dependencies": { + "asn1": "~0.2.3", + "assert-plus": "^1.0.0", + "bcrypt-pbkdf": "^1.0.0", + "dashdash": "^1.12.0", + "ecc-jsbn": "~0.1.1", + "getpass": "^0.1.1", + "jsbn": "~0.1.0", + "safer-buffer": "^2.0.2", + "tweetnacl": "~0.14.0" + }, + "bin": { + "sshpk-conv": "bin/sshpk-conv", + "sshpk-sign": "bin/sshpk-sign", + "sshpk-verify": "bin/sshpk-verify" + }, + "engines": { + "node": ">=0.10.0" + } + }, "node_modules/stackback": { "version": "0.0.2", "resolved": "https://registry.npmjs.org/stackback/-/stackback-0.0.2.tgz", diff --git a/package.json b/package.json index 8acf31cfa..5690d7bb2 100644 --- a/package.json +++ b/package.json @@ -73,6 +73,7 @@ "sharp": "^0.35.4", "socks": "^2.8.10", "ssh2": "^1.17.0", + "sshpk": "^1.18.0", "tar": "^7.5.22", "undici": "^8.10.2", "ws": "^8.21.3" @@ -117,6 +118,7 @@ "@types/react": "^19.2.18", "@types/react-dom": "^19.2.4", "@types/ssh2": "^1.15.6", + "@types/sshpk": "^1.17.5", "@types/supertest": "^7.2.1", "@types/ws": "^8.18.1", "@uiw/react-codemirror": "^4.25.11", diff --git a/src/backend/tests/utils/ssh-key-utils.test.ts b/src/backend/tests/utils/ssh-key-utils.test.ts index 534eb6cc7..16c9b32ca 100644 --- a/src/backend/tests/utils/ssh-key-utils.test.ts +++ b/src/backend/tests/utils/ssh-key-utils.test.ts @@ -1,3 +1,6 @@ +import { generateKeyPairSync } from "node:crypto"; +import ssh2 from "ssh2"; +import { parsePrivateKey } from "sshpk"; import { describe, it, expect } from "vitest"; import { readFileSync } from "node:fs"; import { @@ -158,3 +161,64 @@ describe("validateKeyPair", () => { expect(result.error).toMatch(/private key/i); }); }); + +describe("PKCS#8 runtime key normalization", () => { + it.each(["ed25519", "rsa", "ec"] as const)( + "converts %s without changing its public key", + (type) => { + const { privateKey, publicKey } = + type === "rsa" + ? generateKeyPairSync("rsa", { modulusLength: 2048 }) + : type === "ec" + ? generateKeyPairSync("ec", { namedCurve: "prime256v1" }) + : generateKeyPairSync("ed25519"); + const pem = privateKey + .export({ type: "pkcs8", format: "pem" }) + .toString(); + const prepared = preparePrivateKeyForSSH2(pem); + const parsed = ssh2.utils.parseKey(prepared); + expect(parsed).not.toBeInstanceOf(Error); + if (parsed instanceof Error) throw parsed; + expect( + parsePrivateKey(prepared).toPublic().toBuffer("pkcs8").toString(), + ).toBe(publicKey.export({ type: "spki", format: "pem" }).toString()); + }, + ); + + it("decrypts encrypted PKCS#8 only with the correct passphrase", () => { + const { privateKey } = generateKeyPairSync("ed25519"); + const pem = privateKey + .export({ + type: "pkcs8", + format: "pem", + cipher: "aes-256-cbc", + passphrase: " key passphrase ", + }) + .toString(); + expect( + ssh2.utils.parseKey(preparePrivateKeyForSSH2(pem, " key passphrase ")), + ).not.toBeInstanceOf(Error); + expect(() => preparePrivateKeyForSSH2(pem)).toThrow(); + expect(() => preparePrivateKeyForSSH2(pem, "wrong")).toThrow(); + }); + + it("keeps an already usable OpenSSH key unchanged", () => { + expect(preparePrivateKeyForSSH2(ED25519_PRIVATE).toString()).toBe( + ED25519_PRIVATE, + ); + }); + + it("rejects unsupported X25519 keys", () => { + const { privateKey } = generateKeyPairSync("x25519"); + const pem = privateKey.export({ type: "pkcs8", format: "pem" }).toString(); + expect(() => preparePrivateKeyForSSH2(pem)).toThrow(); + }); + + it("rejects malformed PKCS#8 instead of guessing from its header", () => { + expect(() => + preparePrivateKeyForSSH2( + "-----BEGIN PRIVATE KEY-----\nAAAA\n-----END PRIVATE KEY-----", + ), + ).toThrow(); + }); +}); diff --git a/src/backend/utils/ssh-key-utils.ts b/src/backend/utils/ssh-key-utils.ts index f0463feea..c8834b973 100644 --- a/src/backend/utils/ssh-key-utils.ts +++ b/src/backend/utils/ssh-key-utils.ts @@ -1,3 +1,5 @@ +import { createPrivateKey } from "node:crypto"; +import { parsePrivateKey } from "sshpk"; import { getErrorMessage } from "./error-message.js"; import ssh2Pkg from "ssh2"; const ssh2Utils = ssh2Pkg.utils; @@ -246,7 +248,26 @@ export function preparePrivateKeyForSSH2( privateKeyData: string, passphrase?: string, ): Buffer { - const cleanKey = normalizePrivateKeyText(privateKeyData); + let cleanKey = normalizePrivateKeyText(privateKeyData); + if ( + /^-----BEGIN (?:ENCRYPTED )?PRIVATE KEY-----/.test(cleanKey) && + ssh2Utils.parseKey(cleanKey, passphrase) instanceof Error + ) { + try { + const pem = createPrivateKey({ key: cleanKey, format: "pem", passphrase }) + .export({ type: "pkcs8", format: "pem" }) + .toString(); + cleanKey = parsePrivateKey(pem, "pkcs8").toString("openssh"); + } catch { + throw new Error( + cleanKey.startsWith("-----BEGIN ENCRYPTED PRIVATE KEY-----") + ? "Could not decrypt PKCS#8 private key; check the passphrase" + : "Invalid or unsupported PKCS#8 private key", + ); + } + const parsed = ssh2Utils.parseKey(cleanKey); + if (parsed instanceof Error) throw parsed; + } const keyInfo = parseSSHKey(cleanKey, passphrase); if (!keyInfo.success) {