fix: use RFC 8785 JCS canonicalization for VC signing#5
Closed
HaraldeRoessler wants to merge 1 commit into
Closed
Conversation
Replace json.dumps(sort_keys=True) with jcs.canonicalize() (RFC 8785) for credential signing in credentials.py and endorsement.py. json.dumps(sort_keys=True) is not a proper canonicalization — it does not guarantee consistent output for unicode normalization, floating point representation, or key ordering in nested structures. JCS (JSON Canonicalization Scheme, RFC 8785) provides deterministic serialization. New credentials include canonicalizationAlgorithm: "JCS" in the proof. Verification checks this field and falls back to sort_keys for legacy credentials, ensuring backward compatibility. Note: hash computation functions (sports, signals, fantasy, interaction proofs) are intentionally unchanged — migrating those would invalidate existing commitment hashes stored in the database. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Superseded by #7 which combines JCS + PQC to avoid merge conflicts. |
HaraldeRoessler
added a commit
to HaraldeRoessler/moltrust-api
that referenced
this pull request
May 12, 2026
Follow-up to commit d25e70c (SSRF). After running CodeQL default-setup on the fork, 17 additional findings surfaced. Triage outcome: Already closed by earlier commits this PR: 1 (SSRF) False positives (dismissed via CodeQL UI): 4 Real findings fixed in this commit: 5 Stack-trace-exposure (deferred to design): 7 FIXES IN THIS COMMIT #1 [LOG SANITISATION] credit_middleware exception swallows DB password - app/main.py (logger.error in credit_middleware) `logger.error("…: %s", caller_did, e)` — the raw exception `e` can be an asyncpg ConnectionError whose repr() includes the Postgres connection string (with the password). Log only `type(e).__name__` instead. MoltyCel#2 [DEFENSIVE URL ENCODING] /join?ref= referrer parameter - app/main.py /join handler The redirect target is HARDCODED to https://moltrust.ch — the host is not user-controlled. But `f"https://moltrust.ch?ref={ref}"` interpolates `ref` raw, and a payload like `ref="x&malparam=…"` could corrupt the query string. Use `urllib.parse.quote(ref)` to percent-encode the value before interpolation. MoltyCel#3 [STDOUT TOKEN LEAK] telegram_hn_remind print(r.text) - scripts/telegram_hn_remind.py `print(f'Status: {r.status_code}, Response: {r.text}')` — if Telegram error responses ever echo the request URL (which contains the bot token in the path), the body lands in stdout / CI scrollback. Print only the status code. MoltyCel#4 [ReDoS] mpp authorization header regex - packages/mpp/index.js `auth.match(/^(?:Payment|MPP)\s+(.+)$/i)` on an unbounded header is polynomial-quadratic. This package is published to npm, so consumer servers carry the risk. Cap header at 8 KiB and use bounded `\s{1,8}` with a non-greedy first char. MoltyCel#5 [ReDoS] moltrust-openclaw-v2 base URL trim - moltrust-openclaw-v2/src/client.ts `.replace(/\/+$/, "")` is polynomial on pathological inputs. Replace with a `while (str.endsWith("/")) str = str.slice(0, -1)` loop, which is linear. DISMISSED AS FALSE POSITIVES (no code change) MoltyCel#14 py/clear-text-logging-sensitive-data at SPIFFE bind log Logs spiffe_uri, did, caller_did — none are passwords. CodeQL misfires on the "did" → "id" → "password" name-similarity heuristic. MoltyCel#13, MoltyCel#12 py/clear-text-logging-sensitive-data in scripts/threadwatch.py Telegram bot token flows into the request URL but never into a logger or print() call — only to requests.post (which doesn't log URLs by default). MoltyCel#16 py/weak-sensitive-data-hashing in _reg_tracker This is in-memory rate-limit bucket-key derivation, not password storage. bcrypt/argon2 would be wrong here (slow + salted breaks the lookup). SHA-256 of the full API key is the correct primitive for an O(1) tracker. EXPLICITLY DEFERRED (7 stack-trace-exposure findings) Multiple endpoints currently return `{"error": str(e)[:100]}` to callers. CodeQL flags these as info disclosure. Fixing them means changing the API contract — clients that parse the `error` field would break. This is a design call for the maintainer; deferring to a separate PR + discussion rather than including in this hardening pass. VERIFICATION Python 3.12 AST parse — app/main.py + scripts/telegram_hn_remind.py compile cleanly. `node -c packages/mpp/index.js` clean. The TS file change is a syntactically-trivial loop, not type-impacting. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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
json.dumps(sort_keys=True)withjcs.canonicalize()(RFC 8785) for credential signingcanonicalizationAlgorithm: JCSto proof objects for new credentialssort_keysfor legacy credentials without the fieldWhy
json.dumps(sort_keys=True)is not a proper canonicalization scheme. It does not guarantee consistent output for unicode normalization, floating point representation, or key ordering in nested structures. JCS (RFC 8785) provides deterministic serialization required for cryptographic signatures.The IPR module already uses
jcs— this makes VC signing consistent.Files changed
app/credentials.pyapp/swarm/endorsement.pyNot changed (intentionally)
Hash functions in
sports.py,signals.py,fantasy.py, andinteraction_proof.pystill usejson.dumps(sort_keys=True, separators=...). Changing those would invalidate existing commitment hashes in the database.Test plan
canonicalizationAlgorithm: JCS/credentials/verifyGenerated with Claude Code