Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support verification methods with the same fragment #623

Merged
merged 2 commits into from
Jan 26, 2022

Conversation

cycraig
Copy link
Contributor

@cycraig cycraig commented Jan 25, 2022

Description of change

This updates MethodQuery to match on the DID as well as the fragment when the query includes a full DID-Url.

This fixes the following edge-cases:

  • adding a method to a document with an ID field of a different DID (typically a controller DID Document) but the same fragment as an existing method.
  • resolving such a method by specifying its full ID (previously would return the first method that matched the fragment).
  • verifying a document signed with such a method.

The specification seems to allow adding methods and services to a DID Document which reference a different DID. We added support for signatures from such methods in #458 but duplicate fragments was still an edge-case.

NOTE: this affects all functions that resolve methods (including verifying signatures since that resolves the method used to sign) since it affects the core behaviour of OrderedSet.

Example

The following document has a signing method from a controller DID document (this is a workaround since we do not support signatures from controllers simply referenced as the controller of the DID Document). The signature was created by the private key corresponding to the controller method.

Note how both methods have the same #sign-0 fragment. This is not advised but should still be legal according to the specification.

Before this PR the document would fail verification since the library would incorrectly resolve did:iota:HGE4tecHWL2YiZv5qAGtH7gaeQcaz2Z1CR15GWmMjY1M#sign-0 as the signing method instead of did:iota:A73GoJ7N6qHEdSfoist6YR4MCDteuhiinUmaMV9vSUcZ#sign-0 despite the full ID being specified in "verificationMethod".

{
  "doc": {
    "id": "did:iota:HGE4tecHWL2YiZv5qAGtH7gaeQcaz2Z1CR15GWmMjY1M",
    "capabilityInvocation": [
      {
        "id": "did:iota:HGE4tecHWL2YiZv5qAGtH7gaeQcaz2Z1CR15GWmMjY1M#sign-0",
        "controller": "did:iota:HGE4tecHWL2YiZv5qAGtH7gaeQcaz2Z1CR15GWmMjY1M",
        "type": "Ed25519VerificationKey2018",
        "publicKeyMultibase": "zFJsXMk9UqpJf3ZTKnfEQAhvBrVLKMSx9ZeYwQME6c6tT"
      },
      {
        "id": "did:iota:A73GoJ7N6qHEdSfoist6YR4MCDteuhiinUmaMV9vSUcZ#sign-0",
        "controller": "did:iota:A73GoJ7N6qHEdSfoist6YR4MCDteuhiinUmaMV9vSUcZ",
        "type": "Ed25519VerificationKey2018",
        "publicKeyMultibase": "zEh7rBAVQAFEXXK63Aiku4eqgWJhw5z1VYiVA9tmbveJP"
      }
    ]
  },
  "meta": {
    "created": "2022-01-25T14:11:46Z",
    "updated": "2022-01-25T14:11:46Z",
    "proof": {
      "type": "JcsEd25519Signature2020",
      "verificationMethod": "did:iota:A73GoJ7N6qHEdSfoist6YR4MCDteuhiinUmaMV9vSUcZ#sign-0",
      "signatureValue": "5njR8zSD1setBb135o2SNZ6Zjc58s8xot9nRt5MF2kX9JZXQUxWNxoudJisawG74cXKibSSQdxN11PcsiW5ujqYw"
    }
  }
}

Links to any relevant issues

Fixes issue #457.

Type of change

  • Bug fix (a non-breaking change which fixes an issue)
  • Enhancement (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Fix

How the change has been tested

Tests and Wasm examples pass locally.

Change checklist

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@cycraig cycraig added Wasm Related to Wasm bindings. Becomes part of the Wasm changelog Rust Related to the core Rust code. Becomes part of the Rust changelog. Patch Change without affecting the API that requires a patch release. Part of "Patch" section in changelog labels Jan 25, 2022
@cycraig cycraig linked an issue Jan 25, 2022 that may be closed by this pull request
Copy link
Contributor

@PhilippGackstatter PhilippGackstatter left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for the changes!

Copy link
Contributor

@olivereanderson olivereanderson left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@cycraig cycraig merged commit cec57bc into dev Jan 26, 2022
@cycraig cycraig deleted the fix/method-query-fragments branch January 26, 2022 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Patch Change without affecting the API that requires a patch release. Part of "Patch" section in changelog Rust Related to the core Rust code. Becomes part of the Rust changelog. Wasm Related to Wasm bindings. Becomes part of the Wasm changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] ProofVerificationMethod do not reference the whole key name
3 participants