hyperledger / anoncreds-rs Goto Github PK

Today, when integrating anoncreds-rs in AFJ, CI/CD failed with the following error:

Dynamic Linking Error: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.33' not found (required by /home/runner/work/aries-framework-javascript/aries-framework-javascript/node_modules/@hyperledger/anoncreds-nodejs/native/libanoncreds.so)

CI was set up to use a container based on Ubuntu 20.04, whose libc version is older than 2.33 (as this version was released in 2021). Our particular issue was solved by upgrading to Ubuntu 22.04, but I would like to know if this is on purpose or it is possible to use an older libc version.

I noticed that in both aries-askar and indy-vdr, cross is used when building for Linux targets. There is a comment stating: using cross here to build against an older glibc for compatibility.

Should we use cross here as well to support older platforms?

Right now, the anoncreds library has a static hash map to keep all the anoncreds objects, and their associated handle, in memory for access later on. This works fine for languages with a deconstructor, but can be very tedious in languages without a deconstructor, like JavaScript.

We (@TimoGlastra and I) were wondering if it makes sense to just pass a stringified JSON into the library which would remove this issue entirely. It does make referencing a previous anoncreds object a bit more annoying, but it would prevent any unwanted memory leaks.

Timo also mentioned that this is how the indy-sdk has done it. Was there a specific reason why anoncreds derived from that?

cc: @andrewwhitehead @TimoGlastra

We should do this check, not sure where we would need to do this, but otherwise this is something we would need to handle on a higher level (e.g. AFJ)

can be different between schema and credentialDefinition

There have been some minor changes to the FFI interface when we moved from indy-credx to anoncreds. This has to be updated inside the FFI wrapper for Node.js.

It will mainly consist of passing in the schema_id / cred_def_id when needed instead of generating it inside the library.

• update ffi interface based on changes to public api in anoncreds-rs library
  • ids
  • revocation interface (where already changed)

This should probably be split up into multiple issues

Relevant spec issue: hyperledger/anoncreds-spec#108

As the identifiers can now be any URI, we should update the methods in the AnonCreds library to not generate the id values, but rather allow the user to generate the IDs themselves based on the AnonCreds method they're using.

• Update the credx_create_schema ffi method based on the chosen approach
  • remove origin_did as parameter
  • remove the seq_no parameter from the schema creation (but keep it in the data model for now to not break cred def flow)
  • based on the chosen approach, add an schema_id parameter or not
• Update the create_schema issuer.rs method based on the choses approach
  • remove origin_did parameter
  • remove the seq_no parameter from the schema creation (but keep it in the data model for now to not break cred def flow)
  • based on the chosen approach, add a schema_id parameter or not
  • remove the generation of the schema_id based on the schema values
• If the frist approach is taken, we need to update all validation logic and make the schema_id optional (but only in some cases). This requires some refactoring probably

One things to figure out is how we want to approach the generation of the ID, and whether it already needs to be present when we call the methods that create the objects. There's two approaches we can take:

Approach 1

Call the creation method (e.g. create_schema) without any identifier and then return the created object (the schema) without the id property. The schema is now created and the id property can be added later when the object is written to the ledger.

The advantage of this appraoch is that it allows the id generation process to be based on the contents of the object (schema), or it allows the id to be known after the object has been written to the ledger (if e.g. the ledger generates some identifier).

Approach 2

Call the creation method (e.g. create_schema) with the identifier and return the created object (the schema) with the id property.

The advantage of this appraoch is that is allows the anoncreds library to validate the identifier to be a valid URI / legacy indy identifier and we don't have a in-between representation of the model (all fields except the id).

The anoncreds-data-types module contains an identifiers directory with logic around creation, splitting and validation of the identifiers in anoncreds objects.

The AnonCreds specification has updated identifiers to allow both the legacy indy style identifiers (different per object type) and URIs.

As identifiers don't contain specific elements anymore, but just an URI we need to determine how much validation is still needed.

These are the identifiers that should be updated:

