feat: JCS canonicalization + post-quantum dual-signature (Ed25519 + ML-DSA-65)#7
Closed
HaraldeRoessler wants to merge 1 commit into
Closed
Conversation
…L-DSA-65) Two related changes combined to avoid merge conflicts: 1. RFC 8785 JCS canonicalization for VC signing Replace json.dumps(sort_keys=True) with jcs.canonicalize() for deterministic serialization. Verification supports both JCS (new) and sort_keys (legacy) via canonicalizationAlgorithm field in proof. 2. Post-quantum dual-signature with ML-DSA-65 (Dilithium3) Every credential is signed with both Ed25519 and ML-DSA-65 when Dilithium keys are configured. Falls back to Ed25519-only if not. New files: - app/crypto/dilithium.py: ML-DSA-65 key management, sign, verify - app/crypto/hybrid.py: dual_sign() and verify_proof() - scripts/generate_dilithium_keys.py: keypair generation utility Modified files: - app/credentials.py: uses dual_sign(), verify_proof() handles all formats - app/swarm/endorsement.py: endorsement VCs use dual_sign() - app/main.py: DID document dynamically includes Dilithium key, proof list handling for DB inserts Tested locally with docker-compose: - Ed25519-only mode (no Dilithium keys): works, backward compatible - Dual-signature mode: Ed25519 (64B) + Dilithium (3,309B) both verified - DID document shows both keys when Dilithium configured - Legacy single-proof credentials still verify correctly Migration path: Phase 1 (this PR): dual-signature, Ed25519 + ML-DSA-65 Phase 2 (future): ML-DSA-65 first, Ed25519 deprecated Phase 3 (future): Ed25519 sunset Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 1, 2026
Owner
AI Review Result — 3-Model Consensus: 🔴 BLOCKIERENPR #7 wurde einem Security-fokussierten 3-Modell-Review unterzogen Verdikt pro Modell
Blockierende Findings (je ≥2 Modelle bestätigt)1. Downgrade/Stripping-Angriff — hybrid.py::verify_proof Fix: beide Legs MÜSSEN präsent und valide sein (AND-Logik). 2. Private-Key stdout-Leak — scripts/generate_dilithium_keys.py Fix: Key niemals auf stdout. Nur in sichere Datei (chmod 600) Kritische Nebenbefunde
Zu den 6 Review-Kernfragen
Nächste SchritteBitte Rebase auf aktuelles main + Fixes für die zwei Blocker, Review durchgeführt via ai_review.py (3-Modell-Konsens, |
Contributor
Author
|
Superseded by #209, which is rebased on current main (VC Data Model v2.0) and fixes both blockers from the 3-model review: (1) the downgrade/stripping attack via proof-skeleton binding into the signature, and (2) the private-key stdout leak. The side findings (JCS fail-closed, hardcoded proof[0], liboqs pin) are also addressed. Closing this stale branch in favour of feat/jcs-pqc-dual-signature-v2. |
MoltyCel
added a commit
that referenced
this pull request
Jul 5, 2026
…nt off) (#210) * feat: JCS canonicalization + post-quantum dual-signature v2 (rebased, blockers fixed) Re-implementation of PR #7 against current main (VC Data Model v2.0), fixing both blockers from the 3-model security review and the side findings. Upstream review blockers fixed: 1. Downgrade/stripping attack (app/crypto/hybrid.py::verify_proof) Verification now uses AND-logic: when a credential carries a list of proofs, EVERY proof must be present and valid. A broken Dilithium leg fails the whole credential — an attacker cannot strip the PQC leg and rely on Ed25519 alone. Composite-signature semantics (BSI TR-02102-1). The proof signs the credential body (proof field stripped), so both legs bind the same canonical bytes. 2. Private-key stdout leak (scripts/generate_dilithium_keys.py) The ML-DSA-65 secret key is NEVER printed to stdout/stderr. It is written only to a chmod 600 file (dilithium_secret_key.hex), created with an explicit fchmod so it is never group/world-readable even momentarily. A --kms mode emits base64 raw bytes for piping into `aws kms encrypt`. Side findings fixed: - JCS fail-closed: if the jcs library is missing, dual_sign raises rather than emit a proof labelled canonicalizationAlgorithm=JCS that was actually produced with json.dumps(sort_keys=True) (false-negative/DoS vector). - Hardcoded proof[0] indexing removed: new app/crypto/proof_utils.py extracts proofs by type. The 3 DB-insert sites in main.py + the aae_id fallback now use get_primary_proof_value(), which finds the Ed25519 proof by type — never accidentally persisting a Dilithium proofValue into the proof_value column. - liboqs-python pinned in requirements.txt (>=0.10.2). Imported lazily by dilithium.py so the app starts without it; required only when DILITHIUM_* env vars are set. VC v2.0 integration (the old PR was written against v1): - credentials.py: issue_credential uses dual_sign(); verify_credential uses verify_proof(). Both keep the existing validFrom/validUntil v2.0 shape and the vc_valid_from/vc_valid_until helpers. Legacy sort_keys credentials still verify (backward compat) via the canonicalizationAlgorithm field. - endorsement.py: signing block uses dual_sign() against the v2.0 VC. - main.py: DID_WEB_DOCUMENT is built dynamically (_build_issuer_did_document) and includes the Dilithium verification method + context when configured. /.well-known/did.json rebuilds per-request so a key enabled after startup appears without a restart. #key-1 kept as a legacy alias for verifiers that still resolve the old id. Zero-risk deploy unchanged: without DILITHIUM_PRIVATE_KEY_HEX (and no KMS blob) the system is Ed25519-only, exactly as before. Locally verified (venv with pynacl+jcs, oqs stubbed): - Ed25519-only issue + verify (real JCS) - tampered body rejected - legacy sort_keys credentials still verify - dual-signature list issued - AND-logic: one bad leg fails the whole credential - JCS fail-closed raises when jcs missing - py_compile clean on all 8 files including main.py (8273 lines) * fix(pqc): bind proof skeleton into signature — close downgrade attack The 3-model review blocker #1 (downgrade/stripping) was not fully solved by AND-logic alone: my first implementation stripped the proof field before signing, so an attacker who removed the Dilithium leg left the Ed25519 signature valid over unchanged bytes — the exact OR-downgrade the review flagged. A prior test even asserted the wrong behaviour (stripped single verifies standalone). Fix per the review explicit requirement (proof field must be included in the signed message): - dual_sign now builds a proof SKELETON (every intended proof dict with proofValue blanked) and signs the credential body PLUS that skeleton. Both Ed25519 and Dilithium sign the same canonicalized skeleton bytes. - verify_proof reconstructs the same skeleton from the presented proofs. If a leg is missing, the skeleton differs from what the issuer signed, so the surviving Ed25519 signature fails too. Stripping a leg now breaks every remaining signature — composite-signature semantics (IETF composite-sigs, BSI TR-02102-1). - dual_sign refuses to silently fall back to Ed25519-only when Dilithium is configured but signing fails (that would re-open the downgrade path: an attacker cannot distinguish a deliberate Ed25519-only cred from a failed-dual one). It raises instead. - Legacy sort_keys credentials (no canonicalizationAlgorithm) still verify over the body only — backward compat unchanged, and they only ever had one leg so there was nothing to strip. Also fix blocker #2 fully: scripts/generate_dilithium_keys.py --kms mode previously printed base64(secret_key) to stdout. It now writes the KMS-ready base64 to a chmod 600 file (dilithium_secret_key.kms.b64) instead. Runtime proof: script stdout contains only the PUBLIC key; no secret (hex or base64) reaches stdout/stderr. Secret files are mode 0600. Verified: - Downgrade attack (PQC-enabled issuer -> strip Dilithium -> verify) now FAILS: Ed25519 signature invalid because skeleton changed. - Legit Ed25519-only (non-PQC issuer) still verifies. - Legacy sort_keys credentials still verify. - Dual both-legs-valid -> valid; one bad leg -> whole cred invalid. - JCS fail-closed raises when jcs missing. - py_compile clean on all 8 files. * fix(pqc): harden input validation, exact-match type dispatch, verifyMethod binding Fresh security review with a new model found input-validation gaps and type-confusion risks that the previous version did not cover. None of these are forgery attacks (the attacker still cannot produce a valid signature without the key), but they would cause 500 errors on malformed input and one was a type-confusion code smell. Input validation — all malformed inputs now return valid=False instead of raising (which would surface as 500 errors and be a minor DoS vector): verify_proof: - Non-dict credential (None, str, int, list, bool) rejected - Non-dict/non-list proof field rejected - Non-dict proof entry in a list rejected - None / wrong-type verificationMethod rejected - None / wrong-type proofValue (None, int, float, list, bytes, "") rejected - Non-hex proofValue rejected - Wrong-length proofValue rejected verify_credential: same validations plus type checks for the proof list normalization. proof_utils: get_proofs/find_proof/get_primary_proof_value now accept non-dict credentials and return empty / raise KeyError instead of AttributeError. Type confusion — proof type dispatch changed from substring to exact match. Previously, `"Ed25519" in ptype` would match any string containing "Ed25519" (e.g. "EvilEd25519NotReally"). The attacker still could not forge a signature, but the type field was effectively untrustworthy. Now: `ptype == "Ed25519Signature2020"` exact match. Anything else is rejected as "Unknown proof type". Defense-in-depth — proof verificationMethod is now cross-checked against the key being used: - Ed25519 proof: verificationMethod must start with did:web:api.moltrust.ch#key-ed25519 or #key-1 (legacy alias) - Dilithium proof: verificationMethod must start with did:web:api.moltrust.ch#key-dilithium A valid signature over a valid body is no longer enough if the proof claims to be from a key we are not using. Dead code removed: use_skeleton (set but never used), dil_sig (set to the function object, never called). Coverage: 54 new tests in tests/test_pqc_security.py covering: - Downgrade attack (PQC-enabled -> strip Dilithium -> rejected) - AND-logic (one bad leg fails whole credential) - Swapped proof order rejected - Fake Dilithium added to Ed25519-only rejected - Legitimate credentials verify (Ed25519-only and dual) - Body tamper rejected - Extra field in proof rejected - Legacy sort_keys backward compat - Non-dict / None / wrong-type credential rejected - Non-dict / None / wrong-type proof field rejected - None / wrong-type verificationMethod rejected - None / wrong-type proofValue rejected - Non-hex / short / wrong-length proofValue rejected - Type confusion ("EvilEd25519NotReally", bare "Ed25519", "ed25519signature2020") rejected - proof_utils helpers handle non-dict credentials All 54 tests pass. py_compile clean on all files. The downgrade attack remains blocked (verified again in the new test suite). * test: reload credentials module in stub fixture so verify_credential picks up reloaded hybrid When stub_oqs reloads app.crypto.hybrid, the app.credentials module still holds the old reference to verify_proof. Reload credentials too so the TestVerifyCredentialWrapper tests work correctly. Fixes the 2 failures that appeared when test_credentials_vc_v2.py and test_pqc_security.py run together. * fix(pqc): exact-match verificationMethod and proof type, remove latent substring/prefix bugs Fresh review found three additional hardening gaps: 1. verificationMethod binding used startswith(). An ID like did:web:api.moltrust.ch#key-ed25519-attacker would pass the defense-in-depth check (no forgery without the key, but the type field became untrustworthy). Now exact equality against the allowed key-id sets { #key-ed25519, #key-1 } and { #key-dilithium }. 2. proof_utils.find_proof() still used substring matching. Removed: exact equality only, so "EvilEd25519NotReally" no longer matches a search for "Ed25519Signature2020". No security-critical caller was using it, but it was a latent bug. 3. hybrid.verify_proof() had `isinstance(ed25519_verify_key, object)`, which is always True for any non-None Python value. Removed the useless check; bad keys are caught by the verify() exception handler. Also removed unused get_ed25519_proof imports from credentials.py and main.py. Tests: 71 pass (54 new PQC security + 10 existing VC v2 + 7 new exact-match regressions). Downgrade attack and all input-validation paths remain blocked. * fix(pqc): wrap _load_keypair plaintext hex parsing in try/except The KMS path already had a try/except around bytes.fromhex(), but the plaintext env-var fallback did not. A misconfigured DILITHIUM_PUBLIC_KEY_HEX or DILITHIUM_PRIVATE_KEY_HEX (non-hex string) would raise ValueError, propagate through is_available() -> dual_sign, and crash credential issuance with a 500. Now returns None (PQC off) and logs the error, matching the KMS path pattern. Added 2 tests covering bad public and bad private key hex. * fix(pqc): enforce dual-signature policy for PQC-capable issuers + hard-pin liboqs Addresses both findings from the 3-model review (Gemini + Perplexity): 1. BLOCKER (Perplexity Sonar Pro): verify_proof/verify_credential did not enforce that a PQC-capable issuer MUST dual-sign JCS credentials. has_dual_signature() existed but was never called. An Ed25519-only JCS credential from a PQC-enabled issuer was accepted — a policy downgrade. Fix: verify_proof now checks, before the per-proof loop, whether: (a) the verifier is PQC-capable (dilithium.is_available()), AND (b) the credential uses JCS (new format, detected via _has_skeleton). If both are true and the credential is NOT dual-signed, it is rejected with "PQC policy violation". Legacy credentials (sort_keys, no canonicalizationAlgorithm) are exempt — they predate the PQC policy and only ever had one leg. Threat model documented in the module docstring. 2. NACHBESSERN (Gemini + Perplexity consensus): liboqs-python>=0.10.2 was a minimum version, not a hard pin. Pre-1.0, not FIPS-validated C-binding → supply-chain risk. Hard-pinned to ==0.10.2. Tests: 78 pass (5 new PQC policy tests + 1 new skeleton-binding test). - PQC issuer Ed25519-only JCS → rejected (policy) - PQC issuer dual-signed → accepted - Non-PQC verifier Ed25519-only JCS → accepted - PQC verifier legacy sort_keys → exempt (accepted) - Strip Dilithium breaks Ed25519 signature even without policy (skeleton) * fix(pqc): pin liboqs-python to actual released version + fix policy trigger Two issues caught by CI: 1. liboqs-python==0.10.2 does not exist on PyPI. The 3-model review cited 0.10.2 as the pin target, but the actual released versions are 0.14.1 and 0.15.0 (verified via `pip index versions liboqs-python`). The hard pin to a non-existent version broke `pip install` in CI: "Could not find a version that satisfies the requirement liboqs-python==0.10.2". Fixed to ==0.15.0 (latest stable). 2. The PQC policy trigger in verify_proof used `dilithium.is_available()`, which calls _load_keypair() and therefore requires the SECRET key (or working KMS). If the public key is configured but KMS is temporarily down, is_available() returns False and the policy is silently skipped — an Ed25519-only JCS credential from a PQC-capable issuer would be accepted. The policy should fire on public-key declaration alone, not on full keypair availability. Fix: added `dilithium.public_key_configured()` which checks the env var directly. Updated verify_proof to use it. Updated _setup_pqc test helper to stub the new function. Added 4 tests: - Policy fires when public key configured but is_available() False - public_key_configured returns False when env var unset - public_key_configured returns True when env var set - public_key_configured returns False for whitespace-only env var Tests: 82 pass (was 78). py_compile clean. pip install -r requirements.txt resolves cleanly with ==0.15.0. * fix(pqc): error propagation, canonicalization exception handling, proofValue length cap Fresh cross-check found three issues: 1. verify_credential dropped verify_proof's explicit error when no checks array existed. verify_proof early returns (e.g. PQC policy violation, "credential is not a dict", "No proof found") only set `error`, not `checks`. verify_credential aggregated errors from `checks`, producing an empty `error` string. Fixed by preferring `result["error"]` when present. 2. verify_proof only caught RuntimeError around _signed_payload. A malformed JCS credential (e.g. non-serializable value in the body) could raise ValueError/TypeError from _canonicalize and bubble up, causing a 500 when verify_proof is called directly. Fixed by catching Exception and returning it as a check error. 3. No length limit on proofValue before bytes.fromhex. A malicious multi-megabyte hex string would be accepted and decoded, causing memory DoS. Added _MAX_PROOFVALUE_HEX_LEN = 20000 (generous margin over the largest expected ML-DSA-65 sig of 6618 hex chars). Rejected proofValue returns valid=False with "too long" error. Tests: 85 pass (was 82). Added: - test_verify_credential_preserves_policy_error - test_verify_credential_preserves_early_error - test_very_long_proofvalue_rejected * feat(pqc): make dual-signature policy advisory by default (PQC_ENFORCE) The PQC dual-signature capability stays in the credential format and the verify path, but is no longer hard-enforced by default. A central switch PQC_ENFORCE (env, default off) gates it: - off (default): the policy check runs and its outcome is surfaced (pqc_policy: would_reject) and logged, but a missing Dilithium leg does not fail verification -- existing Ed25519-only issuers keep working. - on: reject a PQC-capable issuer single-signed JCS credential, as before. Legacy sort_keys credentials remain exempt. The liboqs pin, proofValue DoS cap and error propagation from f3817d4 are unchanged. Tests: split the policy test into advisory-accept + enforce-reject; the reject-path tests now run under PQC_ENFORCE=true. Adds an ADR note. Builds on Harald f3817d4 (his commits preserved as ancestors). --------- Co-authored-by: Harald Roessler <harald.roessler@dsncon.com>
Owner
|
Superseded by #210 (squash-merged, rebased auf VC Data Model v2.0). Beide ursprünglichen Blocker adressiert. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two tightly related changes combined into one PR to avoid merge conflicts:
1. RFC 8785 JCS canonicalization — replaces
json.dumps(sort_keys=True)with properjcs.canonicalize()for deterministic VC serialization2. Post-quantum dual-signature — every credential signed with both Ed25519 (64 bytes) and ML-DSA-65/Dilithium3 (3,309 bytes) when configured
Zero-risk deployment
If
DILITHIUM_PRIVATE_KEY_HEXis not set, the system behaves exactly as before (Ed25519-only). Enable PQC by generating keys:Credential format
Without Dilithium (backward compatible):
{"proof": {"type": "Ed25519Signature2020", "canonicalizationAlgorithm": "JCS", ...}}With Dilithium (dual-signature):
{"proof": [ {"type": "Ed25519Signature2020", "verificationMethod": "...#key-ed25519", ...}, {"type": "DilithiumSignature2026", "verificationMethod": "...#key-dilithium", ...} ]}New files
app/crypto/dilithium.pyapp/crypto/hybrid.pydual_sign()andverify_proof()for all proof formatsscripts/generate_dilithium_keys.pyModified files
app/credentials.pydual_sign(),verify_proof()handles legacy + dual + future formatsapp/swarm/endorsement.pydual_sign()app/main.pyLocally tested
Ed25519: valid,Dilithium: valid)#key-ed25519,#key-1legacy,#key-dilithium)Supersedes PRs #5 and #6
Replaces #5 (JCS only) and #6 (PQC only) which would have caused merge conflicts.
Merge order
This PR is independent but recommended to merge after #1 (which adds
jcsto requirements.txt).Generated with Claude Code