Skip to content

Commit e11706a

Browse files
QuantQJclaude
andcommitted
security: fix 3 critical verify vulnerabilities (C1-C3)
C1: verify now checks file hash against sig.sha256 BEFORE signature verification — gives clear "hash mismatch" error on tampered files instead of generic signature failure. C2: verify now calls read_sig with validate_fingerprint=True — detects manually edited sig files where fingerprint doesn't match embedded key. C3: verify without --pubkey now checks keyring first, then warns loudly when falling back to embedded key: "UNVERIFIED — using embedded key, not independently trusted". Previously it silently trusted attacker- controlled keys embedded in forged sig files. Also: fixed stale version test, added regression tests for all 3 issues. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 8a8199f commit e11706a

4 files changed

Lines changed: 161 additions & 25 deletions

File tree

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
44

55
[project]
66
name = "modelsign"
7-
version = "1.0.1"
7+
version = "1.0.2"
88
description = "Sign AI models with identity. Verify anywhere."
99
authors = [{name = "QJ", email = "qj@constantone.ai"}]
1010
license = {text = "Apache-2.0"}

src/modelsign/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"""modelsign — Sign AI models with identity. Verify anywhere."""
22

3-
__version__ = "1.0.1"
3+
__version__ = "1.0.2"
44

55
from modelsign.identity.card import ModelCard, validate_card
66
from modelsign.identity.canonical import canonical_json

src/modelsign/cli.py

Lines changed: 73 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -239,24 +239,42 @@ def verify(model_path, sig_path, pubkey_path, output_json, quiet):
239239
click.echo(f"Error: model not found: {model_path}", err=True)
240240
sys.exit(2)
241241

242-
# Read sig
242+
# Read sig (C2 fix: validate fingerprint matches embedded public key)
243243
try:
244-
sig = read_sig(sig_file_path)
244+
sig = read_sig(sig_file_path, validate_fingerprint=True)
245+
except ValueError as e:
246+
if not quiet:
247+
click.echo(f"FAILED {model_path.name}\n Sig file corrupted: {e}", err=True)
248+
sys.exit(1)
245249
except Exception as e:
246250
if not quiet:
247251
click.echo(f"Error reading sig file: {e}", err=True)
248252
sys.exit(2)
249253

250254
# Reconstruct public key for verification
255+
# C3 fix: warn loudly when using embedded key (no independent trust verification)
256+
using_trusted_key = False
251257
try:
252258
if pubkey_path is not None:
253259
pub_key = load_public_key(Path(pubkey_path))
260+
using_trusted_key = True
254261
else:
255-
# Use embedded public key from sig file
256-
raw_pub = base64.b64decode(sig.public_key)
257-
from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PublicKey
258-
from cryptography.hazmat.primitives import serialization
259-
pub_key = Ed25519PublicKey.from_public_bytes(raw_pub)
262+
# Check keyring for matching key
263+
keyring_dir = DEFAULT_KEY_DIR / "keyring"
264+
trusted_keys = keyring_list(keyring_dir)
265+
matched_trust = None
266+
for entry in trusted_keys:
267+
if entry["fingerprint"] == sig.key_fingerprint:
268+
pub_key = load_public_key(Path(entry["path"]))
269+
using_trusted_key = True
270+
matched_trust = entry["alias"]
271+
break
272+
273+
if not using_trusted_key:
274+
# Fall back to embedded key — but WARN the user
275+
raw_pub = base64.b64decode(sig.public_key)
276+
from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PublicKey
277+
pub_key = Ed25519PublicKey.from_public_bytes(raw_pub)
260278
except Exception as e:
261279
if not quiet:
262280
click.echo(f"Error loading public key: {e}", err=True)
@@ -273,7 +291,20 @@ def verify(model_path, sig_path, pubkey_path, output_json, quiet):
273291
click.echo(f"Error hashing model: {e}", err=True)
274292
sys.exit(2)
275293

