Make minio's link-key secrets 40 characters and keep every mc argument's secret out of its errors (issue 1766) #422
+36
-5
@@ -256,11 +256,22 @@ export class MinioClient {
|
||||
if (argv.some((a) => a.includes(this.rootPassword))) {
|
||||
throw new Error("refused to run mc: its command line would carry the root password (novox/hq issue 282)");
|
||||
}
|
||||
const { stdout } = await execFileAsync(this.mcBin, argv, {
|
||||
timeout: 30_000,
|
||||
env: { ...process.env, MC_HOST_mesh: mcHost(this.baseUrl, this.rootUser, this.rootPassword) },
|
||||
});
|
||||
return stdout.trim();
|
||||
const mcHostValue = mcHost(this.baseUrl, this.rootUser, this.rootPassword);
|
||||
try {
|
||||
const { stdout } = await execFileAsync(this.mcBin, argv, {
|
||||
timeout: 30_000,
|
||||
env: { ...process.env, MC_HOST_mesh: mcHostValue },
|
||||
});
|
||||
return stdout.trim();
|
||||
} catch (err) {
|
||||
// A fresh error with only the redacted message: execFile's own carries the whole command line, and
|
||||
// its cmd, stdout and stderr properties would carry it on (issue 1766).
|
||||
const known = [mcHostValue, this.rootPassword, encodeURIComponent(this.rootPassword), ...secretArgs(argv)];
|
||||
const e = err as { stderr?: unknown; message?: string };
|
||||
const reason = typeof e.stderr === "string" && e.stderr.trim() !== "" ? e.stderr.trim() : (e.message ?? String(err));
|
||||
const shown = argv.filter((a) => a !== this.mcConfigDir).join(" ");
|
||||
throw new Error(redactSecrets(`mc ${shown} failed: ${reason}`, known));
|
||||
}
|
||||
}
|
||||
|
||||
// --- SigV4 request plumbing ---------------------------------------------
|
||||
@@ -360,6 +371,26 @@ function uriEncode(str: string, encodeSlash: boolean): string {
|
||||
return out;
|
||||
}
|
||||
|
||||
/** The values that follow --secret-key and --access-key in an argument list. */
|
||||
function secretArgs(argv: string[]): string[] {
|
||||
const out: string[] = [];
|
||||
argv.forEach((a, i) => { if ((a === "--secret-key" || a === "--access-key") && i + 1 < argv.length) out.push(argv[i + 1]); });
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* The one redaction every `mc` failure goes through (issue 1766): the token after --secret-key or
|
||||
* --access-key (as a separate word or after "="), any MC_HOST_ value, and each known secret, wherever they
|
||||
* appear in the text, become "[redacted]".
|
||||
*/
|
||||
export function redactSecrets(text: string, known: string[] = []): string {
|
||||
let out = text
|
||||
.replace(/(--(?:secret|access)-key(?:=|\s+))('[^']*'|"[^"]*"|\S+)/g, "$1[redacted]")
|
||||
.replace(/(MC_HOST_\w+=)\S+/g, "$1[redacted]");
|
||||
for (const k of known) if (k !== "") out = out.split(k).join("[redacted]");
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* The MC_HOST_<alias> URL `mc` reads an alias from: the endpoint with the credentials in its userinfo,
|
||||
* each percent-encoded so a reserved character in either cannot end it early.
|
||||
|
||||
@@ -36,6 +36,9 @@ export function linkPolicy(bucket: string): string {
|
||||
});
|
||||
}
|
||||
|
||||
/** A link key's secret: 40 hex characters, minio's limit for a secret key (issue 1766). */
|
||||
const LINK_SECRET = /^[0-9a-f]{40}$/;
|
||||
|
||||
export function linkSecretPath(dir: string, bucket: string): string {
|
||||
return join(dir, `${bucket}.secret`);
|
||||
}
|
||||
@@ -47,9 +50,12 @@ export async function ensureLinkKey(admin: LinkAdmin, dir: string, bucket: strin
|
||||
const path = linkSecretPath(dir, bucket);
|
||||
mkdirSync(dir, { recursive: true, mode: 0o700 });
|
||||
let secret = existsSync(path) ? readFileSync(path, "utf8").trim() : "";
|
||||
// A stored secret of the wrong shape (an earlier version made 64 characters; minio takes at most 40) is
|
||||
// replaced, and its key made again (issue 1766).
|
||||
if (!LINK_SECRET.test(secret)) secret = "";
|
||||
if (secret !== "" && (await admin.accessKeyExists(id))) return;
|
||||
if (secret === "") {
|
||||
secret = randomBytes(32).toString("hex");
|
||||
secret = randomBytes(20).toString("hex");
|
||||
writeFileSync(path, secret + "\n", { mode: 0o600 });
|
||||
chmodSync(path, 0o600);
|
||||
}
|
||||
|
||||
@@ -115,3 +115,58 @@ test("the link key id is the one the object-store tool derives (cmd/object-store
|
||||
// 1516/AC1
|
||||
assert.equal(linkKeyId("brother"), "link0b4bd77cec705cf5");
|
||||
});
|
||||
|
||||
// Issue 1766: a link key's secret is minio's length, and no mc failure carries an argument's secret.
|
||||
|
||||
import { chmodSync, writeFileSync, mkdirSync } from "node:fs";
|
||||
import { MinioClient } from "../client.ts";
|
||||
|
||||
test("a link key's secret is 40 characters, and a stored 64-character secret is replaced", async () => {
|
||||
// 1766/AC1
|
||||
const { minio, dir } = setup();
|
||||
await ensureLinkKey(minio, dir, "fresh");
|
||||
assert.equal(minio.keys.get(linkKeyId("fresh"))!.secret.length, 40);
|
||||
assert.match(minio.keys.get(linkKeyId("fresh"))!.secret, /^[0-9a-f]{40}$/);
|
||||
|
||||
mkdirSync(dir, { recursive: true });
|
||||
const old = "ab".repeat(32);
|
||||
writeFileSync(linkSecretPath(dir, "old"), old + "\n");
|
||||
await ensureLinkKey(minio, dir, "old");
|
||||
const made = minio.keys.get(linkKeyId("old"))!.secret;
|
||||
assert.equal(made.length, 40);
|
||||
assert.notEqual(made, old);
|
||||
assert.equal(readFileSync(linkSecretPath(dir, "old"), "utf8").trim(), made);
|
||||
});
|
||||
|
||||
function failingMc(): { client: MinioClient; root: string } {
|
||||
const d = mkdtempSync(join(process.env.TMPDIR ?? tmpdir(), "minio-mc-"));
|
||||
const bin = join(d, "mc");
|
||||
// Says its arguments and its alias on stderr, then fails, the way a refused command does.
|
||||
writeFileSync(bin, '#!/bin/sh\necho "args: $*" >&2\necho "env: MC_HOST_mesh=$MC_HOST_mesh" >&2\nexit 1\n');
|
||||
chmodSync(bin, 0o755);
|
||||
const root = "RootPass-s3cret/ok";
|
||||
const client = new MinioClient({ endpoint: "http://127.0.0.1:9000", rootUser: "root", rootPassword: root, region: "us-east-1", mcBin: bin, mcConfigDir: d });
|
||||
return { client, root };
|
||||
}
|
||||
|
||||
test("an mc failure's message holds no --secret-key value and no MC_HOST value", async () => {
|
||||
// 1766/AC2
|
||||
const { client, root } = failingMc();
|
||||
const secret = "SuperSecretValue0123456789";
|
||||
const err: any = await client.createAccessKey("b", "ACCESSKEYID1234567", secret).then(() => null, (e) => e);
|
||||
assert.ok(err, "the failure is raised");
|
||||
for (const text of [String(err.message), String(err.stack ?? ""), JSON.stringify(Object.getOwnPropertyNames(err).map((k) => err[k]))]) {
|
||||
assert.ok(!text.includes(secret), "no secret key");
|
||||
assert.ok(!text.includes("ACCESSKEYID1234567"), "no access key");
|
||||
assert.ok(!text.includes(root) && !text.includes(encodeURIComponent(root)), "no root password");
|
||||
assert.ok(!text.includes("root:"), "no MC_HOST value");
|
||||
}
|
||||
assert.match(err.message, /\[redacted\]/);
|
||||
});
|
||||
|
||||
test("a consumer key's create passes the minted secret unchanged", async () => {
|
||||
// 1766/AC3
|
||||
const s = setup();
|
||||
await s.h.create(provision);
|
||||
assert.equal(s.minio.keys.get("brother-key")!.secret, "consumer-secret");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user