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

Can issue credential request credential proof property #337

Open
acarnagey opened this issue Feb 24, 2023 · 10 comments
Open

Can issue credential request credential proof property #337

acarnagey opened this issue Feb 24, 2023 · 10 comments
Assignees
Labels
ready for PR Issue ready to be resolved via a Pull Request

Comments

@acarnagey
Copy link

acarnagey commented Feb 24, 2023

Can credential contain the proof property already when issuing a credential?

@OR13
Copy link
Contributor

OR13 commented Feb 24, 2023

https://github.com/w3c-ccg/vc-api/blob/main/issuer.yml#L24

related VCDM PR: w3c/vc-data-model#1014

This behavior is not defined.

https://github.com/w3c-ccg/vc-api/blob/main/components/Credential.yml

Seems like proof MUST NOT be present in credential.

@msporny @dlongley do you agree?

@dlongley
Copy link
Contributor

@OR13,

Seems like proof MUST NOT be present in credential.

@msporny @dlongley do you agree?

No, I think it needs more consideration. Some further experimentation should be done with how to best handle cases with multiple proofs. So I think it's too early to say what the best approach is for that use case.

@OR13
Copy link
Contributor

OR13 commented Feb 24, 2023

How does multiple proofs interact with singular issuer ?

w3c/vc-data-model#932

Seems like we are intentionally leaving a security hole in an API definition for a use case, that has no consensus, and for which there are no examples in the core data model.

@dlongley
Copy link
Contributor

A single issuer could use multiple proofs with different cryptosuites to reduce crypto-fragility.

Seems like we are intentionally leaving a security hole in an API definition for a use case...

Ok, but we aren't. At least, that's not my approach here.

I do think our decisions should stem from use cases and understanding general solutions across those use cases. We know of at least one use case with a single issuer that has multiple proofs with different cryptosuites.

Additionally, we don't want our design to lock out permissionless extensibility, so decisions that create restrictions should be concretely justified. It's true that sometimes security and permissionless extensibility goals can be at odds with one another and could generate a conflict where a choice needs to be made one way or the other. But this should be well considered. Otherwise, we might find ourselves just enforcing side effects of previous decisions with little or no thought to the outcome. Standards work often takes a little more time than "move fast and break things" -- because the scope and impact is much larger than a single product or service for a particular customer.

@OR13
Copy link
Contributor

OR13 commented Feb 24, 2023

We know of at least one use case with a single issuer that has multiple proofs with different cryptosuites.

Is this a VCWG decision or a VC API decision?

Are multiple proofs exclusively for the "same" issuer?

@dlongley
Copy link
Contributor

@OR13,

We know of at least one use case with a single issuer that has multiple proofs with different cryptosuites.

Is this a VCWG decision or a VC API decision?

I'm not sure I understand the question ... because there's not a decision in what you quoted, just a mention of a use case. To hazard some kind of answer to your question, I took this whole issue to be about the VC-API. We're having this discussion in a VC-API issue, and I interpreted it to be about whether or not a credential posted to the /credentials/issue endpoint could have a proof in it or not.

A credential can most certainly have a proof in it if we're talking about the VCDM generally, but I don't think that's what's being discussed here.

Are multiple proofs exclusively for the "same" issuer?

As for data modeling or DI generally, "issuer" is given no consideration at all. But for VCs and using the VC-API to issue credentials? I'd say "yes". At least, if a credential is carrying other proofs (some might not even be assertionMethod proofs), perhaps the best thing for VC-API implementers to do is to ignore them. Or perhaps a better way to put it is: an implementer of the /credentials/issue endpoint should only care that they are issuing a VC with an issuer field that matches what is expected in the issuer instance configuration. I don't know that it matters what else is in the proof field.

The only case where it could matter would be in proof chaining, I'd think, but that's a place where we don't have a clear use case to draw conclusions yet -- and I would expect that to involve different parties. And since multiple issuers doesn't work with the current VCDM, this led to me answering "yes" above. So I don't think proof chaining is relevant here, however, nor do I think we need to close the door on future work there by prematurely prohibiting something.

@OR13
Copy link
Contributor

OR13 commented Feb 24, 2023

