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

Ensure that status messages are committed to at issuance time. #164

Merged
merged 7 commits into from
Apr 16, 2024

Conversation

msporny
Copy link
Member

@msporny msporny commented Apr 6, 2024

This PR is an attempt to address issue #151 by requiring that status messages are committed to by issuers at issuance time. The PR does this by:

  • Moving the statusSize, statusMessage, and statusReference fields to the BitstringStatusListEntry object.
  • Explaining why the commitment is done at issuance time on the original VC itself.

NOTE TO REVIEWERS: Most of the changes in this PR are just moving text around in the spec. The only normative change is the first bullet item above. Everything else should be editorial. It looks like way more changed than it did.

I believe, if this PR is accepted, that it should address @jandrieu and PINGs concerns regarding the issuer changing the types of status messages that are possible without the holder or subjects knowledge.


Preview | Diff

index.html Outdated
The `statusReference` property provides a point for implementers to include a
[[URL]] to material related to the status. An implementer MAY include the
`statusReference` property, and if they do, the value MUST be a [[URL]] or an
array of URLs. Implementers using a `statusPurpose` of `status` are strongly
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
array of URLs. Implementers using a `statusPurpose` of `status` are strongly
array of URLs. Implementers using a `statusPurpose` of `message` are strongly

Comment on lines +595 to +596
`message`, a string used by software developers to assist with debugging
which SHOULD NOT be displayed to end users.
Copy link
Contributor

Choose a reason for hiding this comment

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

How can these messages be committed to for users if they are not to be displayed or understood by them?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a presumption here that the user software will convert the value into something internationalized for the individual based on its programmatic value.

I do think that's excessively hand-wavy, given that the issuer could just do that work here, but given that the Traceability group wants it to work this way, and that there are no other strong opinions on how it should work, the text that's in the spec is the best we have at present.

</td>
</tr>
<tr>
<td id="statusReference">statusReference</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