• CredentialDefinitionId
• RevocationRegistryId
• SchemaId

This task only focusses on adding validation support for the URI type, but doesn't remove logic related to handling the legacy identifier type, this will be handled in separate tasks.

For each of the identifiers the following should happen:

• Update the Validatable implementation for the identifier type to accept both the legacy indy style (currently implemented) and the new URI style according to the AnonCreds specification.
• Remove the Qualifiable implementation for the identifier as it isn't needed for the anoncreds implementation
• Add utility methods to determine if the identifier is of type legacy indy identifier or URI. Can be either through something like an identifier type (uri, legacy_indy), or just adding two methods is_legacy_indy_identifier() and is_uri_identifier() (thinks the second is simplest).
• Rename .parts() to .legacy_indy_identifier_parts(). This should throw an error if the type of the identifier is not a legacy indy identifier

• Api methods are renamed
• Parameters have changed

The data model has been slightly adjusted to be more generic.

The data model is described here: https://anoncreds-wg.github.io/anoncreds-spec/#schema-publisher-publish-schema-object

{
  "id": "did:indy:sovrin:SGrjRL82Y9ZZbzhUDXokvQ/anoncreds/v0/CLAIM_DEF/54177/latest",
  "schema_id": "did:indy:sovrin:SGrjRL82Y9ZZbzhUDXokvQ/anoncreds/v0/SCHEMA/MemberPass/1.0",
  "type": "CL",
  "tag": "latest",
  "value": {
    "primary": {
      "n": "779...397",
      "r": {
            "birthdate": "294...298",
            "birthlocation": "533...284",
            "citizenship": "894...102",
            "expiry_date": "650...011",
            "facephoto": "870...274",
            "firstname": "656...226",
            "master_secret": "521...922",
            "name": "410...200",
            "uuid": "226...757"
      },
      "rctxt": "774...977",
      "s": "750..893",
      "z": "632...005"
    }
  }

• id - (string) The identifier of the [[ref: Credential Definition]]. The format of the identifier is dependent on the [[ref: AnonCreds Objects Method]] used in publishing the [[ref: Credential Definition]].
• schema_id - (string) The identifier of the [[ref: Schema]] on which the [[ref: Credential Definition]] is based. The format of the identifier is dependent on the [[ref: AnonCreds Objects Method]] used in publishing the [[ref: Schema]].
• type - (string) The signature type of the [[ref: Credential Definition]]. For this version of AnonCreds the value is always CL.
• tag (string) - the tag value passed in by the [[ref: Issuer]] to an AnonCred’s [[ref: Credential Definition]] create and store implementation.
• value - (object) an Ursa native object with the primary and revocation fields.
  • primary is the data used for generating credentials.
    • n is a safe RSA-2048 number. A large semiprime number such that n = p.q, where p and q are safe primes. A safe prime p is a prime number such that p = 2p'+ 1, where p' is also a prime. Note: p and q are the private key for the public CL-RSA key this [[ref: Credential Definition]] represents.
    • r is an object that defines a CL-RSA public key fragment for each attribute in the credential. Each fragment is a large number generated by computing s^{xri} where xri is a randomly selected integer between 2 and p'q'-1.
      • master_secret (should be [[ref: link secret]]) is the name of an attribute that can be found in each [[ref: Credential Definition]]. The associated private key is used for signing a blinded value given by the [[ref: Holder]] to the [[ref: Issuer]] during credential issuance, binding the credential to the [[ref: Holder]].
      • The rest of the attributes in the list are those defined in the [[ref: Schema]].
      • The attribute names are normalized (lower case, spaces removed) and listed in the [[ref: Credential Definition]] in alphabetical order.
    • rctxt is equal to s^(xrctxt), where xrctxt is a randomly selected integer between 2 and p'q'-1. (I believe this is used for the commitment scheme, allowing entities to blindly contribute values to credentials.)
    • s is a randomly selected quadratic residue of n. This makes up part of the CL-RSA public key, independent of the message blocks being signed.
    • z is equal to s^(xz), where xz is a randomly selected integer between 2 and p'q'-1. This makes up part of the CL-RSA public key, independent of the message blocks being signed.

This results in the following tasks:

• Remove the to_unqualified method, there's no need to be switching between qualified/unqualified identifiers types, we just use the identifiers as is

Seems this one is quite easy?

The NonRevocationInterval in the PresentationRequest on the request or the attribute/predicate level are not compared with the timestamp of which the prover has updated the RevocationState to in creating the Presentation.

Given the verifier only provides RevocationRegistry (aka Accumulator value) for the timestamps in the NonRevocationInterval, the required RevocationRegistry for the timestamp given in the Presentation will be missing and would not cause a revoked credential during that interval to be verified.
However, if the verifier provides RevocationRegistry mapped to timestamps outside of the interval,
a revoked credential can be verified.

This might also be related to the issued in #36, i.e. if the RevocationState is for a timestamp that is outside of the one defined in the PresentationRequest, then regardless if it has been revoked, the verifier might not supply the Accumulator value for such a timestamp.

Per the AnonCreds Rust Working Group meeting of 2022.11.07, on the topic of Wrappers for the Rust artifacts:

• It would be nice to have generated wrappers if they are of sufficient quality and consistent with the Rust model (which may not be possible). Stephen to talk to Steve McCowan about what generated wrappers would be like to see if the rest of the Devs are supportive.

Let's assume (for now) that generated wrappers are not viable. If we are going have hand-crafted wrappers, where do we put them -- in this repo or have them as a separate repo?

Argument to keep them in

• we can make them a GHAction test to be working before merges
• we can make a GHAction produce release artifacts in-sync when tags are created.
• more likely to keep them up to date
• helps with core feature testing as wrappers are part of the repo vs. an external dependency

Argument to move them out

• unmaintained wrappers become a burden and making releasing features harder.
• Having them in a separate repo can make them easier to update (although that should really not be the case as long as we have active, engaged maintainers).

Proposal:

• Keep core, active wrappers in this repo
  • with named maintainers
  • with GHA tests
  • GHA artifact generation/publishing
• Non-core (no active maintainers) are in separate repos
  • Any non-core wrapper repo that is not maintained for a designated period (suggest 6 months or 1 year) will be archived.
• Policy and process for moving a wrapper into or out of the main repo
  • Suggest:
    • Create issue (optionally with a PR)
    • Posted proposal on Discord
    • Notification to contributors to wrapper
    • Meeting, discussion and vote of the maintainers
  • After the decision is made, the wrapper will be moved into or out of this repo.
    • If moved out, minimal work would be done by maintainers to create a new non-core repo with the wrapper
      • It must have the code and GHActions, but the GHAs do not have to be functional.
• Keep "WRAPPERS.md" file with list of active wrappers, maintainers and policies.
• Suggestion for initial wrappers meeting criteria are:
  • Python
  • Javascript
  • We'll ask the AF-Go team if they want to keep the golang wrapper in the repo
  • We'll ask the .NET team (@CHempel-esatus ) if they want to support a wrapper in this repo

There have been some minor changes to the FFI interface when we moved from indy-credx to anoncreds. This has to be updated inside the FFI wrapper for React Native.

It will mainly consist of passing in the schema_id / cred_def_id when needed instead of generating it inside the library. But also the prover_did, revocation list...

The prover_did property on the credential request must become optional, but we need to leave it in for backwards compatability. We can always ignore it and just generate some entropy on the issuer side (randomBytes, UUID?).

• Throw an error if the prover_did property is present, but we're not using legacy indy identifiers (meaning it is aware of AnonCreds 1.0 instead of 0.1)
• Make property optional
• Always generate random something instead of using it

1. Missing FFI for the below interactions which needs to be implemented

create_revocation_status_list, 
update_revocation_status_list, 
update_revocation_status_list_timestamp_only

2. Current there is no tests that uses update_revocation_status_list_timestamp_only - test to be added

I don't think it is needed for the AnonCreds implementation

See hyperledger/anoncreds-spec#126 and hyperledger/anoncreds-spec#116

Should follow rules as defined in spec (e.g. issuerId from cred def MUST be same as issuerId from revocation registry definition), same with revocation list

We need to rename the indy-data-types module to the anoncreds-data-types module

• Rename indy-data-types directory to anoncreds-data-types
• Update Cargo.toml and remove indy terms, add anoncreds naming/terms

Dependant on some other changes so specified as a separate task

The last FFI method that needs testing Is the verifyPresentation. This ticket is dependent upon #74 and also is included in a subtask.

Once the FFI layer is finished, we just have to pass an already created revocationstatuslist to the function.

The (previously named indy-data-types) anoncreds-data-types contains types that are not relevant to the current AnonCreds implementation.

• Remove all code related to rich schemas
• Remove the merkle_tree subdirectory (I think we don't use this for AnonCreds?)
• Determine other code that is not needed for the anoncreds-data-types module

See #4 for general context and possible approaches

As the identifiers can now be any URI, we should update the methods in the AnonCreds library to not generate the id values, but rather allow the user to generate the IDs themselves based on the AnonCreds method they're using.

• Update the credx_create_revocation_registry ffi method based on the chosen approach
  • remove origin_did as parameter
• Update the create_revocation_registry issuer.rs method based on the chosen approach
  • remove origin_did parameter
  • remove the generation of the rev_reg_id based on the input values. The model will now be created without an id property.
• Remove the id property from the revocation registry definition model

Right now it is a bit messy and adding support for another check (e.g. strict clippy) will be quite annoying.

Main thing would just be separating a lot of the actions, this way we can also run more in parallel. (My idea was something like I have done for Ursa hyperledger-archives/ursa#216)

Tests: https://github.com/hyperledger/anoncreds-rs/blob/main/tests/anoncreds_demos.rs

Need in memory storage implementation (current tests made a start at this)

• Identify what the (un)commented tests do/test
• Pick a list of tests that are important/where it makes sense
• Remove any commented code that doesn't test anything new

3000 of 3500 lines are still commented (85%). The tests are written for the Indy SDK, so we do have to update all code to work with AnonCreds RS. Goal is to reach a good level of testing, it's infeasible to update all tests from indy-sdk

We need to rename the indy-credx module to the anoncreds module

• Rename indy-credx directory to anoncreds
• Update Cargo.toml and remove indy terms, add anoncreds naming/terms
• Rename impl_indy_object to impl_anoncreds_object
• Rename impl_indy_object_from_json to impl_anoncreds_object_from_json
• Rename IndyObject to AnonCredsObject
• Rename AnyIndyObject to AnyAnonCredsObject
• Rename IndyObjectId to AnonCredsObjectId
• Rename IndyObjectList to AnonCredsObjectList

The python has been brought in-sync, but the tests do not reflect the latest FFI interface. A couple things need to be added:

• Internal representation of the RevocationStatusList
• Remove all old revocation types
• Make the binding.py demo in sync with the anoncreds_demo.rs and binding.test.ts

This task is dependent upon #74 and is included as a subtask of that issue.

The indy-shared-rs repository contains an implementation of the wallet query language (WQL). It seems the wql module is used currently used by Aries Askar and Indy CredX. The new AnonCreds library will also need to support .

As Aries Askar now depends on indy_wql, it may make sense to make both AnonCreds and Aries Askar depend on WQL. Need to gather some feedback on this approach.

• Determine where the wql code will live (indy-shared-rs, anoncreds, aries-askar)
• Determine whether the WQL needs to be standardized as part of AnonCreds
  • It could also be a separate specification, with separate implementation

If we keep it in here, we should rename it to anoncreds-wql (or maybe even just wallet_query_language)

Discussed:

• Move implementation to Aries Askar (ask Andrew Whitehead)
• Interface in anoncreds(-utils) that is only used for validation in proof request (can be really simple)

This is an issue with existing anoncreds implementations (indy-sdk and credx) and is illustrated in an aca-py integration test:

https://github.com/hyperledger/aries-cloudagent-python/blob/main/demo/features/0454-present-proof.feature#L120

When a proof includes both revocable and non-revocable credentials, and the request includes the revocation timestamp at the REQUEST level, then the proof will fail (even though it should pass).

Note that when the revocation interval is requested at the ATTRIBUTE level, the proof will pass (see integration test https://github.com/hyperledger/aries-cloudagent-python/blob/main/demo/features/0454-present-proof.feature#L100)

• CI pipeline to release to NPM
• Builds for Linux, Windows, macOS
• Test release

It is something that should be verified by the AnonCreds implementation

Has been renamed to entropy something. holder provides entropy in the signing. For indy identifiers we still need to support prover_did, but it doesn't have to be a did.

related spec issue: hyperledger/anoncreds-spec#107

• Update README

Right now the post install script is disabled as we cannot fetch the library from the releases.

So we first must release the artefacts for all the specified platforms, and afterwards we can add the post install script back.

--- a/wrappers/javascript/nodejs/package.json
+++ b/wrappers/javascript/nodejs/package.json
@@ -5,12 +5,12 @@
     "example": "yarn --cwd example",
     "release": "release-it",
-    "test": "jest"
+    "test": "jest",
+    "install": "node-pre-gyp install --target_arch=$(node build/util/arch.js)"
   },
   "devDependencies": {

The proposed RevocationStatusList field name that contains the current accumulator value is currentAccumulator.

As the accumulator is in the RevocationRegisty struct in libursa, the accum field is private and serde with the same field name.

Current situation is to keep the field name accum but we will need to either update the specs or find a workaround for serde.

Hacky approach, but also create a PR (and think about backwards compatibility).

• CI (react native specific parts)
• iOS & Android builds
• Test release

The data model has been slightly adjusted to be more generic.

The data model is described here, but I think for now we don't have to switch up the structure, casing: https://hyperledger.github.io/anoncreds-spec/#revocation-registry-definition-object-generation

This results in the following tasks:

• Remove revocDefType. With the new revocation list data model that contains all indexes this is not needed anymore in the data model.
• Remove RevocationRegistryConfig as it isn't used (not 100% about this?)
• Remove the to_unqualified method, there's no need to be switching between qualified/unqualified identifiers types, we just use the identifiers as is

As @genaris has highlighted in # the API for parsing objects could be improved.

currently you have to do the following to load a credential definition:

rsCredentialDefinitions[credDefId] = CredentialDefinition.load(JSON.stringify(credentialDefinitions[credDefId]))

It would be nice if load took a json object instead of a string. We also discussed that e.g. Presentation.create could just take json objects as input, and do the loading in the Presentation class so the user doesn't need to care about the object handles etc..

I think we should make the API just json objects wherever we can

The data model has been slightly adjusted to be more generic.

The data model is described here: anoncreds-wg.github.io/anoncreds-spec/#schema-publisher-publish-schema-object. We diverge a bit to keep it consistent with the current implementation

{
    "id": "https://www.did.example/schema.json",
    "name": "Example schema",
    "version": "0.0.1",
    "attrNames": ["name", "age", "vmax"]
}

• id - (string) The identifier of the Schema. The format of the identifier is dependent on the [AnonCreds Objects Method](anoncreds-wg.github.io/anoncreds spec/#term:anoncreds-objects-method) used in publishing the Schema.
• name (string) - the name of the schema
• version (string) - the schema version
• attrNames (str[]) - an array of strings with each string being the name of an attribute of the schema

This results in the following tasks:

• Keep the seq_no property in for now to not break the cred def flow. This will be removed in a separate task
  Update the serde encoding of attrNames property to attr_names (maybe we should keep it attrNames to not change unneeded values?)
• Update Validatable for SchemaV1 to only validate the schema name and version against the identifier if is_legacy_indy_identifier is true on self.id
• Remove the to_unqualified method, there's no need to be switching between qualified/unqualified identifiers types, we just use the identifiers as is

See #4 for general context and possible approaches

As the identifiers can now be any URI, we should update the methods in the AnonCreds library to not generate the id values, but rather allow the user to generate the IDs themselves based on the AnonCreds method they're using.

• Update the credx_create_credential_definition ffi method based on the chosen approach
  • remove origin_did as parameter
  • based on the chosen approach, add a cred_def_id parameter or not
• Update the create_credential_definition issuer.rs method based on the choses approach
  • remove origin_did parameter
  • based on the chosen approach, add a cred_def_id parameter or not
  • remove the generation of the cred_def_id based on the credential definition values
  • remove the logic of extracting the seq_no from the schema for the schema_id property in the cred_def, but rather use the schema.id directly for the schema_id property.
• If the frist approach is taken, we need to update all validation logic and make the cred_def_id optional (but only in some cases). This requires some refactoring probably

• Add @hyperledger prefix
• Make sure the registration is consistent Askar & Indy VDR
• Set the correct version
• add the anoncreds prefix to the folder names

Related to this Pull Request 2055 in ACA-Py. The ACA-Py team has tested the scenario below and found it is failing, even though it should pass. Adding this as an issue in this repo as it is likely the best place to address it.

This test "fails" - the proof doesn't verify even though it should:

@T003-RFC0454.1f - two credentials, one revocable one not, neither revoked, "non_revoked" is requested at therequest level
For the failing scenario, it fails with indy-sdk and credx. I believe the underlying library is not creating the proof properly, and/or the underlying anoncreds itself has a bug.

    We should probalby update this to work the same as the indy vdr registration (for another PR, just so we don't forgot)

Originally posted by @TimoGlastra in #58 (comment)

    We can extract the schema_name and schema_version from the passed schemas in the `verify_presentation` method and pass them down (`verify_presentation`  -> `verify_requested_restrictions` -> `gather_filter_info`).

For the other ones, maybe we need to look at adding an issuerId property to anoncreds objects after all? I think there's no harm in it, and it would be backwards compatible with current anoncreds (issuerId is always the indy did in that case). Relevant issue: hyperledger/anoncreds-spec#102

For the indy method it would be the indy did, for cheqd it would be the cheqd did. For non ledger based methods there must be some point of reference of who created the schema/credential definition. For https it could be the host (e.g. schema https://hyperledger.org/membershipSchema would have issuerId of https://hyperledger.org

• support both _did and id for restrictions

@swcurran thoughts?

Originally posted by @TimoGlastra in #25 (comment)

Right now we still have the indy-utils package which is partially being used.

• Do we want to create a anoncreds-utils package or bring them inside the anoncreds folder?
• Which modules can we keep inside the anoncreds package?
• Which of the crypto packages to we want to bring?
• How can we keep the implementation crypto agile (where it makes sense)?
• Do we need to keep the did module?

See https://github.com/animo/indy-shared-rs/tree/wrapper/javascript

• Move from Animo's fork to AnonCreds repo
• Rename methods to anondreds instead of indy

Right now the verify presentation method takes a list called RevocationEntries. This is not up-to-date with the specification and it should take the new RevocationStatusList.

Tasks

• Update the verifier::verify_presentation method to verify revocation based on the status list. (@whalelephant)
• Update the FFI layer to supply a revocation status list.
• Make sure the tests in Node.js are working when this is fixed in the FFI layer (separate issue #75 )
• Make sure the tests in Python are working when this is fixed in the FFI layer (separate issue #76)

@whalelephant I can pick up the FFI layer and FFI tests, but I am not aware of the changes need to be made for verify_presentation to verify with the new status list.

The credential_defintion must use master secret for backwards compat. But the spec uses link secret. The api currently also uses master. What should we use?

    Will you do the renaming of the folders (to anoncreds-nodejs etc..) in a follow up PR?

We should make sure that change is also made in the askar repo

Originally posted by @TimoGlastra in #58 (comment)