From 8818dc98d8a1bf1c81813064199cdf7d475883a4 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 14 Jul 2026 10:02:51 -0600 Subject: [PATCH 1/3] feat: encrypt stored SSH deploy keys at rest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `add_ssh_key` wrote the private key plaintext to `/ssh/.key` and replicated the raw key in the operation body, so the key material transited replication and landed on every peer's disk. Log redaction only ever covered the log surface. The key is now sealed into an `enc:v1:` envelope (the same envelope encryption backing the hdb_secret store) before it touches disk or the replicated op body, closing both the at-rest and in-op-body exposures. A key that arrives already sealed — replicated from a peer, or cloned from the leader — is stored verbatim and never decrypted to forward it, so `get_ssh_key`/`cloneSSHKeys` carry ciphertext too. Core decrypts to a transient 0600 file only for the lifetime of a git invocation (`materializeGitSSH`). Degraded mode: with no secret custody registered there is no key to seal against, so the key is stored as plaintext — today's behavior — with a loud WARN. This follows core's `ingestRegistryAuth`, which likewise passes a literal credential through rather than failing the operation when custody is absent; SSH keys predate custody and must keep working on a node that has none. `update_ssh_key` rotation and `list_ssh_keys` (names only) are externally unchanged. Refs #581 Co-Authored-By: Claude Opus --- security/sshKeyOperations.ts | 96 +++++++- unitTests/security/sshKeyOperations.test.mjs | 228 +++++++++++++++++++ 2 files changed, 312 insertions(+), 12 deletions(-) create mode 100644 unitTests/security/sshKeyOperations.test.mjs diff --git a/security/sshKeyOperations.ts b/security/sshKeyOperations.ts index 5a7b19706..f75b0b105 100644 --- a/security/sshKeyOperations.ts +++ b/security/sshKeyOperations.ts @@ -7,6 +7,9 @@ import harperLogger from '../core/utility/logging/harper_logger.js'; import { ClientError } from '../core/utility/errors/hdbError.js'; import { CONFIG_PARAMS } from '../core/utility/hdbTerms.ts'; import * as env from '../core/utility/environment/environmentManager.js'; +import { getSecretCustody } from '../core/resources/secretDecryptor.ts'; +import { encryptEnvelope, parseEnvelopeFields } from '../core/utility/secretEnvelope.ts'; +import { ENV_ENCRYPTED_PREFIX } from '../core/utility/envFile.ts'; import { replicateOperation } from '../replication/replicator.ts'; // SSH key name can only be alphanumeric, dash and underscores @@ -20,9 +23,57 @@ const exists = async (path: string): Promise => .catch(() => false); // Helper function to write a file ensuring the directory exists -async function writeFileEnsureDir(filePath: string, data: string) { +async function writeFileEnsureDir(filePath: string, data: string, mode?: number) { await mkdir(dirname(filePath), { recursive: true }); - await writeFile(filePath, data); + await writeFile(filePath, data, mode === undefined ? undefined : { mode }); +} + +/** + * Seal a private key for storage and replication. The `enc:v1:` envelope is what lands on disk and + * what goes into the replicated operation body, so the plaintext key reaches neither a peer's disk + * nor the wire. + * + * A key that arrives already sealed — replicated from a peer, or fetched from the leader by + * `cloneSSHKeys` — is stored verbatim and never decrypted here, so forwarding a key never requires + * holding its plaintext. Its `kid` is checked against this node's custody exactly the way + * `set_secret` vets an ingested envelope. + * + * Degraded mode: with no custody registered there is no key to seal against, so the key is stored + * as plaintext — today's behavior — and a WARN says so plainly. This follows the precedent set by + * core's `ingestRegistryAuth`, which likewise passes a literal credential through rather than + * failing the operation when custody is absent: SSH keys predate custody and must keep working on + * a node that has none. Custody is present by default (the file tier generates a cluster keypair on + * first boot), so this is the exception rather than the path. + */ +function sealSSHKey(name: string, key: string): string { + const custody = getSecretCustody(); + + if (key.startsWith(ENV_ENCRYPTED_PREFIX)) { + let kid: string | undefined; + try { + kid = parseEnvelopeFields(key.slice(ENV_ENCRYPTED_PREFIX.length)).kid; + } catch (error) { + throw new ClientError(`Invalid SSH key envelope: ${(error as Error).message}`); + } + const fingerprint = custody?.getPublicKey().fingerprint; + if (fingerprint && kid && kid !== fingerprint) { + throw new ClientError( + `SSH key envelope kid '${kid}' does not match this cluster's secrets key (expected '${fingerprint}')` + ); + } + return key; + } + + if (!custody) { + harperLogger?.warn( + `SSH key '${name}' is being stored and replicated in PLAINTEXT: no secret custody is registered on this node. ` + + 'Configure secret custody (`secretCustody` in the Harper config) so deploy keys are encrypted at rest.' + ); + return key; + } + + const { publicKey, fingerprint } = custody.getPublicKey(); + return ENV_ENCRYPTED_PREFIX + encryptEnvelope(key, publicKey, fingerprint); } const addValidationSchema = Joi.object({ @@ -78,9 +129,13 @@ interface AddSSHKeyRequest { * known_hosts entries. If the hostname is `github.com`, GitHub's public SSH * keys are automatically fetched and added to the known_hosts file. * + * The private key is sealed (`sealSSHKey`) before it reaches disk or the replicated operation + * body; core decrypts it to a transient file only for the lifetime of a git invocation + * (`materializeGitSSH`). + * * @param req - The request object containing the SSH key details. * @param req.name - The name of the SSH key to add. - * @param req.key - The SSH key contents to write to disk. + * @param req.key - The SSH key contents, either plaintext or an `enc:v1:` envelope. * @param req.host - The Host alias to use in the SSH config block. * @param req.hostname - The HostName (real hostname) to use in the SSH config block. * @param req.known_hosts - Optional known_hosts entries to append to the known_hosts file. @@ -100,8 +155,13 @@ export async function addSSHKey(req: AddSSHKeyRequest): Promise<{ message: strin throw new ClientError('Key already exists. Use update_ssh_key or delete_ssh_key and then add_ssh_key'); } + // Seal before anything durable or replicated happens, and replicate the envelope rather than + // the plaintext the caller supplied. + const storedKey = sealSSHKey(name, key); + req.key = storedKey; + // Create the key file - await writeFileEnsureDir(filePath, key); + await writeFileEnsureDir(filePath, storedKey, 0o600); await chmod(filePath, 0o600); // Build the config block string @@ -160,12 +220,17 @@ Host ${host} * Retrieves an SSH key by name, along with any associated Host and HostName * configuration from the SSH config file. * + * `key` is returned exactly as stored — an `enc:v1:` envelope on a node with secret custody, or + * plaintext on one without (see `sealSSHKey`). It is deliberately NOT decrypted: the only consumer + * is `cloneSSHKeys`, which feeds it straight back into `add_ssh_key` on the cloning node, and + * returning the envelope keeps the private key off the clone's wire as well as off its disk. + * * @param req - The request object containing the key name. * @param req.name - The name of the SSH key to retrieve. - * @returns An object containing the key name, the key contents, and optionally - * the Host and HostName from the SSH config file. + * @returns An object containing the key name, the stored key (sealed envelope or plaintext), and + * optionally the Host and HostName from the SSH config file. */ -async function getSSHKey(req: { +export async function getSSHKey(req: { name: string; }): Promise<{ name: string; key: string; host?: string; hostname?: string }> { const validation = validateBySchema(req, getSSHKeyValidationSchema); @@ -194,14 +259,18 @@ async function getSSHKey(req: { } /** - * Updates an existing SSH key by overwriting the key file with new contents. + * Updates an existing SSH key by overwriting the key file with new contents. Rotation semantics are + * unchanged; only the stored representation is sealed (see `sealSSHKey`). * * @param req - The request object containing the updated key details. * @param req.name - The name of the SSH key to update. - * @param req.key - The new SSH key contents to write to disk. + * @param req.key - The new SSH key contents, either plaintext or an `enc:v1:` envelope. * @returns An object containing a success message and optional replication results. */ -async function updateSSHKey(req: { name: string; key: string }): Promise<{ message: string; replicated?: unknown[] }> { +export async function updateSSHKey(req: { + name: string; + key: string; +}): Promise<{ message: string; replicated?: unknown[] }> { const validation = validateBySchema(req, updateSSHKeyValidationSchema); if (validation) throw new ClientError(validation.message); @@ -213,7 +282,10 @@ async function updateSSHKey(req: { name: string; key: string }): Promise<{ messa throw new ClientError(`SSH key '${name}' does not exist. Use add_ssh_key to create it.`); } - await writeFileEnsureDir(filePath, key); + const storedKey = sealSSHKey(name, key); + req.key = storedKey; + + await writeFileEnsureDir(filePath, storedKey, 0o600); await chmod(filePath, 0o600); const response = await replicateOperation(req); @@ -262,7 +334,7 @@ async function deleteSSHKey(req: { name: string }): Promise<{ message: string; r * @returns An array of objects containing the key name and optionally * the Host and HostName from the SSH config file. */ -async function listSSHKeys(): Promise<{ name: string; host?: string; hostname?: string }[]> { +export async function listSSHKeys(): Promise<{ name: string; host?: string; hostname?: string }[]> { const { sshDir, configFile } = getSSHPaths(undefined); if (!(await exists(sshDir))) return []; diff --git a/unitTests/security/sshKeyOperations.test.mjs b/unitTests/security/sshKeyOperations.test.mjs new file mode 100644 index 000000000..d10ede5d8 --- /dev/null +++ b/unitTests/security/sshKeyOperations.test.mjs @@ -0,0 +1,228 @@ +/** + * SSH deploy keys are sealed at rest (harper-pro#581). Before this, `add_ssh_key` wrote the private + * key plaintext to `/ssh/.key` AND replicated the raw key in the operation body, so + * the key material landed on every peer's disk. + * + * Coverage here is the ingest half — the key is sealed into an `enc:v1:` envelope before it touches + * disk or the replicated op body: + * - add/update seal, and the object handed to `replicateOperation` carries the envelope, not the key + * - an already-sealed key (replicated from a peer, or cloned from the leader via `get_ssh_key`) + * is stored verbatim rather than double-sealed or decrypted + * - an envelope sealed under a different cluster key is refused + * - `update_ssh_key` rotation and `list_ssh_keys` (names only) are externally unchanged + * - degraded mode: no custody → plaintext (today's behavior) plus a loud WARN + * + * The at-use half — decrypting to a transient 0600 file for the git invocation — lives in core + * (`materializeGitSSH`) and is covered by core's Application tests. + */ +import assert from 'node:assert/strict'; +import { mkdtempSync, rmSync, readFileSync, statSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { generateKeyPairSync } from 'node:crypto'; + +const PRIVATE_KEY = '-----BEGIN OPENSSH PRIVATE KEY-----\nb3BlbnNzaC1rZXktdjEAAAA\n-----END OPENSSH PRIVATE KEY-----\n'; +const ROTATED_KEY = + '-----BEGIN OPENSSH PRIVATE KEY-----\ncm90YXRlZC1rZXktbWF0ZXJpYWw\n-----END OPENSSH PRIVATE KEY-----\n'; + +function makePem() { + return generateKeyPairSync('rsa', { + modulusLength: 2048, + publicKeyEncoding: { type: 'spki', format: 'pem' }, + privateKeyEncoding: { type: 'pkcs8', format: 'pem' }, + }).privateKey; +} + +describe('sshKeyOperations sealing', () => { + let rootDir; + let sshDir; + let ops; + let core; // secretDecryptor + let custodyModule; + let envMgr; + let terms; + let server; + let harperLogger; + let warnings; + let originalWarn; + + // addSSHKey/updateSSHKey call replicateOperation, which dispatches to `server.nodes`. An empty + // peer list exercises the real replication path (including the `req` it would have sent) without + // a network. + const request = (fields) => ({ ...fields }); + + before(async function () { + this.timeout(60000); + envMgr = await import('#src/core/utility/environment/environmentManager'); + terms = await import('#src/core/utility/hdbTerms'); + ({ server } = await import('#src/core/server/Server')); + core = await import('#src/core/resources/secretDecryptor'); + custodyModule = await import('#src/security/keyCustody'); + harperLogger = (await import('#src/core/utility/logging/harper_logger')).default; + ops = await import('#src/security/sshKeyOperations'); + }); + + beforeEach(() => { + rootDir = mkdtempSync(join(tmpdir(), 'ssh-key-ops-test-')); + sshDir = join(rootDir, 'ssh'); + envMgr.setProperty(terms.CONFIG_PARAMS.ROOTPATH, rootDir); + server.nodes = []; + + warnings = []; + originalWarn = harperLogger.warn; + harperLogger.warn = (...args) => warnings.push(args.join(' ')); + + // the file tier is what a real node runs: a cluster-shared keypair every peer holds + custodyModule.resetKeyCustodyForTests(); + core.registerSecretCustody(custodyModule.buildCustody(custodyModule.custodyKeysFromPem(makePem()))); + }); + + afterEach(() => { + harperLogger.warn = originalWarn; + core.clearSecretCustody(); + custodyModule.resetKeyCustodyForTests(); + rmSync(rootDir, { recursive: true, force: true }); + }); + + const storedKeyFor = (name) => readFileSync(join(sshDir, `${name}.key`), 'utf8'); + const decrypt = (value) => core.getSecretCustody().decrypt(value); + + it('seals the private key before it reaches disk, and replicates the envelope rather than the key', async () => { + const req = request({ name: 'deploy', key: PRIVATE_KEY, host: 'gh', hostname: 'example.com' }); + const response = await ops.addSSHKey(req); + + assert.equal(response.message, 'Added ssh key: deploy'); + + // on disk: an envelope, never the key material + const stored = storedKeyFor('deploy'); + assert.ok(stored.startsWith('enc:v1:'), 'key file should hold an enc:v1: envelope'); + assert.ok(!stored.includes('OPENSSH PRIVATE KEY'), 'plaintext key must not be on disk'); + assert.equal(decrypt(stored), PRIVATE_KEY, 'the envelope must round-trip to the original key'); + assert.equal(statSync(join(sshDir, 'deploy.key')).mode & 0o777, 0o600); + + // on the wire: replicateOperation is handed this same `req`, so the op body must already + // carry the envelope — this is the peer's-disk exposure the issue is about + assert.ok(req.key.startsWith('enc:v1:'), 'the replicated op body must carry the envelope'); + assert.equal(req.key, stored); + assert.notEqual(req.key, PRIVATE_KEY); + }); + + it('writes the ssh config block pointing at the durable key path (core repoints it at the transient copy)', async () => { + await ops.addSSHKey(request({ name: 'deploy', key: PRIVATE_KEY, host: 'gh', hostname: 'example.com' })); + + const config = readFileSync(join(sshDir, 'config'), 'utf8'); + assert.match(config, /IdentityFile .*deploy\.key/); + assert.ok(!config.includes('OPENSSH PRIVATE KEY')); + }); + + it('stores an already-sealed key verbatim (peer replication / clone from leader) without double-sealing', async () => { + // what a peer receives, or what cloneSSHKeys reads back from the leader via get_ssh_key + await ops.addSSHKey(request({ name: 'origin', key: PRIVATE_KEY, host: 'gh', hostname: 'example.com' })); + const envelope = storedKeyFor('origin'); + + const req = request({ name: 'replica', key: envelope, host: 'gh', hostname: 'example.com' }); + await ops.addSSHKey(req); + + assert.equal(storedKeyFor('replica'), envelope, 'the envelope should be stored as-is'); + assert.equal(req.key, envelope, 'and forwarded as-is — never decrypted to forward'); + assert.equal(decrypt(storedKeyFor('replica')), PRIVATE_KEY); + }); + + it('refuses an envelope sealed under a different cluster key', async () => { + const foreign = await import('#src/core/utility/secretEnvelope'); + const otherPem = makePem(); + const otherKeys = custodyModule.custodyKeysFromPem(otherPem); + const fileModule = await import('#src/security/fileKeyCustody'); + const foreignEnvelope = + 'enc:v1:' + foreign.encryptEnvelope(PRIVATE_KEY, fileModule.publicPemOf(otherPem), otherKeys.activeKid); + + await assert.rejects( + ops.addSSHKey(request({ name: 'foreign', key: foreignEnvelope, host: 'gh', hostname: 'example.com' })), + /does not match this cluster's secrets key/ + ); + }); + + it('refuses a malformed envelope', async () => { + await assert.rejects( + ops.addSSHKey(request({ name: 'bad', key: 'enc:v1:not-an-envelope', host: 'gh', hostname: 'example.com' })), + /Invalid SSH key envelope/ + ); + }); + + it('update_ssh_key rotates to a new sealed key, unchanged externally', async () => { + await ops.addSSHKey(request({ name: 'deploy', key: PRIVATE_KEY, host: 'gh', hostname: 'example.com' })); + const before = storedKeyFor('deploy'); + + const req = request({ name: 'deploy', key: ROTATED_KEY }); + const response = await ops.updateSSHKey(req); + + assert.equal(response.message, 'Updated ssh key: deploy'); + const after = storedKeyFor('deploy'); + assert.notEqual(after, before, 'rotation must replace the stored envelope'); + assert.ok(after.startsWith('enc:v1:')); + assert.equal(decrypt(after), ROTATED_KEY); + assert.ok(req.key.startsWith('enc:v1:'), 'rotation must not replicate the raw key either'); + }); + + it('list_ssh_keys still returns names (and host config) only — never key material', async () => { + await ops.addSSHKey(request({ name: 'alpha', key: PRIVATE_KEY, host: 'gh', hostname: 'example.com' })); + await ops.addSSHKey(request({ name: 'beta', key: ROTATED_KEY, host: 'gl', hostname: 'example.org' })); + + const listed = await ops.listSSHKeys(); + + assert.deepEqual(listed.map((entry) => entry.name).sort(), ['alpha', 'beta']); + for (const entry of listed) { + assert.equal(entry.key, undefined); + assert.ok('host' in entry && 'hostname' in entry); + } + }); + + it('get_ssh_key returns the stored envelope, so the clone path carries ciphertext too', async () => { + await ops.addSSHKey(request({ name: 'deploy', key: PRIVATE_KEY, host: 'gh', hostname: 'example.com' })); + + const fetched = await ops.getSSHKey({ name: 'deploy' }); + + assert.ok(fetched.key.startsWith('enc:v1:')); + assert.ok(!fetched.key.includes('OPENSSH PRIVATE KEY')); + assert.equal(fetched.host, 'gh'); + assert.equal(fetched.hostname, 'example.com'); + }); + + describe('degraded mode (no secret custody on this node)', () => { + beforeEach(() => { + core.clearSecretCustody(); + }); + + it('falls back to the plaintext key file and says so at WARN', async () => { + const req = request({ name: 'deploy', key: PRIVATE_KEY, host: 'gh', hostname: 'example.com' }); + await ops.addSSHKey(req); + + assert.equal(storedKeyFor('deploy'), PRIVATE_KEY, 'degraded mode keeps the pre-#581 behavior'); + assert.equal(statSync(join(sshDir, 'deploy.key')).mode & 0o777, 0o600); + + const warned = warnings.find((line) => line.includes('PLAINTEXT')); + assert.ok(warned, `expected a plaintext WARN, got: ${JSON.stringify(warnings)}`); + assert.ok(warned.includes('deploy'), 'the WARN should name the key'); + assert.ok(!warned.includes('OPENSSH PRIVATE KEY'), 'the WARN must not contain key material'); + }); + + it('still accepts a sealed key from a peer, storing it opaquely for a node that can decrypt it', async () => { + // a cluster where the custody key has not reached this node yet (e.g. mid-clone) must not + // reject replicated keys — it stores the envelope it cannot read + const custodyKeys = custodyModule.custodyKeysFromPem(makePem()); + const envelopeModule = await import('#src/core/utility/secretEnvelope'); + const fileModule = await import('#src/security/fileKeyCustody'); + const envelope = + 'enc:v1:' + + envelopeModule.encryptEnvelope( + PRIVATE_KEY, + fileModule.publicPemOf(custodyKeys.keys.get(custodyKeys.activeKid)), + custodyKeys.activeKid + ); + + await ops.addSSHKey(request({ name: 'replica', key: envelope, host: 'gh', hostname: 'example.com' })); + + assert.equal(storedKeyFor('replica'), envelope); + }); + }); +}); From cac4a38fc5195533fc1a5fe72614794a4af28c6f Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 14 Jul 2026 11:57:57 -0600 Subject: [PATCH 2/3] fix: guard against undefined getPublicKey() result in sealSSHKey Chain optional access through getPublicKey() so a custody implementation returning undefined fails gracefully instead of throwing a TypeError, matching the existing optional-chaining intent on custody itself. Addresses gemini-code-assist review comment on PR #582. --- security/sshKeyOperations.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/sshKeyOperations.ts b/security/sshKeyOperations.ts index f75b0b105..fa3eff9b4 100644 --- a/security/sshKeyOperations.ts +++ b/security/sshKeyOperations.ts @@ -55,7 +55,7 @@ function sealSSHKey(name: string, key: string): string { } catch (error) { throw new ClientError(`Invalid SSH key envelope: ${(error as Error).message}`); } - const fingerprint = custody?.getPublicKey().fingerprint; + const fingerprint = custody?.getPublicKey()?.fingerprint; if (fingerprint && kid && kid !== fingerprint) { throw new ClientError( `SSH key envelope kid '${kid}' does not match this cluster's secrets key (expected '${fingerprint}')` From 868407ad9b2be91bafa3138dfc5f5c29340541e0 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 14 Jul 2026 17:09:19 -0600 Subject: [PATCH 3/3] fix: update SSH key integration tests for encrypted-at-rest reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit get_ssh_key now returns the sealed enc:v1: envelope by design (#581) — update the test's expectation to match instead of the stale plaintext assertion. The delete_ssh_key failure was a side effect: the prior assertion failure skipped that test's cleanup, leaving a stray key for the next test to trip over. Co-Authored-By: Claude Sonnet 5 --- integrationTests/security/sshKeyOperations.test.mjs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/integrationTests/security/sshKeyOperations.test.mjs b/integrationTests/security/sshKeyOperations.test.mjs index 7bdcd1da0..0628f1777 100644 --- a/integrationTests/security/sshKeyOperations.test.mjs +++ b/integrationTests/security/sshKeyOperations.test.mjs @@ -59,7 +59,13 @@ suite('SSH Key Operations', (ctx) => { ({ status, data } = await sendOperation(ctx.harper, { operation: 'get_ssh_key', name: 'testkey1' })); equal(status, 200); - deepEqual(data, { name: 'testkey1', host: 'testkey1.gitlab.com', hostname: 'gitlab.com', key: 'random\nstring' }); + equal(data.name, 'testkey1'); + equal(data.host, 'testkey1.gitlab.com'); + equal(data.hostname, 'gitlab.com'); + // Keys are sealed at rest (harper-pro#581) and get_ssh_key returns the envelope as-is — + // the only consumer is cloneSSHKeys, which never needs the plaintext. + ok(data.key.startsWith('enc:v1:'), 'expected key to be returned as an enc:v1: envelope'); + ok(!data.key.includes('random\nstring'), 'expected key to not be returned in plaintext'); // cleanup await sendOperation(ctx.harper, { operation: 'delete_ssh_key', name: 'testkey1' });