Security considerations for client_metadata parameters - #735
dpostnikov wants to merge 12 commits into
Conversation
Co-authored-by: Frederik Krogsdal Jacobsen <fkj@users.noreply.github.com>
| Wallet implementers need to choose a source of client metadata they can trust within their own ecosystem or implementation. This should determine whether it is safe to use or display to a User. For example, a client name obtained from an X.509 certificate can be trusted, whereas a client name from the `client_metadata` parameter cannot. | ||
|
|
||
| It is recommended to use `client_metadata` parameters only for client-supplied parameters that can be used untrusted, for example, a public key. | ||
|
|
||
| Ecosystems can use other ecosystem-specific trusted data sources as long as they conduct a proper trust evaluation. |
There was a problem hiding this comment.
I think we have to be careful not to conflict with this text:
"It is recommended" probably needs to be reworded, and 'Ecosystems' we used 'profiles' in the other text.
There was a problem hiding this comment.
@jogu What specifically is the problem with the "it is recommended" text? Just that it reads as a normative statement?
And what about something like this:
| Wallet implementers need to choose a source of client metadata they can trust within their own ecosystem or implementation. This should determine whether it is safe to use or display to a User. For example, a client name obtained from an X.509 certificate can be trusted, whereas a client name from the `client_metadata` parameter cannot. | |
| It is recommended to use `client_metadata` parameters only for client-supplied parameters that can be used untrusted, for example, a public key. | |
| Ecosystems can use other ecosystem-specific trusted data sources as long as they conduct a proper trust evaluation. | |
| Ecosystems can define other ecosystem-specific trusted data sources in a profile as long as they conduct a proper trust evaluation. |
There was a problem hiding this comment.
@jogu where do you think it conflicts? Is it the example of the public key and should we be more specific that it is about specific, ephemeral public keys as described in the section?
In the spec I also often notice we use "ecosystems or profiles" instead of the one or the other. Maybe that applies here as well.
There was a problem hiding this comment.
@jogu Would be great to have some more detail on this.
There was a problem hiding this comment.
I think my concern was in the conflict between
It is recommended (for Wallet implementers) to use
client_metadataparameters...
and
Other metadata parameters MUST be ignored unless a profile <...>'
If the recommendation is scoped to profiles that's better. But maybe something like:
Profiles MAY define additional
client_metadataparameters for client-supplied parameters that can be used untrusted, for example, a public key.
(Pretty sure that can be improved upon)
| Wallet implementers need to choose a source of client metadata they can trust within their own ecosystem or implementation. This should determine whether it is safe to use or display to a User. For example, a client name obtained from an X.509 certificate can be trusted, whereas a client name from the `client_metadata` parameter cannot. | ||
|
|
||
| It is recommended to use `client_metadata` parameters only for client-supplied parameters that can be used untrusted, for example, a public key. | ||
|
|
||
| Ecosystems can use other ecosystem-specific trusted data sources as long as they conduct a proper trust evaluation. |
There was a problem hiding this comment.
@jogu What specifically is the problem with the "it is recommended" text? Just that it reads as a normative statement?
And what about something like this:
| Wallet implementers need to choose a source of client metadata they can trust within their own ecosystem or implementation. This should determine whether it is safe to use or display to a User. For example, a client name obtained from an X.509 certificate can be trusted, whereas a client name from the `client_metadata` parameter cannot. | |
| It is recommended to use `client_metadata` parameters only for client-supplied parameters that can be used untrusted, for example, a public key. | |
| Ecosystems can use other ecosystem-specific trusted data sources as long as they conduct a proper trust evaluation. | |
| Ecosystems can define other ecosystem-specific trusted data sources in a profile as long as they conduct a proper trust evaluation. |
| Wallet implementers need to choose a source of client metadata they can trust within their own ecosystem or implementation. This should determine whether it is safe to use or display to a User. For example, a client name obtained from an X.509 certificate can be trusted, whereas a client name from the `client_metadata` parameter cannot. | ||
|
|
||
| It is recommended to use `client_metadata` parameters only for client-supplied parameters that can be used untrusted, for example, a public key. | ||
|
|
||
| Ecosystems can use other ecosystem-specific trusted data sources as long as they conduct a proper trust evaluation. |
There was a problem hiding this comment.
I think my concern was in the conflict between
It is recommended (for Wallet implementers) to use
client_metadataparameters...
and
Other metadata parameters MUST be ignored unless a profile <...>'
If the recommendation is scoped to profiles that's better. But maybe something like:
Profiles MAY define additional
client_metadataparameters for client-supplied parameters that can be used untrusted, for example, a public key.
(Pretty sure that can be improved upon)
|
@dpostnikov can you respond to reviews here before the WG calls this week? |
|
@dpostnikov we need your response to the comments and requests for changes |
Co-authored-by: Frederik Krogsdal Jacobsen <fkj@users.noreply.github.com> Co-authored-by: Joseph Heenan <joseph@heenan.me.uk> Co-authored-by: Lukasz Jaromin <lukasz.jaromin@raidiam.com>
dpostnikov
left a comment
dpostnikov
left a comment
There was a problem hiding this comment.
Apply suggestions from code review
Co-authored-by: Frederik Krogsdal Jacobsen fkj@users.noreply.github.com
Co-authored-by: Joseph Heenan joseph@heenan.me.uk
Co-authored-by: Lukasz Jaromin lukasz.jaromin@raidiam.com
|
@dpostnikov looks like there are changes for you to review |
Co-authored-by: Joseph Heenan <joseph@authlete.com>
Co-authored-by: Joseph Heenan <joseph@authlete.com>
Co-authored-by: Joseph Heenan <joseph@authlete.com>
Co-authored-by: Joseph Heenan <joseph@authlete.com>
Co-authored-by: Joseph Heenan <joseph@authlete.com>
Co-authored-by: Joseph Heenan <joseph@authlete.com>
Co-authored-by: Joseph Heenan <joseph@authlete.com>
|
Discussed today. @jogu will review this week. |
10 participants
resolves #670