-
Notifications
You must be signed in to change notification settings - Fork 31
Client_id requirements and clarifications #484
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
Comments
I have also a client_id clarification question, that is somehow connected:
After second thought, i think having no fixed binding (compared to flexible expected_origins) between client_id and origin could cause some issues.
For example To limit the potential damage after key compromise or by a bad actor, i think
|
Yes, we raised this with OAuth WG and this led to the changes on client identifier in this section in OAuth 2.1 draft. Specifically,
and we also already say in client identifier prefix section in openid4vp that
having said htat, @aaronpk does it make sense to point to OAuth 2.1, instead of OAuth 2.0 throughout OpenID4VP? |
I think we already intend to say that in the introduction of client identifier prefix section: we could improve it to something like "A certain Client Identifier Prefix sets the requirements whether the Verifier needs to sign the Authorization Request as means of authentication and/or pass additional parameters and require the Wallet to process them" |
I think we intend to clarify that in the introduction of client identifier prefix section: here too, we can clarify "This specification defines the concept of a Client Identifier Prefix that dictates how the Wallet needs to interpret the Client Identifier and associated data in the process of Client identification, authentication, and authorization."
so in short, all processing rules for client identifier prefixes are mandatory for the wallet. |
client identifier not being assigned by AS (#484 (comment)) is i believe being clarified in #465 |
With the recent changes to the client_id and the multi-rp PR merged, there are some clarifications necessary for the client_id definition and processing.
Some of this could also be handled by #465 but I'm not sure whether it's clearer to split these comments or not.
Some of the ambiguity comes from the definition. The client_id is currently defined through a reference to RFC 6749. However RFC 6749 defines the client_id as being issued by the authorization server as part of a registration process. client_ids aren't always part of a registration process (e.g. they can be derived from other information like a certificate) and they aren't always issued by the authorization server.
Can we change the reference, or add language that these requirements from RFC 6749 do not apply?
For some of the requirements we have to state more explicitly who has to verify those:
For x509_san_dns: "The Wallet MUST validate the signature and the trust chain of the X.509 certificate." Other schemes have similar requirements.
Can we add a general statement along the lines of "Parsing and verification requirements for client identifiers are only applicable to the Wallet when the Wallet uses the information from that client identifier."
For x509_hash: "When the Client Identifier Scheme is x509_hash, the Client Identifier MUST be a hash and match the hash of the leaf certificate passed with the request." Other schemes have similar requirements.
Is it required, recommended or optional for the Wallet to verify this? For example when using the x509_hash and the DC api, the client_id is not included in the response, and all the information used to verify the signature is in the certificate itself. Therefore the hash is effectively duplicating information of the certificate. Since both are signed over, I don't think there is a security benefit from actually matching the x509_hash against the certificate itself.
To make sure that there is agreement on verification requirements, we have to state explicitly whether and when these checks must be done or not by the Wallet.
The text was updated successfully, but these errors were encountered: