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

Revocation request flow and Non-Revocation Attestation issuance flow #143

Merged
merged 29 commits into from
Feb 29, 2024

Conversation

ruphy
Copy link
Member

@ruphy ruphy commented Oct 15, 2023

This draft PR aims to facilitate a discussion on the technical details of the proposed revocation mechanism. It represents our initial attempt to implement ideas generated from informal discussions, with the goal of exploring this approach (or alternative ones) thoroughly to reach a consensus.

After considering status lists, dynamic status lists, and bearer tokens, this PR presents an approach that currently appears most feasible. In this approach, the authenticity of a Verifiable Credential (VC) can be independently verified. However, for scenarios where the current validity of the VC is crucial (e.g., checking the validity of a driving license), we propose the presentation of a "recent" non-revocation attestation (suggested minimum duration of 350 hours).

This approach allows all credentials to be long-lived (e.g., a proof that a person is over 18 years old), providing all the associated benefits. Simultaneously, it gives Relying Parties (RPs) the flexibility to determine whether they require a validity attestation and how recent it needs to be. This flexibility enables a trade-off between factors such as user experience and accuracy.

Please note that this PR is a starting point for discussion and not a final implementation. Feedback and suggestions are highly welcomed.

@peppelinux peppelinux marked this pull request as draft October 16, 2023 11:17
Copy link
Contributor

@balanza balanza left a comment

Choose a reason for hiding this comment

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

Great job @ruphy, thanks for starting this conversation.

I left some comments in the review

docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
@peppelinux peppelinux changed the title Draft PR for Discussing Technical Aspects of Revocation Mechanism Revocation request flow and Non-Revocation Attestation issuance flow Oct 16, 2023
docs/en/defined-terms.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
Co-authored-by: Giuseppe De Marco <[email protected]>
Co-authored-by: Giuseppe De Marco <[email protected]>
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

credential_proof -> credential_pop

Copy link
Member

Choose a reason for hiding this comment

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

credential_proof -> credential_pop

Copy link
Contributor

@balanza balanza left a comment

Choose a reason for hiding this comment

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

Thanks @fmarino-ipzs for continuing @ruphy's work, it's excellent!

I'm happy with how we define responsibilities and the mechanisms you set up.

The only issue I see is about API design, I have some concerns on how we make use of HTTP verbs and codes. Please note that such concerns does not invalid the overall solution!

TL;DR Avoid using 4xx to comunicate the validity of a resource, let the response payload carry such information.
on.

HTTP codes don't usually refer to the content of the response, but give semantic to the request/response connection itself:

  • 2xx mean the request is successful
  • 4xx mean the request is malformed or using wrong assumptions (ex: wrong authz o pointing to wrong resource)
  • 5xx mean technical issues arise in the server

Instead, we are using 4xx-family codes to comunicate something about the content of the response. This is doable but a bit out-of-standard and common use, and it could make it cumbersome to use typical HTTP clients.

Instead, I propose the following approach:

  • 2xx when the request succeed, and we get a consistent response from the server; the payload ships the details about the operation being positive or negative.
  • 400 when the request is malformed (ex: passing unsupported value)
  • 401 when the authz fails (ex: the provided PoP token is not valid)
  • 403 404 when the provided authz mean is valid, but not related to the requested resource (why 404?)
  • 404 when requesting something about a non-existing credential
  • 5xx for server errors

This is an overview, we can go in detail for each endpoint if needed.

Having said that, I'm in favor of merging this PR as-is and make this refactoring in the next PR, for not messing up too much with the whole proposition.

What do you think?

Co-authored-by: Giuseppe De Marco <[email protected]>
Co-authored-by: Giada Sciarretta <[email protected]>
Copy link
Member

Choose a reason for hiding this comment

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

credential_proof -> credential_pop

Copy link
Member

Choose a reason for hiding this comment

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

credential_proof -> credential_pop

Copy link
Collaborator

@fmarino-ipzs fmarino-ipzs left a comment

Choose a reason for hiding this comment

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

fixed credential_proof --> credential_pop in the text and in the images. I also clarified the aud parameter definition.

@peppelinux peppelinux merged commit e19c177 into versione-corrente Feb 29, 2024
8 of 9 checks passed
@balanza
Copy link
Contributor

balanza commented Mar 1, 2024

Me and @fmarino-ipzs had further discussion on #143 (review)

We agreed that given the resource of the HTTP request is the Status Attestation, it's ok to consider any issue related to the Credential as a bad request. Hence, requesting a Status Attestation for a revoked Credential is not different from mistyping one parameter.

Error codes are there to provide hints to clients so they can trace errors and decide to inform Users accordingly, but they add no semantics to the response itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

6 participants