A credential can most certainly have a proof in it if we're talking about the VCDM generally, but I don't think that's what's being discussed here.

This is not true, see: w3c/vc-data-model#1014

There is no consensus that credential can have proof or MUST NOT have proof.

Seems directly relevant to this API, since if VCWG says it MUST NOT, this API would be "non conformant".

Noting that you could communicate that credential.proof is a valid member by updating this file:

https://github.com/w3c-ccg/vc-api/blob/main/components/Credential.yml

Saying its allowed to be present, while that file does not include it... seems... harmful.

But for VCs and using the VC-API to issue credentials? I'd say "yes".

It would be good to see that made explicit, or I think the answer is conclusively "undefined".

Something like:

"IF a verifiable credential has multiple proof THEN, they MUST be from the same issuer".

So I don't think proof chaining is relevant here, however, nor do I think we need to close the door on future work there by prematurely prohibiting something.

I agree, I think proof chaining is also likely out of scope for VCDM 2.0 at this point, with feature freeze coming.

@dlongley
Copy link
Contributor

@OR13,

A credential can most certainly have a proof in it if we're talking about the VCDM generally, but I don't think that's what's being discussed here.

This is not true, see: w3c/vc-data-model#1014

There is no consensus that credential can have proof or MUST NOT have proof.

I think that's a discussion around the media type application/credential+ld+json and whether it will reflect what the VCDM currently allows (since 1.1) or if it will place an additional restriction on data tagged with that media type. I'm on record saying that I expect it to be confusing to developers do that, but that's a separate issue.

Seems directly relevant to this API, since if VCWG says it MUST NOT, this API would be "non conformant".

This API doesn't yet use the media type application/credential+ld+json. I do think it should -- but if the VCWG decides that you can't put a proof in data tagged with that media type, that opinion might change. If there's a restriction added to the media type like that -- it needs to be convincing to developers or they just won't use it ... and I wouldn't recommend it here without there being a good argument for the prohibition.

If the WG makes that decision without a good, well-articulated argument in the specification, they should expect developers to think it's one of those "why did they do that in the spec?" situations where they are having to add if/else conditions in various places and having to work around it as an annoyance. Some will say "yeah, this is what happens in standards sometimes, people just throw stuff in there without good arguments that they add to the spec and now we're stuck doing it". It can be hard to avoid it happening because of the politics or timelines in standards now and then -- but if we can avoid it, we should.

It's important to note that people may just start violating the spec under those circumstances too -- and then a VCWG 3.0 could just change it to reflect current usage in the wild or enable more wide spread use (by those that didn't violate).

Noting that you could communicate that credential.proof is a valid member by updating this file:

https://github.com/w3c-ccg/vc-api/blob/main/components/Credential.yml

Saying its allowed to be present, while that file does not include it... seems... harmful.

Yeah, after a healthy discussion, if people agree it should be able to be present, we should add that, I agree.

Something like:

"IF a verifiable credential has multiple proof THEN, they MUST be from the same issuer".

Here, in this spec? I don't know that implementers need to process proof here. Of course, verifiers could also just ignore such proofs. What's the argument for the extra processing and complexity? It may be better to leave that responsibility to callers to ensure that they aren't generating VCs that may be flagged by verifiers as unacceptable for any variety of reasons.

If the VCWG decides it wants to add extra prohibitions around proofs like that -- then everyone will need to check them and reject them, sure (or they'll revolt if insufficiently convinced of the rationale). I'm not sure it's valuable to do or not at this time. There are, of course, any number of other things that could be added to a VC (because it's an extensible data model) that we would never know to tell people to do / not do. Adding extra processing, again, should have a good rationale and defense in the spec or it will end up irritating the people that have to implement it without understanding why.

@msporny
Copy link
Contributor

msporny commented Jul 11, 2023

