feat(anoncreds): use legacy prover did #1374

genaris · 2023-03-08T12:41:08Z

Adds useLegacyProverDid flag for credential request creation, which is always set when dealing with legacy indy identifiers. Some checks were added in LegacyIndyCredentialFormatService to make sure we are using legacy ones.

When using IndySdk, only legacy prover_did can be used, while in AnonCredsRs through this flag we can select between prover_did and entropy, unless we are using new identifiers where only entropy can be generated.

Signed-off-by: Ariel Gentile <[email protected]>

packages/anoncreds/src/formats/LegacyIndyCredentialFormatService.ts

      )
    }

+    if (!credentialDefinitionId.match(legacyIndyCredentialDefinitionIdRegex)) {


packages/anoncreds/src/formats/LegacyIndyCredentialFormatService.ts

-    if (!credOffer.schema_id || !credOffer.cred_def_id) {
+    if (
+      !credOffer.schema_id.match(legacyIndySchemaIdRegex) ||
+      !credOffer.cred_def_id.match(legacyIndyCredentialDefinitionIdRegex)


packages/anoncreds/src/formats/LegacyIndyCredentialFormatService.ts

@@ -226,6 +234,11 @@

    const credentialOffer = offerAttachment.getDataAsJson<AnonCredsCredentialOffer>()

+    if (!credentialOffer.cred_def_id.match(legacyIndyCredentialDefinitionIdRegex)) {


packages/anoncreds-rs/src/services/AnonCredsRsHolderService.ts

-        credentialDefinition: CredentialDefinition.load(JSON.stringify(credentialDefinition)),
-        credentialOffer: CredentialOffer.load(JSON.stringify(credentialOffer)),
-        masterSecret: MasterSecret.load(JSON.stringify({ value: { ms: linkSecretRecord.value } })),
+      const isLegacyIdentifier = credentialOffer.cred_def_id.match(legacyIndyCredentialDefinitionIdRegex)


codecov-commenter · 2023-03-08T12:59:41Z

Codecov Report

Merging #1374 (5f17ee8) into main (0351eec) will decrease coverage by 0.03%.
The diff coverage is 68.18%.

@@            Coverage Diff             @@
##             main    #1374      +/-   ##
==========================================
- Coverage   87.23%   87.20%   -0.03%     
==========================================
  Files         780      780              
  Lines       18728    18747      +19     
  Branches     3211     3221      +10     
==========================================
+ Hits        16337    16349      +12     
- Misses       2384     2391       +7     
  Partials        7        7

Impacted Files	Coverage Δ
...sdk/src/anoncreds/services/IndySdkHolderService.ts	`8.40% <0.00%> (-0.15%)`	⬇️
...ncreds/src/formats/LegacyIndyProofFormatService.ts	`72.83% <50.00%> (-0.39%)`	⬇️
...s/src/formats/LegacyIndyCredentialFormatService.ts	`74.86% <75.00%> (-0.14%)`	⬇️
...ncreds-rs/src/services/AnonCredsRsHolderService.ts	`91.72% <85.71%> (-0.42%)`	⬇️
packages/anoncreds/src/index.ts	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

…ver-did

…vice Signed-off-by: Ariel Gentile <[email protected]>

packages/anoncreds/src/formats/LegacyIndyProofFormatService.ts

@@ -499,6 +505,10 @@
    const credentialDefinitions: { [key: string]: AnonCredsCredentialDefinition } = {}

    for (const credentialDefinitionId of credentialDefinitionIds) {
+      if (!credentialDefinitionId.match(legacyIndyCredentialDefinitionIdRegex)) {


TimoGlastra · 2023-03-15T10:07:09Z

What's the blocker for this PR @genaris?

genaris · 2023-03-15T10:18:27Z

What's the blocker for this PR @genaris?

It was hyperledger/anoncreds-rs#137

Now it's merged but waiting for the release in npm integrating it.

TimoGlastra · 2023-03-15T10:36:10Z

Cool! Release is cooking now: https://github.com/hyperledger/anoncreds-rs/releases/tag/v0.1.0-dev.10

Signed-off-by: Ariel Gentile <[email protected]>

genaris · 2023-03-15T14:03:06Z

packages/anoncreds-rs/src/services/AnonCredsRsHolderService.ts

      createReturnObj = CredentialRequest.create({
-        entropy: anoncreds.generateNonce(), // FIXME: find a better source of entropy
+        entropy: !useLegacyProverDid || !isLegacyIdentifier ? anoncreds.generateNonce() : undefined, // FIXME: find a better source of entropy


Another option for generating entropy can be calling agentContext.wallet.generateWalletKey, which in AskarWallet creates an ephemeral key and converts its private key to a base58 string (I guess IndySdkWallet does more or less the same thing). However I'm not sure if it's good to rely on wallet implementation for that (unless we strictly specify the requirements for such generateWalletKey in API).

It feels like we're misusing the generateWalletKey in that case. I think a nonce should provide enough randomness right?

IMHO the nonce generated by anoncreds, apart from being convenient for us, is enough for the purposes of giving some entropy in credential generation. But just trying to find an alternative that provides a true 'alphanumeric' random string as stated in the spec.

TimoGlastra · 2023-03-15T17:29:25Z

packages/anoncreds-rs/src/services/AnonCredsRsHolderService.ts

      createReturnObj = CredentialRequest.create({
-        entropy: anoncreds.generateNonce(), // FIXME: find a better source of entropy
+        entropy: !useLegacyProverDid || !isLegacyIdentifier ? anoncreds.generateNonce() : undefined, // FIXME: find a better source of entropy
+        proverDid: useLegacyProverDid ? generateLegacyProverDidLikeString() : undefined,


As it should be random, do wr need to make this a secure ranom generation? E.g. nonce and then encode it as base58?

We could also add a getRandomBytes method to the wallet?

I copied it from anoncreds/indy-sdk package implementation, but also thought that could be a good idea to rely in anoncreds native library instead of uuid as it's being done currently in generateLegacyProverDidLikeString.

Any 22-bytes long base58 string will be good for that purpose, right?

As it's used for entropy, it may be needed for the string to be generated with actual secure random bytes instead of a random string?

TimoGlastra · 2023-03-15T17:34:21Z

packages/anoncreds/tests/InMemoryAnonCredsRegistry.ts


+    const legacySchemaId = getLegacySchemaId(legacyIssuerId, options.schema.name, options.schema.version)


The old implementation supported both for convenience, why is this needed?

If schema issuerId is an unqualified did, parseIndyDid threw an error. Maybe I can do it 'automatic' (i.e. without config flag and a try-catch?)

It's for tests, so whatever works
Is fine. Just curious

Signed-off-by: Ariel Gentile <[email protected]>

TimoGlastra · 2023-03-17T12:49:22Z

LGTM!!

Signed-off-by: Ariel Gentile <[email protected]>

genaris added 5 commits March 7, 2023 15:51

feat(anoncreds-rs): use new API methods for json conversion

e553b1e

Signed-off-by: Ariel Gentile <[email protected]>

clear object handles after usage

d989f10

Signed-off-by: Ariel Gentile <[email protected]>

fix test

44bd6e5

Signed-off-by: Ariel Gentile <[email protected]>

feat(anoncreds): add useLegacyProverDid for credential requests

b3bb7ee

Signed-off-by: Ariel Gentile <[email protected]>

fix: useLegacyProverDid logic

5a22d86

Signed-off-by: Ariel Gentile <[email protected]>

github-advanced-security bot found potential problems Mar 8, 2023

View reviewed changes

genaris added 2 commits March 13, 2023 21:41

Merge remote-tracking branch 'upstream/main' into feat/use-legacy-pro…

0378ed4

…ver-did

feat(anoncreds: check for legacy dids in legacy indy proof format ser…

32f17d7

…vice Signed-off-by: Ariel Gentile <[email protected]>

genaris mentioned this pull request Mar 14, 2023

Add new changes from AnonCreds RS to AFJ #1371

Closed

3 tasks

github-advanced-security bot found potential problems Mar 14, 2023

View reviewed changes

update anoncreds + don't run rs tests in node < 18

cc03c05

Signed-off-by: Ariel Gentile <[email protected]>

genaris commented Mar 15, 2023

View reviewed changes

genaris marked this pull request as ready for review March 15, 2023 14:03

genaris requested a review from a team as a code owner March 15, 2023 14:03

TimoGlastra approved these changes Mar 15, 2023

View reviewed changes

genaris force-pushed the feat/use-legacy-prover-did branch from c7d385a to cc03c05 Compare March 15, 2023 20:48

TimoGlastra and others added 2 commits March 16, 2023 09:08

Merge branch 'main' into feat/use-legacy-prover-did

5f17ee8

feat: create legacy prover did from a secure random string

b8a586e

Signed-off-by: Ariel Gentile <[email protected]>

TimoGlastra approved these changes Mar 17, 2023

View reviewed changes

TimoGlastra enabled auto-merge (squash) March 17, 2023 13:13

TimoGlastra merged commit c17013c into openwallet-foundation:main Mar 17, 2023

karimStekelenburg pushed a commit to karimStekelenburg/aries-framework-javascript that referenced this pull request Mar 17, 2023

feat(anoncreds): use legacy prover did (openwallet-foundation#1374)

d860045

Signed-off-by: Ariel Gentile <[email protected]>

genaris deleted the feat/use-legacy-prover-did branch August 17, 2023 17:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(anoncreds): use legacy prover did #1374

feat(anoncreds): use legacy prover did #1374

genaris commented Mar 8, 2023 •

edited

Loading

codecov-commenter commented Mar 8, 2023 •

edited

Loading

TimoGlastra commented Mar 15, 2023

genaris commented Mar 15, 2023

TimoGlastra commented Mar 15, 2023

genaris Mar 15, 2023

TimoGlastra Mar 15, 2023

genaris Mar 15, 2023

TimoGlastra Mar 15, 2023

genaris Mar 15, 2023

TimoGlastra Mar 16, 2023

TimoGlastra Mar 15, 2023

genaris Mar 15, 2023

TimoGlastra Mar 15, 2023

TimoGlastra commented Mar 17, 2023

		@@ -226,6 +234,11 @@

		const credentialOffer = offerAttachment.getDataAsJson<AnonCredsCredentialOffer>()

		if (!credentialOffer.cred_def_id.match(legacyIndyCredentialDefinitionIdRegex)) {


		const legacySchemaId = getLegacySchemaId(legacyIssuerId, options.schema.name, options.schema.version)

feat(anoncreds): use legacy prover did #1374

feat(anoncreds): use legacy prover did #1374

Conversation

genaris commented Mar 8, 2023 • edited Loading

codecov-commenter commented Mar 8, 2023 • edited Loading

Codecov Report

TimoGlastra commented Mar 15, 2023

genaris commented Mar 15, 2023

TimoGlastra commented Mar 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TimoGlastra commented Mar 17, 2023

genaris commented Mar 8, 2023 •

edited

Loading

codecov-commenter commented Mar 8, 2023 •

edited

Loading