feat(circuits): add GCP JWT verifier with TEE attestation claims extraction #1317
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new GCP JWT Verifier circuit and supporting tooling: Circom templates for JWT parsing and full x5c certificate-chain verification, certificate/public-key utilities, JSON-field extraction, a TypeScript input-preparation script, README and example fixture, plus build script and build-system path/ptau updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as prepare.ts (CLI)
participant JWT as JWT File (header.payload.sig + x5c[])
participant Circuit as GCPJWTVerifier Circuit
participant CertUtils as Cert Utilities
participant SigVer as SignatureVerifier
Note over CLI,JWT: prepare.ts parses JWT + x5c chain and builds circuit inputs
CLI->>JWT: read JWT + x5c[0..2]
CLI->>CertUtils: parse certs -> DER, pubkeys, offsets, signatures
CLI->>Circuit: write circuit_inputs.json
Note over Circuit,CertUtils: in-circuit verification flow
Circuit->>SigVer: SHA-256(message) then RSA verify (leaf pubkey)
Circuit->>CertUtils: ExtractAndValidatePubkey(x5c[0]) -> leaf pubkey
Circuit->>SigVer: VerifyCertificateSignature(x5c[0] signed by x5c[1])
Circuit->>SigVer: VerifyCertificateSignature(x5c[1] signed by x5c[2])
Circuit->>CertUtils: ExtractAndValidatePubkey(x5c[2]) -> root pubkey -> compute hash
Circuit->>Circuit: ExtractAndVerifyJSONField(payload, eat_nonce)
Circuit->>Circuit: ExtractAndVerifyJSONField(payload, image_digest)
Circuit-->>CLI: public outputs (publicKeyHash, eat_nonce, image_hash)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
circuits/circuits/gcp_jwt_verifier/README.md(1 hunks)circuits/circuits/gcp_jwt_verifier/example_jwt.txt(1 hunks)circuits/circuits/gcp_jwt_verifier/gcp_jwt_verifier.circom(1 hunks)circuits/circuits/gcp_jwt_verifier/jwt_verifier.circom(1 hunks)circuits/circuits/gcp_jwt_verifier/prepare.ts(1 hunks)circuits/circuits/utils/gcp_jwt/extractAndValidatePubkey.circom(1 hunks)circuits/circuits/utils/gcp_jwt/verifyCertificateSignature.circom(1 hunks)circuits/circuits/utils/gcp_jwt/verifyExtractedString.circom(1 hunks)circuits/package.json(2 hunks)circuits/scripts/build/build_gcp_jwt_verifier.sh(1 hunks)circuits/scripts/build/common.sh(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use Yarn only for package management (yarn install/add/remove); do not use npm or pnpm in scripts
Files:
circuits/package.json
circuits/**/*.circom
⚙️ CodeRabbit configuration file
circuits/**/*.circom: Review ZK circuit code for:
- Circuit correctness and completeness
- Constraint efficiency
- Input validation
- Security considerations for zero-knowledge proofs
Files:
circuits/circuits/gcp_jwt_verifier/gcp_jwt_verifier.circomcircuits/circuits/utils/gcp_jwt/verifyExtractedString.circomcircuits/circuits/utils/gcp_jwt/extractAndValidatePubkey.circomcircuits/circuits/gcp_jwt_verifier/jwt_verifier.circomcircuits/circuits/utils/gcp_jwt/verifyCertificateSignature.circom
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
circuits/circuits/gcp_jwt_verifier/prepare.ts
🧬 Code graph analysis (1)
circuits/scripts/build/build_gcp_jwt_verifier.sh (1)
circuits/scripts/build/common.sh (2)
download_ptau(28-40)build_circuit(50-153)
🔇 Additional comments (3)
circuits/package.json (1)
11-11: Yarn-only usage looks good; script entry is fine.
No issues spotted with the new script from a package manager policy standpoint.circuits/scripts/build/common.sh (1)
75-79: Ensure all circuit include roots are covered.If any includes resolve like "@zk-email/..." you may also need a linker path for that package’s circuits directory, or rely on "-l node_modules" resolving module roots correctly. Verify in CI to avoid linker surprises.
circuits/circuits/gcp_jwt_verifier/prepare.ts (1)
47-56: Repository unavailable for verification—manual review required.The codebase could not be accessed to verify the concerns about the hex substring offset detection logic (lines 47-56). While the review identifies a potentially legitimate risk—that
indexOf()searches on hex-encoded DER data can produce incorrect offsets due to encoding variations—I cannot confirm:
- Whether the current implementation actually causes failures or is production-tested
- If ASN.1 parsing libraries are already available as dependencies
- How the offset is used downstream in circuit validation
- The actual severity relative to other code risks
Action required: Manually verify that this offset calculation is resilient across all expected JWT certificate encodings from GCP, or implement proper ASN.1 parsing with verified SubjectPublicKeyInfo extraction.
| // Verify x5c[0] signature using x5c[1] public key | ||
| VerifyCertificateSignature(signatureAlgorithm, n, k, MAX_CERT_LENGTH)( | ||
| leaf_cert, | ||
| leaf_cert_padded_length, | ||
| intermediate_pubkey, | ||
| leaf_signature | ||
| ); | ||
|
|
||
| // Extract and validate x5c[2] public key | ||
| ExtractAndValidatePubkey(signatureAlgorithm, n, k, MAX_CERT_LENGTH, MAX_PUBKEY_PREFIX, MAX_PUBKEY_LENGTH)( | ||
| root_cert, | ||
| root_pubkey_offset, | ||
| root_pubkey_actual_size, | ||
| root_pubkey | ||
| ); | ||
|
|
||
| // Verify x5c[1] signature using x5c[2] public key | ||
| VerifyCertificateSignature(signatureAlgorithm, n, k, MAX_CERT_LENGTH)( | ||
| intermediate_cert, | ||
| intermediate_cert_padded_length, | ||
| root_pubkey, | ||
| intermediate_signature | ||
| ); |
There was a problem hiding this comment.
No root‑of‑trust anchor; chain is unanchored and therefore forgeable.
You verify leaf←intermediate←root signatures but never pin the root to a trusted key. A prover can pick any self‑consistent chain. Anchor root (e.g., SPKI hash) as a constant or public input checked in‑circuit, and bind x5c[] used here to the JWT header’s x5c.
- Add a public input expected_root_spki_hash (or hardcode for GCP CA) and constrain Poseidon(root_pubkey) to match.
- Optionally also output header hash and assert header.x5c[0..2] digests correspond to the provided certificates to prevent chain substitution.
🤖 Prompt for AI Agents
In circuits/circuits/gcp_jwt_verifier/gcp_jwt_verifier.circom around lines
121-143 the chain is unanchored (you verify leaf←intermediate←root but never pin
the root), so add a root-of-trust check: introduce a public input (e.g.
expected_root_spki_hash) or hardcode the known GCP CA SPKI hash, compute the
Poseidon hash (or existing digest primitive used elsewhere) over root_pubkey (or
its SPKI encoding) and constrain that hash to equal expected_root_spki_hash;
additionally, bind the x5c entries to the JWT header by hashing the
header.x5c[0..2] (or header hash) and asserting those digests correspond to the
certificate blobs used above to prevent chain substitution.
| // Extract substring from payload at the claimed offset | ||
| signal extracted_eat_nonce[MAX_EAT_NONCE_B64_LENGTH] <== SelectSubArray( | ||
| maxPayloadLength, | ||
| MAX_EAT_NONCE_B64_LENGTH | ||
| )( | ||
| payload, | ||
| eat_nonce_0_offset, | ||
| eat_nonce_0_b64_length | ||
| ); | ||
|
|
||
| // GCP spec: nonce must be 10-74 bytes decoded | ||
| // Base64url encoding: 10 bytes = 14 chars, 74 bytes = 99 chars | ||
| // https://cloud.google.com/confidential-computing/confidential-space/docs/connect-external-resources | ||
|
|
||
| // Make sure nonce is not empty | ||
| component length_nonzero = IsZero(); | ||
| length_nonzero.in <== eat_nonce_0_b64_length; | ||
| length_nonzero.out === 0; // Must NOT be zero | ||
|
|
||
| // Validate nonce minimum length (10 bytes decoded = 14 base64url chars) | ||
| component length_min_check = GreaterEqThan(log2Ceil(MAX_EAT_NONCE_B64_LENGTH)); | ||
| length_min_check.in[0] <== eat_nonce_0_b64_length; | ||
| length_min_check.in[1] <== 14; | ||
| length_min_check.out === 1; | ||
|
|
||
| // Validate nonce maximum length (74 bytes decoded = 99 base64url chars) | ||
| component length_max_check = LessEqThan(log2Ceil(MAX_EAT_NONCE_B64_LENGTH)); | ||
| length_max_check.in[0] <== eat_nonce_0_b64_length; | ||
| length_max_check.in[1] <== 99; | ||
| length_max_check.out === 1; | ||
|
|
There was a problem hiding this comment.
Nonce length logic inconsistent with spec and MAX length; add charset check and structural binding.
- MAX_EAT_NONCE_B64_LENGTH is 88 (64 bytes), but you check ≤ 99 chars (74 bytes). Decide one spec and enforce it.
- No base64url charset enforcement; any bytes pass.
- Offset isn’t tied to the "eat_nonce" JSON key; substring could be from anywhere in payload.
- var MAX_EAT_NONCE_B64_LENGTH = 88; // ...
+ var MAX_EAT_NONCE_B64_LENGTH = 99; // 74 bytes → 99 base64url chars (spec), or lower if you intend 64 bytes
@@
- component length_max_check = LessEqThan(log2Ceil(MAX_EAT_NONCE_B64_LENGTH));
- length_max_check.in[0] <== eat_nonce_0_b64_length;
- length_max_check.in[1] <== 99;
+ component length_max_check = LessEqThan(log2Ceil(MAX_EAT_NONCE_B64_LENGTH));
+ length_max_check.in[0] <== eat_nonce_0_b64_length;
+ length_max_check.in[1] <== MAX_EAT_NONCE_B64_LENGTH;
length_max_check.out === 1;Additionally:
- Use zk‑regex to enforce base64url: ^[A-Za-z0-9_-]+$.
- Match the JSON key context (e.g., a small regex around
"eat_nonce":[\s]*\["...]) rather than trusting an offset.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In circuits/circuits/gcp_jwt_verifier/gcp_jwt_verifier.circom around lines
145-175, the nonce length checks and provenance are incorrect and there's no
base64url charset enforcement: align the length bounds with the chosen
MAX_EAT_NONCE_B64_LENGTH (decide whether MAX_EAT_NONCE_B64_LENGTH represents
74-byte(=99-char) or 64-byte(=88-char) max and use that constant consistently
for min/max comparisons), replace the current numeric checks that misuse
log2Ceil with direct comparisons using the actual max/min constants, add a
zk‑regex check to extracted_eat_nonce enforcing base64url charset
^[A-Za-z0-9_-]+$ and non-empty, and instead of trusting eat_nonce_0_offset
alone, verify the offset/length fall inside a matched JSON context by matching
the surrounding payload bytes against a small zk‑regex that locates the
"eat_nonce" key (or minimal JSON pattern) and assert the substring lies within
that match.
| template VerifyExtractedString(MAX_LENGTH) { | ||
| signal input extracted[MAX_LENGTH]; // String extracted from payload | ||
| signal input input_string[MAX_LENGTH]; // String provided as input (padded) | ||
| signal input length; // Actual length of the string | ||
|
|
||
| // Verify each character matches (with padding) | ||
| component char_match[MAX_LENGTH]; | ||
| for (var i = 0; i < MAX_LENGTH; i++) { | ||
| char_match[i] = GreaterThan(log2Ceil(MAX_LENGTH)); | ||
| char_match[i].in[0] <== length; | ||
| char_match[i].in[1] <== i; | ||
|
|
||
| // If within length: extracted must equal input | ||
| // If beyond length: input must be 0 (padding) | ||
| char_match[i].out * (extracted[i] - input_string[i]) === 0; | ||
| (1 - char_match[i].out) * input_string[i] === 0; | ||
| } |
There was a problem hiding this comment.
Add length upper‑bound and constrain extracted bytes beyond length to 0.
Without a bound, length can exceed MAX_LENGTH (fits into log2Ceil bits), and bytes in extracted[i] for i ≥ length remain unconstrained. Tighten constraints.
template VerifyExtractedString(MAX_LENGTH) {
signal input extracted[MAX_LENGTH]; // String extracted from payload
signal input input_string[MAX_LENGTH]; // String provided as input (padded)
signal input length; // Actual length of the string
+ // Enforce 0 <= length <= MAX_LENGTH
+ component length_max = LessEqThan(log2Ceil(MAX_LENGTH));
+ length_max.in[0] <== length;
+ length_max.in[1] <== MAX_LENGTH;
+ length_max.out === 1;
// Verify each character matches (with padding)
component char_match[MAX_LENGTH];
for (var i = 0; i < MAX_LENGTH; i++) {
char_match[i] = GreaterThan(log2Ceil(MAX_LENGTH));
char_match[i].in[0] <== length;
char_match[i].in[1] <== i;
// If within length: extracted must equal input
// If beyond length: input must be 0 (padding)
char_match[i].out * (extracted[i] - input_string[i]) === 0;
(1 - char_match[i].out) * input_string[i] === 0;
+ // Also force extracted padding to zero to avoid unconstrained witness bytes
+ (1 - char_match[i].out) * extracted[i] === 0;
}
}🤖 Prompt for AI Agents
In circuits/circuits/utils/gcp_jwt/verifyExtractedString.circom around lines 10
to 26, add an explicit upper-bound on length and constrain extracted bytes
beyond length to zero: ensure length is constrained to the valid range (0 <=
length <= MAX_LENGTH) by adding a comparator/constraint that enforces length <=
MAX_LENGTH (and optionally non-negative via bit-size or comparator), and inside
the loop add the complementary conditional so that when i >= length you enforce
extracted[i] === 0 while keeping the existing checks for i < length
(extracted[i] === input_string[i]) — this prevents length from exceeding
MAX_LENGTH and ensures bytes past length are constrained to 0.
| eat_nonce_0_b64_output_length <== eat_nonce_0_b64_length; | ||
|
|
||
| // Extract image digest from payload | ||
| signal extracted_image_digest[MAX_IMAGE_DIGEST_LENGTH] <== SelectSubArray( |
There was a problem hiding this comment.
can you check the prefix here too?
Release to Staging - 2025-10-24
Release to Staging - 2025-11-01
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on November 17
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
circuits/circuits/utils/gcp_jwt/verifyJSONFieldExtraction.circom
Outdated
Show resolved
Hide resolved
circuits/circuits/utils/gcp_jwt/verifyJSONFieldExtraction.circom
Outdated
Show resolved
Hide resolved
circuits/circuits/utils/gcp_jwt/verifyJSONFieldExtraction.circom
Outdated
Show resolved
Hide resolved
| colon_after_key === 58; // ASCII code for ':' | ||
|
|
||
| // Verify value comes after colon (prevents pointing to colon itself) | ||
| component offset_check = GreaterThan(log2Ceil(maxJSONLength)); |
There was a problem hiding this comment.
why not just check if they're equal here?
There was a problem hiding this comment.
JSON allows whitespace, so strict equality would incorrectly reject valid JSON with minor formatting differences. Currently, we use a loose GreaterThan check but we can tighten this by enforcing a closer match. For example, by measuring structural similarity within a reasonable threshold
There was a problem hiding this comment.
yeah let's check if value offset is the next index or the next to next one (if there's a space) and leave it at that
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
circuits/circuits/utils/gcp_jwt/verifyJSONFieldExtraction.circom(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
circuits/**/*.circom
⚙️ CodeRabbit configuration file
circuits/**/*.circom: Review ZK circuit code for:
- Circuit correctness and completeness
- Constraint efficiency
- Input validation
- Security considerations for zero-knowledge proofs
Files:
circuits/circuits/utils/gcp_jwt/verifyJSONFieldExtraction.circom
🔇 Additional comments (3)
circuits/circuits/utils/gcp_jwt/verifyJSONFieldExtraction.circom (3)
34-69: Strong improvements to JSON structure validation.The key_offset underflow protection (lines 34-38), colon validation (lines 67-69), and the quote/key validation logic effectively address the previous security concerns about quote offset underflow and missing structural validation.
71-109: Excellent array structure enforcement.The two-pattern validation (
:["vs.: [") with strict value_offset enforcement successfully addresses previous concerns about JSON array structure bypass and unsafe value extraction. The constraint at line 109 ensures value_offset cannot be arbitrarily chosen by the prover.
41-41: ItemAtIndex performs internal bounds checking—concern is mitigated but code clarity could improve.ItemAtIndex enforces bounds by comparing the requested index against the array length and using a conditional multiplexer to either output the selected element when index < maxArrayLen or output zero otherwise. The code already validates
key_offset >= 1before the line 41 access. However, the remaining ItemAtIndex calls at lines 64, 75, 94, and 102 rely on implicit constraints through character matching (quotes, colons, brackets) rather than explicit bounds checks.In a zero-knowledge circuit, returning zero for out-of-bounds is safe—the prover cannot exploit this to hide invalid witnesses. The character assertions provide indirect validation that offsets land within the JSON structure. Still, adding explicit bounds validation for derived offsets (like
key_offset + key_lengthandcolon_position + N) would strengthen code clarity and defense-in-depth, particularly for maintainability.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
circuits/circuits/utils/gcp_jwt/verifyJSONFieldExtraction.circom(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
circuits/**/*.circom
⚙️ CodeRabbit configuration file
circuits/**/*.circom: Review ZK circuit code for:
- Circuit correctness and completeness
- Constraint efficiency
- Input validation
- Security considerations for zero-knowledge proofs
Files:
circuits/circuits/utils/gcp_jwt/verifyJSONFieldExtraction.circom
🧠 Learnings (1)
📚 Learning: 2025-09-01T13:28:11.251Z
Learnt from: mitschabaude
Repo: selfxyz/self PR: 949
File: circuits/circuits/register/register_aadhaar.circom:39-47
Timestamp: 2025-09-01T13:28:11.251Z
Learning: In zk-email-verify's Sha256Bytes circuit, paddedInLength is already constrained to be ≤ maxDataLength, so additional bounds checking for this parameter is not needed when using this component.
Applied to files:
circuits/circuits/utils/gcp_jwt/verifyJSONFieldExtraction.circom
🔇 Additional comments (2)
circuits/circuits/utils/gcp_jwt/verifyJSONFieldExtraction.circom (2)
34-42: Good: Underflow protection added.The
key_offset >= 1constraint properly prevents the underflow issue flagged in previous reviews. The subsequent access atkey_offset - 1is now safe.
71-109: Good JSON structure validation, but verify bounds for intermediate positions.The pattern-matching logic for
"key":["value"]vs"key": ["value"]is well-designed and addresses previous structural validation concerns. The enforcement ofvalue_offsetbased on the detected pattern prevents offset manipulation attacks.However,
colon_position + 2andcolon_position + 3(lines 94, 102) could also exceedmaxJSONLength. Consider adding bounds validation for these intermediate positions as well, similar to the suggestions for other offset calculations.
| signal input key_length; | ||
| signal input value_offset; | ||
| signal input value_length; |
There was a problem hiding this comment.
Critical: Missing validation on length inputs.
key_length and value_length are accepted as inputs without constraints. A prover could set these to 0 or values exceeding their respective maximums, causing out-of-bounds access in subsequent ItemAtIndex calls (e.g., lines 64, 118-122).
Add range validation for both lengths:
signal input key_offset;
signal input key_length;
signal input value_offset;
signal input value_length;
+ // Validate key_length is within bounds and non-zero
+ component key_length_valid = LessEqThan(log2Ceil(maxKeyNameLength));
+ key_length_valid.in[0] <== key_length;
+ key_length_valid.in[1] <== maxKeyNameLength;
+ key_length_valid.out === 1;
+ component key_length_nonzero = GreaterThan(log2Ceil(maxKeyNameLength));
+ key_length_nonzero.in[0] <== key_length;
+ key_length_nonzero.in[1] <== 0;
+ key_length_nonzero.out === 1;
+
+ // Validate value_length is within bounds and non-zero
+ component value_length_valid = LessEqThan(log2Ceil(maxValueLength));
+ value_length_valid.in[0] <== value_length;
+ value_length_valid.in[1] <== maxValueLength;
+ value_length_valid.out === 1;
+ component value_length_nonzero = GreaterThan(log2Ceil(maxValueLength));
+ value_length_nonzero.in[0] <== value_length;
+ value_length_nonzero.in[1] <== 0;
+ value_length_nonzero.out === 1;| // Verify closing quote after key | ||
| signal key_quote_after <== ItemAtIndex(maxJSONLength)(json, key_offset + key_length); | ||
| key_quote_after === 34; // ASCII code for " | ||
|
|
||
| // Verify colon after closing quote (ensures valid JSON key:value structure) | ||
| signal colon_after_key <== ItemAtIndex(maxJSONLength)(json, key_offset + key_length + 1); | ||
| colon_after_key === 58; // ASCII code for ':' |
There was a problem hiding this comment.
Critical: Missing bounds validation on key position calculations.
While the colon validation addresses previous concerns, key_offset + key_length + 1 could exceed maxJSONLength, causing undefined behavior in ItemAtIndex. In field arithmetic, out-of-bounds access may wrap around, allowing malformed JSON to pass validation.
Add bounds validation before these checks:
+ // Validate key position is within bounds
+ component key_end_bounds = LessThan(log2Ceil(maxJSONLength));
+ key_end_bounds.in[0] <== key_offset + key_length + 1;
+ key_end_bounds.in[1] <== maxJSONLength;
+ key_end_bounds.out === 1;
+
// Verify closing quote after key
signal key_quote_after <== ItemAtIndex(maxJSONLength)(json, key_offset + key_length);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Verify closing quote after key | |
| signal key_quote_after <== ItemAtIndex(maxJSONLength)(json, key_offset + key_length); | |
| key_quote_after === 34; // ASCII code for " | |
| // Verify colon after closing quote (ensures valid JSON key:value structure) | |
| signal colon_after_key <== ItemAtIndex(maxJSONLength)(json, key_offset + key_length + 1); | |
| colon_after_key === 58; // ASCII code for ':' | |
| // Validate key position is within bounds | |
| component key_end_bounds = LessThan(log2Ceil(maxJSONLength)); | |
| key_end_bounds.in[0] <== key_offset + key_length + 1; | |
| key_end_bounds.in[1] <== maxJSONLength; | |
| key_end_bounds.out === 1; | |
| // Verify closing quote after key | |
| signal key_quote_after <== ItemAtIndex(maxJSONLength)(json, key_offset + key_length); | |
| key_quote_after === 34; // ASCII code for " | |
| // Verify colon after closing quote (ensures valid JSON key:value structure) | |
| signal colon_after_key <== ItemAtIndex(maxJSONLength)(json, key_offset + key_length + 1); | |
| colon_after_key === 58; // ASCII code for ':' |
🤖 Prompt for AI Agents
In circuits/circuits/utils/gcp_jwt/verifyJSONFieldExtraction.circom around lines
63-69, the code directly indexes ItemAtIndex(maxJSONLength)(json, key_offset +
key_length) and +1 without validating bounds; add explicit range checks for both
key_offset + key_length and key_offset + key_length + 1 against maxJSONLength
(e.g. compute endIndex = key_offset + key_length and endIndexPlusOne = endIndex
+ 1, use a LessThan comparator to assert endIndex < maxJSONLength and
endIndexPlusOne < maxJSONLength, convert those boolean signals to constraints
and then only call ItemAtIndex when the checks pass), and if the checks fail
produce a validation failure signal (or constrain to 0/1) so out-of-bounds
access cannot occur or wrap around.
| // Validate value ends with closing quote and bracket: "value"] | ||
| signal closing_quote <== ItemAtIndex(maxJSONLength)(json, value_offset + value_length); | ||
| closing_quote === 34; // ASCII code for " | ||
|
|
||
| signal closing_bracket <== ItemAtIndex(maxJSONLength)(json, value_offset + value_length + 1); | ||
| closing_bracket === 93; // ASCII code for ] |
There was a problem hiding this comment.
Critical: Missing bounds validation on value end position.
The closing quote and bracket validation correctly checks the structure, but value_offset + value_length + 1 could exceed maxJSONLength, causing out-of-bounds access in ItemAtIndex.
Add bounds validation:
+ // Validate value end position is within bounds
+ component value_end_bounds = LessThan(log2Ceil(maxJSONLength));
+ value_end_bounds.in[0] <== value_offset + value_length + 1;
+ value_end_bounds.in[1] <== maxJSONLength;
+ value_end_bounds.out === 1;
+
// Validate value ends with closing quote and bracket: "value"]
signal closing_quote <== ItemAtIndex(maxJSONLength)(json, value_offset + value_length);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In circuits/circuits/utils/gcp_jwt/verifyJSONFieldExtraction.circom around lines
117 to 122, the code reads json at value_offset + value_length and +1 without
ensuring those indices are within maxJSONLength; add explicit bounds checks
(signals/assertions) that value_offset + value_length < maxJSONLength and
value_offset + value_length + 1 < maxJSONLength before calling ItemAtIndex, and
use those checks to guard or conditionally require the closing_quote and
closing_bracket comparisons so no out-of-bounds access can occur.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 19414827 | Triggered | Generic Password | 12bdd72 | packages/mobile-sdk-alpha/ios/Frameworks/NFCPassportReader.xcframework/ios-arm64/NFCPassportReader.framework/Modules/NFCPassportReader.swiftmodule/arm64-apple-ios.swiftinterface | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
3 participants
Description
This PR implements a circuit for verifying Google Cloud Platform Confidential Space JWT attestations with X.509 certificate chain validation and TEE-specific claim extraction (EAT nonce and Docker image hash).
Changes
Updated Powers of Tau download source URL since the previous one was down (e16daba)
Fixed circom compilation by adding correct
node_moduleslibrary paths for@zk-kitandcircomlibdependencies (0f963573)Added JWT verification circuit dependencies:
@zk-email/jwt-tx-builder-circuits,@zk-email/jwt-tx-builder-helpers,node-forge(b532bd98)Created
extractAndValidatePubkeycircuit to extract and validate X.509 certificate public keys with ASN.1 structure verification (eb497f62)Created
verifyCertificateSignaturecircuit to hash TBS certificates and verify RSA signatures (8356718b)Created
verifyExtractedStringcircuit for generic string extraction with padding validation, used by EAT nonce and image digest extraction (2fd97583)Implemented GCP JWT verifier circuit with complete 3-signature certificate chain validation, EAT nonce
extraction, and container image digest verification as public outputs (13ae2194)
Added build script for GCP JWT verifier that compiles circuit, generates proving/verification keys, and
exports Solidity verifier (ad0c626a)
Summary by CodeRabbit
New Features
Documentation
Chores
Note
Adds a GCP JWT verifier circuit that validates the full x5c certificate chain and extracts EAT nonce and container image hash, with input prep tooling, build script, and dependencies.
circuits/gcp_jwt_verifier/gcp_jwt_verifier.circom: New main circuit verifying JWT (RSA) and fullx5c[0..2]chain; outputs root CA pubkey hash,eat_nonce[0](b64url), and image SHA256 hash.jwt_verifier.circom,utils/gcp_jwt/extractAndValidatePubkey.circom,utils/gcp_jwt/verifyCertificateSignature.circom,utils/gcp_jwt/verifyJSONFieldExtraction.circom.prepare.tsto parse JWT, extract x5c certs/keys/signatures, locateeat_nonceandimage_digest, and emit circuit inputs; addsexample_jwt.txt.scripts/build/build_gcp_jwt_verifier.shand npm scriptbuild-gcp-jwt-verifierto compile and generate keys.@zk-email/jwt-tx-builder-circuits,@zk-email/jwt-tx-builder-helpers, andnode-forgeinpackage.json.Written by Cursor Bugbot for commit fcb40e9. This will update automatically on new commits. Configure here.