kaleido-io / kaleido-iden3-samples

Sample code for using the iden3 protocol to issue verifiable claims
Apache License 2.0
5 stars 4 forks source link

Holder wallet's sendCallback is failing with checksum error on holder ID #14

Closed nedgar closed 1 year ago

nedgar commented 1 year ago

With @jimthematrix's latest changes in https://github.com/kaleido-io/kaleido-iden3-samples/pull/12, if I go through the steps from scratch, in final step the holder wallet's callback invocation (new) fails with Error: fromBytes error: checksum error in sendCallback line: from: Id.fromBigInt(BigInt(holderId)).string(),

I added a console.log of the holderId, seen in output below, so it's reading it OK.

e.g.

node index.js --holder AliceWonder --qrcode ~/Downloads/challenge-qr2.png

...
Calculated witness successfully written to file /Users/nedgar/iden3/AliceWonder/witness-079da6b78545c4e08c02.wtns
Successfully generated proof!
holderId: 316342200244053077361837322580837170633470168946294832427234421230335098880
Error: fromBytes error: checksum error
    at Function.fromBytes (/Users/nedgar/src/github.com/kaleido-io/kaleido-iden3-samples/holder/wallet/node_modules/@iden3/js-iden3-core/dist/cjs/id.js:60:19)
    at Function.fromBigInt (/Users/nedgar/src/github.com/kaleido-io/kaleido-iden3-samples/holder/wallet/node_modules/@iden3/js-iden3-core/dist/cjs/id.js:70:19)
    at sendCallback (/Users/nedgar/src/github.com/kaleido-io/kaleido-iden3-samples/holder/wallet/index.js:228:14)
    at generateProof (/Users/nedgar/src/github.com/kaleido-io/kaleido-iden3-samples/holder/wallet/index.js:212:9)

FYI @Chengxuan

Chengxuan commented 1 year ago

Hi @nedgar , I hit the same error when I tried it, there is a workaround:

replace from: Id.fromBigInt(BigInt(holderId)).string(), with the string id. e.g. from: "11C3BYGvF9QaTBGCYfV3tiKQ5tQh1Fpu7YtnazFczS", That should get past it.

I'm looking at what's going on, as Jim has it working in his setup. I tried using the test data in https://github.com/iden3/js-iden3-auth/blob/develop/test/core.test.ts#L41 locally and I hit the same error, which made me think this is an environment-specific issue.

jimthematrix commented 1 year ago

@nedgar @Chengxuan what version of node do you have? I have tested with v19.0.0 and v18.12.1 locally

nedgar commented 1 year ago

I was running Node v16.17.0. I was able to reproduce the problem using v18.13.0 and v19.4.0.

I think it's a big- vs little-endian issue. e.g. my AliceWonder has ID 114fm2GYvyzqgmvkkd9BLhJrWzbjAp5hVpvkMqX4CL, stored as "userID": "192683142140422890149398643334790695435250728937015592017523328898481258496" in genesis_state.json.

In node v19.4.0 with package @iden3/js-iden3core 0.0.2:

> idStr = '114fm2GYvyzqgmvkkd9BLhJrWzbjAp5hVpvkMqX4CL'
> userID = BigInt('192683142140422890149398643334790695435250728937015592017523328898481258496')

> core.Id.fromString(idStr)
Uncaught Error: fromBytes error: checksum error
    at Id.fromBytes (/Users/nedgar/src/github.com/kaleido-io/kaleido-iden3-samples/holder/wallet/node_modules/@iden3/js-iden3-core/dist/cjs/id.js:60:19)
    at Id.fromString (/Users/nedgar/src/github.com/kaleido-io/kaleido-iden3-samples/holder/wallet/node_modules/@iden3/js-iden3-core/dist/cjs/id.js:66:19)

> bytes = base58.base58_to_binary(idStr)
Uint8Array(31) [
    0,   0,  80, 205, 208, 136, 142, 243,
  138, 145,  34, 128, 154,  71, 183, 218,
  226,  51, 139, 214, 112, 200, 114,  85,
   41,   6, 169, 245,   6,  14, 109
]

> core.fromLittleEndian(bytes.slice())
192683142140422890149398643334790695435250728937015592017523328898481258496n
> core.fromLittleEndian(bytes.slice()) === userID
true

> id = core.Id.getFromBytes(bytes)  // ignores checksum from bytes and recalculates it
> id.string()
'114fm2GYvyzqgmvkkd9BLhJrWzbjAp5hVpvkMqXBQ1' // last 3 chars differ (checksum encoding)

> id.checksum()
Uint8Array(2) [ 109, 14 ]  // reverse order from last two bytes