276-
# Rebuild message and verify
294+
# C1 fix: check file hash against sig BEFORE verifying signature
295+
# sig.sha256 is stored as "sha256:<hex>", model_hash is raw hex
296+
expected_hash = sig.sha256
297+
if expected_hash.startswith("sha256:"):
298+
expected_hash = expected_hash[7:]
299+
if model_hash != expected_hash:
300+
if output_json:
301+
click.echo(json.dumps({"verified": False, "error": "hash mismatch",
302+
"detail": "file has been modified since signing"}))
303+
elif not quiet:
304+
click.echo(f"FAILED {model_path.name}\n Hash mismatch — file has been modified since signing.")
305+
sys.exit(1)
306+
307+
# Rebuild message and verify signature
277308
identity_bytes = canonical_json(sig.identity)
278309
if model_path.is_dir():
279310
message = build_dir_message(model_hash, identity_bytes)
@@ -289,33 +320,53 @@ def verify(model_path, sig_path, pubkey_path, output_json, quiet):
289320

290321
ok = verify_bytes(message, signature_bytes, pub_key)
291322

323+
if not ok:
324+
if output_json:
325+
click.echo(json.dumps({"verified": False, "error": "invalid signature",
326+
"detail": "signature or identity card has been tampered with"}))
327+
elif not quiet:
328+
click.echo(f"FAILED {model_path.name}\n Invalid signature — sig file may have been tampered with.")
329+
sys.exit(1)
330+
331+
# Determine trust level for display
332+
fp = compute_fingerprint(pub_key)
333+
if pubkey_path:
334+
trust_label = "TRUSTED (--pubkey)"
335+
elif using_trusted_key:
336+
trust_label = f"TRUSTED (keyring: {matched_trust})"
337+
else:
338+
trust_label = "UNVERIFIED — using embedded key, not independently trusted"
339+
292340
if output_json:
293341
result = {
294-
"verified": ok,
342+
"verified": True,
295343
"model": str(model_path),
296344
"sha256": f"sha256:{model_hash}",
297-
"fingerprint": sig.key_fingerprint,
345+
"fingerprint": fp,
346+
"trust": trust_label,
298347
"signed_at": sig.signed_at,
299348
"identity": sig.identity,
300349
}
301350
click.echo(json.dumps(result))
302-
if not ok:
303-
sys.exit(1)
304351
return
305352

306353
if quiet:
307-
if not ok:
308-
sys.exit(1)
309354
return
310355

311-
if ok:
312-
click.echo(f"VERIFIED: {model_path}")
313-
click.echo(f" Identity: {sig.identity.get('name', '(unnamed)')}")
314-
click.echo(f" Fingerprint: {sig.key_fingerprint}")
315-
click.echo(f" Signed at: {sig.signed_at} (unverified — no RFC 3161 timestamp)")
316-
else:
317-
click.echo(f"FAILED: signature verification failed for {model_path}")
318-
sys.exit(1)
356+
click.echo(f"VERIFIED {model_path.name}")
357+
click.echo(f" Name: {sig.identity.get('name', '(unnamed)')}")
358+
if sig.identity.get("creator"):
359+
click.echo(f" Creator: {sig.identity['creator']}")
360+
if sig.identity.get("architecture"):
361+
click.echo(f" Architecture: {sig.identity['architecture']}")
362+
if sig.identity.get("base_model"):
363+
click.echo(f" Base model: {sig.identity['base_model']}")
364+
if sig.identity.get("license"):
365+
click.echo(f" License: {sig.identity['license']}")
366+
click.echo(f" Signed: {sig.signed_at} (unverified — no RFC 3161 timestamp)")
367+
click.echo(f" Key: {fp} ({trust_label})")
368+
if not using_trusted_key:
369+
click.echo(f" WARNING: key not in keyring. Run 'modelsign keyring add <pubkey> <alias>' to trust it.")
319370

320371

321372
@main.command()

