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

Add examples for multiple status lists and multiple entries in a single status list #122

Merged
merged 8 commits into from
Jan 13, 2024

Conversation

msporny
Copy link
Member

@msporny msporny commented Dec 30, 2023

This PR is an attempt at addressing some comments in issue #73 by adding examples for multiple status lists and multiple entries in a single status list.

Note: This PR contains a normative change to support the last example with multiple status types stored in a single list. It also has an at-risk marker for the feature as the more I wrote about the feature, the less of a benefit there seemed to be. The strongest benefit seems to be a privacy argument (if you mix in multiple statuses in a single list, and the bits for each status don't sit beside one another, it's hard to tell which type of status bit is being flipped to an outside observer -- which improves the privacy characteristics of the list).


Preview | Diff

Copy link
Contributor

@dlongley dlongley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, but generally not a fan of the approach, I think it adds unnecessary complexity.

index.html Outdated Show resolved Hide resolved
Copy link
Contributor

@David-Chadwick David-Chadwick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why dont we remove the multiple entries feature before CR in the interests of simplification?

index.html Outdated Show resolved Hide resolved
index.html Outdated
Comment on lines 1151 to 1152
`proofPurpose` values in the status list credentials (again, a meager savings in
complexity).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a meager savings in complexity seems to go against the rest of this note, which discusses meager savings in time and/or data size, gained at the expense of more complex implementations...

Suggested change
`proofPurpose` values in the status list credentials (again, a meager savings in
complexity).
`proofPurpose` values in the status list credentials (again, a meager savings in
complexity).

index.html Outdated Show resolved Hide resolved
Copy link

@KDean-GS1 KDean-GS1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see no reason for having multiple status lists; I'm hard-pressed to imagine a scenario where a verifier would be interested in "this" status but not "that" status. I'm sure such a scenario exists, but it's probably in a very small minority.

I'm neutral on this change.

Copy link

@longpd longpd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also not persuaded by this requested change. The security benefits and time savings seem minor. As KDean-GS1 noted "I'm hard-pressed to imagine a scenario where a verifier would be interested in "this" status but not "that" status.". Not sure the complexity is worth it - though will remain neutral and not object.

@msporny
Copy link
Member Author

msporny commented Jan 6, 2024

@KDean-GS1 wrote:

I see no reason for having multiple status lists; I'm hard-pressed to imagine a scenario where a verifier would be interested in "this" status but not "that" status. I'm sure such a scenario exists, but it's probably in a very small minority.

Here's a fairly significant set of use cases that care about revocation, but probably don't care about suspension: using any government issued identification card to verify:

  • name
  • birthday
  • gender
  • country of domicile
  • current address
  • height
  • weight
  • biometric photo
  • sight restrictions

IOW, just because the document is suspended (but not revoked) probably doesn't affect the claims above. This can be true for a variety of other document types as well.

I'm offering this as a counter-point, not arguing for any particular direction on this PR.

@KDean-GS1
Copy link

@KDean-GS1 wrote:

I see no reason for having multiple status lists; I'm hard-pressed to imagine a scenario where a verifier would be interested in "this" status but not "that" status. I'm sure such a scenario exists, but it's probably in a very small minority.

Here's a fairly significant set of use cases that care about revocation, but probably don't care about suspension: using any government issued identification card to verify:

  • name
  • birthday
  • gender
  • country of domicile
  • current address
  • height
  • weight
  • biometric photo
  • sight restrictions

IOW, just because the document is suspended (but not revoked) probably doesn't affect the claims above. This can be true for a variety of other document types as well.

I'm offering this as a counter-point, not arguing for any particular direction on this PR.

I'm not sold on that. If the government has suspended an identification credential, it could be because they are investigating the holder for some fraudulent declaration. For example, Toronto, Ontario has a law requiring that short-term renters (e.g., via Airbnb) reside at the address they're renting out, and proving it via a driver's license. This has led to families with multiple properties having individuals within the family each declare a property as their primary residence and reflecting it on their driver's license. If the government believes that the address declarations are fraudulent, they could suspend the credentials pending proper proof of residency.

Basically, unless specific use case rules say otherwise, any credential that with a status other than "active" should be treated at least as suspect. I think the process should be as follows: check the status, and once the status is known, whatever it might be, then and only then apply the validation rules.

@TallTed
Copy link
Member

TallTed commented Jan 10, 2024

I think the process should be as follows: check the status, and once the status is known, whatever it might be, then and only then apply the validation rules.

That would be dictating business logic, which is beyond our purview.

I think we can legitimately say that Verifiers SHOULD check the status, but we cannot say that they MUST nor SHOULD do anything based on the result.

@KDean-GS1
Copy link

That would be dictating business logic, which is beyond our purview.

Agreed. This is more about a generic approach; I can't imagine building a library that would selectively ignore any status instead of "get everything and pass it to the caller to figure out".

I think we can legitimately say that Verifiers SHOULD check the status, but we cannot say that they MUST nor SHOULD do anything based on the result.

Definitely.


<pre class="example nohighlight vc"
title="Associating multiple status lists with a single Verifiable Credential"
data-vc-vm='https://example.edu/issuers/565049/keys/1'>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason that ' is used to wrap data-vc-vm values (lines 1092 and 1157), instead of " as on all other attribute values? It just seems odd to differ like this...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, no reason, just a typo.

Copy link
Member Author

@msporny msporny Jan 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8d234df.

Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor

index.html Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@msporny
Copy link
Member Author

msporny commented Jan 13, 2024

Editorial, multiple reviews, changes requested and made, no objections, merging.

@msporny msporny merged commit c3607c5 into main Jan 13, 2024
1 of 2 checks passed
@msporny msporny deleted the msporny-multilist branch January 13, 2024 21:20
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

Successfully merging this pull request may close these issues.

7 participants