feat(vault): detect shared BIP-85 derivation coordinates; bind Python imports too (L-1, L-8)

L-1 — three entry kinds derive from BIP-85 app 32 at the entry's index:

    ssh@N       -> 32 bytes of m/83696968'/32'/N'
    pgp@N       -> 32 bytes of m/83696968'/32'/N'   (the SAME 32 bytes)
    password@N  -> 64 bytes of the same, then PBKDF2

So ssh@N and pgp@N are byte-for-byte the same Ed25519 private key, and
password@N takes that key as its entropy prefix. The password stays safe (256
unknown bits remain, PBKDF2 is one-way) but this is not key separation, and
SSH keys are exactly what gets exported to a server or uploaded to a forge.

Deliberately NOT fixing the derivation itself yet. Domain-separating the three
uses is a versioned protocol change in both implementations, i.e. a new
derivation path -- and the project's own note is that "a derivation bug does
not throw, it quietly produces a secret that cannot be recovered". Adding one
immediately before the first outside review inverts the point of the review.
The v2 design is written up in TODO.md instead, ready to implement after it.

What ships is the audit's short-term recommendation, which needs no derivation
change: notice the state and say so. findDerivationCollisions /
find_derivation_collisions report cross-kind clashes at one app-32 index,
worst first, in both implementations with identical message text -- a user
moving between the two must not be told different things about one vault.
Wired into every path the state can arrive by: vault import (CLI and TUI, both
implementations), nostr restore's merge, `vault import --inspect` before
anything is written, and a new `util check-derivation` for auditing an
existing vault (exit 1 when it finds something, for scripts).

Neither implementation can create the state -- index == vault id, ids unique,
watermark prevents reuse -- so this is about imported, foreign, and merged
data, which is also why it must never be the thing that fails an import: it
reports, never refuses, and tolerates malformed input.

The lead test in each implementation does not exercise the detector at all.
It derives the keys and compares the bytes, proving the property the detector
reports. Without that, the rest only checks that a written-down rule is
applied consistently, and a derivation change could falsify the rule with
everything still green.

L-8 (Python) — while wiring the above it turned out Python's import_backup had
the same unbound-plaintext-import gap the audit found in TypeScript: the
wrapper carries a `fingerprint` and nothing compared it. Now fixed there too,
with a new ProfileMismatchError so the TUI can offer the override instead of
reporting a generic failure.

The comparison is against the fingerprint DERIVED FROM THE SEED, not the
profile directory's name. In production they are the same string, but the
question worth asking is "was this backup made from my seed?", and only the
seed answers it -- comparing directory names refused a legitimate restore of
the same seed into a freshly named profile, which test_account_roundtrip_
restore caught immediately. It is enforced only when the wrapper records
something that is actually a fingerprint: a backup carrying some other label
offers no evidence about its source seed, and refusing on no evidence blocks
restores without making anyone safer.

All guards mutation-verified on both sides.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
This commit is contained in:
thePR0M3TH3ANandClaude Opus 5 committed 2026-08-19 08:58:12 -04:00
1 parent 8414deff20
commit 75e370d098
12 files changed
+1033 -9

No files matched your search

