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

Interactions between documentLoader and security context #80

Open
OR13 opened this issue Jun 19, 2020 · 16 comments
Open

Interactions between documentLoader and security context #80

OR13 opened this issue Jun 19, 2020 · 16 comments

Comments

@OR13
Copy link
Contributor

OR13 commented Jun 19, 2020

When a signature suite is not registered here: https://w3id.org/security/v2

match proof must be modified like so:

  async matchProof({
    proof,
  }: // document,
  // purpose,
  // documentLoader,
  // expansionMap,
  any) {
    // console.log(proof, this.type);
    return proof.type === this.type || proof.type === '/' + this.type;
  }

This modification needs to happen in classes which are normally internal, and thus, this behavior prevents the use of vc-js with custom signature suites, unless the implementer does not extend DB classes (forces implementer to not use inheritance)... this seems undesirable for digital bazaar, but I'm not really sure...

There are various other approaches possible, like experimenting with adding contexts, overriding contexts, compactProof, or changing SECURITY_CONTEXT_URL...

however, all of them are more intrusive than changing matchProof... and since any new suite is obviously not going to be registered in SECURITY_CONTEXT_URL yet... it seems logical to make the change suggested...

For example, this one change, allows for a new suite: https://github.com/w3c-ccg/lds-jws2020/pull/19/files#diff-e886e78ec0726474ae672dcfa71f28c7R16

and an old suite: https://github.com/w3c-ccg/lds-jws2020/pull/19/files#diff-ed30fefd45db38ae8b8b0e692510803fR16

to both pass tests... but it required the new suite to make the change noted on this ticket... no additional changes were required, see the document loader:

https://github.com/w3c-ccg/lds-jws2020/pull/19/files#diff-e25e8ce8b4ce985e43c848580e547d55R13

@OR13
Copy link
Contributor Author

OR13 commented Jun 19, 2020

I opened this issue here, but its really about inheritance assumptions of jsonld-signatures and their assumed interfaces by this library: https://github.com/digitalbazaar/jsonld-signatures/blob/master/lib/suites/LinkedDataProof.js#L50

@OR13
Copy link
Contributor Author

OR13 commented Jun 20, 2020

I have confirmed this issue by reimplementing the necessary parts, and observing that my new implementation that requires a custom documentLoader (and will not use a "default") hits https://w3id.org/security/v2 whereas vc-js / jsonld-signatures today, somehow does not hit that context... even when a custom documentLoader is provided...

I recommend verifying this issue by testing Ed25519Signature2020 as an alias of Ed25519Signature2018.

@OR13
Copy link
Contributor Author

OR13 commented Jun 20, 2020

When issuing a vc, and passing a custom document loader (that only supports local contexts), you should be forced to have local copies of the following:

const localOverrides: any = {
  [unlockedDidDocument.id]: unlockedDidDocument,
  'https://w3id.org/did/v0.11': require('./contexts/did-v0.11.json'),
  'https://www.w3.org/2018/credentials/v1': require('./contexts/credentials-v1.json'),
  'https://www.w3.org/2018/credentials/examples/v1': require('./contexts/examples-v1.json'),
  'https://www.w3.org/ns/odrl.jsonld': require('./contexts/odrl.json'),
  'https://w3id.org/security/v1': require('./contexts/security-v1.json'),
  'https://w3id.org/security/v2': require('./contexts/security-v2.json'),
};

@msporny
Copy link
Member

msporny commented Jul 2, 2020

@dlongley mentioned in a drive-by IRC comment that "you get a / before a type when you're not defining properties and it's being treated as a relative URL" ... which may point to something not being used properly. @dlongley, @dmitrizagidulin, or @davidlehn will chime in when they get a few free cycles to look at the issue more deeply.

@dlongley
Copy link
Member

dlongley commented Jul 5, 2020

This issue is a little hard to parse. Could we get a simple runnable example that demonstrates the problem whereby changing matchProof in the way highlighted above would fix it? We'd like to be able to analyze what's going on -- and it may be that there's a better solution -- or one that fits in with the current design.

