decentralized-identity / jwt-vc-presentation-profile

https://identity.foundation/jwt-vc-presentation-profile/
Apache License 2.0
15 stars 15 forks source link

Wallets should not be required to check revocation status for linked domain credentials #24

Closed dwight-holman closed 1 year ago

dwight-holman commented 2 years ago

See title.

Easier for the issuers to remove them from .well-known endpoints.

vlanard commented 2 years ago

The ask here is to explicitly cite in the profile that this particular check is not required, so that we remove this identified instance of ambiguity that could lead to unneeded work. Let's tell implementers that they can assume that resolving well known did creds are unrevoked.

dtmcg commented 2 years ago

@nklomp to do a PR to clarify

dangodb commented 2 years ago

Can we clarify the use case. When would I have a VC in the wellKnown that is revoked as opposed to it being removed from the wellknown.

nklomp commented 2 years ago

Ideally .well-known is automatically updated from DIDs it is referring to in the DomainLinkage VCs.

I can see at least one use case and another personal dislike of just leaving out Status List revocations if they are part of the VC. To start with the last one:

Any RP using .well-known decides for themselves whether their DomainLinkage credentials have a StatusList to begin with or not. If you want to follow the suggested approach of this ticket, simply issue them without a StatusList to begin with. Then removal from the .well-known location is enough. If however the RP decides to use a StatusList in their DomainLinkage VC, then always validate it. If you ask me it would be an anti-pattern to start making exceptions on something which is meant to be used to check revocations. You don't know the reason why the RP decided to include it in the first place.

Example use case: Ideally the KMS/Did management is automated. However in a lot of implementation DID management is separate from VC issuance, which again is seperate from the .well-known publishing. Nothing is guaranteeing that deactivating a DID-Key or revoking the DomainLinkage VC will result in an actual update of the .well-known location. It could be that this is handled by different departments in an organization if it is not automated. Certainly no assumptions can be made about attomicity even if it would be automated.

So instead of creating a simplification in a spec that already mentions that a VC MAY contain a StatusList2021, and if present should be checked for non-expiry, it think it wouldn't be wise to make exceptions to status lists checks. IMO any VC no matter where/how it was implemented by default should take the StatusList into account. The Issuer of the VC included it for a reason.

Don't want that for DomainLinkage VCs, then don't include the StatusList to begin with.

nklomp commented 2 years ago

Forgot to add. If people do feel strongly about not having to do StatusList checks on DomainLinkage VCs. Then I would suggest to bring up the issue in the DID Well-known spec repo to begin with.

Sakurann commented 2 years ago

I just checked and I don't think anything in .well-known spec requires StatusList check in a DomainLinkage VC..? https://identity.foundation/specs/did-configuration/

nklomp commented 2 years ago

No. So my suggestion is that I will add some non normative text stating that DomainLinkage VCs typically should not contain a StatusList and thus removal from the well-known location suffices

dwight-holman commented 1 year ago

Per @Sakurann 's comment, and https://identity.foundation/specs/did-configuration/#validation-of-domain-linkage-assertions , I think it's clearest to add language like

Validation of domain credentials by the wallet MUST follow the steps given in the Well Known DID Configuration standard. Additional checks, e.g. of revocation, are not required by this profile.

nklomp commented 1 year ago

I am not sure I follow the logic here. The response of @Sakurann is about .well-known spec not requiring StatusList checks. That is something different than saying it should not be required to validate if present.

Again, simply don't use a StatusList in the DomainLinkage credential to begin with. The issuer of the VC is in full control of that property being present or not.

I really don't get this whole ticket to be honest. It doesn't make sense to me and reeks like an anti-pattern.

dwight-holman commented 1 year ago

I really don't get this whole ticket to be honest. It doesn't make sense to me and reeks like an anti-pattern.

Sure, I'll elaborate. My experience with decentralized identity is a bit unusual in that I'm not trying to write specifications and standards, I'm trying to solve a problem by making a running program implementing those standards. As I read the standards, if I find myself asking questions arising from the combination of standards being referenced by the profile, I suggest modifications to the profile so that the right answer is obvious.

Again, simply don't use a StatusList in the DomainLinkage credential to begin with. The issuer of the VC is in full control of that property being present or not.

Although the issuer is in control of the property being present, the wallet controls the interpretation of the credential. The profile is saying in one place that credential revocation is handled by status lists, and in another that credentials are used for domain linkage.

The relation of one to the other for a wallet implementation lead to several questions on my part, and adding a sentence saying one does not apply to the other answers them. If that's not the answer we want? Okay. But something about the question would be helpful for people implementing wallets.

nklomp commented 1 year ago

Yes but then I suggest to add non-normative text that a DomainLinkage VC normally shouldn't contain a StatusList as in most circumstances simply removing it from the .well-known location suffices. I am really worried about a spec basically saying to not have to inspect something. The more exceptions like these you make the more things that can go wrong. Typically a wallet will have StatusList support anyway, as you also want to be aware of VCs you poses which might have been revoked. Or VCs you interpret from other parties during an exchange

dwight-holman commented 1 year ago

The standard, as linked above, has a specific and limited set of checks it does, none of which are revocation checks. Adding a revocation check diverges from the specs we're referencing.

Perhaps a two-sentence reminder that one does not apply to the other is out of scope for the profile. We can close this with no corresponding changes to the profile if that's the case.

nklomp commented 1 year ago

Yes, but CredentialStatus is part of the VC-Data model and it is not explicitly excluded from the interop profile. Actually the whole revocation chapter is about it. https://identity.foundation/jwt-vc-presentation-profile/#revocation So then it might become confusing for some people why certain parts of the spec require credentialStatus checks to be implemented but then another part of the spec is saying "just ignore it here".

To not make it really explicit, I suggest some non-normative text. Something to the extent of:

Since Domain Linkage Credentials are being managed by the Verifier/RP by updating the DID Configuration Resource directly, the usage of a credentialStatus property in a Domain Linkage Credential typically is of little use.

nklomp commented 1 year ago

Per @Sakurann 's comment, and https://identity.foundation/specs/did-configuration/#validation-of-domain-linkage-assertions , I think it's clearest to add language like

Validation of domain credentials by the wallet MUST follow the steps given in the Well Known DID Configuration standard. Additional checks, e.g. of revocation, are not required by this profile.

Sorry missed this comment. Maybe merging both of our suggestions a bit might be the solution 😅

nklomp commented 1 year ago

I merged both texts, but added a little text about expiration. Thas has to do with the well-known DIDs normatively mentioning that expirationDate has to be present: https://identity.foundation/.well-known/resources/did-configuration/#domain-linkage-credential.

Validation of domain credentials by the wallet MUST follow the steps given in the Well Known DID Configuration standard. Additional checks, e.g. of revocation, are not required by this profile. Since Domain Linkage Credentials are being managed by the Verifier/RP by updating the DID Configuration Resource directly, the usage of a credentialStatus property for revocation in a Domain Linkage Credential typically is of little use. Expiration of the Domain Linkage Credential does need to be taken into account.

Sakurann commented 1 year ago

I agree with the proposed text above

jischr commented 1 year ago

consensus: following steps in well known did configuration and does not included status list in credential.

nklomp commented 1 year ago

Updated PR with latest comment. Should be ready for approval