Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions circuits/circuits/gcp_jwt_verifier/gcp_jwt_verifier.circom
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ include "../utils/gcp_jwt/verifyCertificateSignature.circom";
include "../utils/gcp_jwt/verifyJSONFieldExtraction.circom";
include "circomlib/circuits/comparators.circom";
include "@openpassport/zk-email-circuits/utils/array.circom";
include "@openpassport/zk-email-circuits/utils/bytes.circom";

/// @title GCPJWTVerifier
/// @notice Verifies GCP JWT signature and full x5c certificate chain
Expand Down Expand Up @@ -70,11 +71,11 @@ template GCPJWTVerifier(


// 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
// EAT nonce (payload.eat_nonce[0])
var MAX_EAT_NONCE_B64_LENGTH = 99; // Max length for base64url string (74 bytes decoded = 99 b64url chars)
var MAX_EAT_NONCE_B64_LENGTH = 74; // Max length for base64url string (74 bytes decoded = 99 b64url chars)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Incorrect base64 length constant truncates valid nonces

The MAX_EAT_NONCE_B64_LENGTH was changed from 99 to 74, but the variable represents base64url character length, not decoded byte length. The comment even states "74 bytes decoded = 99 b64url chars". This causes the circuit to reject valid nonces with base64 lengths between 75-99 characters. The prepare.ts file still uses 99, creating an inconsistency. The length check at line 167 and the ExtractAndVerifyJSONField template instantiation will fail for full-length nonces that are valid per the GCP spec.

Additional Locations (1)

Fix in Cursor Fix in Web

var MAX_EAT_NONCE_KEY_LENGTH = 10; // Length of "eat_nonce" key (without quotes)
var EAT_NONCE_PACKED_CHUNKS = computeIntChunkLength(MAX_EAT_NONCE_B64_LENGTH);
signal input eat_nonce_0_b64_length; // Length of base64url string
signal input eat_nonce_0_key_offset; // Offset in payload where "eat_nonce" key starts (after opening quote)
signal input eat_nonce_0_value_offset; // Offset in payload where eat_nonce[0] value appears
Expand All @@ -83,6 +84,7 @@ template GCPJWTVerifier(
var MAX_IMAGE_DIGEST_LENGTH = 71; // "sha256:" + 64 hex chars
var IMAGE_HASH_LENGTH = 64; // Just the hex hash portion
var MAX_IMAGE_DIGEST_KEY_LENGTH = 12; // Length of "image_digest" key (without quotes)
var IMAGE_HASH_PACKED_CHUNKS = computeIntChunkLength(IMAGE_HASH_LENGTH);
signal input image_digest_length; // Length of full string (should be 71)
signal input image_digest_key_offset; // Offset in payload where "image_digest" key starts (after opening quote)
signal input image_digest_value_offset; // Offset in payload where image_digest value appears
Expand All @@ -91,8 +93,8 @@ template GCPJWTVerifier(
var maxPayloadLength = (maxB64PayloadLength * 3) \ 4;

signal output rootCAPubkeyHash; // Root CA (x5c[2]) pubkey, trust anchor
signal output eat_nonce_0_b64_output[MAX_EAT_NONCE_B64_LENGTH]; // eat_nonce[0] base64url string
signal output image_hash[IMAGE_HASH_LENGTH]; // Container image SHA256 hash (without "sha256:" prefix)
signal output eat_nonce_0_b64_packed[EAT_NONCE_PACKED_CHUNKS]; // eat_nonce[0] base64url string packed with PackBytes
signal output image_hash_packed[IMAGE_HASH_PACKED_CHUNKS]; // Container image SHA256 hash (64 hex chars) packed with PackBytes

// Verify JWT Signature (using x5c[0] public key)
component jwtVerifier = JWTVerifier(n, k, maxMessageLength, maxB64HeaderLength, maxB64PayloadLength);
Expand Down Expand Up @@ -162,7 +164,7 @@ template GCPJWTVerifier(
// 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.in[1] <== MAX_EAT_NONCE_B64_LENGTH;
length_max_check.out === 1;

// Validate nonce offset bounds (prevent reading beyond payload)
Expand Down Expand Up @@ -195,7 +197,7 @@ template GCPJWTVerifier(
eatNonceExtractor.expected_key_name <== expected_eat_nonce_key;

// Output the extracted base64url string
eat_nonce_0_b64_output <== eatNonceExtractor.extracted_value;
eat_nonce_0_b64_packed <== PackBytes(MAX_EAT_NONCE_B64_LENGTH)(eatNonceExtractor.extracted_value);

// Validate length is exactly 71 ("sha256:" + 64 hex chars)
image_digest_length === 71;
Expand Down Expand Up @@ -244,9 +246,12 @@ template GCPJWTVerifier(
extracted_image_digest[6] === 58; // ':'

// Extract and output only the 64-char hash (skip "sha256:" prefix)
signal image_hash_bytes[IMAGE_HASH_LENGTH];
for (var i = 0; i < IMAGE_HASH_LENGTH; i++) {
image_hash[i] <== extracted_image_digest[7 + i];
image_hash_bytes[i] <== extracted_image_digest[7 + i];
}

image_hash_packed <== PackBytes(IMAGE_HASH_LENGTH)(image_hash_bytes);
}

component main = GCPJWTVerifier(1, 120, 35);
31 changes: 27 additions & 4 deletions circuits/circuits/gcp_jwt_verifier/prepare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,6 @@ async function main() {
);
}

// Decode for verification/logging (not used in circuit)
const eatNonce0Buffer = Buffer.from(eatNonce0Base64url, 'base64url');
console.log(`[INFO] eat_nonce[0] decoded: ${eatNonce0Buffer.length} bytes`);

// Find offset of eat_nonce[0] in the decoded payload JSON
// Decode the payload from base64url to get the exact JSON string
const payloadJSON = Buffer.from(payloadB64, 'base64url').toString('utf8');
Expand Down Expand Up @@ -285,6 +281,30 @@ async function main() {
eatNonce0CharCodes[i] = eatNonce0Base64url.charCodeAt(i);
}

const eatNonce1Base64url = payload.eat_nonce[1];
console.log(`[INFO] eat_nonce[1] (base64url): ${eatNonce1Base64url}`);
console.log(`[INFO] eat_nonce[1] string length: ${eatNonce1Base64url.length} characters`);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Missing array length check before accessing second nonce

The code accesses payload.eat_nonce[1] at line 284 without verifying the array has at least 2 elements. The existing validation at line 240 only checks payload.eat_nonce.length === 0, not length < 2. If a JWT contains only one nonce in the eat_nonce array, eatNonce1Base64url will be undefined, and accessing .length on line 286 will throw a TypeError and crash the preparation script.

Fix in Cursor Fix in Web


if (eatNonce1Base64url.length > MAX_EAT_NONCE_B64_LENGTH) {
throw new Error(
`[ERROR] eat_nonce[1] length ${eatNonce1Base64url.length} exceeds max ${MAX_EAT_NONCE_B64_LENGTH}`
);
}

const eatNonce1ValueOffset = payloadJSON.indexOf(eatNonce1Base64url);
if (eatNonce1ValueOffset === -1) {
console.error('[ERROR] Could not find eat_nonce[1] value in decoded payload JSON');
console.error('[DEBUG] Payload JSON:', payloadJSON);
console.error('[DEBUG] Looking for:', eatNonce1Base64url);
throw new Error('[ERROR] Could not find eat_nonce[1] value in decoded payload JSON');
}
console.log(`[INFO] eat_nonce[1] value offset in payload: ${eatNonce1ValueOffset}`);

const eatNonce1CharCodes = new Array(MAX_EAT_NONCE_B64_LENGTH).fill(0);
for (let i = 0; i < eatNonce1Base64url.length; i++) {
eatNonce1CharCodes[i] = eatNonce1Base64url.charCodeAt(i);
}

// Extract image_digest from payload.submods.container.image_digest
if (!payload.submods?.container?.image_digest) {
throw new Error('[ERROR] No image_digest found in payload.submods.container');
Expand Down Expand Up @@ -378,6 +398,9 @@ async function main() {
eat_nonce_0_key_offset: eatNonce0KeyOffset.toString(),
eat_nonce_0_value_offset: eatNonce0ValueOffset.toString(),

// EAT nonce[1] (circuit will extract value directly from payload)
eat_nonce_1_b64_length: eatNonce1Base64url.length.toString(),

// Container image digest (circuit will extract value directly from payload)
image_digest_length: imageDigest.length.toString(),
image_digest_key_offset: imageDigestKeyOffset.toString(),
Expand Down
53 changes: 27 additions & 26 deletions circuits/circuits/utils/gcp_jwt/verifyJSONFieldExtraction.circom
Original file line number Diff line number Diff line change
Expand Up @@ -74,50 +74,51 @@ template ExtractAndVerifyJSONField(
// Check character at colon+1: must be '[' (91) or space (32)
signal char_after_colon <== ItemAtIndex(maxJSONLength)(json, colon_position + 1);

signal value_start <== ItemAtIndex(maxJSONLength)(json, value_offset);

// is_bracket: 1 if char is '[', 0 otherwise
component is_bracket = IsEqual();
is_bracket.in[0] <== char_after_colon;
is_bracket.in[1] <== 91; // '['

// is_space: 1 if char is space, 0 otherwise
component is_space = IsEqual();
is_space.in[0] <== char_after_colon;
is_space.in[1] <== 32; // ' '

// Exactly one must be true: char is either '[' or space
is_bracket.out + is_space.out === 1;
// is_quote: 1 if char is quote, 0 otherwise
component is_quote = IsEqual();
is_quote.in[0] <== char_after_colon;
is_quote.in[1] <== 34; // "

// If bracket at colon+1: check quote at colon+2, value at colon+3
// If space at colon+1: check bracket at colon+2, quote at colon+3, value at colon+4
// Exactly one must be true: char is either [ or quote
is_bracket.out + is_quote.out === 1;

// When is_bracket=1 (no space): expect quote at colon+2
// When is_bracket=1 : expect quote at colon+2
signal char_at_plus2 <== ItemAtIndex(maxJSONLength)(json, colon_position + 2);
// When is_space=1: expect bracket at colon+2
// Constraint: if is_bracket=1, char_at_plus2 must be quote(34)
// if is_space=1, char_at_plus2 must be bracket(91)
// if is_quote=1, char_at_plus2 must be value[0]
is_bracket.out * (char_at_plus2 - 34) === 0; // If bracket at +1, quote at +2
is_space.out * (char_at_plus2 - 91) === 0; // If space at +1, bracket at +2

// When is_space=1: check quote at colon+3
signal char_at_plus3 <== ItemAtIndex(maxJSONLength)(json, colon_position + 3);
is_space.out * (char_at_plus3 - 34) === 0; // If space at +1, quote at +3

// Enforce value_offset based on pattern
// Pattern 1 (no space): :[" -> value at colon+3
// Pattern 2 (space): : [" -> value at colon+4
signal expected_value_offset <== colon_position + 3 + is_space.out;
value_offset === expected_value_offset;
component is_value_after_quote = IsEqual();
is_value_after_quote.in[0] <== char_at_plus2;
is_value_after_quote.in[1] <== value_start;
is_quote.out * (1 - is_value_after_quote.out) === 0;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Missing value_offset constraint allows arbitrary value extraction

The constraint value_offset === expected_value_offset was removed, which previously enforced that the extracted value position matched the expected location relative to the verified key. For the array format (:["value"]), there's now no constraint linking value_offset to the key position. A malicious prover could provide a key_offset pointing to the correct key while using a value_offset pointing to a different valid-looking value elsewhere in the JSON, effectively extracting an unintended value while passing verification.

Fix in Cursor Fix in Web


// Extract value from JSON and output directly
extracted_value <== SelectSubArray(
maxJSONLength,
maxValueLength
)(json, value_offset, value_length);

// Validate value ends with closing quote and bracket: "value"]
// Validate value ends with closing quote and then either ']' or ',' after
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 ]
// The character following the closing quote must be either ']' (93) or ',' (44)
signal char_after_quote <== ItemAtIndex(maxJSONLength)(json, value_offset + value_length + 1);
component is_closing_bracket = IsEqual();
is_closing_bracket.in[0] <== char_after_quote;
is_closing_bracket.in[1] <== 93; // ']'

component is_comma = IsEqual();
is_comma.in[0] <== char_after_quote;
is_comma.in[1] <== 44; // ','

// Exactly one of the two must be true
is_closing_bracket.out + is_comma.out === 1;
}
1 change: 1 addition & 0 deletions circuits/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"nice": "prettier --write .",
"test": "yarn test-base 'tests/**/*.test.ts' --exit",
"test-base": "yarn ts-mocha -n import=tsx --max-old-space-size=8192 --paths -p tsconfig.json",
"test-gcp-jwt-verifier": "yarn test-base 'tests/gcp_jwt_verifier/gcp_jwt_verifier.test.ts' --exit",
"test-custom-hasher": "yarn test-base 'tests/other_circuits/custom_hasher.test.ts' --exit",
"test-disclose": "yarn test-base 'tests/disclose/vc_and_disclose.test.ts' --exit",
"test-disclose-aadhaar": "yarn test-base 'tests/disclose/vc_and_disclose_aadhaar.test.ts' --exit",
Expand Down
Loading