So it looks like the Go libraries encode the checksum as big endian, while the TS/JS libraries encode it as little endian. Think the Go libraries may have it backwards.

nedgar commented 1 year ago

Think the Go libraries may have it backwards. Yup.

The spec says the ID's checksum should encoded as little endian: https://docs.iden3.io/protocol/spec/#identifier-format


> checksum = bytes.slice(0,-2).reduce((a, b) => a+b)
3693

> bigEndian = [checksum >> 8, checksum & 0xFF]
[ 14, 109 ]

> littleEndian = bigEndian.reverse()
[ 109, 14 ]

So the JS library has it right, the Go library has it wrong. 

@OBrezhniev is this a known issue?
nedgar commented 1 year ago

In the go-iden3-core library v0.1.0 (latest tag), it calculates and stores the checksum in Big Endian, but in the latest on master branch it's been changed to use Little Endian.

nedgar commented 1 year ago

I changed the ID parsing to use:

  console.log("holderId:", holderId)
  const holderIdBytes = toLittleEndian(BigInt(holderId));
  const fromId = Id.getFromBytes(holderIdBytes); // workaround: this ignores the encoded checksum and recalculates it instead
  console.log("fromId:", fromId.string());

and then use from: fromId.string().

But, because of the ID mismatch, the verifier callback fails with:

Error in callback: Error: sender is not used for proof creation, expected 1162xYfDAjp5P5JmEZ9YoqJQCCA1NX7MS4Bnv8m3K3, user from public signals: 1162xYfDAjp5P5JmEZ9YoqJQCCA1NX7MS4Bnv8kvwj
at AtomicQuerySigPubSignals.verifyIdOwnership (/Users/nedgar/src/github.com/kaleido-io/kaleido-iden3-samples/verifier/server/node_modules/@iden3/js-iden3-auth/dist/cjs/circuits/ownershipVerifier.js:7:19) at Verifier.verifyAuthResponse (/Users/nedgar/src/github.com/kaleido-io/kaleido-iden3-samples/verifier/server/node_modules/@iden3/js-iden3-auth/dist/cjs/auth/auth.js:71:28) at processTicksAndRejections (node:internal/process/task_queues:96:5) at async callback (/Users/nedgar/src/github.com/kaleido-io/kaleido-iden3-samples/verifier/server/app.js:125:5)

If I just use the stored ID as @Chengxuan suggested, e.g. from: "1162xYfDAjp5P5JmEZ9YoqJQCCA1NX7MS4Bnv8kvwj", it does authenticate / verify OK.

OBrezhniev commented 1 year ago

@nedgar

In the go-iden3-core library v0.1.0 (latest tag), it calculates and stores the checksum in Big Endian, but in the latest on master branch it's been changed to use Little Endian.

Yes, we've fixed that to match the spec, but js-iden3-core is a new library we just released to match go-iden3-core functionality for the upcoming release. It's incompatible with the current tagged versions because of this fix. This change also impacts circuits, and is bundled with a lot of other changes to circuits (like merklezation of the claim, introduction of DID method, identity profiles, etc).

So for now I would advise to use tagged versions of go libs for issuance, and go/js-iden3-auth for verification.

nedgar commented 1 year ago

Jim @jimthematrix asked in the Polygon Discord's #polygon-id channel when the next release of go-iden3-core might be published. No response yet other than "Checking with team..."

I tried running through the samples using the latest from master (go get github.com/iden3/go-iden3-core@master) but it still fails with a checksum error in the verifier's callback endpoint, even when I fix the ID used for audience:

-  const audience = '1125GJqgw6YEsKFwj63GY87MMxPL9kwDKxPUiwMLNZ';
+  const audience = '1125GJqgw6YEsKFwj63GY87MMxPL9kwDKxPUiwMToR';

I'm not sure which ID it's failing on, but it seems to be the userID in the challenge.json, though the value there is the correct integer serialization of the ID. I'll revert to the workaround of using a hardwired 'from' string for now.

nedgar commented 1 year ago

it seems to be the userID in the challenge.json

Well, it's that value but it's hit when parsing the public signals in js-iden3-auth's constructor AtomicQuerySigPubSignals (the userID is at pubSignals[1]).

Looks like a bug in js-iden3-auth's implementation of ID. I've filed https://github.com/iden3/js-iden3-auth/issues/37

nedgar commented 1 year ago

So it looks like two wrongs were actually making a right! :)

jimthematrix commented 1 year ago

thanks for all the investigations @nedgar. Looks like the best option for now, until the fix for the checksum calculation propagates through the stack, is re-implementing the BytesHelper.calculateChecksum() locally on the JS side, fixing the checksum bits to match the wrong encoding from go-iden3-core.