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

Not able to match suites using JSON ld signatures #139

Open
vongohren opened this issue Jun 16, 2021 · 14 comments
Open

Not able to match suites using JSON ld signatures #139

vongohren opened this issue Jun 16, 2021 · 14 comments

Comments

@vongohren
Copy link

vongohren commented Jun 16, 2021

Intro

My verification method fails because JSON ld is not able to match suit and type. With a credential I have made earlier.
But as you can see, there are some mismatch when jsonld wants to match the proof.
Trying to match with a precursor called sec:

Screenshot 2021-06-16 at 14 13 08

That leads to 0 results and fails with no proofs to match.

Screenshot 2021-06-16 at 14 14 19

Where is this problem laying?

Solution

Doing return this.type.includes(proof.type); makes my software happy again, but I know that that is not an accepted solution, because the semantics of things becomes diffuse becuase of that. But it is the only issue I have, this missmatch of matching coming from somewhere

Data for testing

My current document for verification looks like this

{
  '@context': [
    'https://www.w3.org/2018/credentials/v1',
    'https://w3id.org/traceability/v1',
    'https://w3id.org/security/bbs/v1'
  ],
  id: 'https://www.diwala.io/4eeope1d1d',
  issuer: 'did:key:zUC76TFzdrf6kvBFoe1kQg3QZsupm6aatPr46eXMnPm86p5LooBf7ieWLerrNsvSVJtpujpXq4XPYSdc9fViBmAAmxAhdW8meRyJ2GDzAa5WKL7ja7EH6HiUxhEuK4dhxXLdFuN',
  type: [ 'VerifiableCredential', 'EmailVerification' ],
  name: 'Verified email',
  description: 'This is a credential of email attribute that has been verified through sending an email to the owner',
  issuanceDate: 'Mon Jun 14 2021',
  credentialSubject: { type: [ 'Person' ], email: 'bbs@vongohren.me' },
  proof: {
    type: 'BbsBlsSignature2020',
    created: '2021-06-14T20:59:15Z',
    proofPurpose: 'assertionMethod',
    proofValue: 'tHGx5dZ+CUBp2FFRyziz57Rn1Fmp1hMKu1hBkg4kkwStgLy1+zYH066j1+CgXTFIYUHlKfzccE4m7waZyoLEkBLFiK2g54Q2i+CdtYBgDdkUDsoULSBMcH1MwGHwdjfXpldFNFrHFx/IAvLVniyeMQ==',
    verificationMethod: 'did:key:zUC76TFzdrf6kvBFoe1kQg3QZsupm6aatPr46eXMnPm86p5LooBf7ieWLerrNsvSVJtpujpXq4XPYSdc9fViBmAAmxAhdW8meRyJ2GDzAa5WKL7ja7EH6HiUxhEuK4dhxXLdFuN#zUC76TFzdrf6kvBFoe1kQg3QZsupm6aatPr46eXMnPm86p5LooBf7ieWLerrNsvSVJtpujpXq4XPYSdc9fViBmAAmxAhdW8meRyJ2GDzAa5WKL7ja7EH6HiUxhEuK4dhxXLdFuN'
  }
}

Environment

Im using @mattrglobal/jsonld-signatures-bbs -- 0.10.0 -- BbsBlsSignature2020 on both sides, Issuance and Verifications.
Using "jsonld-signatures": "^9.0.2" directly on verification side and on the issuer side im using

yarn why v1.22.10
[1/4] 🤔  Why do we have the module "jsonld-signatures"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "jsonld-signatures@9.0.2"
info Has been hoisted to "jsonld-signatures"
info This module exists because it's specified in "dependencies".
info Disk size without dependencies: "136KB"
info Disk size with unique dependencies: "2.81MB"
info Disk size with transitive dependencies: "7.27MB"
info Number of shared dependencies: 12
=> Found "@mattrglobal/jsonld-signatures-bbs#jsonld-signatures@5.0.1"
info This module exists because "@mattrglobal#jsonld-signatures-bbs" depends on it.
info Disk size without dependencies: "2.34MB"
info Disk size with unique dependencies: "6.93MB"
info Disk size with transitive dependencies: "9.74MB"
info Number of shared dependencies: 16
✨  Done in 9.85s.
@OR13
Copy link

OR13 commented Jun 16, 2021

I have seen this before.

your did is resolvable.. but it might not have the correct context yet:

Make sure it does: https://ns.did.ai/suites/bls12381-2020/

here is a similar test:

https://github.com/OR13/didme.me/blob/master/test/bbs-issue-verify.test.js

when I hit this kind of error, i tend to start from scratch, with the simplest vc I can make, and a document loader that throws errors for any iri... then I fix errors one by one.

now to answer you actual question, sec: is from security vocab and its common to see the following "equivalent" things:

What match proof is doing is trying to filter on this type... depending on how compact is used, you might see each of these values...

IMO the root cause for this, is a confusing "proof verification pipeline" inside the suite... probably split across multiple inheritance or packages....

When we wrote JsonWebSignature2020, we specifically did not do that, and move everything into a single class:

https://github.com/transmute-industries/verifiable-data/blob/main/packages/json-web-signature-2020/src/JsonWebSignature.ts

notice our matchProof hard codes looking for the prefixed term:

https://github.com/transmute-industries/verifiable-data/blob/main/packages/json-web-signature-2020/src/JsonWebSignature.ts#L93

@OR13
Copy link

