search

w3cping / privacy-request

tracking privacy reviews of W3C specifications
9 stars 2 forks source link

Securing Verifiable Credentials using JOSE and COSE 2022-09-15 #125

Closed awoie closed 7 months ago

awoie commented 12 months ago

We have conducted a self-review of our spec Securing Verifiable Credentials using JOSE and COSE, and the results can be found at https://github.com/w3c/vc-jose-cose/issues/93.

Please check our findings.

kdenhartog commented 10 months ago

VC JOSE/COSE spec

Notes for PING Reviewers

Noting that the terms First Party is not shared between the different VC data model specs. This isn't really an issue, but in general I think of the First party as the issuer, the holder as a UA, and the verifier as a third party so will refer to them within this context. This is the model that was used in the VC Data Model self review as well, but doesn't necessarily align with the specific definitions that are normally used within the browser and PING. It appears based on previous reviews that they're aligning these concepts more without delineating between 3P/1P and instead sticking with their roles and recognizing each role has a UA associated with it.

This spec is one of two classes of proof suites (the other is VC-Data Integrity reviewed here https://github.com/w3cping/privacy-request/issues/120) that is used to produce a signature of the claims. The proofs can be used either to prove the assertions came from the issuer, or to assert the presentation by a holder to the verifier. Specifically with the VC-JOSE-COSE spec, this spec is standardizing the method of using JOSE and COSE related specs to produce signatures to assert claims about the subject in a VC.

Summary of Review

  1. The spec should point back to the vc-data-model privacy and security sections, as well as the JWT and CWT privacy and security considerations, and the SD-JWT security considerations sections. I see the privacy considerations does this and the sec considerations points back to RF7515/7519. This would avoid needing to repeat a lot of the same things across Data-integrity/VC-Data-Model/This spec.

  2. Editorial nit: It would be useful to split these considerations up with separate headers

  3. The specification should highlight the tradeoff discussed in: https://medium.com/@markus.sabadello/json-ld-vcs-are-not-just-json-4488d279be43 and how semantic security is not a guarantee and considered an accepted tradeoff here.

  4. The processing details of section 3.2 and 3.2.1 seem rather loose. Is it using COSE or is it using CBOR? It currently says it MAY use COSE, but if it's using just CBOR it's rather ambiguous how that signature should be produced and verified. I'd recommend sticking with COSE only here to narrow the focus of the spec and not allow unnecessary ambiguity.

    • I'm noting that the similar issue with JOSE definitions in section 3.1. It should be more explicit about the signature production and verification and reference these specs directly or otherwise specify what is happening if this is not the case.
    • As @jyasskin noted in https://github.com/w3c/vc-data-model/issues/1285#issuecomment-1731724138 we really should be confident about the processing models here to have a high confidence of interoperability and ensure no security or privacy issues arise from improper validation of signatures.

  5. The spec should forbid the usage of algorithms which are "Prohibited" in https://www.iana.org/assignments/jose/jose.xhtml and "Deprecated" in https://www.iana.org/assignments/cose/cose.xhtml

  6. Editorial nit: does it make sense to redefine the terminology in this spec or reference back to it in other specs? Most of these are defined elsewhere anyways and look to exactly match. FWIW, I should have noted that in the DI review as well. cc @msporny

jyasskin commented 9 months ago

The group has started to address (4) in https://github.com/w3c/vc-jose-cose/pull/190.

jyasskin commented 9 months ago

The biggest privacy risk that I noticed is that the spec doesn't rule out the possibility that a verifier will fetch some of the JWT fields while verifying the credential. Some of the VC use cases want to avoid telling an issuer that a particular subject has visited a particular verifier, and without seeing the algorithms, we can't evaluate whether those use cases are satisfied. It looks like other use cases don't require this, and may even require that fields be fetched dynamically, so I don't think it should be a requirement that the algorithms never fetch anything during verification, but they should be clear how callers can enforce the requirements of each type of use case.

kdenhartog commented 9 months ago

Sorry, I wasn't able to make the call today. I still need to read through the notes. One of the broader points that needs to be made though goes along with @jyasskin point is that currently this spec is non-declaritive about it's purpose to the point it creates privacy risks. For this reason, I do think we should be considering whether or not this spec is even ready for CR status in the first place if it's not possible to be certain that an implementation is using specifically JWTs or CWTs. Based on this current language specified in 3.2:

COSE [[RFC9052](https://www.w3.org/TR/vc-jose-cose/#bib-rfc9052)] is a common approach to encoding and securing 
information using CBOR [[RFC8949](https://www.w3.org/TR/vc-jose-cose/#bib-rfc8949)]. Verifiable credentials MAY be 
secured using COSE [[RFC9052](https://www.w3.org/TR/vc-jose-cose/#bib-rfc9052)] and SHOULD be identified through 
use of content types as outlined in this section.

This could be interpreted that a conformant implementation chooses to support CBOR, but not use RFC9052 as long as it uses the content types. This doesn't seem specific enough to me to produce a conformant implementation let alone one that we can reasonably deduce the privacy properties of it. This is largely what I was trying to point out in number 4, but I didn't do a good enough job articulating in my previous comment.

Hopefully others in the group were able to understand this based on what @pes10k relayed for me on the call, but if not I wanted to make sure it was clear this is what I meant by number 4.

jyasskin commented 9 months ago

I think that was fairly clear. I'm pretty sure the WG means that an issuer MUST secure each VC (from the core data model spec), and one of the ways it MAY secure them is COSE. If the issuer does that, it SHOULD identify that fact using content types.

+1 for saying in our wide review comments that the WG needs to write algorithms that actually specify this. We can also formally object through our individual AC reps if the WG doesn't do that by AC review time, but I'm hopeful that we won't have to.

msporny commented 9 months ago

We can also formally object through our individual AC reps if the WG doesn't do that by AC review time, but I'm hopeful that we won't have to.

Comments below are with my VCWG Member hat on (I'm not an editor of this specification):

I expect that certain members of the VCWG will object to taking the vc-jose-cose specification to CR if it does not address all of the PING review comments. It was made clear to the group by W3M that the expectation is that we address ALL Horizontal Review comments to the satisfaction of the reviewing group before transitioning any specification to CR.

I don't expect that you will have to FO through your individual AC rep unless the VCWG disagrees with the review comments, and to date, the WG has agreed with and made changes based on all the prior PING reviews. I don't expect this specification to be different in that regard.

msporny commented 9 months ago

Editorial nit: does it make sense to redefine the terminology in this spec or reference back to it in other specs? Most of these are defined elsewhere anyways and look to exactly match. FWIW, I should have noted that in the DI review as well. cc @msporny

Yes, it makes sense.

That said, the Editors of the vc-jose-cose spec have been adamant about copy-pasting content into their spec to make it as independent as possible from the other specifications. IOW, the vc-jose-cose spec should reference other specs, but it doesn't and instead copy-pastes content into their document, which is an anti-pattern.

I'll also note this conversation, where we're trying to figure out a better way of "sharing" these definitions across W3C specifications.

https://github.com/w3c/respec/issues/4522#issuecomment-1831981202

There is hope that we might be able to do something like this before any of the VC specs transition to the next stage:

https://github.com/w3c/respec/issues/4522#issuecomment-1842628036

kdenhartog commented 9 months ago

I think that was fairly clear. I'm pretty sure the WG means that an issuer MUST secure each VC (from the core data model spec), and one of the ways it MAY secure them is COSE. If the issuer does that, it SHOULD identify that fact using content types.

Ok, that's good to hear. Yeah I could definitely see someone going "Hey I don't want to use COSE here for X, Y, and Z reason, but CBOR looks nice. I'll just use this unspecified content type that I made up and isn't defined anywhere and call it conformant with the VC-JOSE-COSE spec". Based on the usage of MAY and SHOULD here, they'd technically still be conformant even though it's wildly divergent from what's defined and I wanted to make sure that was called out to spec authors. This one should be as simple as just making the spec language a bit stronger. One example would be:

"If an implementer wishes to use CBOR as a serialization format they MUST USE RFC9052 and MUST specify the content type as...".

My language might still face tradeoffs here, but it at least remains narrowly defined to make it easy for someone to implement later. I'll open an issue on the spec to make sure people are aware of this concern.

pes10k commented 7 months ago

This was discussed on PING call on Dec 7, 2023. Closing as discussed

https://github.com/w3c/ping/blob/main/summaries/PING-minutes-20231207.md