Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 48 additions & 3 deletions electron/bridges/privateKeyNormalizer.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,38 @@ class UnsupportedPrivateKeyError extends Error {
}
}

// Matches a private-key PEM block by its BEGIN/END markers (which survive even
// when the surrounding newlines are lost), capturing the label and raw body.
const PEM_BLOCK_RE =
/-----BEGIN ((?:RSA |DSA |EC |OPENSSH |ENCRYPTED )?PRIVATE KEY)-----([\s\S]*?)-----END \1-----/;

/**
* Rebuild clean PEM framing for a key whose text was mangled in transit —
* newlines collapsed to spaces, turned into literal "\n", or lines indented.
* Returns the repaired PEM, or null when it isn't a recoverable block.
*
* The base64 body is preserved byte-for-byte (only non-base64 characters are
* stripped before re-wrapping), so this can never produce a different key.
* Encrypted legacy PEM (Proc-Type / DEK-Info header lines inside the body) is
* left alone — those lines aren't base64 and can't be safely re-wrapped.
*/
function repairMalformedPem(text) {
// Newlines flattened into literal "\n" / "\r\n" escape sequences.
const unescaped = text.replace(/\\r\\n|\\n|\\r/g, "\n");
const match = PEM_BLOCK_RE.exec(unescaped);
if (!match) return null;

const label = match[1];
const body = match[2];
if (/Proc-Type:|DEK-Info:/i.test(body)) return null;

const base64 = body.replace(/[^A-Za-z0-9+/=]/g, "");
if (!base64) return null;

const wrapped = base64.replace(/.{1,64}/g, "$&\n").trimEnd();
return `-----BEGIN ${label}-----\n${wrapped}\n-----END ${label}-----\n`;
}

/**
* Normalize a private key into a form ssh2 can parse.
*
Expand All @@ -60,17 +92,29 @@ function normalizePrivateKeyForSsh2(privateKey, passphrase) {
return { privateKey, passphrase, converted: false };
}

// The key text may have been mangled before it reached us — newlines lost,
// turned into literal "\n", or lines indented. Rebuild clean PEM framing and
// retry; a repaired key also feeds cleanly into the PKCS#8 path below.
const repaired = repairMalformedPem(privateKey);
if (repaired && repaired !== privateKey) {
const reparsed = sshUtils.parseKey(repaired, passphrase);
if (reparsed && !(reparsed instanceof Error)) {
return { privateKey: repaired, passphrase, converted: true };
}
Comment on lines +99 to +103
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve repaired encrypted OpenSSH keys before prompting

For an encrypted OpenSSH key whose line breaks became literal \n, this path repairs the PEM but then discards it because ssh2.parseKey(repaired, undefined) fails without a passphrase, and the non-PKCS#8 fallback returns the original mangled text. In the first-party auth flow, that same literal-\n form is not reliably detected as encrypted before resolveKeyForAuth is called, so the user never gets a passphrase retry with the repaired key even though normalizePrivateKeyForSsh2(mangled, passphrase) would work. Please keep or surface the repaired candidate for encrypted non-PKCS#8 keys instead of falling back to the unrepaired input when the only missing piece is a passphrase.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified. The repaired candidate was indeed being discarded, but the reason there's no passphrase retry is one layer up: isKeyEncrypted() couldn't read the cipher from the mangled OpenSSH blob — the literal \n corrupts the base64 decode of the openssh-key-v1 header — so the key was routed to the unencrypted branch and never prompted. Surfacing the repaired candidate alone wouldn't help, since resolveKeyForAuth re-validates without a passphrase and drops it.

Fixed in 81e02ca by repairing the PEM framing before reading the cipher name in isKeyEncrypted, so the key is detected as encrypted and reaches the passphrase prompt — where normalizePrivateKeyForSsh2(key, passphrase) already repairs and validates it (exactly the path you noted would work). Added a regression test: a literal-\n encrypted OpenSSH key now triggers exactly one prompt and parses with the passphrase.

}
const candidate = repaired || privateKey;

// We can only rescue PKCS#8 keys, which Node's crypto can read.
if (!PKCS8_HEADER_RE.test(privateKey)) {
if (!PKCS8_HEADER_RE.test(candidate)) {
return { privateKey, passphrase, converted: false };
}

const encrypted = privateKey.includes("-----BEGIN ENCRYPTED PRIVATE KEY-----");
const encrypted = candidate.includes("-----BEGIN ENCRYPTED PRIVATE KEY-----");

let keyObject;
try {
keyObject = crypto.createPrivateKey(
passphrase ? { key: privateKey, passphrase } : privateKey,
passphrase ? { key: candidate, passphrase } : candidate,
);
} catch (err) {
if (encrypted) {
Expand Down Expand Up @@ -98,6 +142,7 @@ function normalizePrivateKeyForSsh2(privateKey, passphrase) {

module.exports = {
normalizePrivateKeyForSsh2,
repairMalformedPem,
PrivateKeyPassphraseError,
UnsupportedPrivateKeyError,
};
49 changes: 49 additions & 0 deletions electron/bridges/privateKeyNormalizer.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,52 @@ test("passes through content that is not a PKCS#8 key", () => {
assert.equal(result.converted, false);
assert.equal(result.privateKey, junk);
});

const indentLines = (key) =>
key.split("\n").map((line) => (line ? " " + line : line)).join("\n");
const dropEndLine = (key) => key.split("\n").slice(0, -2).join("\n");
const legacyEncryptedRsa = (passphrase) =>
crypto.generateKeyPairSync("rsa", {
modulusLength: 2048,
privateKeyEncoding: { type: "pkcs1", format: "pem", cipher: "aes-128-cbc", passphrase },
publicKeyEncoding: { type: "spki", format: "pem" },
}).privateKey;

test("repairs an RSA key whose newlines were collapsed to spaces", () => {
const result = normalizePrivateKeyForSsh2(rsaPkcs1().replace(/\n/g, " "));
assert.equal(result.converted, true);
assert.ok(parseOk(result.privateKey), "repaired key should be parseable by ssh2");
});

test("repairs an RSA key whose newlines became literal backslash-n", () => {
const result = normalizePrivateKeyForSsh2(rsaPkcs1().replace(/\n/g, "\\n"));
assert.equal(result.converted, true);
assert.ok(parseOk(result.privateKey));
});

test("repairs an RSA key whose lines are indented", () => {
const result = normalizePrivateKeyForSsh2(indentLines(rsaPkcs1()));
assert.equal(result.converted, true);
assert.ok(parseOk(result.privateKey));
});

test("repairs and converts a collapsed PKCS#8 key", () => {
const result = normalizePrivateKeyForSsh2(rsaPkcs8().replace(/\n/g, " "));
assert.equal(result.converted, true);
const parsed = parseOk(result.privateKey);
assert.ok(parsed);
assert.equal(parsed.type, "ssh-rsa");
});

test("cannot repair a truncated key and leaves it unchanged", () => {
const truncated = dropEndLine(rsaPkcs1());
const result = normalizePrivateKeyForSsh2(truncated);
assert.equal(result.converted, false);
assert.equal(result.privateKey, truncated);
});

test("does not attempt to repair an encrypted legacy PEM (DEK-Info)", () => {
const collapsed = legacyEncryptedRsa("secret").replace(/\n/g, " ");
const result = normalizePrivateKeyForSsh2(collapsed, "secret");
assert.equal(result.converted, false);
});
6 changes: 5 additions & 1 deletion electron/bridges/sshAuthHelper.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const keyboardInteractiveHandler = require("./keyboardInteractiveHandler.cjs");
const passphraseHandler = require("./passphraseHandler.cjs");
const {
normalizePrivateKeyForSsh2,
repairMalformedPem,
PrivateKeyPassphraseError,
} = require("./privateKeyNormalizer.cjs");

Expand Down Expand Up @@ -86,8 +87,11 @@ function isKeyEncrypted(keyContent) {
// Check for OpenSSH format keys
if (keyContent.includes("-----BEGIN OPENSSH PRIVATE KEY-----")) {
try {
// Repair mangled framing (lost or escaped newlines) first, so the cipher
// name can be read from the base64 blob even when the key was flattened.
const source = repairMalformedPem(keyContent) || keyContent;
// Extract the base64 content between the markers
const base64Match = keyContent.match(
const base64Match = source.match(
/-----BEGIN OPENSSH PRIVATE KEY-----\s*([\s\S]*?)\s*-----END OPENSSH PRIVATE KEY-----/
);
if (base64Match) {
Expand Down
42 changes: 42 additions & 0 deletions electron/bridges/sshAuthHelper.pkcs8.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const crypto = require("node:crypto");
const fs = require("node:fs");
const os = require("node:os");
const path = require("node:path");
const { spawnSync } = require("node:child_process");
const { utils: sshUtils } = require("ssh2");

const {
Expand Down Expand Up @@ -84,3 +85,44 @@ test("loadIdentityFileForAuth converts an unencrypted PKCS#8 identity file", asy
assert.ok(result, "expected a loaded identity file");
assert.ok(isParseable(result.privateKey), "prepared key should be parseable by ssh2");
});

test("preparePrivateKeyForAuth recovers a mangled encrypted OpenSSH key via passphrase prompt", async (t) => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "netcatty-mangled-openssh-"));
t.after(() => fs.rmSync(dir, { recursive: true, force: true }));
const keyPath = path.join(dir, "id_ed25519");
const gen = spawnSync(
"ssh-keygen",
["-q", "-t", "ed25519", "-N", "secret", "-f", keyPath, "-C", "netcatty-test"],
{ encoding: "utf8" },
);
if (gen.status !== 0) {
t.skip("ssh-keygen is unavailable");
return;
}
// Simulate a key whose line breaks were flattened into literal "\n" on paste.
const mangled = fs.readFileSync(keyPath, "utf8").replace(/\n/g, "\\n");

const originalRequest = passphraseHandler.requestPassphrase;
t.after(() => {
passphraseHandler.requestPassphrase = originalRequest;
});
let prompts = 0;
passphraseHandler.requestPassphrase = async () => {
prompts += 1;
return { passphrase: "secret" };
};

const result = await preparePrivateKeyForAuth({
sender,
privateKey: mangled,
keyName: "id_ed25519",
hostname: "example.test",
logPrefix: "[Test]",
});

assert.ok(result, "expected a prepared key");
assert.equal(prompts, 1, "the encrypted key should trigger exactly one passphrase prompt");
assert.equal(result.passphrase, "secret");
const parsed = sshUtils.parseKey(result.privateKey, result.passphrase);
assert.ok(parsed && !(parsed instanceof Error), "prepared key + passphrase should parse in ssh2");
});
Loading