OR13 commented Jun 16, 2021

I would recommend not writing suite classes that are more than 1 file, or split across npm packages... and the amount of code in the current suites is problematic, ideally the suite would be rewritten to make this much simpler, and removing all uneeded code / minimizing the code paths for review.

@vongohren
Copy link
Author

Great information @OR13!
From what I understand, some of these thoughts are directed to the library creator and contributors?
How much can I do without changing up the library, or is it anything I can do?

Because when you say that I should have my did method with a context, which comes out of did-key libraries at transmute, what can I do to improve that? Or we are "converting" between mattrs blskeypar and transmutes, maybe we are loosing that context which can leade to this issue?

So im still a bit lost into where a change should begin. Im going to dig more into this tomorrow to see where my dids are missing the context and where this lag of lable pops up.

@OR13
Copy link

OR13 commented Jun 16, 2021

@vongohren I am just giving your all the context I can to help you.

You need to understand how this stuff works to debug the issue.

as far as I can tell, there is no change needed to use transmute and mattr libraries together.

I am not sure why you are encountering the issue you are, but I would recommend a minimal example that will help the library maintainers debug the issue.

ideally, one that included static everything and made no network requests.

for example, can you issue and verify a VC from did:example:123#key, where the document loader has been updated to support that?

@vongohren
Copy link
Author

vongohren commented Jun 16, 2021 via email

@OR13
Copy link

OR13 commented Jun 16, 2021

@vongohren its possible there is a problem in either our libraries or Mattr's... did core changes broke a number of things for us recently, and we have not finished pushing our fixes all the way through our did-key implementation.

@tplooker
Copy link
Member

@vongohren as suggested by @OR13 could you create a small sample so I can debug this some more?

@vongohren
Copy link
Author

Coming up, give me an hour or so

@vongohren
Copy link
Author

vongohren commented Jun 17, 2021

Intro

@tplooker @OR13 here is a repro: https://github.com/vongohren/json-ld-error-repro

I have splitted it into issuer and verifier because this is where the mistake is I believe.

In the issuer there is no direct dependency on "jsonld-signatures"
It uses the intrinsic "jsonld-signatures" dependency of "@mattrglobal/jsonld-signatures-bbs" which uses version 5.0.1.
I need to not use the 9.0.2 version of "jsonld-signatures" because that complains on the suite is missing a method

So when the verifier is trying to consume this, the verifier has made a dependency on "jsonld-signatures", using 9.0.2.

So here the error occurs.

If we remove the 9.0.2 dependency from the verifier and use the intrisic dependency of "@mattrglobal/jsonld-signatures-bbs"it works. On the same earlier issued VC. If we introduce the newest version of json ld into issuer, it fails because of missing methods.

You can add and remove the dependency on the verifier side to see it fail and work again

Concern

This is a legit concern in the sense that issuer and verifiers are not always the same code bases right? Meaning that an org that is issuing with the intrinsic "jsonld-signatures" dependency of "@mattrglobal/jsonld-signatures-bbs" which uses version 5.0.1. Issues then a VC that is not verifiable to a different verifier down the line? Just because of package differences?

Should the VCs that easily break/fail? That does not support a whole lot of interoperability i guess?

We cannot demand two different sides of the party to rely on the same version, except the latest and keep it up to date

@OR13
Copy link

OR13 commented Jun 17, 2021

Should the VCs that easily break/fail? That does not support a whole lot of interoperability i guess?

The VC isn't broken, as far as I can tell.

The issue is likely version drift between libraries maintained by 3 independent companies.

From a developer perspective, you should avoid any solution that takes on a lot of 3rd party dependencies, its worth asking could we make @mattrglobal/jsonld-signatures-bbs not depend on jsonld-signatures?

You can see that this library does not: https://github.com/transmute-industries/verifiable-data/blob/main/packages/json-web-signature-2020/package.json#L62

However, it does depend on jsonld... if we look more closely, it relies on cannonize, compact, and frame... probably, this could be cut down to just cannonize, by reducing the allowed input formats, and throwing better errors.

I suspect the inheretence here: https://github.com/mattrglobal/jsonld-signatures-bbs/blob/master/src/BbsBlsSignature2020.ts#L32

could / should be removed from this library to prevent such issues in the future... The best way of preventing version drift in a dependency from breaking things, is to not have any unnecessary dependencies.

We cannot demand two different sides of the party to rely on the same version, except the latest and keep it up to date

We should not need too. Whatever is failing here, is very likely not at the crypto layer, its at the key lookup and proof matching layers... In my experience this is often the place where things break.

There were recent changes to JSON-LD Signatures that impacted the way that documentLoaders behave:

https://github.com/digitalbazaar/jsonld-signatures/pull/139/files

^ this might be related.

@vongohren you are right to complain when things like this happen, it helps us invest the time where we need to, in improving dependency management and reducing version drift breakage risk.

@vongohren
Copy link
Author

vongohren commented Jun 17, 2021

@OR13 thanks for great explanation and suggestion, Im still learning about many of these thoughts so its very helpful to get a good explanation. I do not have an immediate suggestion to complement your explanation and suggestion, but I look forward to hear @tplooker `s thoughts

@kuzdogan
Copy link

Might be related to digitalbazaar/jsonld-signatures#143 ?

@vongohren
Copy link
Author

I think it is pretty much the same, just dont know what the solution might be

@vongohren
Copy link
Author

This is probably related to this issue aswell: #142

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants