Re: [OAUTH-WG] some WGLC comments on draft-ietf-oauth-jwsreq-06

[Justin Richer <[e-mail]>]

The errata/update version of Connect’s registration spec need to register a bunch of stuff in the Dyn-Reg IANA registry, but I don’t think that’s been done yet.

 — Justin

> On Oct 27, 2015, at 3:41 PM, Brian Campbell <[e-mail]> wrote:
> 
> Sakimura-san, Senior Bradley, and distinguished members of the OAUTH WG,
> 
> I re-read draft-ietf-oauth-jwsreq (-06 anyway, it'd been a while) for WGLC and, while I feel it's in pretty good shape, I did have a few comments on the draft. Those are listed below in roughly the order I came across things while reading the document. 
> 
> To my eye, "oauth-jar" looks a bit awkward. I'd suggest changing the abbrev to something like 
> "OAuth JAR".
> 
> From the wording in Section 3 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-3>, it is unclear whether the Request Object can be a JWE only or if a JWS is always used (with alg:none for unsigned) and is nested within a JWE when encryption but not singing is needed. To my reading there is text that suggest both cases. Which is it? I think some clarification is needed around this. Section 5.1 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-5.1> doesn't help me as I'm unsure if "unsigned (plaintext) Request Object" is an out-of-date usage of the old way to refer to the "none" JWS alg or is intended to mean the straight up JSON of the request object parameters. 
> 
> Also in Section 3 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-3>, the sentence 'The Authorization Request Object MAY alternatively be sent by reference using the "request_uri" parameter.' reads a little awkward because the other alternative isn't explicit discussed anywhere nearby. Perhaps something like "The Authorization Request Object may be sent by value as described in section 4.1 or by reference as described in section 4.2."?
> 
> Also in Section 3 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-3>, the sentence "If a required parameter is not present in neither the query parameter nor the Request Object, it forms a malformed request." made my brain hurt. Maybe reword to something like, "If a required parameter is missing from both the query parameters and the Request Object, the request is considered malformed."?
> 
> Also in Section 3 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-3>, '... the Authorization Request Object SHOULD contain the Claims "iss" (issuer) and "aud" (audience) as members ...', however, that will produce a parameter name conflict with the "aud" parameter from OAuth 2.0 Proof-of-Possession: Authorization Server to Client Key Distribution. <https://tools.ietf.org/html/draft-ietf-oauth-pop-key-distribution-02> Seems like draft-ietf-oauth-pop-key-distribution will need to change its parameter name (aud in JWT is pretty well established). And shouldn't draft-ietf-oauth-jwsreq register some of the JWT's Registered Claim Names <http://tools.ietf.org/html/rfc7519#section-4.1> (at least iss and aud but maybe exp and others) as authorization request OAuth parameters <https://www.iana.org/assignments/oauth-parameters/oauth-parameters.xhtml#parameters>?
> 
> Also in Section 3 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-3>, I personally feel like the "extension variables" detract from the example being able to easily convey the concept. I would suggest using only 'vanilla' OAuth parameters. Or, at the very least, removing the "claims" stuff, which doubles the space used by the example and is a very OpenID Connect specific thing. This applies to the examples throughout the draft including those that have the encoded JWT Request Object. 
> 
> In Section 4 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-4>, the whole "The client constructs the authorization request URI by ..." followed by the list of parameters is somewhat awkward to me - especially with the "REQUIRED"s. Why list "state" here and none of the others here? I think I know why (you expect state to vary on each request but not others) but a lot of inferring needs to happen from the reader.  For the extension defined in this document, one uses either request_uri or request (but not both, right? I'm not sure it's explicitly stated anywhere) and whatever other parameters are needed in the given situation. Maybe just some text towards that end rather than the list format?
> 
> Also in Section 4 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-4>, the request_uri in the example is a lot like the URI that's used for redirect_uri in a lot of places (the /cb path, I think, was intended to short for callback). I mean, it *could* happen but the example is more confusing than needs to be. It's also too long/wide for the page - add some line breaks, which are for display purposes only. Perhaps move this example to 4.2(.1) somewhere and match the value with the value shown there? 
> 
> Also in Section 4 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-4>, the sentence that starts with 'When the "request" parameter is used...' suggests that the text about JWT parameters superseding others is only applicable to the pass by value method but it applies to both. Perhaps change '"request" parameter' to 'Request Object'?
> 
> The second paragraph of Section 4.2.1 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-4.2.1> talks about "requested values for Claims" which is an OpenID Connect thing that isn't really applicable to this draft. Can this paragraph be dropped or made more general to be about any potentially sensitive or PII data?
> 
> Also in Section 4.2.1 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-4.2.1>, is "... at a URL the Server can access... " - assume the "Authorization Sever"? 
> 
> Isn't it a bit odd in Section 5.2 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-5.2> to talk about the "request_object_signing_alg" which is defined in OpenID Connect Dynamic Client Registration <http://openid.net/specs/openid-connect-registration-1_0.html> (which is not referenced) and kinda mentioned in OpenID Connect Core <http://openid.net/specs/openid-connect-core-1_0.html> (informative referenced)? I don't know that it belongs here? Or if it does, what about request_object_encryption_alg and request_object_encryption_enc? I suppose related to that is the question of if/how to use the client_secret for HMAC and symmetric encryption algorithms and if that needs to be discussed here? Indicating public key(s) too for that matter. Seems like all the metadata/registration stuff should be in here. Or it should all be out of scope. But just the request_object_signing_alg seems odd to me.
> 
> Section 6 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-6> says that "... this document defines additional error values ..." but those were previously defined in 3.1.2.6 of OpenID Connect Core <http://openid.net/specs/openid-connect-core-1_0.html#AuthError>. Maybe just change the wording to reflect the real state of things?   
> 
> Section 7 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-7> says that "The request_object_signing_alg OAuth Dynamic Client Registration Metadata is pending registration by OpenID Connect Dynamic Registration specification." But is that true? The registry <https://www.iana.org/assignments/oauth-parameters/oauth-parameters.xhtml#client-metadata> doesn't have it and Connect's Registration <http://openid.net/specs/openid-connect-registration-1_0.html#IANA> "makes no requests of IANA". 
> 
> As I'm sure reading this has been a ton of fun for the editors, I'm sure you will be happy to please add me to the Acknowledgements Section <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-9> ;) 
> 
> 
> 
> 
> _______________________________________________
> OAuth mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/oauth