Re: [OAUTH-WG] Benjamin Kaduk's Yes on draft-ietf-oauth-par-08: (with COMMENT)

[Brian Campbell <[e-mail]>]

Hi Ben,

I was a little over-optimistic with the "next week" estimate (apologies for
that) but a few days later and just under the wire for the impending I-D
submission cut-off, I've pushed a -09 revision addressing your feedback
here as well as some other comments received in IESG evaluation.

https://datatracker.ietf.org/doc/draft-ietf-oauth-par/
https://www.ietf.org/archive/id/draft-ietf-oauth-par-09.html
https://www.ietf.org/rfcdiff?difftype=--hwdiff&url2=draft-ietf-oauth-par-09.txt
or https://www.ietf.org/rfcdiff?url2=draft-ietf-oauth-par-09.txt


Thanks,
Brian

On Wed, Jun 30, 2021 at 7:48 PM Benjamin Kaduk <[e-mail]> wrote:

> Hi Brian,
>
> On Wed, Jun 30, 2021 at 01:10:37PM -0600, Brian Campbell wrote:
> > Thanks Ben, I've endeavored to respond to your comments inline below.
> And I
> > will/am working on related document updates but have some time off
> planned
> > around the 4th of July so likely won't have a new revision out until next
> > week sometime.
>
> Understandable!
>
> > On Mon, Jun 28, 2021 at 1:45 PM Benjamin Kaduk via Datatracker <
> > [e-mail]> wrote:
> >
> > > Benjamin Kaduk has entered the following ballot position for
> > > draft-ietf-oauth-par-08: Yes
> > >
> > > The document, along with other ballot positions, can be found here:
> > > https://datatracker.ietf.org/doc/draft-ietf-oauth-par/
> > >
> > >
> > > ----------------------------------------------------------------------
> > > COMMENT:
> > > ----------------------------------------------------------------------
> > >
> > > I made a github PR with some editorial suggestions, at
> > > https://github.com/oauthstuff/draft-oauth-par/pull/70 .
> > >
> >
> > Merged on Monday. Thank you.
> >
> >
> > Section 1
> > >
> > >                                        Those apps typically invoke a
> > >    custom tab with an URL that is translated into a GET request.  Using
> > >    "POST" would require the app to first open a web page under its
> > >    control in the custom tab that in turn would initiate the form
> "POST"
> > >    towards the authorization server.  PAR is simpler to use and has
> > >    additional security benefits as described above.
> > >
> > > This description leaves me with a feeling that I only have an
> incomplete
> > > picture of why POST is "prohibitively difficult" to use with mobile
> > > apps.  It seems like the setup is describing a scenario where the
> > > authorization logic is operating inside a framework or environment
> > > provided by some other entity that is unwilling or unable to change the
> > > framework to accomodate new OAuth behavior.  Is this other entity the
> > > author of the mobile app, or an app framework, or somethint else?
> > > Having a bit more description of the multiple entities involved and
> > > which one is trying to control the specific mechanics of the
> > > authorization request would make this depiction a more compelling
> > > argument that POST is unusable.
> > >
> >
> > I think perhaps myself and co-authors are so familiar with it that it
> seems
> > almost self-evident. This is the second ballot comment on this paragraph,
> > however, so it seems some more work on the text is warranted. I'll try to
> > elaborate and better explain why it isn't particularly viable for a
> native
> > mobile app to cause the user's browser on the device to make a POST
> request
> > (with sizable body content).
>
> I think that even mentioning that there are ecosystem pressures to use the
> native (OS?) browser for an otherwise independent app (that acts as OAuth
> client) would be helpful.  The fact that the interface between native apps
> and the system browser has limitations should be fairly easy to accept as a
> fact of life, whereas the current text invites the reader to ask "but can't
> you just make a POST request yourself?  Why is that so hard?".
>
> >
> >
> >
> > > Section 1.1
> > >
> > >      POST /as/par HTTP/1.1
> > >      Host: as.example.com
> > >      Content-Type: application/x-www-form-urlencoded
> > >      Authorization: Basic czZCaGRSa3F0Mzo3RmpmcDBaQnIxS3REUmJuZlZkbUl3
> > >
> > > My personal preference would be to have examples that just want a
> > > generic HTTP authentication mechanism to use something stronger than
> > > Basic, though I don't think this is something that I can insist on, at
> > > present.  (Applies throughout.)
> > >
> >
> > I wavered on this very point when putting together the examples but
> > eventually took the easy route and went with Basic (aka
> > client_secret_basic). The three such examples just want to show some form
> > of OAuth client authentication happening. I think JWT based client
> > assertion authentication using asymmetric keys
> > https://datatracker.ietf.org/doc/html/rfc7523#section-2.2 (aka
> > private_key_jwt) is the only stronger mechanism that would show well in
> > examples (MTLS/RFC8705 is hard to show in a meaningful way in an HTTP
> > example). I could update the examples to use RFC7523
> > client_assertion/client_assertion_type parameter auth?
>
> That would address my concern, but I understand that it's a nontrivial
> amount of work and definitely am not insisting on it.
>
> >
> >
> >
> > >      HTTP/1.1 201 Created
> > >      Cache-Control: no-cache, no-store
> > >      Content-Type: application/json
> > >
> > >      {
> > >        "request_uri": "urn:example:bwc4JK-ESC0w8acc191e-Y1LTC2",
> > >        "expires_in": 90
> > >      }
> > >
> > > The "request_uri" element's semantics feels like a very natural fit for
> > > the native HTTP "Location" response header field (vs requiring a
> > > specific element in the JSON body of the response).  I am less sure
> that
> > > there's a native HTTP element for the expiration time, as the HTTP
> > > timeouts tend to be associated with HTTP caching vs the actual lifetime
> > > of the resource.
> > >
> >
> > The semantics feel subtly different in that the "request_uri" value is
> not
> > used as-is by the client but rather as a query string parameter in the
> > construction of the full authorization request URI to which it will
> direct
> > the user's browser. I was also unsure whether the "Location" response
> > header field had to have a resolvable URI value (I didn't find text
> saying
> > so explicitly but it seems somewhat logical). Also, to me anyway, it made
> > sense and was more consistent to keep the OAuth parameters at the same
> > layer they show up in other OAuth exchanges. And, as you note, there
> isn't
> > an obvious HTTP field to convey the expiration time and putting some
> > protocol data in the headers and some in the JSON content payload seemed
> > awkward.
> >
> > That's some of the reasoning behind not using the "Location" header.
> > Avoiding a breaking change at this stage is another reason.
>
> Indeed.  And to be honest, the last one is pretty compelling.
> (I am not an HTTP expert and should not opine on whether the Location
> response value has to be resolvable or not.  I do think that "the location
> of the resource that was created" is a good fit even if the resource itself
> is not accessed directly by the entity that receives the HTTP response with
> the location header field -- a URI is a way to identify a resource, after
> all, and that identifier can be passed around to other entities.)
>
> >
> >
> > > Section 2
> > >
> > > It feels a little unfortunate that we have to reuse the metadata
> > > parameters relating to the token_endpoint_auth_method for PAR, but
> > > creating new metadata parameters that are basically always going to
> hold
> > > the same values is probably worse...
> > >
> >
> > I believe the reuse is exactly what's wanted here. The unfortunate part
> is
> > in the naming that, in retrospect, is overly specific.
>
> Er, yes, exactly that.
>
> > Section 2.1
> > >
> > >                     Any token endpoint parameters that are not related
> > >    to client authentication have no defined meaning for a pushed
> > >    authorization request.  [...]
> > >
> > > I suppose that in most cases, whether or not a token endpoint parameter
> > > is related to client authentication should be fairly unambiguous ...
> but
> > > the registry at
> > >
> > >
> https://www.iana.org/assignments/oauth-parameters/oauth-parameters.xhtml#parameters
> > > has a dedicated column for "parameter usage location".  It seems like
> it
> > > would be fairly straightforward to add "pushed authorization request"
> as
> > > an additional possible value there and remove the ambiguity.  We could
> > > enumerate the existing parameters that are applicable and update the
> > > references for the registry to include this document.
> > >
> >
> > I believe the text describes the situation pretty well and resolves the
> > ambiguities sufficient for interoperable implementations/deployments in
> > practice. A registry update is possible but would be a significant change
> > with ongoing maintenance needs that aren't readily apparent while not
> > providing practical value.
>
> I won't insist on anything; the benefit would mostly be "ideological
> cleanliness" which is not very tangible.
>
> >
> >
> > > Section 2.1
> > >
> > >    The authorization server MUST process the request as follows:
> > >
> > > We often see descriptions along the lines of "this is not intended to
> > > constrain implementation techniques; any procedure that results in an
> > > equivalent outcome is permissible".  This procedure is simple enough
> > > that we may not need to have concerns of that nature, though.
> > >
> > > Section 2.2
> > >
> > >    *  "request_uri" : The request URI corresponding to the
> authorization
> > >       request posted.  This URI is used as reference to the respective
> > >       request data in the subsequent authorization request only.  [...]
> > >
> > > Would it be appropriate to use the phrase "single-use" in this
> > > description?
> > >
> >
> > Yes, I think so. I'll add it.
> >
> >
> >
> > > Section 2.4
> > >
> > >    It is at the discretion of the authorization server to apply
> > >    restrictions on supplied "redirect_uri" values, e.g. the
> > >    authorization server MAY require a certain URI prefix or allow only
> a
> > >    query parameter to vary at runtime.
> > >
> > > Is this expected to be discoverable by the client?  Would there be a
> > > particular error code used, for example?
> > >
> >
> > It's not so much intended to be discoverable but more to just note the
> > possible exception to the tightening restrictions on redirect_uri values
> in
> > the security BCP and OAuth 2.1. It's allowance/utilization will likely be
> > determined for a whole deployment or ecosystem, which will also dictate
> the
> > specific URI allowances and how clients are configured or registered with
> > such info.
>
> Okay.
>
> >
> > Section 3
> > >
> > >                                    When the "application/x-www-form-
> > >    urlencoded" HTTP entity-body "request" parameter is used, the
> request
> > >    object MUST contain all the authorization request parameters as
> > >    claims of the JWT.  Additional request parameters as required by the
> > >    given client authentication method are to be included as
> > >    'application/x-www-form-urlencoded' parameters in the HTTP request
> > >    entity-body [...]
> > >
> > > I might flip this around and start off with "authentication request
> > > parameters required by a given client authentication method are
> included
> > > in the application/x-www-form-urlencoded request directly, and are the
> > > only parameters other than 'request' in the form body.  All other
> > > request parameters, i.e., those pertaining to the authorization request
> > > itself, MUST appear as claims of the JWT representing the authorization
> > > request".  The current formulation kind of hides the specifics of how
> to
> > > determine which parameter goes where -- the spec for the client
> > > authentication method in use is the authoritative one, IIUC.
> > >
> >
> > That makes sense, I'll flip it around like you suggest (modulo a few
> > wording tweaks).
> >
> >
> >
> > >    1.  If applicable, decrypt the request object as specified in JAR
> > >        [I-D.ietf-oauth-jwsreq], section 6.1.
> > >
> > >    2.  Validate the request object signature as specified in JAR
> > >        [I-D.ietf-oauth-jwsreq], section 6.2.
> > >
> > > Do we want to say anything more about error handling if decryption or
> > > signature validation fails?
> > >
> >
> > The referenced sections in JAR indicate "invalid_request_object" as the
> > error code, which seems sufficient and doesn't warrant repeating here.
>
> Okay, makes sense.
>
> >
> > Section 4
> > >
> > > I think we should explicitly state that expired "request_uri"s are
> > > rejected as invalid.
> > >
> >
> > Sure, I can add that.
> >
> >
> > Section 5
> > >
> > >    Note that the presence of "pushed_authorization_request_endpoint" is
> > >    sufficient for a client to determine that it may use the pushed
> > >    authorization request flow.  A "request_uri" value obtained from the
> > >    PAR endpoint is usable at the authorization endpoint regardless of
> > >    other authorization server metadata such as
> > >    "request_uri_parameter_supported" or
> > >    "require_request_uri_registration".
> > >
> > > Since this is the only mention of it in this document, and it's an OIDC
> > > parameter, maybe we should provide a reference for
> > > "require_request_uri_registration"?
> > >
> >
> > Sure, I'll add an informational reference to
> > https://openid.net/specs/openid-connect-discovery-1_0.html
> >
> >
> >
> > > Section 7.1
> > >
> > >    An attacker could attempt to guess and replay a valid request URI
> > >    value and try to impersonate the respective client.  [...]
> > >
> > > Just to confirm my analysis here: an attacker that does guess a valid
> > > request URI value would probably also need to guess the corresponding
> > > "client_id", and if the request used PKCE, would also need to guess the
> > > PKCE value in order to successfully obtain a token response?  I'm not
> > > sure whether any of that is worth mentioning or not here, though.
> > >
> >
> >  While true, it's immaterial when the URI value itself is not guessable.
> So
> > I don't think it needs mentioning.
>
> Okay.
>
> >
> >
> > > Section 10.3
> > >
> > > Section 12
> > >
> > > Why is RFC 6749 not a normative reference?
> > >
> >
> > Just a mistake due to a tooling misunderstanding - see the reply to the
> > same issue raised by Zaheduzzaman Sarker
> > https://mailarchive.ietf.org/arch/msg/oauth/Mx_6jDnlE4vjn4UvywNJ5QV4Y_M/
> >
> >
> > NITS
> > >
> > > Section 2
> > >
> > >    The pushed authorization request endpoint is an HTTP API at the
> > >    authorization server that accepts HTTP "POST" requests with
> > >    parameters in the HTTP request entity-body using the "application/x-
> > >    www-form-urlencoded" format with a character encoding of UTF-8 as
> > >
> > > I'm not sure where "entity-body" is defined as the name for the request
> > > body in this case; draft-ietf-httpbis-semantics would seem to just have
> > > us refer to this as the "message body".
> > >
> >
> > Sure, I'll change it and the few other occurrences to "message body"
> (FWIW
> > I think "entity-body" came from text borrowed from RFC 6749, which in
> turn
> > used the term from RFC 2616).
> >
> >
> > Section 2.1
> > >
> > >    The client adds the parameters in "x-www-form-urlencoded" format
> with
> > >
> > > I'd suggest a different word than "adds"; these paratmers are the
> entire
> > > body, not appended to some earlier structure.
> > >
> >
> >  Okay. Maybe "constructs the request with parameters" or similar ...
> >
> > > Some parameters of the example request (e.g., "state") are duplicated
> > > across the multiple examples in the document.  For those which are
> > > typically single-use in practice, we might consider using different
> > > values in the different examples (including in the JWT payload of the
> > > example request in Section 3).
> > >
> >
> > The JWT payload of the example request in Section 3 was kinda sorta meant
> > to show what the same request from an earlier example in 2.1 would look
> > like in JWT request form. So I might keep that. But in general the
>
> If you do, please note that it's intended to match up!
>
> > suggestion makes sense and I'll take a pass through the examples to try
> and
> > ensure things that should be unique are unique.
>
> Thanks!
>
> > Section 7.4
> > >
> > > The section heading is "Client Policy Change", which to me indicates a
> > > change in the policy of the client, that is, what the client requires.
> > > The text of the section indicates that this is discussing the AS's
> > > per-client policy, though, so I wonder if there is a better phrasing
> > > that could be used.
> > >
> >
> >  Maybe configuration rather than policy? I dunno. I'm tempted to just
> > remove 7.4
>
> I'd prefer leaving it alone over removing it, if we can't find any wordings
> that we like better than what's there.
>
> Thanks for all the responses and updates!
>
> -Ben
>

-- 
_CONFIDENTIALITY NOTICE: This email may contain confidential and privileged 
material for the sole use of the intended recipient(s). Any review, use, 
distribution or disclosure by others is strictly prohibited.  If you have 
received this communication in error, please notify the sender immediately 
by e-mail and delete the message and any file attachments from your 
computer. Thank you._