If a type value is being expressed as /Foo internally by jsonld-signatures, after the data has been compacted to the security v2 context, that indicates that no definition was provided for the type in the input context. If no base URL is given and no definition for a type of Foo is given, then Foo compacts to /Foo as it is treated as a relative URL. This implies an error in the input context (a failure to define the type). It could be that a simpler remedy to the above problem is to ensure that the type is defined -- and to use the fully qualified type (https://example.com/my-suites/Foo) when constructing the suite and setting type in the class instance.

But I think we need a runnable example to easily explore this further.

@OR13
Copy link
Contributor Author

OR13 commented Jul 6, 2020

https://github.com/w3c-ccg/lds-jws2020#usage

^ has examples. As i noted here: #80 (comment), we don't see the security context v2 getting loaded by the document loader... so a custom document loader cannot be used to "preview" what the suite would look like with all terms in that context.

@dlongley
Copy link
Member

dlongley commented Jul 6, 2020

@OR13, Ok, we'll look into this further as soon as we can. I do see right away that the "JsonWebKey2020" term isn't defined in any context that is used in the first example. If we cooked up a context that would work with the example, determined that setting the type in the suite to the fully qualified URI made it function properly, and linked in the README to the lds-jws2020 repo as an example of how to extend proofs for VCs -- would that resolve this issue?

@OR13
Copy link
Contributor Author

OR13 commented Jul 6, 2020

@dlongley yes, i think so.. but it won't resolve the issue of wanting to simulate an updated security context with those terms... which is what I was hoping to be able to show here, because my goal was to get this suite to the same state as Ed25519Signature2018 from a vocab perspective... no additional context should be needed.... essentially, I'm wanting to show 2 things.

  1. What would signatures and verification look like if JsonWebKey2020 and JwsSignature2020 were in security-v2
  2. How can linked data proofs look the same as (1) with a custom context.

The way the documentLoader in this repo is handling security context-v2 seems to be preventing me from accomplishing 1 without implementing 2... which seems like a bug to me.

when a custom document loader is provided, it should be possible to see the security-v2 context resolved... my testing indicated that the way overloading was happening, custom document loaders cannot override the security context.

https://github.com/digitalbazaar/jsonld-signatures/blob/master/lib/documentLoader.js#L19

* extendContextLoader extends another JSON-LD document loader.
   * Given a document loader to extend, this method will return a
   * new document loader that will first check for a URL in
   * jsonld-signature's built-in context map and, if not found,
   * it will fall back to using the passed document loader.
   * This utility method can be used to ensure that any local,
   * in-memory, immutable context documents provided by
   * jsonld-signatures will be used prior to using another
   * document loader to load other documents.

https://github.com/digitalbazaar/jsonld-signatures/blob/ee999c814fcbac15ea9ae86e56f9b127c544c970/lib/ProofSet.js#L75

This logic implies that it's not possible to simulate changes to the security context with the libraries as is.

@dlongley
Copy link
Member

This is still on my radar, just been too busy to get to it. In the meantime, I have some other comments in another issue that you may find useful for your understanding around jsonld-signatures that I thought I'd link to: digitalbazaar/jsonld.js#402

@dlongley
Copy link
Member

@OR13 -- sorry, forgot to tag you in the above comment.

@OR13
Copy link
Contributor Author

OR13 commented Jul 16, 2020

no worries, I'm hoping that I can take another stab at debugging it now that I have a better understanding of the internals.... I believe the BBS+ signature suite is having similar issues. essentially the problem is "How to use new signature suites with the vc data model".

@OR13
Copy link
Contributor Author

OR13 commented Jul 16, 2020

I expect we will have the same issue with Ed25519Signature2020

@antonimmo
Copy link

no worries, I'm hoping that I can take another stab at debugging it now that I have a better understanding of the internals.... I believe the BBS+ signature suite is having similar issues. essentially the problem is "How to use new signature suites with the vc data model".

That's exactly what brought me here. I'm signing VCs with secp384k1, for which I'm using a custom schema (basically a copy-paste of https://www.w3.org/2018/credentials/v1 and adding EcdsaSecp384r1Signature2020 with ID "https://w3id.org/security#EcdsaSecp384r1Signature2020"), but still get "type": "/EcdsaSecp384r1Signature2020".

Any clue on how to extend the schema?

@antonimmo
Copy link

Thanks, @OR13! I'll take a look into it. I've already implemented custom LDKeyPair and LinkedDataSignature, so I hope I'm almost there.

@antonimmo
Copy link

It worked! I just missed the final two adjustments on type and matchProof.

Thanks!

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

No branches or pull requests

4 participants