Clarify the security implications of using query parameters in client_id to dynamically generate metadata documents

[birdhalfbaked]

While reading the draft, it currently seems to allow broad use of query parameters in the client_id for sending information that a guides the use of metadata that should be retrieved in the flows.

Client identifier URLs MAY contain a query string component and MAY contain a port.
This specification places no restrictions on what URL is used as a client identifier. A short URL is RECOMMENDED, since the URL may be displayed to the end user in the authorization interface or in management interfaces. Usage of a stable URL that does not frequently change for the client is also RECOMMENDED.

(emphasis mine)

We already see some implementations using this to facilitate development by passing OAuth metadata (e.g. scope and redirect_uris) such as with ATProto's use of OAuth in the localhost exception case, and as written this may mistakenly encourage the use of dynamically-generated metadata documents from user-given inputs even in non-development use cases which can undermine the mechanisms herein.

e.g. client_id: https://someurl.com?scope=scope1%20scope2 is allowed, but should be strongly discouraged outside of local development contexts.

This discouragement could fit well in the additional Security Considerations I am thinking. Something along the lines "Authorization servers SHOULD reject the use of query params to dynamically generate metadata outside of development contexts where other methods of generating metadata documents are not practical."

Wording probably needs work :P Also reinforces some good ideas I've seen around setting up a service for development metadata documents like in #12

[ThisIsMissEm]

I would probably say that we should discourage the use of query string arguments in the client_id document URL. Instead, services that allow creating a Client ID Metadata Document should instead generate a stable identifier for that document. e.g., instead of https://client_id.example.com/client.json?scopes=read&client_name=example, it would be better to have these registered with that service as: https://client_id.example.com/WtzJCPeDSGs4FsKPlNy3.json where WtzJCPeDSGs4FsKPlNy3 identifies the record that stores the metadata.

This does make such a service stateful, rather than stateless, but has better security properties, in my opinion.

I would also think it'd be an idea to introduce a new property in the OAuth Dynamic Client Registration Metadata IANA registry of client_expires_at which is the timestamp at which the entire client should be deemed no longer valid (such to allow cache purging of development clients), this is necessary as Client ID Metadata Documents don't have a client secret, therefore cannot use client_secret_expires_at.

[aaronpk]

Couldn't we use http cache headers to let the client indicate its intended lifetime of the document? Obvs the AS can cache or not cache as long as it wants, but this is an existing mechanism that could be used to indicate the doc lifetime. Also would be worth checking if AS metadata or RS metadata have any mention of http cache headers.

[ThisIsMissEm]

I think this could be reasonable, but my concern is that the client metadata document could be considered "expired" before the AS decides to revalidate its cache. That is, if the client metadata document cache header says "cache me for a month" and the AS goes "nah, I cache things for a 6 months", then we'll hit into issues where the AS will keep hold of a client metadata document that the provider of the document thinks is expired/no-longer valid.

[matthieusieben]

I like @birdhalfbaked 's proposal to add the following into the security considerations:

"Authorization servers SHOULD reject the use of query params to dynamically generate metadata outside of development contexts where other methods of generating metadata documents are not practical."

I don't think Atproto will get rid of the localhost exception just yet but I can see us adding a restriction forbidding use of query strings for non "loopback clients".

[ThisIsMissEm]

I'd recommend going the route of having a service that registers and serves documents, rather than generating them through query parameters. That way it's just a bit harder to create multiple documents to confuse just by changing URL parameters.

E.g., by storing that information you can perform validation and moderation on it as necessary.

That's why I've gone the route of removing query parameters from the client metadata document URIs.

[ThisIsMissEm]

@aaronpk yeah, I think we can use the Expires header for this: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Expires

Perhaps we just need particular language around this? "A server responding to a request for a client metadata document MAY return the Expires header, indicating that the given client metadata document expires after a given time, at which point the client should be considered revoked if re-requesting the metadata document URI fails"

[ThisIsMissEm]

Opened #29 with the above.

[ThisIsMissEm]

@matthieusieben I would say that what Bluesky is doing with its localhost client_id's is side-stepping this internet draft completely. It is its own thing entirely. This internet draft relies on the ability for the authorization server to request and validate the client_id as being a client metadata document, what you're doing isn't that.

[matthieusieben]

I think you're right. Considering them separate things makes a lot of sense.

[ThisIsMissEm]

I think this can be closed as complete based on the changes in #24.