Does statusReference pose a privacy problem in the same way that uncommitted messages might? Perhaps a hash is also required (e.g., statusReference: {id: "https://example.com/foo", digestMultibase: "<hash>"}?

If statusReference were committed to via a hash, the statusMessage array could potentially be omitted entirely. This might be more attractive than always including all possible messages, now that they must be present in the VC itself. This may also help address the i18n + commitment issue (that I think might exist now as mentioned above) because a more rich response can be returned in the status reference document.

Copy link
Contributor

Choose a reason for hiding this comment

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

could we just treat this identical to an external reference as in the core data model?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's the basic idea, it was just being given a more specific relationship name since it's well known how it relates.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought the statusReference was human-readable documentation, and so committing to it using a hash would prevent it being updated in ways that would harm readability?

Again, the design here is strange in that we have a unique value for each message (statusMessage.status), but then we have another "developer value" (statusMessage.message), that could just as easily live in statusReference (if it were a machine-readable file).

It's up to Traceability folks if they want to change the design to something simpler. I'm not going to let that hold up this PR.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated
Comment on lines 627 to 630
Status list entries can be used to associate a single status type,
such as `revocation` or `suspension`, with a [=verifiable credential=] by using
the `statusPurpose` property. The example below demonstrates the association
of simple status list entries:
Copy link
Member

Choose a reason for hiding this comment

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

There's some difficulty of alignment between "status types" which are here blurred with "statusPurposes". "Revocation" and "suspension" are fine "status purposes", but they're terrible "status types". "Revoked" and "suspended" are OK "status types" (or "status statuses"), but terrible "status purposes". Maybe these "purposes" are associated with BitStringStatusLists, rather than with Statuses, per se? Partial text patch below; conversation may lead to modifications/additions.

Suggested change
Status list entries can be used to associate a single status type,
such as `revocation` or `suspension`, with a [=verifiable credential=] by using
the `statusPurpose` property. The example below demonstrates the association
of simple status list entries:
Status list entries can be used to associate a single status type,
such as `revocation` or `suspension`, with each [=verifiable credential=]
by using the `statusPurpose` property. The example below demonstrates the
association of simple status list entries:

Copy link
Contributor

Choose a reason for hiding this comment

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

"a status type" => "a status purpose"?

Copy link
Contributor

@dlongley dlongley Apr 16, 2024

Choose a reason for hiding this comment

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

My review below includes a suggestion that incorporates Ted's suggestion with some edits.

index.html Outdated
Comment on lines 666 to 673
Status list entries can be used to associate many status types with a
[=verifiable credential=] by using the `message` status purpose. The set of
messages associated with a particular entry is committed to by the [=issuer=]
by using the `statusSize`, `statusMessage`, and optional `statusReference`
properties. This commitment is done at the time of issuance to ensure that
the [=holder=] knows what sort of information might be associated with a
particular [=verifiable credential=] that is in their possession, and would
then be discoverable by a [=verifier=] that receives that credential.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Status list entries can be used to associate many status types with a
[=verifiable credential=] by using the `message` status purpose. The set of
messages associated with a particular entry is committed to by the [=issuer=]
by using the `statusSize`, `statusMessage`, and optional `statusReference`
properties. This commitment is done at the time of issuance to ensure that
the [=holder=] knows what sort of information might be associated with a
particular [=verifiable credential=] that is in their possession, and would
then be discoverable by a [=verifier=] that receives that credential.
Status list entries can be used to associate multiple status types with a
[=verifiable credential=] through the single `message` status purpose. The
[=issuer=] commits to the set of messages that may be associated with a
particular entry (i.e., with a particular [=verifiable credential=]) through
the `statusSize`, `statusMessage`, and optional `statusReference` properties,
at the time of [=verifiable credential=] issuance. This is to ensure that
the [=holder=] knows what sort of information might be associated with a
particular [=verifiable credential=] they keep in their possession, that
could then be discoverable by a [=verifier=] that later receives that
credential.

Copy link
Contributor

Choose a reason for hiding this comment

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

So "message" is just another "status purpose", it doesn't enable "multiple status types" (I don't think). It is just the case that just about any "message" about the status of a credential can be expressed. The purpose of a "message" status list expands to: "to express a specific message (one from a list of precommitted messages) describing the status of a credential". We should probably just avoid saying "status type" and stick to "status purpose".

So we have:

  1. A status purpose of "revocation", which means that a status list is used to express the revocation status of a particular credential.
  2. A status purpose of "suspension", which means that a status list is used to express the suspension status of a particular credential.
  3. A status purpose of "message", which means that a status list is used to express a specific message, from a predefined list of messages, describing the status of a particular credential.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe some text like this can be incorporated into the suggestions above:

Status list entries can be used to express the purpose of a
status associated with a [=verifiable credential=] by using the
`statusPurpose` property.

The use of `revocation` or `suspension` as the status purpose
includes the semantics of the status, with `revocation`
indicating that a status bit expresses whether a
[=verifiable credential=] has been revoked and `suspension`
indicating that a status bit expresses whether a
[=verifiable credential=] has been suspended:

/* revocation and suspension examples */

The use of `message` as the status purpose enables an issuer to
define an arbitrary number of custom, descriptive messages about
the status of the [=verifiable credential=]. The [=issuer=] commits...
...

Copy link
Contributor

@dlongley dlongley Apr 16, 2024

Choose a reason for hiding this comment

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

I added a suggestion below (in my review) that attempts to combine Ted's suggestion with my own.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Copy link
Member

@brentzundel brentzundel left a comment

Choose a reason for hiding this comment

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

approved with Ted's suggested changes

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

msporny commented Apr 16, 2024

Normative, multiple reviews, changes requested and made, no objections (but non-blocking design concerns raised), merging.

@msporny msporny merged commit c97dc95 into main Apr 16, 2024
1 of 2 checks passed
@msporny msporny deleted the msporny-slm-commitment branch April 16, 2024 20:11
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.

9 participants