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

(Q)EAA issuance and data model #70

Merged
merged 41 commits into from
Jul 24, 2023
Merged

(Q)EAA issuance and data model #70

merged 41 commits into from
Jul 24, 2023

Conversation

fmarino-ipzs
Copy link
Collaborator

Title

(Q)EAA issuance and data model

Content

This PR generalized the PID data model and issuance flow to the (Q)EAAs, under the following assumption:

  • the User has a valid PID stored in its own Wallet Instance;
  • the (Q)EAA requires a high-security implementation profile.

Some editorial improvements are also given to the PID data model and PID issuance.

Review

  • Ensure your files are written following RST specs (not MD!)
  • Italian version
  • English version
  • Example files
  • Ask for review

@fmarino-ipzs fmarino-ipzs added the feature A new feature label Jul 14, 2023
Copy link
Member

@peppelinux peppelinux left a comment

Choose a reason for hiding this comment

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

This review must continue

docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Show resolved Hide resolved
docs/en/pid-eaa-issuance.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-issuance.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-issuance.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-issuance.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-issuance.rst Outdated Show resolved Hide resolved
@peppelinux peppelinux requested a review from ruphy July 15, 2023 09:55
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
{
"verified_claims": {
"verification": {
"trust_framework": "eidas2",
Copy link
Member

Choose a reason for hiding this comment

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

how it will be there?

docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-issuance.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-issuance.rst Outdated Show resolved Hide resolved
@@ -192,13 +233,13 @@ The JWS payload of the request object is represented below:
}


**Steps 16-18:** The Wallet Instance creates a new key pair to which the new credential shall be bound. Then, it creates a proof of possession with the new key and the *c_nonce* obtained in **Step 15** and it creates a DPoP proof for the request to the PID credential issuance endpoint.
**Steps 16-18 (DPoP Proof for Credential Endpoint):** The Wallet Instance creates a new key pair to which the new credential SHALL be bound. Then, it creates a proof of possession with the new key and the *c_nonce* obtained in **Step 15** and it creates a DPoP proof for the request to the PID/(Q)EAA credential issuance endpoint.
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about using the same previously generated key for DPoP to bind to the new credential? Since both operations take place within seconds of each other, in my opinion, there is no need to generate a new key pair.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the evidence I have in mind is that we should not mandate which key should be used, it's up to the wallet solution, the issuer doesn't have the control of this and I would not put any constraints but just a normative SHOULD indicating a good practice to use different keys for different scopes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In principle, using the same key breaks the key separation principle of using different keys for different purposes. However in this case, I don't see any security issues. I will change in SHOULD as suggested by @peppelinux .

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks!


**Step 19:** The Wallet Instance sends a PID issuance request to the PID Provider credential endpoint. It contains the *access token*, the *DPoP proof*, the *credential type*, the *proof* (proof of possession of the key) and the *format*.
**Step 19 (Credential Request):** The Wallet Instance sends a PID/(Q)EAA issuance request to the PID/(Q)EAA credential endpoint. It contains the *access token*, the *DPoP proof*, the *credential type*, the *proof* (proof of possession of the key) and the *format*.
Copy link
Collaborator

Choose a reason for hiding this comment

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

In fact, here there could be confusion between DPoP proof and proof. See previous tip

docs/en/pid-eaa-issuance.rst Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
- **organization_name**: Name of the Organization handling the eID used by the User
- **organization_id**: Identification code for the Organization. It MUST be set to the *IPA Code* of the Organization
- **organization_name**: Name of the Organization acting as Authentic Source.
- **organization_id**: Identification code for the Organization. For public Organization, it MUST be set to the *IPA Code*, following the URN namespace ``urn:eudi:it:organization_id:ipa_code:<that-value>``.
Copy link
Member

Choose a reason for hiding this comment

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

urn:eudi:it:organization_id:ipa_code:<that-value>
must be included in the expert group sprints, can we do a review of the defined epics and push this accordingly?

@asharif1990 @giadas

docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-issuance.rst Show resolved Hide resolved
docs/en/pid-eaa-issuance.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-issuance.rst Show resolved Hide resolved
docs/en/pid-eaa-issuance.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-issuance.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@grausof grausof left a comment

Choose a reason for hiding this comment

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

LGTM

docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-data-model.rst Outdated Show resolved Hide resolved
docs/en/pid-eaa-issuance.rst Outdated Show resolved Hide resolved
Co-authored-by: Giuseppe De Marco <[email protected]>
@peppelinux peppelinux merged commit 7f74a95 into versione-corrente Jul 24, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature issuance
Projects
Development

Successfully merging this pull request may close these issues.

3 participants