mattrglobal / jsonld-signatures-bbs

A linked data proof suite for BBS+ signatures
Apache License 2.0
140 stars 42 forks source link

Derive proofs for documents signed by more than one key #79

Closed dysbulic closed 3 years ago

dysbulic commented 4 years ago

I am working on a digital contract system where more than one party will be signing a document. Currently, it seems that generating multiple signatures is handled by the underlying libraries, but not for deriving proofs. In the sample app, I can copy:

const signedDocument = await sign(inputDocument, {
  suite: new BbsBlsSignature2020({ key: keyPair }),
  purpose: new purposes.AssertionProofPurpose(),
  documentLoader,
  compactProof: false
});

to

const signedDocument2 = await sign(signedDocument, {
  suite: new BbsBlsSignature2020({ key: keyPair }),
  purpose: new purposes.AssertionProofPurpose(),
  documentLoader,
  compactProof: false
});

And it will generate a document with an array for proof element. Verification also succeeds. The sample continues to:

const derivedProof = await deriveProof(signedDocument, revealDocument, {
  suite: new BbsBlsSignatureProof2020(),
  documentLoader
})

Where it fails in BbsBlsSignatureProof2020 because proof.type is undefined. proof is a hash with keys '0' and '1'. I added the following shim to get by that error:

if(proof['0']) proof = proof['0']

There is then an error that the message and signature don't match unless you add the line:

proof['@context'] = 'https://w3id.org/security/v2'

With these changes, a single proof can be derived and verified. The proper behavior though is to return an array of proofs in the derived document and to verify them all.

tplooker commented 4 years ago

Hi @dysbulic thanks for filling this issue, I believe the cause was due to the sample you were using was out of date with the latest release of the library, in #80 I have now updated the sample and tested it with your double proof scenario with a derivation and it worked as expected, so please let me know if that doesnt resolve this issue.

dysbulic commented 4 years ago

@tplooker, currently if there are multiple BBS signatures, a derived proof will only be generated for the first one.

Is this the correct behavior? It seems like proof should be an array with a derived proof for each signature.

tplooker commented 4 years ago

@dysbulic yes this is correct and by default it is selecting the first proof that matches the supported proof type at the moment. We could potentially modify the logic in deriveProof to generate a derived proof per matched proof, could you elaborate more on why this usage is important, do you want to be able to prove information revealed in a derived proof was signed by multiple issuers/signers?

dysbulic commented 4 years ago

I am working on a bonded courier service. I want to have the sender, recipient, and courier all sign a JSON-LD contract describing: • the appearance of the parcel • where it is coming from • where it goes • bond amount staked against loss of the parcel • bid amount the courier will receive

An escrow covering the courier's bid for the job and a stake of the courier's bond are created.

If a party faults ‒ the sender or recipient is unreachable, the courier absconds with the package, etc. ‒ then a moderator can be employed to decide the disbursement of funds. (I'm working off the OpenBazaar escrow.)

I would like to reveal as little about the contract as possible to the moderator, specifically I want to keep the locations of the parties private while proving that the parties all signed a contract.

dysbulic commented 4 years ago

I've got code that passes all the existing tests. The sample, however, is failing, and I'm still narrowing down why.

The basic refactor that I did was for studio.deriveProof and studio.verifyProof, I moved the bulk of the computation into a derive (or verify) method which I call appropriately depending on the result of Array.isArray(proof).

It changed the indentation which makes it look like I touched alot more than I actually did.

tplooker commented 4 years ago

@dysbulic thanks for the link and description of the usecase, I think we can refactor the deriveProof function to instead of selecting the first valid proof, enumerate through matched proofs and create a derivedProof

kdenhartog commented 4 years ago

@dysbulic we've got this code into master and are fixing the CI pipeline so an unstable release can be used to work with this functionality shortly. Samples are in PR #83 as well if you'd like to look at some examples.

Thanks for filing this issue.

tplooker commented 4 years ago

@dysbulic this functionality is now available in the unstable version of 0.7.1-unstable.55c8eaa we will cut a formal release in the next week or two after implementing several other pending features

dysbulic commented 4 years ago

@tplooker, that's awesome. I'll play around with it some.

You all's turnaround time was great. Thanks for being so responsive to the community.

All this crypto stuff is evolving so quickly, especially the zero-knowledge arena. It's nice to have solid implementations to build on.

tplooker commented 4 years ago

@tplooker, that's awesome. I'll play around with it some.

You all's turnaround time was great. Thanks for being so responsive to the community.

@dysbulic you're welcome, glad we could help.

All this crypto stuff is evolving so quickly, especially the zero-knowledge arena. It's nice to have solid implementations to build on.

Yes watch this space, we have many more features coming down the pipeline.

We will keep this issue open until this feature is cut into a formal release.