From fef9a634e8349b8dd9bd4319f9c4c0f7116ded51 Mon Sep 17 00:00:00 2001 From: Tyler Brockmeyer <95704252+tbrockmeyer-a@users.noreply.github.com> Date: Thu, 26 Sep 2024 18:17:49 -0500 Subject: [PATCH] add support for the mac_only_encrypted flag of the sops config (#18) --- src/decode.ts | 78 +++++++++++++++++++++------------ test/integration/decode.test.ts | 77 ++++++++++++++++++++++++++++---- 2 files changed, 117 insertions(+), 38 deletions(-) diff --git a/src/decode.ts b/src/decode.ts index 9888177..97211c9 100644 --- a/src/decode.ts +++ b/src/decode.ts @@ -5,6 +5,16 @@ import { AssumeRoleCommand, STSClient } from "@aws-sdk/client-sts"; import { DecryptCommand, KMSClient } from "@aws-sdk/client-kms"; const UNENCRYPTED_SUFFIX = "_unencrypted"; +/** + * used to assure that MACs created with mac_only_encrypted + * are unique from those created without it. + * + * const value = crypto.createHash('sha256').update('sops').digest(); + */ +const MAC_ONLY_ENCRYPTED_INITIALIZATION = Buffer.from( + "8a3fd2ad54ce66527b1034f3d147be0b0b975b3bf44f72c6fdadec8176f27d69", + "hex", +); // Why doesn't the JSON methods return this... export type Json = string | number | boolean | null | JsonObject | JsonArray; @@ -33,6 +43,7 @@ export interface SopsMetadata { encrypted_suffix?: string; unencrypted_regex?: string; encrypted_regex?: string; + mac_only_encrypted?: boolean; } export interface EncodedTree { @@ -100,7 +111,12 @@ export async function decrypt(tree: JsonObject) { const digest = crypto.createHash("sha512"); - const result = walkAndDecrypt(tree, key, "", digest, [], shouldBeEncrypted); + const macOnlyEncrypted = sops.mac_only_encrypted; + if (macOnlyEncrypted) { + digest.update(MAC_ONLY_ENCRYPTED_INITIALIZATION); + } + const settings = { key, digest, shouldBeEncrypted, macOnlyEncrypted }; + const result = walkAndDecrypt(tree, [], settings); if (sops.mac) { const hash = String(decryptScalar(sops.mac, key, sops.lastmodified)); @@ -188,43 +204,42 @@ export function decryptScalar( } } +interface WalkSettings { + key: Uint8Array; + digest: crypto.Hash; + shouldBeEncrypted: EncryptionModifier; + macOnlyEncrypted?: boolean; +} + function walkAndDecrypt( value: Json, - key: Uint8Array, - aad: string, - digest: crypto.Hash, path: string[], - shouldBeEncrypted: EncryptionModifier, + settings: WalkSettings, ): unknown { + const { key, digest, shouldBeEncrypted, macOnlyEncrypted } = settings; if (value === null) { return value; } if (Array.isArray(value)) { - return value.map((innerValue) => - walkAndDecrypt(innerValue, key, aad, digest, path, shouldBeEncrypted), - ); + return value.map((v) => walkAndDecrypt(v, path, settings)); } if (typeof value === "object") { - return Object.fromEntries(Object.entries(value) - .filter(([k, v]) => v !== undefined && (path.length > 0 || k !== "sops")) - .map(([k, innerValue]) => [ - k, - walkAndDecrypt( - innerValue, - key, - `${aad}${k}:`, - digest, - [...path, k], - shouldBeEncrypted, - ) - ])); + return Object.fromEntries( + Object.entries(value) + .filter(([k, v]) => v !== undefined && (path.length || k !== "sops")) + .map(([k, v]) => [k, walkAndDecrypt(v, [...path, k], settings)]), + ); + } + const isEncrypted = shouldBeEncrypted(path); + const plaintext = + typeof value === "string" && isEncrypted + ? decryptScalar(value, key, path.join(":") + ":") + : value; + if (!macOnlyEncrypted || isEncrypted) { + digest.update(toBytes(plaintext)); } - const plaintext = typeof value === 'string' && shouldBeEncrypted(path) - ? decryptScalar(value, key, aad) - : value; - digest.update(toBytes(plaintext)); return plaintext; -}; +} /** * Get the key from the 'sops.kms' node of the tree @@ -250,7 +265,9 @@ async function getKey(tree: EncodedTree): Promise { for (const entry of kmsTree) { try { if (!entry.enc || !entry.arn) { - throw new SopsError(`Invalid format for KMS node: ${JSON.stringify(entry)}`); + throw new SopsError( + `Invalid format for KMS node: ${JSON.stringify(entry)}`, + ); } // eslint-disable-next-line no-await-in-loop @@ -269,8 +286,11 @@ async function getKey(tree: EncodedTree): Promise { } return response.Plaintext; - } catch(error) { - const [errorType, errorText] = error instanceof Error ? [error.name, error.message] : ['UnknownError', JSON.stringify(error)]; + } catch (error) { + const [errorType, errorText] = + error instanceof Error + ? [error.name, error.message] + : ["UnknownError", JSON.stringify(error)]; errors.push(`${entry.arn} - ${errorType}: ${errorText}`); } } diff --git a/test/integration/decode.test.ts b/test/integration/decode.test.ts index a0f8407..4fa7d16 100644 --- a/test/integration/decode.test.ts +++ b/test/integration/decode.test.ts @@ -1,11 +1,11 @@ -import { deepEqual } from "assert"; +import * as assert from "assert"; import { execFile } from "child_process"; import { randomUUID } from "crypto"; import * as test from "node:test"; import { tmpdir } from "os"; import * as path from "path"; import { decodeFile } from "../../src/decode"; -import { rm, writeFile } from "fs/promises"; +import { readFile, rm, writeFile } from "fs/promises"; const mustEnv = (name: string) => { const value = process.env[name]; @@ -26,12 +26,14 @@ type SopsArgs = { unencrypted_regex?: string; encrypted_regex?: string; }; + macOnlyEncrypted?: boolean; }; const createEncryptedFile = async ({ data, encryptionMethod, keyEncryptionBasis, + macOnlyEncrypted, }: SopsArgs) => { const filepath = path.join(tmpdir(), `${randomUUID()}.json`); const opts: string[] = []; @@ -50,6 +52,9 @@ const createEncryptedFile = async ({ if (keyEncryptionBasis.encrypted_regex) { opts.push("--encrypted-regex", keyEncryptionBasis.encrypted_regex); } + if (macOnlyEncrypted) { + opts.push("--mac-only-encrypted"); + } await writeFile(filepath, JSON.stringify(data)); const child = execFile("sops", [...opts, "-e", "-i", filepath]); await new Promise((res, rej) => @@ -59,7 +64,7 @@ const createEncryptedFile = async ({ return filepath; }; -test.test("decodeFile", { concurrency: true, timeout: 1000 }, async (t) => { +test.test("decodeFile", { concurrency: true, timeout: 2000 }, async (t) => { // We cannot test without this if (!process.env["KMS_KEY_ARN"]) { return; @@ -103,9 +108,8 @@ test.test("decodeFile", { concurrency: true, timeout: 1000 }, async (t) => { myObject_encrypted: simpleObject, }; - const encryptionMethods: SopsArgs["encryptionMethod"][] = [ - { kms: [mustEnv("KMS_KEY_ARN")] }, - ]; + const kmsEncryption = { kms: [mustEnv("KMS_KEY_ARN")] }; + const encryptionMethods: SopsArgs["encryptionMethod"][] = [kmsEncryption]; const keyEncryptionBases: SopsArgs["keyEncryptionBasis"][] = [ { unencrypted_suffix: "_plain" }, { encrypted_suffix: "_encrypted" }, @@ -125,7 +129,7 @@ test.test("decodeFile", { concurrency: true, timeout: 1000 }, async (t) => { t.test( "key encryption bases (arbitrarily choosing encryption method)", async (t) => { - const encryptionMethod = encryptionMethods[0]; + const encryptionMethod = kmsEncryption; await Promise.all( keyEncryptionBases.map((keyEncryptionBasis) => t.test(`using ${JSON.stringify(keyEncryptionBasis)}`, async (t) => { @@ -138,7 +142,7 @@ test.test("decodeFile", { concurrency: true, timeout: 1000 }, async (t) => { keyEncryptionBasis, }); const decoded = await decodeFile(filepath); - deepEqual(decoded, data); + assert.deepEqual(decoded, data); }), ), ); @@ -165,7 +169,7 @@ test.test("decodeFile", { concurrency: true, timeout: 1000 }, async (t) => { keyEncryptionBasis, }); const decoded = await decodeFile(filepath); - deepEqual(decoded, data); + assert.deepEqual(decoded, data); }), ), ); @@ -175,5 +179,60 @@ test.test("decodeFile", { concurrency: true, timeout: 1000 }, async (t) => { ); }, ), + t.test("mac only encrypted", async (t) => { + const encryptionMethod = kmsEncryption; + const keyEncryptionBasis = { encrypted_suffix: "a" }; + type Data = { a: string; b: number }; + const data: Data = { a: "string", b: 2 }; + const options = [ + { macOnlyEncrypted: true, changes: true, error: false }, + { macOnlyEncrypted: true, changes: false, error: false }, + { macOnlyEncrypted: false, changes: true, error: true }, + { macOnlyEncrypted: false, changes: false, error: false }, + ] as const; + await Promise.all( + options.map(({ macOnlyEncrypted, changes, error }) => + t.test( + `${macOnlyEncrypted ? "enabled" : "disabled"}, ${ + changes ? "with" : "without" + } changes to unencrypted data, ${ + error ? "expecting error" : "expecting success" + }`, + async (t) => { + const filepath = await createEncryptedFile({ + data, + encryptionMethod, + keyEncryptionBasis, + macOnlyEncrypted, + }); + if (changes) { + const fileContents = JSON.parse( + await readFile(filepath, "utf8"), + ) as Data; + fileContents.b = 42; + await writeFile(filepath, JSON.stringify(fileContents)); + } + if (error) { + try { + await decodeFile(filepath); + throw new Error("expected an error"); + } catch (e) { + assert( + e instanceof Error && e.message.includes("Hash mismatch"), + "expected a hash-mismatch error", + ); + } + } else { + const decoded = (await decodeFile(filepath)) as Data; + assert( + decoded.b === (changes ? 42 : data.b), + "expected the decoded data to match the file contents", + ); + } + }, + ), + ), + ); + }), ]); });