OAuth 2.0 Token Exchange
draft-ietf-oauth-token-exchange-19

Note: This ballot was opened for revision 16 and is now closed.

Roman Danyliw Yes

Comment (2019-07-18 for -18)
A few nits:

** Section 2 and 4.  Reference the figure numbers in the text  when introducing an example.

** Figure 6.  Invalid JSON.  line ‘"sub":"https://service77.example.com”, ‘ should not end with a comma

Benjamin Kaduk (was Discuss) Yes

Comment (2019-07-05 for -17)
I'm balloting Yes; this document is solid and well-written.  I do have a
few additional (largely editorial) suggestions and a question or two,
though.

Section 2.1

   The client makes a token exchange request to the token endpoint with
   an extension grant type using the HTTP "POST" method and including
   the following parameters using the "application/x-www-form-
   urlencoded" format with a character encoding of UTF-8 in the HTTP
   request entity-body as described in Appendix B of RFC6749 [RFC6749].

This is a really long sentence.  I see how it got that way, and the RFC
Editor staff will probably have some thoughts on how to reword it, but
if you happen to have thoughts as well, feel free to have at it.

Section 2.2.1

   expires_in
      RECOMMENDED.  The validity lifetime, in seconds, of the token
      issued by the authorization server.  Oftentimes the client will
      not have the inclination or capability to inspect the content of
      the token and this parameter provides a consistent and token type
      agnostic indication of how long the token can be expected to be
      valid.  For example, the value 1800 denotes that the token will

nit: hyphenate "token-type-agnostic".

Section 4.4

Refresh my memory: did we already have a discussion about may_act as an
object vs. an array of objects?

Section 5

I'd consider also mentioning/linking the OAuth 2.0 security
considerations -- the fact that the STS is colocated with the token
endpoint takes care of ensuring a lot of its security properties.

Section 7

It's common (but not required, since it will not be relevant upon
publication as an RFC) to note that the indicated values are reflected
in early allocations from the indicated IANA registries.  In this case
I'd say "don't bother"...

Appendix B

Uh-oh, now we are up to five security ADs that have been around for this
document...

(Eric Rescorla) Yes

Deborah Brungard No Objection

(Ben Campbell) No Objection

Alissa Cooper (was Discuss) No Objection

Comment (2019-07-08 for -18)
Thanks for addressing my DISCUSS and COMMENT. Apologies for the delay on my part.

(Spencer Dawkins) No Objection

Suresh Krishnan No Objection

Barry Leiba No Objection

Comment (2019-07-18 for -18)
I have comments below, a couple of which might have been DISCUSS except that there have been enough eyes on this and enough DISCUSSes already, and I trust the authors and responsible AD to do the right thing.  So I’ve divided the comments between “higher importance” and “purely editorial”.

Of higher importance:

— Section 1.1 —
Given the extensive discussion of impersonation here, what strikes me as missing is pointing out that impersonation here is still controlled, that “A is B” but only to the extent that’s allowed by the token.  First, it might be limited by number of instances (one transaction only), by time of day (only for 10 minutes), and by scope (in regard to B’s address book, but not B’s email).  Second, there is accountability: audit information still shows that the token authorized acting as B.  Is that not worth clarifying?

— Section 8.2 —
RFC 8174 needs to be normative, along with 2119.

— Section 2.2.2 —

   If the authorization server is unwilling or unable to issue a token
   for all the target services indicated by the "resource" or "audience"
   parameters, the "invalid_target" error code SHOULD be used in the
   error response.

I always trip when I read “all” in a context like this.  I think you mean “any”, because you have “a token”.  You could arguably make it “tokens” and leave “all”, but I think changing to “any” is clearer:

NEW
   If the authorization server is unwilling or unable to issue a token
   for any target service indicated by the "resource" or "audience"
   parameters, the "invalid_target" error code SHOULD be used in the
   error response and no tokens are returned.
END

The danger with “all” is having a reader interpret the error as only occurring when the server fails *all* services, and thinking that failing one out of three still constitutes success.  I have seen this be an issue often (not with OAuth, but in general).  If you want to be absolutely clear you could even add to the end, “A request is successful only when all requested tokens are issued.”

— Section 5 —

   In addition, both delegation and impersonation introduce unique
   security issues.  Any time one principal is delegated the rights of
   another principal, the potential for abuse is a concern.  The use of
   the "scope" claim is suggested to mitigate potential for such abuse,
   as it restricts the contexts in which the delegated rights can be
   exercised.

I’m ambivalent here: is it worth also mentioning limiting the time a token is valid and possibly make it a one-time-use token?  Or is it that that’s adequately covered in the other references and shouldn’t be repeated here?

Also, is it worth referring (not copying) here to the advice in section 2.1 about the importance of authentication?

— Section 6 —
Should “TLS” here have a citation and normative reference?

=====

Purely editorial:

— Section 1 —

   An OAuth resource server, for example, might assume
   the role of the client during token exchange in order to trade an
   access token, which it received in a protected resource request, for
   a new token that is appropriate to include in a call to a backend
   service.

A suggestion: I think this would work better using a restrictive clause, rather than a non-restrictive one.

NEW
   An OAuth resource server, for example, might assume
   the role of the client during token exchange in order to trade an
   access token that it received in a protected resource request for
   a new token that is appropriate to include in a call to a backend
   service.
END

   The scope of this specification is limited to the definition of a
   basic request and response protocol for an STS-style token exchange

There should be hyphens in “request-and-reaponse protocol” (compound modifier).

— Sections 4.1 and 4.4 —

Section 4.1 says this:
   Consequently, non-
   identity claims (e.g., "exp", "nbf", and "aud") are not meaningful
   when used within an "act" claim, and therefore must not be used.

Section 4.4 says this:
   Consequently,
   claims such as "exp", "nbf", and "aud" are not meaningful when used
   within a "may_act" claim, and therefore should not be used.

I agree that neither of these should be BCP 14 key words, but I still think that being consistent is important, and I urge you to make them the same: both “must not be used” or both “should not be used” (or perhaps both “are not used”).

(I did not review the appendices.)

(Terry Manderson) No Objection

Alexey Melnikov No Objection

Alvaro Retana No Objection

Adam Roach (was Discuss) No Objection

Comment (2019-07-05 for -17)
Thanks for addressing my discuss and comment points.

Martin Vigoureux No Objection