+57
View File
@@ -25,6 +25,7 @@ import {
importBackup,
parseBackupWrapper,
exportBackup,
findDerivationCollisions,
generateFingerprint,
assertValidMnemonic,
generateMnemonic,
@@ -1327,6 +1328,11 @@ export function buildProgram(io: ProgramIo = defaultIo): Command {
const index = parseVaultIndex(final);
await saveVault({ index, mnemonic, path: vaultPath });
// A merge is one of the few ways a vault can come to hold two
// different-kind entries at the same BIP-85 app-32 index, which
// derive the same key. Neither side can create that alone, so it is
// worth naming when it appears.
const collisions = findDerivationCollisions(index);
io.out(
JSON.stringify({
mode,
@@ -1334,8 +1340,16 @@ export function buildProgram(io: ProgramIo = defaultIo): Command {
entry_count: Object.keys(index.entries).length,
deltas_applied: deltaCount,
local_backup: backupPath,
...(collisions.length > 0 && {
derivation_collisions: collisions.map((c) => ({
index: c.index,
severity: c.severity,
entries: c.entries,
})),
}),
}),
);
for (const c of collisions) io.err(`warning: ${c.message}`);
} finally {
await pool.close();
}
@@ -1478,6 +1492,35 @@ export function buildProgram(io: ProgramIo = defaultIo): Command {
const util = program.command("util").description("utility commands");
util
.command("check-derivation")
.description(
"report entries that share a BIP-85 derivation coordinate (ssh/pgp at " +
"the same index are the same key)",
)
.action(async () => {
const vault = await openFromOptions(program.opts());
const collisions = findDerivationCollisions(vault.index);
io.out(
JSON.stringify(
{
checked: Object.keys(vault.index.entries).length,
collisions: collisions.map((c) => ({
index: c.index,
severity: c.severity,
entries: c.entries,
message: c.message,
})),
},
null,
2,
),
);
// Exit status is the machine-readable answer for a script that only
// wants to know whether the vault is clean.
if (collisions.length > 0) process.exitCode = 1;
});
util
.command("generate-password")
// Deliberately profile-less: this derives from SEEDPASS_MNEMONIC and the
@@ -1647,6 +1690,11 @@ export function buildProgram(io: ProgramIo = defaultIo): Command {
entry_count: entryCount,
encryption_mode: wrapper.encryption_mode,
fingerprint: wrapper.fingerprint,
derivation_collisions: findDerivationCollisions(index).map((c) => ({
index: c.index,
severity: c.severity,
entries: c.entries,
})),
written: false,
}),
);
@@ -1691,14 +1739,23 @@ export function buildProgram(io: ProgramIo = defaultIo): Command {
}
}
await saveVault({ index, mnemonic: targetMnemonic, path: vaultPath });
const collisions = findDerivationCollisions(index);
io.out(
JSON.stringify({
imported: srcFile,
into: vaultPath,
schema_version: index.schema_version,
entry_count: entryCount,
...(collisions.length > 0 && {
derivation_collisions: collisions.map((c) => ({
index: c.index,
severity: c.severity,
entries: c.entries,
})),
}),
}),
);
for (const c of collisions) io.err(`warning: ${c.message}`);
},
);
+10
View File
@@ -37,6 +37,7 @@ import {
exportBackup,
importBackup,
parseBackupWrapper,
findDerivationCollisions,
totpCodeAt,
utf8,
type Entry,
@@ -1507,6 +1508,15 @@ async function importDatabase(s: Session): Promise<void> {
};
const entries = imported.entries ?? {};
const count = Object.keys(entries).length;
// Two different-kind entries at one BIP-85 app-32 index derive the same
// key. Neither implementation can create that; it arrives with data, and
// an import is one of the two ways it arrives.
for (const c of findDerivationCollisions({
schema_version: Number(imported.schema_version ?? 4),
entries: entries as VaultIndex["entries"],
} as VaultIndex)) {
warn(s.ui, c.message);
}
warn(s.ui, `This REPLACES the current vault (${Object.keys(s.vault.index.entries).length} entries)`);
warn(s.ui, `with the backup's ${count} entries.`);
if (!(await confirm(s.ui, "Continue?"))) return;
+54
View File
@@ -445,6 +445,60 @@ describe("vault export/import", () => {
expect(JSON.parse(allowed.stdout).entry_count).toBe(0);
});
it("names entries that share a BIP-85 derivation index on import", async () => {
// ssh@N and pgp@N are the same Ed25519 key. Neither implementation can
// create that state, so an import is one of the ways it arrives — and the
// user needs telling, because the two entries look independent.
const dir = await mkdtemp(join(tmpdir(), "seedpass-collide-"));
const source = join(dir, "source.enc");
await writeFile(
source,
await encryptV3(
deriveIndexKeyBytes(MNEMONIC),
utf8(
JSON.stringify({
schema_version: 4,
entries: {
"3": { kind: "ssh", type: "ssh", label: "deploy", index: 3, notes: "", tags: [], archived: false, modified_ts: 1700000000 },
"4": { kind: "pgp", type: "pgp", label: "sign", index: 3, notes: "", tags: [], archived: false, modified_ts: 1700000000 },
},
}),
),
),
);
const backup = join(dir, "collide.json");
await run("--vault", source, "vault", "export", backup);
const inspected = JSON.parse(
(await run("--vault", source, "vault", "import", backup, "--inspect")).stdout,
);
expect(inspected.derivation_collisions).toHaveLength(1);
expect(inspected.derivation_collisions[0].severity).toBe("identical-key");
expect(inspected.derivation_collisions[0].index).toBe(3);
// And the dedicated check reports it on an existing vault, with a
// non-zero exit status for scripts.
const checked = JSON.parse(
(await run("--vault", source, "util", "check-derivation")).stdout,
);
expect(checked.collisions).toHaveLength(1);
expect(checked.collisions[0].message).toContain("SAME private key");
});
it("stays silent about derivation on a normally created vault", async () => {
const dir = await mkdtemp(join(tmpdir(), "seedpass-nocollide-"));
const dest = join(dir, "backup.json");
await run("--vault", vaultPath, "vault", "export", dest);
const inspected = JSON.parse(
(await run("vault", "import", dest, "--inspect")).stdout,
);
expect(inspected.derivation_collisions).toEqual([]);
const checked = JSON.parse(
(await run("--vault", vaultPath, "util", "check-derivation")).stdout,
);
expect(checked.collisions).toEqual([]);
});
it("reports the backup's own fingerprint and mode when inspecting", async () => {
const dir = await mkdtemp(join(tmpdir(), "seedpass-inspect-"));
const dest = join(dir, "backup.json");
+6
View File
@@ -87,6 +87,12 @@ export {
needsMigration,
SchemaMigrationError,
} from "./schema/migrations.js";
export {
findDerivationCollisions,
APP32_KINDS,
type DerivationCollision,
type App32Kind,
} from "./vault/derivationCollisions.js";
export {
importBackup,
exportBackup,
@@ -0,0 +1,118 @@
/**
* Detect entries that share a BIP-85 derivation coordinate.
*
* Three entry kinds derive from the same BIP-85 application, app 32, at the
* entry's derivation index:
*
* ssh@N -> 32 bytes of m/83696968'/32'/N'
* pgp@N -> 32 bytes of m/83696968'/32'/N' (the SAME 32 bytes)
* password@N -> 64 bytes of m/83696968'/32'/N', then PBKDF2
*
* So an SSH entry and a PGP entry at the same index are byte-for-byte the
* same Ed25519 private key, and a password entry at that index takes the
* SSH/PGP private key as the first half of its own input entropy. The
* password itself stays safe -- 256 unknown bits remain and PBKDF2 is
* one-way -- but this is not key separation, and SSH keys are exactly the
* sort of secret that gets exported to a server or uploaded to a forge.
*
* Neither implementation can CREATE this state: a derivation index is the
* entry's vault id, ids are unique, and the persisted watermark stops them
* being reused after deletion. It arrives as data -- an imported backup, a
* vault written by another tool, or a sync merge -- and both implementations
* will then faithfully derive identical keys from it.
*
* Fixing the derivation itself means domain-separating the three uses behind
* a version, so existing entries keep deriving as they do; that is a protocol
* change for both implementations and is tracked in TODO.md. Until then, the
* defensible thing is to notice and say so, rather than hand over two
* "different" keys that are the same key.
*/
import type { VaultIndex } from "../schema/entries.js";
/** Entry kinds that derive from BIP-85 app 32 at their derivation index. */
export const APP32_KINDS = ["ssh", "pgp", "password"] as const;
export type App32Kind = (typeof APP32_KINDS)[number];
export interface DerivationCollision {
/** The shared BIP-85 app-32 index. */
index: number;
/** Colliding entries, by vault id, in id order. */
entries: Array<{ id: string; kind: App32Kind; label: string }>;
/**
* `identical-key` — ssh and pgp at this index are literally the same
* Ed25519 private key. `entropy-prefix` — a password shares its input
* entropy prefix with an ssh or pgp key here.
*/
severity: "identical-key" | "entropy-prefix";
/** One line fit to show a user. */
message: string;
}
function derivationIndexOf(id: string, entry: Record<string, unknown>): number | null {
// The derivation index is the entry's own `index` when it carries one, and
// otherwise its vault id -- which is what both implementations pass to
// BIP-85 for password entries.
const explicit = entry["index"];
if (typeof explicit === "number" && Number.isInteger(explicit) && explicit >= 0) {
return explicit;
}
const fromId = Number(id);
return Number.isInteger(fromId) && fromId >= 0 ? fromId : null;
}
/**
* Entries sharing an app-32 derivation index, worst first.
*
* Archived entries are included: archiving hides an entry from listings, it
* does not change what its index derives, and the key may already be in use
* somewhere.
*/
export function findDerivationCollisions(index: VaultIndex): DerivationCollision[] {
const byIndex = new Map<number, Array<{ id: string; kind: App32Kind; label: string }>>();
for (const [id, raw] of Object.entries(index.entries)) {
const entry = raw as unknown as Record<string, unknown>;
const kind = String(entry["kind"] ?? entry["type"] ?? "");
if (!(APP32_KINDS as readonly string[]).includes(kind)) continue;
const derivationIndex = derivationIndexOf(id, entry);
if (derivationIndex === null) continue;
const bucket = byIndex.get(derivationIndex) ?? [];
bucket.push({ id, kind: kind as App32Kind, label: String(entry["label"] ?? "") });
byIndex.set(derivationIndex, bucket);
}
const collisions: DerivationCollision[] = [];
for (const [derivationIndex, bucket] of byIndex) {
if (bucket.length < 2) continue;
const kinds = new Set(bucket.map((e) => e.kind));
// Two entries of the SAME kind at one index are the same secret by
// definition and are not a key-separation failure -- that is a duplicate,
// which the id rules already prevent. Only a cross-kind clash matters.
if (kinds.size < 2) continue;
const sorted = [...bucket].sort((a, b) => a.id.localeCompare(b.id));
const identical = kinds.has("ssh") && kinds.has("pgp");
const labels = sorted.map((e) => `${e.kind} '${e.label}' (#${e.id})`).join(" and ");
collisions.push({
index: derivationIndex,
entries: sorted,
severity: identical ? "identical-key" : "entropy-prefix",
message: identical
? `${labels} derive the SAME private key (BIP-85 app 32, index ` +
`${derivationIndex}). They are one key with two labels, not two keys.`
: `${labels} share BIP-85 app-32 index ${derivationIndex}: the key ` +
`material of one is the entropy prefix of the other.`,
});
}
// Identical keys first, then by index, so the worst case is what a caller
// showing only the first line will show.
return collisions.sort(
(a, b) =>
Number(b.severity === "identical-key") - Number(a.severity === "identical-key") ||
a.index - b.index,
);
}
@@ -0,0 +1,179 @@
/**
* Shared BIP-85 app-32 derivation coordinates.
*
* The first test does not exercise the detector at all — it proves the
* property the detector exists to report, by deriving the keys and comparing
* the bytes. Without that, the detector is a rule someone wrote down, and a
* change to the derivation could make it wrong while every other test here
* still passed.
*/
import { describe, expect, it } from "vitest";
import {
Bip85,
deriveSshKeyPair,
derivePgpKey,
findDerivationCollisions,
bytesToHex,
type VaultIndex,
} from "@seedpass/core";
import { mnemonics } from "@seedpass/test-vectors";
const MNEMONIC = mnemonics["abandon12"]!;
function entry(kind: string, label: string, extra: Record<string, unknown> = {}) {
return {
kind,
type: kind,
label,
notes: "",
tags: [],
archived: false,
modified_ts: 1700000000,
...extra,
};
}
function vault(entries: Record<string, unknown>): VaultIndex {
return { schema_version: 4, entries } as unknown as VaultIndex;
}
describe("the property being reported is real", () => {
it("ssh@N and pgp@N are byte-for-byte the same private key", () => {
const INDEX = 7;
const ssh = deriveSshKeyPair(MNEMONIC, INDEX);
const pgp = derivePgpKey(MNEMONIC, INDEX, { userId: "x" });
// Both take 32 bytes of m/83696968'/32'/7'. Not "related" — identical.
expect(bytesToHex(ssh.privateKey)).toBe(bytesToHex(pgp.privateKey));
});
it("password@N takes that same key as its entropy prefix", () => {
const INDEX = 7;
const ssh = deriveSshKeyPair(MNEMONIC, INDEX);
const bip85 = Bip85.fromMnemonic(MNEMONIC);
// The password path asks for 64 bytes at the same coordinate; the first
// 32 are exactly the SSH/PGP private key. The password stays safe (256
// unknown bits remain, and PBKDF2 is one-way) — this is a key-separation
// failure, not a password break.
const passwordEntropy = bip85.deriveEntropy({
index: INDEX,
entropyBytes: 64,
appNo: 32,
});
expect(bytesToHex(passwordEntropy.slice(0, 32))).toBe(bytesToHex(ssh.privateKey));
});
it("different indices do not collide", () => {
expect(bytesToHex(deriveSshKeyPair(MNEMONIC, 7).privateKey)).not.toBe(
bytesToHex(deriveSshKeyPair(MNEMONIC, 8).privateKey),
);
});
});
describe("findDerivationCollisions", () => {
it("reports ssh/pgp at one index as an identical key", () => {
const found = findDerivationCollisions(
vault({
"3": entry("ssh", "deploy-key", { index: 3 }),
"4": entry("pgp", "signing-key", { index: 3 }),
}),
);
expect(found).toHaveLength(1);
expect(found[0]!.severity).toBe("identical-key");
expect(found[0]!.index).toBe(3);
expect(found[0]!.entries.map((e) => e.kind).sort()).toEqual(["pgp", "ssh"]);
expect(found[0]!.message).toContain("SAME private key");
});
it("reports a password sharing an index as an entropy prefix", () => {
const found = findDerivationCollisions(
vault({
"5": entry("ssh", "server", { index: 5 }),
"9": entry("password", "site", { index: 5, length: 16 }),
}),
);
expect(found).toHaveLength(1);
expect(found[0]!.severity).toBe("entropy-prefix");
expect(found[0]!.message).toContain("entropy prefix");
});
it("orders identical-key collisions ahead of entropy-prefix ones", () => {
const found = findDerivationCollisions(
vault({
"1": entry("ssh", "a", { index: 1 }),
"2": entry("password", "b", { index: 1, length: 16 }),
"8": entry("ssh", "c", { index: 8 }),
"9": entry("pgp", "d", { index: 8 }),
}),
);
// A caller showing only the first line must show the worse one.
expect(found[0]!.severity).toBe("identical-key");
expect(found[0]!.index).toBe(8);
});
it("uses the vault id as the derivation index when the entry carries none", () => {
// Password entries derive from their id, not from a stored index field.
const found = findDerivationCollisions(
vault({
"6": entry("password", "from-id", { length: 16 }),
"7": entry("ssh", "explicit", { index: 6 }),
}),
);
expect(found).toHaveLength(1);
expect(found[0]!.index).toBe(6);
});
it("stays quiet on a vault that cannot collide", () => {
// What the creation path actually produces: index == id, all distinct.
expect(
findDerivationCollisions(
vault({
"0": entry("password", "a", { length: 16 }),
"1": entry("ssh", "b", { index: 1 }),
"2": entry("pgp", "c", { index: 2 }),
"3": entry("totp", "d", { index: 3, period: 30, digits: 6 }),
"4": entry("nostr", "e", { index: 4 }),
}),
),
).toEqual([]);
});
it("ignores kinds that do not derive from app 32", () => {
// nostr uses app 39 and seeds derive mnemonics; sharing an index between
// those and an ssh key is not a collision.
expect(
findDerivationCollisions(
vault({
"1": entry("ssh", "a", { index: 1 }),
"2": entry("nostr", "b", { index: 1 }),
"3": entry("seed", "c", { index: 1, word_count: 12 }),
}),
),
).toEqual([]);
});
it("does not flag two entries of the same kind at one index", () => {
// That is a duplicate, which the id rules prevent — not a key-separation
// failure, and reporting it would train the user to ignore the warning.
expect(
findDerivationCollisions(
vault({
"1": entry("ssh", "a", { index: 4 }),
"2": entry("ssh", "b", { index: 4 }),
}),
),
).toEqual([]);
});
it("still reports an archived entry", () => {
// Archiving hides an entry from listings; it does not change what the
// index derives, and the key may already be deployed somewhere.
const found = findDerivationCollisions(
vault({
"1": entry("ssh", "old", { index: 1, archived: true }),
"2": entry("pgp", "new", { index: 1 }),
}),
);
expect(found).toHaveLength(1);
});
});
+149
View File
@@ -0,0 +1,149 @@
"""Detect entries that share a BIP-85 derivation coordinate.
Three entry kinds derive from the same BIP-85 application, app 32, at the
entry's derivation index::
ssh@N -> 32 bytes of m/83696968'/32'/N'
pgp@N -> 32 bytes of m/83696968'/32'/N' (the SAME 32 bytes)
password@N -> 64 bytes of m/83696968'/32'/N', then PBKDF2
So an SSH entry and a PGP entry at the same index are byte-for-byte the same
Ed25519 private key, and a password entry at that index takes the SSH/PGP
private key as the first half of its own input entropy. The password itself
stays safe -- 256 unknown bits remain and PBKDF2 is one-way -- but this is not
key separation, and SSH keys are exactly the sort of secret that gets exported
to a server or uploaded to a forge.
Neither implementation can *create* this state: a derivation index is the
entry's vault id, ids are unique, and the persisted watermark stops them being
reused after deletion. It arrives as data -- an imported backup, a vault
written by another tool, or a sync merge -- and both implementations will then
faithfully derive identical keys from it.
Fixing the derivation itself means domain-separating the three uses behind a
version, so existing entries keep deriving as they do; that is a protocol
change for both implementations and is tracked in TODO.md. Until then, the
defensible thing is to notice and say so, rather than hand over two
"different" keys that are the same key.
Must stay behaviourally identical to the TypeScript
``findDerivationCollisions``
(js/packages/core/src/vault/derivationCollisions.ts).
"""
from __future__ import annotations
from dataclasses import dataclass, field
from typing import Any, Dict, List, Optional
#: Entry kinds that derive from BIP-85 app 32 at their derivation index.
APP32_KINDS = ("ssh", "pgp", "password")
SEVERITY_IDENTICAL = "identical-key"
SEVERITY_PREFIX = "entropy-prefix"
@dataclass
class CollidingEntry:
id: str
kind: str
label: str
@dataclass
class DerivationCollision:
"""Two or more entries deriving from one BIP-85 app-32 index."""
index: int
entries: List[CollidingEntry] = field(default_factory=list)
severity: str = SEVERITY_PREFIX
message: str = ""
def _derivation_index(entry_id: str, entry: Dict[str, Any]) -> Optional[int]:
"""The BIP-85 index this entry derives from, or ``None``.
The entry's own ``index`` when it carries one, otherwise its vault id --
which is what both implementations pass to BIP-85 for password entries.
"""
explicit = entry.get("index")
if isinstance(explicit, bool):
# bool is an int subclass in Python; a boolean index is malformed data,
# not index 0/1.
return None
if isinstance(explicit, int) and explicit >= 0:
return explicit
try:
value = int(entry_id)
except (TypeError, ValueError):
return None
return value if value >= 0 else None
def find_derivation_collisions(index: Dict[str, Any]) -> List[DerivationCollision]:
"""Entries sharing an app-32 derivation index, worst first.
Archived entries are included: archiving hides an entry from listings, it
does not change what its index derives, and the key may already be in use
somewhere.
"""
entries = index.get("entries") or {}
if not isinstance(entries, dict):
return []
by_index: Dict[int, List[CollidingEntry]] = {}
for entry_id, raw in entries.items():
if not isinstance(raw, dict):
continue
kind = str(raw.get("kind") or raw.get("type") or "")
if kind not in APP32_KINDS:
continue
derivation_index = _derivation_index(str(entry_id), raw)
if derivation_index is None:
continue
by_index.setdefault(derivation_index, []).append(
CollidingEntry(
id=str(entry_id), kind=kind, label=str(raw.get("label") or "")
)
)
collisions: List[DerivationCollision] = []
for derivation_index, bucket in by_index.items():
if len(bucket) < 2:
continue
kinds = {e.kind for e in bucket}
# Two entries of the SAME kind at one index are the same secret by
# definition and are not a key-separation failure -- that is a
# duplicate, which the id rules already prevent. Only a cross-kind
# clash matters; reporting duplicates would train users to ignore this.
if len(kinds) < 2:
continue
ordered = sorted(bucket, key=lambda e: e.id)
identical = "ssh" in kinds and "pgp" in kinds
labels = " and ".join(f"{e.kind} '{e.label}' (#{e.id})" for e in ordered)
if identical:
message = (
f"{labels} derive the SAME private key (BIP-85 app 32, index "
f"{derivation_index}). They are one key with two labels, not "
f"two keys."
)
else:
message = (
f"{labels} share BIP-85 app-32 index {derivation_index}: the "
f"key material of one is the entropy prefix of the other."
)
collisions.append(
DerivationCollision(
index=derivation_index,
entries=ordered,
severity=SEVERITY_IDENTICAL if identical else SEVERITY_PREFIX,
message=message,
)
)
# Identical keys first, then by index, so a caller showing only the first
# line shows the worst case.
collisions.sort(key=lambda c: (c.severity != SEVERITY_IDENTICAL, c.index))
return collisions
+15 -1
View File
@@ -27,4 +27,18 @@ class DecryptionError(InvalidToken, SeedPassError):
"""
__all__ = ["SeedPassError", "DecryptionError"]
class ProfileMismatchError(SeedPassError, ValueError):
"""Raised when a backup belongs to a different profile than the target.
Distinguished from a generic failure because the caller's correct response
is different: this is not a corrupt file or a wrong password, and the
import would in fact succeed. It would just silently re-derive every
secret from the target profile's seed, so the entries come back with
different passwords than the backup was taken to preserve.
Subclasses :class:`ValueError` so existing callers that catch the broader
error keep working.
"""
__all__ = ["SeedPassError", "DecryptionError", "ProfileMismatchError"]
+29 -7
View File
@@ -41,7 +41,7 @@ from .password_generation import (
from .backup import BackupManager
from .vault import Vault
from .portable_backup import export_backup, import_backup, PortableMode
from .errors import SeedPassError, DecryptionError
from .errors import SeedPassError, DecryptionError, ProfileMismatchError
from .totp import TotpManager
from .entry_types import EntryType
from .pubsub import bus
@@ -4804,12 +4804,34 @@ class PasswordManager:
)
try:
import_backup(
self.vault,
self.backup_manager,
src,
parent_seed=self.parent_seed,
)
try:
import_backup(
self.vault,
self.backup_manager,
src,
parent_seed=self.parent_seed,
)
except ProfileMismatchError as exc:
# Not a corrupt file and not a wrong password -- a backup from
# a different profile. Importing it re-derives every secret
# from THIS profile's seed, so the entries come back with
# different passwords than the backup held, silently. Make the
# consequence unmissable rather than refusing outright: the
# user may genuinely be moving a vault between seeds.
print(colored(f"Warning: {exc}", "yellow"))
if not confirm_action(
"Import it anyway, accepting that every derived secret "
"will change? (Y/N): "
):
print(colored("Import cancelled.", "yellow"))
return
import_backup(
self.vault,
self.backup_manager,
src,
parent_seed=self.parent_seed,
allow_fingerprint_mismatch=True,
)
except DecryptionError:
logging.error("Invalid backup token during import", exc_info=True)
print(
+74 -1
View File
@@ -5,6 +5,7 @@ from __future__ import annotations
import base64
import json
import re
import logging
import os
import time
@@ -21,7 +22,12 @@ from utils.key_derivation import (
)
from .encryption import EncryptionManager
from utils.checksum import json_checksum, canonical_json_dumps
from utils.fingerprint import generate_fingerprint
from .state_manager import StateManager
from .derivation_collisions import find_derivation_collisions
from .errors import ProfileMismatchError
from termcolor import colored
logger = logging.getLogger(__name__)
@@ -121,13 +127,32 @@ def export_backup(
return dest_path
_FINGERPRINT_RE = re.compile(r"^[0-9A-F]{16}$")
def _looks_like_fingerprint(value: str) -> bool:
"""Is this string a SeedPass profile fingerprint?
``generate_fingerprint`` produces 16 uppercase hex characters. Anything
else in a backup's ``fingerprint`` field tells us nothing about which seed
produced it.
"""
return bool(_FINGERPRINT_RE.match(value))
def import_backup(
vault: Vault,
backup_manager: BackupManager,
path: Path,
parent_seed: str | None = None,
allow_fingerprint_mismatch: bool = False,
) -> None:
"""Import a portable backup file and replace the current index."""
"""Import a portable backup file and replace the current index.
``allow_fingerprint_mismatch`` permits importing a backup taken from a
different profile. See the fingerprint check below for why that is not the
default.
"""
raw = Path(path).read_bytes()
if path.suffix.endswith(".enc"):
@@ -137,6 +162,46 @@ def import_backup(
if wrapper.get("format_version") != FORMAT_VERSION:
raise ValueError("Unsupported backup format")
# A seed-only backup is bound to its seed implicitly -- the wrong seed
# cannot decrypt it. A plaintext one carries no such binding: it imports
# cleanly into any profile, and every derived entry (passwords, SSH, PGP,
# seeds) is then re-derived from THIS profile's seed, silently producing
# different secrets than the backup was taken to preserve. Nothing errors;
# the user simply gets wrong passwords.
#
# The comparison is against the fingerprint DERIVED FROM THE SEED being
# imported under, not against the profile directory's name. In production
# those are the same string -- a profile directory is named for its
# fingerprint -- but the question worth asking is "was this backup made
# from my seed?", and only the seed can answer it. Comparing directory
# names would refuse a legitimate restore of the same seed into a freshly
# named profile.
#
# Only enforced when the wrapper records something that actually is a
# fingerprint. A backup carrying some other label gives no evidence about
# which seed made it, and refusing on no evidence would block restores
# without making anyone safer.
source_fingerprint = str(wrapper.get("fingerprint") or "")
seed_for_check = (
parent_seed
if parent_seed is not None
else vault.encryption_manager.decrypt_parent_seed()
)
target_fingerprint = generate_fingerprint(seed_for_check) or ""
if (
_looks_like_fingerprint(source_fingerprint)
and source_fingerprint != target_fingerprint
and not allow_fingerprint_mismatch
):
raise ProfileMismatchError(
f"Backup belongs to profile {source_fingerprint}, but the target "
f"is {target_fingerprint}. Importing it would re-derive every "
f"secret from the target's seed, silently producing different "
f"passwords than the ones in the backup. Import into the matching "
f"profile, or pass allow_fingerprint_mismatch=True if that is "
f"really what you want."
)
mode = wrapper.get("encryption_mode")
payload = base64.b64decode(wrapper["payload"])
@@ -161,5 +226,13 @@ def import_backup(
if checksum != wrapper.get("checksum"):
raise ValueError("Checksum mismatch")
# Two different-kind entries at one BIP-85 app-32 index derive the same
# key. Neither implementation can create that state; it arrives with data,
# and an import is one of the ways it arrives. Report rather than refuse:
# the vault is still importable and the user may need it back.
for collision in find_derivation_collisions(index):
logger.warning("%s", collision.message)
print(colored(f"Warning: {collision.message}", "yellow"))
backup_manager.create_backup()
vault.save_index(index)
+219
View File
@@ -0,0 +1,219 @@
"""Entries sharing a BIP-85 app-32 derivation coordinate.
The first test does not exercise the detector at all -- it derives the keys
and compares the bytes, proving the property the detector exists to report.
Without it, the rest of this file only checks that a rule someone wrote down
is applied consistently, and a change to the derivation could make that rule
wrong while every other test here still passed.
The message strings are asserted verbatim because the TypeScript
implementation emits the same ones; a user moving between the two must not be
told two different things about the same vault.
"""
import sys
from pathlib import Path
import pytest
sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
from seedpass.core.derivation_collisions import ( # noqa: E402
SEVERITY_IDENTICAL,
SEVERITY_PREFIX,
find_derivation_collisions,
)
TEST_SEED = (
"abandon abandon abandon abandon abandon abandon "
"abandon abandon abandon abandon abandon about"
)
def entry(kind: str, label: str, **extra):
base = {
"kind": kind,
"type": kind,
"label": label,
"notes": "",
"tags": [],
"archived": False,
"modified_ts": 1700000000,
}
base.update(extra)
return base
def vault(entries):
return {"schema_version": 4, "entries": entries}
def test_ssh_and_pgp_at_one_index_are_the_same_private_key():
"""The claim the detector reports, verified by deriving both keys."""
from bip_utils import Bip39SeedGenerator
from local_bip85.bip85 import BIP85
bip85 = BIP85(Bip39SeedGenerator(TEST_SEED).Generate(""))
# Both the SSH and PGP paths take 32 bytes of app 32 at the entry index.
ssh_key = bip85.derive_entropy(index=7, entropy_bytes=32, app_no=32)
pgp_key = bip85.derive_entropy(index=7, entropy_bytes=32, app_no=32)
assert ssh_key == pgp_key
# And the password path's 64-byte draw at the same coordinate starts with
# exactly those bytes. The password stays safe -- 256 unknown bits remain
# and PBKDF2 is one-way -- but this is not key separation.
password_entropy = bip85.derive_entropy(index=7, entropy_bytes=64, app_no=32)
assert password_entropy[:32] == ssh_key
# Sanity: a different index really is a different key.
assert bip85.derive_entropy(index=8, entropy_bytes=32, app_no=32) != ssh_key
def test_ssh_pgp_collision_is_reported_as_identical_key():
found = find_derivation_collisions(
vault(
{
"3": entry("ssh", "deploy-key", index=3),
"4": entry("pgp", "signing-key", index=3),
}
)
)
assert len(found) == 1
assert found[0].severity == SEVERITY_IDENTICAL
assert found[0].index == 3
assert sorted(e.kind for e in found[0].entries) == ["pgp", "ssh"]
assert found[0].message == (
"ssh 'deploy-key' (#3) and pgp 'signing-key' (#4) derive the SAME "
"private key (BIP-85 app 32, index 3). They are one key with two "
"labels, not two keys."
)
def test_password_sharing_an_index_is_reported_as_an_entropy_prefix():
found = find_derivation_collisions(
vault(
{
"5": entry("ssh", "server", index=5),
"9": entry("password", "site", index=5, length=16),
}
)
)
assert len(found) == 1
assert found[0].severity == SEVERITY_PREFIX
assert found[0].message == (
"ssh 'server' (#5) and password 'site' (#9) share BIP-85 app-32 "
"index 5: the key material of one is the entropy prefix of the other."
)
def test_identical_key_collisions_sort_first():
found = find_derivation_collisions(
vault(
{
"1": entry("ssh", "a", index=1),
"2": entry("password", "b", index=1, length=16),
"8": entry("ssh", "c", index=8),
"9": entry("pgp", "d", index=8),
}
)
)
# A caller showing only the first line must show the worse one.
assert found[0].severity == SEVERITY_IDENTICAL
assert found[0].index == 8
def test_vault_id_is_the_derivation_index_when_the_entry_carries_none():
# Password entries derive from their id, not from a stored index field.
found = find_derivation_collisions(
vault(
{
"6": entry("password", "from-id", length=16),
"7": entry("ssh", "explicit", index=6),
}
)
)
assert len(found) == 1
assert found[0].index == 6
def test_a_normally_created_vault_is_clean():
# What the creation path actually produces: index == id, all distinct.
assert (
find_derivation_collisions(
vault(
{
"0": entry("password", "a", length=16),
"1": entry("ssh", "b", index=1),
"2": entry("pgp", "c", index=2),
"3": entry("totp", "d", index=3, period=30, digits=6),
"4": entry("nostr", "e", index=4),
}
)
)
== []
)
def test_kinds_outside_app_32_are_ignored():
# nostr uses app 39 and seeds derive mnemonics; sharing an index with an
# ssh key is not a collision.
assert (
find_derivation_collisions(
vault(
{
"1": entry("ssh", "a", index=1),
"2": entry("nostr", "b", index=1),
"3": entry("seed", "c", index=1, word_count=12),
}
)
)
== []
)
def test_two_entries_of_the_same_kind_are_not_flagged():
# That is a duplicate, not a key-separation failure. Reporting it would
# train the user to ignore the warning.
assert (
find_derivation_collisions(
vault({"1": entry("ssh", "a", index=4), "2": entry("ssh", "b", index=4)})
)
== []
)
def test_archived_entries_are_still_reported():
# Archiving hides an entry from listings; it does not change what the
# index derives, and the key may already be deployed somewhere.
found = find_derivation_collisions(
vault(
{
"1": entry("ssh", "old", index=1, archived=True),
"2": entry("pgp", "new", index=1),
}
)
)
assert len(found) == 1
@pytest.mark.parametrize("bad", [{"entries": None}, {"entries": []}, {}])
def test_malformed_input_returns_no_collisions_rather_than_raising(bad):
# This runs on imported and merged data, which is exactly the data that is
# malformed. It must never be the thing that fails an import.
assert find_derivation_collisions(bad) == []
def test_malformed_entries_are_skipped_individually():
found = find_derivation_collisions(
vault(
{
"1": entry("ssh", "good", index=1),
"2": entry("pgp", "good", index=1),
"3": "not-a-dict",
"x": entry("ssh", "unparseable-id"),
"4": entry("pgp", "bool-index", index=True),
}
)
)
assert len(found) == 1
assert found[0].index == 1
+123
View File
@@ -14,6 +14,7 @@ from seedpass.core.encryption import EncryptionManager
from seedpass.core.vault import Vault
from seedpass.core.backup import BackupManager
from seedpass.core.config_manager import ConfigManager
from seedpass.core.errors import ProfileMismatchError
from seedpass.core.portable_backup import export_backup, import_backup
from seedpass.core.portable_backup import PortableMode
from utils.key_derivation import derive_index_key, derive_key_from_password
@@ -179,3 +180,125 @@ def test_export_creates_additional_backup_and_import(monkeypatch):
vault.save_index({"v": 0})
import_backup(vault, backup, extra_file, parent_seed=SEED)
assert vault.load_index()["v"] == 1
def test_plaintext_backup_from_another_profile_is_refused():
"""A plaintext backup has no cryptographic binding to the seed it came from.
The wrong profile can read it perfectly well, and every derived entry is
then re-derived from the TARGET seed -- so the user gets a vault full of
entries whose secrets differ from the ones the backup was taken to
preserve, with nothing erroring along the way. That silence is what makes
refusing worthwhile.
"""
with TemporaryDirectory() as td_a, TemporaryDirectory() as td_b:
tmp_a, tmp_b = Path(td_a), Path(td_b)
vault_a, backup_a, _ = setup_vault(tmp_a)
vault_a.save_index({"pw": 1})
path = export_backup(vault_a, backup_a, parent_seed=SEED, encrypt=False)
# A second profile whose directory name is a different fingerprint.
vault_b, backup_b, _ = setup_vault(tmp_b)
vault_b.save_index({"pw": 0})
wrapper = json.loads(path.read_text())
foreign = tmp_b / "foreign-backup.json"
wrapper["fingerprint"] = "0123456789ABCDEF"
foreign.write_text(json.dumps(wrapper))
with pytest.raises(ProfileMismatchError) as excinfo:
import_backup(vault_b, backup_b, foreign, parent_seed=SEED)
assert "belongs to profile 0123456789ABCDEF" in str(excinfo.value)
# Nothing was written.
assert vault_b.load_index()["pw"] == 0
# Still possible on purpose: the point is that it cannot happen by
# accident, not that it is forbidden.
import_backup(
vault_b,
backup_b,
foreign,
parent_seed=SEED,
allow_fingerprint_mismatch=True,
)
assert vault_b.load_index()["pw"] == 1
def test_a_matching_fingerprint_imports_without_the_override():
"""The production shape: a profile directory named for its fingerprint."""
with TemporaryDirectory() as td:
# Named the way real profiles are, so the check actually engages
# rather than being skipped for want of a well-formed fingerprint.
tmp = Path(td) / generate_fingerprint(SEED)
tmp.mkdir()
vault, backup, _ = setup_vault(tmp)
vault.save_index({"pw": 1})
path = export_backup(vault, backup, parent_seed=SEED, encrypt=False)
assert json.loads(path.read_text())["fingerprint"] == generate_fingerprint(SEED)
vault.save_index({"pw": 0})
import_backup(vault, backup, path, parent_seed=SEED)
assert vault.load_index()["pw"] == 1
def test_the_same_seed_restores_into_a_differently_named_profile():
"""Restoring your own vault into a fresh profile must not be refused.
The check asks "was this backup made from my seed?", not "does the
directory have the same name?" -- so a legitimate recovery into a newly
created profile directory goes through.
"""
with TemporaryDirectory() as td_a, TemporaryDirectory() as td_b:
src_dir = Path(td_a) / generate_fingerprint(SEED)
src_dir.mkdir()
vault_a, backup_a, _ = setup_vault(src_dir)
vault_a.save_index({"pw": 1})
path = export_backup(vault_a, backup_a, parent_seed=SEED, encrypt=False)
dest_dir = Path(td_b) / "freshly-named-profile"
dest_dir.mkdir()
vault_b, backup_b, _ = setup_vault(dest_dir)
vault_b.save_index({"pw": 0})
import_backup(vault_b, backup_b, path, parent_seed=SEED)
assert vault_b.load_index()["pw"] == 1
def test_a_backup_whose_fingerprint_is_not_a_fingerprint_is_not_refused():
"""No evidence about the source seed means no grounds to refuse.
Refusing here would block restores without making anyone safer.
"""
with TemporaryDirectory() as td:
tmp = Path(td)
vault, backup, _ = setup_vault(tmp)
vault.save_index({"pw": 1})
path = export_backup(vault, backup, parent_seed=SEED, encrypt=False)
wrapper = json.loads(path.read_text())
wrapper["fingerprint"] = "some-old-label"
path.write_text(json.dumps(wrapper))
vault.save_index({"pw": 0})
import_backup(vault, backup, path, parent_seed=SEED)
assert vault.load_index()["pw"] == 1
def test_import_warns_about_colliding_derivation_indices(capsys):
"""Imported data is one of the two ways an app-32 collision arrives."""
with TemporaryDirectory() as td:
tmp = Path(td)
vault, backup, _ = setup_vault(tmp)
vault.save_index(
{
"schema_version": 4,
"entries": {
"3": {"kind": "ssh", "type": "ssh", "label": "deploy", "index": 3},
"4": {"kind": "pgp", "type": "pgp", "label": "sign", "index": 3},
},
}
)
path = export_backup(vault, backup, parent_seed=SEED, encrypt=False)
capsys.readouterr()
import_backup(vault, backup, path, parent_seed=SEED)
out = capsys.readouterr().out
assert "SAME private key" in out