The group discussed this on 2023-07-11. The group reviewed the request around multiple proofs on a VC. The use case of interest was https://w3c.github.io/vc-data-integrity/#proof-sets. @jandrieu noted that we might want to understand how the caller expects to happen. @dlongley agreed, and noted that status list credential signature also matters and not mixing cryptography between status list credential and original VC. So, if you have two types of signatures on VC, then status list credential also needs two of the same types of signatures. There was concern that we don't have enough implementation experience to understand what to do here. There seemed to be agreement that caller intent was important (on what to do with existing proofs). @PatStLouis noted on /credentials/issue, you have credential and options, if we have a credential w/ proof already attached, you would need to remove proof first before signing again? @dlongley noted that's how proof sets work, that's how proof sets work... you remove the proof, sign, and add the proof. @dlongley noted that there are also proof chains, so add proof that references another proof. Sign over content and refer to previous proof. @jandrieu said that how we're attaching semantics are "of the moment"... status list is a bit more weird, could attach proofs at 3 different issuers and have 3 different status lists, we need to work through that. @dlongley noted that credentials can have multiple issuers... currently there is only a single issuer for a VC, one of the problems w/ having many issuers is this sort of explosion in complexity.

While Data Integrity spec allows different proofs to come from different parties, VC API might be scoped to a single party with different types of cryptography attached to a VC... so single status list for VC. @jandrieu status list is signed over by the proof... you can't change status list w/ 2nd proof. Adding a proof doesn't let you add a status list -- not an operation that works. @dlongley are we going to recommend people have different issuing instances, sign, and that manages status list, and next instance adds proof (one way to do it -- clunky). Another way to do it is single issuing instance that attaches all proofs at once, does credential status management. @jandrieu recommended separating out status mechanism from issuance mechanism. @dlongley noted that for a lot of cases that might be challenging to do -- partitioning in the system that is doing issuing, tracking status, complexities can be avoided when they're done together vs. apart, especially in allocation of indexes into status list.

@msporny asked how we're going to convey the semantics? Default in options object to remove proof? @dlongley noted simplest way might be to have a single instance that attaches multiple proofs. Another case to consider is attaching proofs, expiration date on VC, open question is "should you just issue a new VC or should you be able to add a new proof w/ new validity period?" -- another use case to consider. With respect to "proof set", make an issuer instance that does all N proofs when you issue the VC (instead of trying to coordinate multiple issuing instances). @msporny asked about proof chains and how those are supported? For proof chains, we need a use case that involve the same issuer (solved same way), if we're talking about something else, we need to understand what the use case is. Notary and endorsement -- we might have /credentials/endorse or /credentials/notorize (to support proof chains). @jandrieu is concerned about the semantics being different for "endorsement" or "notarize" -- it's not just attaching another signature. Changing validity period, that's signed over -- can't do it w/ proof set. @dlongley two different validity periods -- validity period of VC itself, and then validity of proof. One is validFrom/validUntil, the other is created/expires on Data Integrity Proof. In the vast majority of cases, they line up, but people have talked about allowing them to diverge for various reasons... how would it be supported.

@msporny noted that all examples we have would throw error if previous proof. @dlongley noted that if we have something like a timestamp proof, that shouldn't be removed. What we're saying is "if proof is on VC when it arrives", that's ok, that doesn't need to be an error case. The instance will do what the Data Integrity spec says, remove proof, add whatever new proofs, then add existing proofs back in. We might say that "the way you do proof sets is through a single instance that does that". @jandrieu noted that maybe we address this by clarifying that this is not a security problem -- content can't be changed on previous proof.

The specification should mention that the default behavior for an instance is to add a proof to a VC that has an existing proof (per the Data Integrity specification). The instance can be configured to reject the default behavior and not add a proof to a VC that has an existing proof.

@msporny msporny added the ready for PR Issue ready to be resolved via a Pull Request label Jul 11, 2023
@msporny
Copy link
Contributor

msporny commented Mar 5, 2024

The group discussed this on the 2024-03-05 call:

We should add text to the spec that says a few things:

  • The general assumption is that if multiple proofs are required, an instance will add multiple proofs in one go.
  • Allow issuing instances to attach proofs to VCs that contain existing proofs.
  • The instance can be configured to reject the default behavior and not add a proof to a VC that has an existing proof.
  • If a VC already contains a single proof, the instance can convert the proof value to an array and add the existing proof and the new proof to the array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for PR Issue ready to be resolved via a Pull Request
Projects
None yet
Development

No branches or pull requests

5 participants