diff --git a/packages/cli/src/daemon/http-utils.ts b/packages/cli/src/daemon/http-utils.ts index 462c71a57..6ef0f9384 100644 --- a/packages/cli/src/daemon/http-utils.ts +++ b/packages/cli/src/daemon/http-utils.ts @@ -576,10 +576,8 @@ export function shouldBypassRateLimitForLoopbackTraffic(ip: string, pathname: st } export function isValidContextGraphId(id: string): boolean { - if (!id || typeof id !== "string") return false; - if (id.length > 256) return false; - // Allow URNs, DIDs, simple slug-like identifiers, and URIs - return /^[\w:/.@\-]+$/.test(id); + if (typeof id !== "string") return false; + return validateContextGraphId(id).valid; } export function shortId(peerId: string): string { diff --git a/packages/cli/src/daemon/routes/context-graph.ts b/packages/cli/src/daemon/routes/context-graph.ts index 74c77cff7..65b01698d 100644 --- a/packages/cli/src/daemon/routes/context-graph.ts +++ b/packages/cli/src/daemon/routes/context-graph.ts @@ -669,7 +669,9 @@ export async function handleContextGraphRoutes(ctx: RequestContext): Promise MAX_SCRYPT_MEMORY_BYTES) { + throw new Error('Unsupported scrypt N for keystore encryption'); + } + SCRYPT_N = n; +} -function deriveKey(passphrase: string, salt: Buffer): Buffer { +function isPowerOfTwo(value: number): boolean { + return Number.isInteger(value) && value > 0 && Number.isInteger(Math.log2(value)); +} + +function assertSafeKdfParams(kdfparams: EncryptedKeystore['crypto']['kdfparams']): void { + if (!Number.isSafeInteger(kdfparams.n) || !isPowerOfTwo(kdfparams.n) || kdfparams.n < MIN_SCRYPT_N) { + throw new Error('KDF parameters below minimum: scrypt N too low'); + } + if (!Number.isSafeInteger(kdfparams.r) || kdfparams.r < MIN_SCRYPT_R) { + throw new Error('KDF parameters below minimum: scrypt r too low'); + } + if (!Number.isSafeInteger(kdfparams.p) || kdfparams.p < MIN_SCRYPT_P) { + throw new Error('KDF parameters below minimum: scrypt p too low'); + } + const estimatedMemoryBytes = 128 * kdfparams.n * kdfparams.r; + if (!Number.isSafeInteger(estimatedMemoryBytes) || estimatedMemoryBytes > MAX_SCRYPT_MEMORY_BYTES) { + throw new Error('Unsupported keystore KDF parameters: scrypt memory cost too high'); + } + if (kdfparams.p > MAX_SCRYPT_P) { + throw new Error('Unsupported keystore KDF parameters: scrypt p too high'); + } + if (kdfparams.dklen !== DKLEN) { + throw new Error(`Invalid dklen: dklen must be ${DKLEN}`); + } + if (!/^[0-9a-fA-F]+$/.test(kdfparams.salt) || kdfparams.salt.length % 2 !== 0 || kdfparams.salt.length < MIN_SALT_BYTES * 2) { + throw new Error(`KDF parameters below minimum: salt too short (minimum ${MIN_SALT_BYTES} bytes)`); + } +} + +function deriveKey( + passphrase: string, + salt: Buffer, + params: Pick, +): Buffer { return scryptSync(passphrase, salt, DKLEN, { - N: SCRYPT_N, - r: SCRYPT_R, - p: SCRYPT_P, + N: params.n, + r: params.r, + p: params.p, maxmem: 256 * 1024 * 1024, }); } @@ -56,7 +101,12 @@ export async function encryptKeystore( passphrase: string, ): Promise { const salt = randomBytes(32); - const key = deriveKey(passphrase, salt); + const key = deriveKey(passphrase, salt, { + n: SCRYPT_N, + r: SCRYPT_R, + p: SCRYPT_P, + dklen: DKLEN, + }); const iv = randomBytes(12); const cipher = createCipheriv('aes-256-gcm', key, iv); @@ -96,8 +146,9 @@ export async function decryptKeystore( } const { kdfparams } = keystore.crypto; + assertSafeKdfParams(kdfparams); const salt = Buffer.from(kdfparams.salt, 'hex'); - const key = deriveKey(passphrase, salt); + const key = deriveKey(passphrase, salt, kdfparams); const iv = Buffer.from(keystore.crypto.iv, 'hex'); const tag = Buffer.from(keystore.crypto.tag, 'hex'); diff --git a/packages/cli/test/daemon-http-behavior-extra.test.ts b/packages/cli/test/daemon-http-behavior-extra.test.ts index d311541aa..d50af07c7 100644 --- a/packages/cli/test/daemon-http-behavior-extra.test.ts +++ b/packages/cli/test/daemon-http-behavior-extra.test.ts @@ -634,6 +634,29 @@ describe('CLI-16 — Path traversal in context-graph IDs', () => { expect(body.error).toMatch(/context graph id|invalid/i); }); } + + it('rejects encoded traversal in context-graph path-param routes', async () => { + const d = daemon!; + const encoded = encodeURIComponent('../etc/passwd'); + const res = await fetch(urlFor(d, `/api/context-graph/${encoded}/participants`), { + method: 'GET', + headers: authHeaders(d), + }); + expect(res.status).toBe(400); + const body = await res.json().catch(() => ({})); + expect(body.error).toMatch(/contextGraphId|context graph id|invalid/i); + }); + + it('rejects malformed percent-encoding in context-graph path-param routes', async () => { + const d = daemon!; + const res = await fetch(urlFor(d, '/api/context-graph/%E0%A4%A/participants'), { + method: 'GET', + headers: authHeaders(d), + }); + expect(res.status).toBe(400); + const body = await res.json().catch(() => ({})); + expect(body.error).toMatch(/percent-encoding|malformed/i); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/cli/test/daemon-keystore-extra.test.ts b/packages/cli/test/daemon-keystore-extra.test.ts index 830325be9..ac2f51c65 100644 --- a/packages/cli/test/daemon-keystore-extra.test.ts +++ b/packages/cli/test/daemon-keystore-extra.test.ts @@ -80,31 +80,15 @@ describe('CLI-1 — scrypt KDF parameter floor (PROD-BUG: not enforced)', () => _setScryptN(SAFE_N); const ks = await encryptKeystore(PRIVKEY, PASSPHRASE); - // Re-encrypt using a toy N so we can faithfully construct a forged - // "weak" keystore (same ciphertext+IV+tag would not decrypt if we just - // mutated kdfparams because the derived key would differ). - _setScryptN(WEAK_N); - const weakKs = await encryptKeystore(PRIVKEY, PASSPHRASE); - _setScryptN(SAFE_N); - - // Sanity: this really is a weak keystore — the advertised N is the one we - // encrypted with. - expect(weakKs.crypto.kdfparams.n).toBe(WEAK_N); + await expect(() => _setScryptN(WEAK_N)).toThrow(/Unsupported scrypt N/); - // Sanity: the "strong" keystore is rejected if we lie about its N - // (tampered kdfparams → wrong key → GCM auth failure). + // Sanity: the "strong" keystore is rejected if we lie about its N. + // The loader should fail at KDF validation before attempting GCM. await expect( decryptKeystore(withKdfParams(ks, { n: WEAK_N }), PASSPHRASE), - ).rejects.toThrow(/Decryption failed/); - - // PROD-BUG: the below call SHOULD throw "KDF parameters below minimum" - // (or any rejection tied to the cost floor). Instead it returns the - // plaintext — which means any attacker who can write a keystore file - // can force an O(1)-to-brute-force KDF. See issue #11. - // - // This assertion stays RED until `decryptKeystore` enforces N >= 2**15, - // r >= 8, p >= 1. Leaving red-on-purpose. - await expect(decryptKeystore(weakKs, PASSPHRASE)).rejects.toThrow( + ).rejects.toThrow(/KDF parameters below minimum|scrypt N too low|weak keystore/i); + + await expect(decryptKeystore(withKdfParams(ks, { n: WEAK_N }), PASSPHRASE)).rejects.toThrow( /KDF parameters below minimum|scrypt cost too low|weak keystore/i, ); }); @@ -145,6 +129,18 @@ describe('CLI-1 — scrypt KDF parameter floor (PROD-BUG: not enforced)', () => ); }); + it('refuses KDF parameters above the supported envelope before calling scrypt', async () => { + _setScryptN(SAFE_N); + const ks = await encryptKeystore(PRIVKEY, PASSPHRASE); + + await expect(decryptKeystore(withKdfParams(ks, { n: 2 ** 30 }), PASSPHRASE)) + .rejects.toThrow(/memory cost too high|unsupported keystore/i); + await expect(decryptKeystore(withKdfParams(ks, { r: 65 }), PASSPHRASE)) + .rejects.toThrow(/memory cost too high|unsupported keystore/i); + await expect(decryptKeystore(withKdfParams(ks, { p: 64 }), PASSPHRASE)) + .rejects.toThrow(/scrypt p too high|unsupported keystore/i); + }); + it('refuses to decrypt a keystore with a short salt (<16 bytes)', async () => { _setScryptN(SAFE_N); const ks = await encryptKeystore(PRIVKEY, PASSPHRASE); diff --git a/packages/cli/test/http-utils.test.ts b/packages/cli/test/http-utils.test.ts new file mode 100644 index 000000000..38ddaeec9 --- /dev/null +++ b/packages/cli/test/http-utils.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from 'vitest'; +import { isValidContextGraphId } from '../src/daemon/http-utils.js'; + +describe('isValidContextGraphId', () => { + it('rejects traversal path segments', () => { + for (const id of [ + '../etc/passwd', + '../../root', + './../_private', + 'legit-cg/../../other-cg', + 'legit-cg/%2e%2e/other-cg', + ]) { + expect(isValidContextGraphId(id)).toBe(false); + } + }); + + it('keeps existing slug, DID, URN, and URL-style identifiers valid', () => { + for (const id of [ + 'devnet-test', + 'did:dkg:context-graph:devnet-test', + 'urn:dkg:project:smart-contracts', + 'https://example.org/context-graphs/devnet-test', + 'agent@example.org/context.graph-v1', + ]) { + expect(isValidContextGraphId(id)).toBe(true); + } + }); +}); diff --git a/packages/cli/test/keystore.test.ts b/packages/cli/test/keystore.test.ts index 17c896b91..b3f8105cb 100644 --- a/packages/cli/test/keystore.test.ts +++ b/packages/cli/test/keystore.test.ts @@ -8,7 +8,7 @@ import { } from '../src/keystore.js'; beforeAll(() => { - _setScryptN(2 ** 14); + _setScryptN(2 ** 15); }); const TEST_KEY = 'aabbccdd11223344aabbccdd11223344aabbccdd11223344aabbccdd11223344'; diff --git a/packages/core/src/constants.ts b/packages/core/src/constants.ts index 6ce86e3ef..27d2be7ed 100644 --- a/packages/core/src/constants.ts +++ b/packages/core/src/constants.ts @@ -122,6 +122,9 @@ export function validateContextGraphId(id: string): { valid: boolean; reason?: s if (!id || id.length === 0) return { valid: false, reason: 'Context graph ID cannot be empty' }; if (id.length > 256) return { valid: false, reason: 'Context graph ID exceeds 256 characters' }; if (!/^[\w:/.@\-]+$/.test(id)) return { valid: false, reason: 'Context graph ID contains disallowed characters (allowed: alphanumeric, _, :, /, ., @, -)' }; + if (id.split('/').some((segment) => segment === '.' || segment === '..')) { + return { valid: false, reason: 'Context graph ID cannot contain path traversal segments' }; + } return { valid: true }; } diff --git a/packages/core/test/constants.test.ts b/packages/core/test/constants.test.ts index 9d2831ac8..256bd21ce 100644 --- a/packages/core/test/constants.test.ts +++ b/packages/core/test/constants.test.ts @@ -110,6 +110,11 @@ describe('validateContextGraphId', () => { expect(validateContextGraphId('a'.repeat(257)).valid).toBe(false); expect(validateContextGraphId('a'.repeat(256)).valid).toBe(true); }); + + it('rejects literal traversal path segments', () => { + expect(validateContextGraphId('../etc/passwd').valid).toBe(false); + expect(validateContextGraphId('legit-cg/../../other-cg').valid).toBe(false); + }); }); describe('validateAssertionName', () => {