-
Notifications
You must be signed in to change notification settings - Fork 15
signed issuer metadata #176
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
Conversation
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
…nto 156-signed-metadata
cre8
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommended is fine over must since the policy of the eco system can define it.
For eIdAS relevant ones it is a must, but maybe not for all eco systems.
| To support signed Issuer Metadata to allow for Issuer authentication, it is | ||
| RECOMMENDED to use the `signed_metadata` parameter in the Issuer Metadata. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want to say "it's recommended to do issuer authentication, and if you do it then use the signed_metadata parameter so this might be better:
| To support signed Issuer Metadata to allow for Issuer authentication, it is | |
| RECOMMENDED to use the `signed_metadata` parameter in the Issuer Metadata. | |
| It is RECOMMENDED for Issuers to support signed Issuer Metadata, using the `signed_metadata` parameter in the Issuer Metadata, to allow for Issuer authentication. |
This kind of feels too strong to be honest though. Validating the TLS certificate seems a perfect valid way of authenticating the issuer, I think we're probably suggesting something beyond authentication?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to have authentication on the VCI layer and not the TLS one.
Enterprises will mostly have a TLS proxy, but for eIdAS you need to use a specific access certificate for authentication. Also while using signed metadata with the embedded x509 in the header, it is decoupled from Https and allows better flexibility in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HAIP isn't just for eIdAS so we should figure out if there's a useful benefit for other ecosystems too, and how to concisely describe what the benefit is. If we can't figure out how to do that then we probably shouldn't recommend it :)
| When required by ecosystem policy, signed Issuer Metadata MUST be supported by | ||
| both the Wallet and the Issuer. Key resolution to validate the signed Issuer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something here feels awkward for an interoperability profile. We'd generally mandate the wallet to support so it can work with any issuer I think?
I'm not sure if we need to be explicit about what should be in signed_metadata. Is it okay for it to just have iat, iss & sub & everything else to be in the unsigned section? (That's all that VCI mandates as far as I can see.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think requiring the wallet to support signed metadata makes sense. We already require it for OpenID4VP. By making it optional for the issuer you still provide flexibility, but don't create interoperability issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think with the new approach discussed at last WGs meeting it'll be either fully signed or fully unsigned, so that simplifies the decision as what to include in the signed section
| RECOMMENDED to use the `signed_metadata` parameter in the Issuer Metadata. | ||
| When required by ecosystem policy, signed Issuer Metadata MUST be supported by | ||
| both the Wallet and the Issuer. Key resolution to validate the signed Issuer | ||
| Metadata MUST be supported via the `x5c` JOSE header parameter as defined in [@!RFC7515]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should call out that how the wallet gets the root certificate is out of scope, so this isn't a complete key resolution strategy.
|
@jogu in a comment somewhere said, "Validating the TLS certificate seems a perfect valid way of authenticating the issuer" and that seems a perfectly valid comment. |
|
Is there any implementation experience with signed metadata? |
All the OpenID Federation implementations exclusively use signed metadata. By my count during the interop last week, there were at least 14 implementations. So yes, there's implementation and deployment experience with signed metadata. |
|
Just for clarity, I believe Paul is talking about The two different |
|
Yes. In particular: is OpenID federation using exclusively signed metadata? OpenID4VCI is using a mixture of signed and unsigned metadata that makes me slightly uncomfortable and I was interested if anybody has implementation experience with what OpenID4CVI specifies. |
TimoGlastra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in a comment, to promote interoperability it's not a big ask for a wallet to require support for signed metadata, especially if the changes are incorporated into VCI to either require all signed or all unsigned.
Co-authored-by: Joseph Heenan <joseph@authlete.com>
|
Discussed on today's WG call - Paul to review then we plan to merge. |
|
did we discuss Timo's comment, which I think effectively suggests making it mandatory for the wallet to support signed issuer metadata? |
|
when we discussed in the WG, rough consensus was that mandating this for the wallet was a disproportionate burden on the wallets. hence current text. merging as agreed in the WG, but feel free to open an issue, we can keep discussing. (for example for 1.1 even) |
Closes #156
I added the RECOMMENDED part - happy to remove if WG is not comfortable with it.