tests/test_cli.py

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ def test_version():
1010
runner = CliRunner()
1111
result = runner.invoke(main, ["version"])
1212
assert result.exit_code == 0
13-
assert "1.0.0" in result.output
13+
import modelsign
14+
assert modelsign.__version__ in result.output
1415

1516

1617
def test_keygen(tmp_path):
@@ -192,3 +193,87 @@ def test_verify_json_output(tmp_path):
192193
data = json.loads(result.output)
193194
assert data["verified"] is True
194195
assert data["identity"]["name"] == "json-verify"
196+
197+
198+
def test_verify_without_pubkey_warns_untrusted(tmp_path):
199+
"""C3 regression: verify without --pubkey must warn about untrusted embedded key."""
200+
runner = CliRunner()
201+
202+
key_dir = tmp_path / "keys"
203+
key_dir.mkdir()
204+
runner.invoke(main, ["keygen", "--out", str(key_dir / "private.pem")])
205+
206+
model_path = tmp_path / "model.safetensors"
207+
model_path.write_bytes(b"fake-model-untrusted-test")
208+
209+
runner.invoke(main, [
210+
"sign", str(model_path),
211+
"--name", "untrusted-test",
212+
"--key", str(key_dir / "private.pem"),
213+
])
214+
215+
# Verify WITHOUT --pubkey — should still verify but warn about untrusted key
216+
result = runner.invoke(main, ["verify", str(model_path)])
217+
assert result.exit_code == 0
218+
assert "VERIFIED" in result.output
219+
assert "UNVERIFIED" in result.output or "WARNING" in result.output or "not independently trusted" in result.output
220+
221+
222+
def test_verify_tampered_file_shows_hash_mismatch(tmp_path):
223+
"""C1 regression: tampered file must show 'hash mismatch', not generic sig failure."""
224+
runner = CliRunner()
225+
226+
key_dir = tmp_path / "keys"
227+
key_dir.mkdir()
228+
runner.invoke(main, ["keygen", "--out", str(key_dir / "private.pem")])
229+
230+
model_path = tmp_path / "model.safetensors"
231+
model_path.write_bytes(b"original-weights")
232+
233+
runner.invoke(main, [
234+
"sign", str(model_path),
235+
"--name", "hash-test",
236+
"--key", str(key_dir / "private.pem"),
237+
])
238+
239+
model_path.write_bytes(b"tampered-weights")
240+
241+
result = runner.invoke(main, [
242+
"verify", str(model_path),
243+
"--pubkey", str(key_dir / "public.pem"),
244+
])
245+
assert result.exit_code != 0
246+
assert "Hash mismatch" in result.output or "hash mismatch" in result.output
247+
248+
249+
def test_verify_forged_sig_fails(tmp_path):
250+
"""C3 regression: attacker-signed file with attacker key must still fail with --pubkey."""
251+
runner = CliRunner()
252+
253+
# Legitimate key
254+
legit_dir = tmp_path / "legit"
255+
legit_dir.mkdir()
256+
runner.invoke(main, ["keygen", "--out", str(legit_dir / "private.pem")])
257+
258+
# Attacker key
259+
attacker_dir = tmp_path / "attacker"
260+
attacker_dir.mkdir()
261+
runner.invoke(main, ["keygen", "--out", str(attacker_dir / "private.pem")])
262+
263+
model_path = tmp_path / "model.safetensors"
264+
model_path.write_bytes(b"attacker-controlled-weights")
265+
266+
# Attacker signs with their key
267+
runner.invoke(main, [
268+
"sign", str(model_path),
269+
"--name", "legit-looking-model",
270+
"--key", str(attacker_dir / "private.pem"),
271+
])
272+
273+
# Verify with legitimate pubkey — must FAIL
274+
result = runner.invoke(main, [
275+
"verify", str(model_path),
276+
"--pubkey", str(legit_dir / "public.pem"),
277+
])
278+
assert result.exit_code != 0
279+
assert "FAILED" in result.output

0 commit comments

Comments
 (0)