fix: EA-1 clean error on malformed flag/doc; EA-2 fingerprint ambiguity + fd-leak
EA-1: main dispatch catches KeyError/TypeError so a structurally-malformed flag/doc prints a clean [x] line instead of a traceback. EA-2: fingerprint revoke rejects an empty prefix and an ambiguous prefix (was: silently revoked the first match). json_store closes the raw fd if os.fdopen raises before taking ownership (was: leaked). init TOCTOU documented as by-design (trusted-DEK model, save upserts by _id). list '?' wording clarified. Signed-off-by: disqualifier <dev@disqualifier.me>
This commit is contained in:
parent
130c62e31c
commit
88e1eaef39
@ -94,9 +94,10 @@ def main() -> int:
|
||||
return 0
|
||||
except InvalidTag:
|
||||
return _fail("capability flag failed authentication — tampered or wrong DEK")
|
||||
except (ConfigError, CommandError, RuntimeError, ValueError, OSError) as error:
|
||||
# OSError covers the FileNotFoundError/PermissionError/IsADirectoryError family
|
||||
# so an i/o failure prints a clean [✘] line instead of a traceback
|
||||
except (ConfigError, CommandError, RuntimeError, ValueError, OSError, KeyError, TypeError) as error:
|
||||
# OSError covers the FileNotFoundError/PermissionError/IsADirectoryError family;
|
||||
# KeyError/TypeError cover a structurally-malformed flag/doc (unguarded indexing
|
||||
# of ['iv']/['meta']['authorizer']/['key']) — all print a clean [✘] line, not a traceback
|
||||
return _fail(str(error))
|
||||
|
||||
|
||||
|
||||
@ -11,7 +11,14 @@ from . import CommandError, build_doc, find_by_friendly, make_flag
|
||||
|
||||
|
||||
def run(config, storage, args) -> None:
|
||||
"""initialize the key system on this machine as the first authorizer"""
|
||||
"""initialize the key system on this machine as the first authorizer
|
||||
|
||||
the already-initialized / duplicate-friendly checks are non-atomic (a check-then-act
|
||||
TOCTOU under two concurrent CLIs), but this is a one-shot human admin tool and `save`
|
||||
upserts by `_id`, so key material can never collide — the worst case is a cosmetic
|
||||
double-init under a race, which carries no security consequence in the trusted-
|
||||
DEK-holder threat model. left non-atomic by design.
|
||||
"""
|
||||
if storage.get_all():
|
||||
raise CommandError(
|
||||
"already initialized; use `authorizer list` to see existing keys"
|
||||
|
||||
@ -11,7 +11,12 @@ from . import boot_local, read_flag
|
||||
|
||||
|
||||
def _can_authorize(crypto, doc) -> str:
|
||||
"""decrypted authority of a doc as Yes/No, or `?` if not readable here"""
|
||||
"""decrypted authority of a doc as Yes/No, or `?` if not readable here
|
||||
|
||||
`?` means the flag could not be read for ANY reason — the local key can't unwrap it,
|
||||
or the doc is missing/malformed — so the table always renders rather than crashing on
|
||||
one bad row. it is not specifically a corruption signal.
|
||||
"""
|
||||
try:
|
||||
return "Yes" if read_flag(crypto, doc["meta"]["authorizer"]) else "No"
|
||||
except Exception:
|
||||
|
||||
@ -18,11 +18,18 @@ _WARNING = (
|
||||
|
||||
|
||||
def _find_by_fingerprint(storage, prefix: str):
|
||||
"""return the doc whose `_id` starts with the given prefix, or None"""
|
||||
for doc in storage.get_all():
|
||||
if doc.get("_id", "").startswith(prefix):
|
||||
return doc
|
||||
return None
|
||||
"""return the single doc whose `_id` starts with the given prefix, or None
|
||||
|
||||
rejects an empty prefix (which would match every key) and an ambiguous prefix
|
||||
that matches more than one key, rather than silently revoking the first match.
|
||||
"""
|
||||
if not prefix:
|
||||
raise CommandError("fingerprint prefix must not be empty")
|
||||
matches = [doc for doc in storage.get_all() if doc.get("_id", "").startswith(prefix)]
|
||||
if len(matches) > 1:
|
||||
ids = ", ".join(d["_id"][:16] for d in matches)
|
||||
raise CommandError(f"fingerprint prefix '{prefix}' is ambiguous; matches: {ids}")
|
||||
return matches[0] if matches else None
|
||||
|
||||
|
||||
def run(config, storage, args) -> None:
|
||||
|
||||
@ -45,12 +45,21 @@ class JsonStore(StorageBackend):
|
||||
fd, tmp = tempfile.mkstemp(
|
||||
dir=self.path.parent, prefix=self.path.name + ".", suffix=".tmp"
|
||||
)
|
||||
wrapped = False
|
||||
try:
|
||||
with os.fdopen(fd, "w", encoding="utf-8") as handle:
|
||||
wrapped = True # fdopen took ownership of fd; its close() handles it
|
||||
json.dump(docs, handle, indent=2)
|
||||
handle.write("\n")
|
||||
os.replace(tmp, self.path)
|
||||
except BaseException:
|
||||
if not wrapped:
|
||||
# fdopen raised before taking ownership — close the raw fd ourselves so
|
||||
# it isn't leaked (the `with` only closes once fdopen returns a file object)
|
||||
try:
|
||||
os.close(fd)
|
||||
except OSError:
|
||||
pass
|
||||
try:
|
||||
os.unlink(tmp)
|
||||
except OSError:
|
||||
|
||||
Loading…
Reference in New Issue
Block a user