Subject: Re: [OAUTH-WG] Benjamin Kaduk's Discuss on draft-ietf-oauth-token-exchange-16: (with DISCUSS and COMMENT)

From: Brian Campbell <[email-masked]>
Date: Tue, 4 Dec 2018 14:54:36 -0700

I apologize for the slow response, Ben. I was on vacation with my family
around the Thanksgiving holiday when the ballot position came in. And even
on returning and starting to work on it, there's an awful lot here to get
through and this kind of thing is very time consuming for me. But thank you
for the review - I've attempted to reply, as best I can, to your
comments/questions inline below.

On Wed, Nov 21, 2018 at 6:43 AM Benjamin Kaduk <[email-masked]> wrote:

> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-oauth-token-exchange-16: Discuss
>
> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
>
>
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-oauth-token-exchange/
>
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
>
> It looks like allocations in the OAuth URIs registry are merely
> "Specification Required", so we should not have the expectation of WG
> exclusivity and thus are squatting on unallocated values here.
> Process-wise, that's not great and the IESG shouldn't approve a document
> that is squatting on codepoints.
>

In retrospect, RFC 6755 <https://tools.ietf.org/html/rfc6755> should have
used "RFC Required" for the OAuth URIs registry. But that was 2012 and 6755
was my first RFC and I was even more clueless back then than I am now. And
what's done is done.

In practice the only entries that have been made to the registry have been
from RFCs and the only prospective entries (that I'm aware of anyway) are
in documents that are on track to be RFCs. This document has followed the
same procedures with respect to the OAuth URIs registrations as those other
documents.

Having said all that, I'm unsure what action you are expecting to see as a
result of this DISCUSS comment?

> why do we allow both client authentication (i.e., using an
> actor token) and a distinct actor_token request parameter?  Is it
> supposed to be the case that the actor_token parameter is only supplied
> for delegation flows?  If so, that needs to be made explicit in the
> document.
>

Client authentication is inherited from RFC 6749. It's optional but can be
useful for deployments that want to "lock down" who can invoke token
exchange.

The actor_token and subject_token are inputs into the exchange. They have
to be validated but that is not exactly authentication per se. Honestly, I
struggle with the wording and how to describe it all (here and in not
dissimilar contexts of the authorization grants of RFC 7522
<https://tools.ietf.org/html/rfc7522> and 7523
<https://tools.ietf.org/html/rfc7523>). I've done the best I can in the
document. If you can propose some text that you think would make things
more clear or explicit, that'd help progress this. But I honestly don't
know what to add or change here.

>
> Are the privacy considerations (e.g., risk of a tailed per-request
> error_uri) relating to the use of error_uri discussed in some other
> document that we can refer to from this document's security
> considerations?  (I say a bit more about this in my COMMENT.)
>

I am not aware of any document with such considerations and I've searched
the likely suspects of RFC 6749 and RFC 6819 but don't find anything.

The error_uri token endpoint response parameter was defined in the original
OAuth 2.0 framework document (RFC 6749) and any considerations around it
are applicable to considerably more than this document. It's also very
rarely used in practice as far as I know. I don't think that this document,
which is a narrow extension of a whole framework with a series of other
documents that use error_uri, is the appropriate place to add privacy or
security considerations about error_uri.  Perhaps
https://datatracker.ietf.org/doc/draft-ietf-oauth-security-topics/ would be
more appropriate in scope and content?

I could remove the one mention of error_uri in this document? It's usage
would still be possible/valid by virtue of this document being an extension
of RFC 6749 but, out of sight and out of mind, and this doc wouldn't then
encourage new usage of it anyway. While usage isn't really happening anyway.

>
> Section 2.1 has:
>    audience
>       OPTIONAL.  The logical name of the target service where the client
>       intends to use the requested security token.  This serves a
>       purpose similar to the "resource" parameter, but with the client
>       providing a logical name rather than a location.  Interpretation
>       of the name requires that the value be something that both the
>       client and the authorization server understand.  An OAuth client
>       identifier, a SAML entity identifier [OASIS.saml-core-2.0-os], an
>       OpenID Connect Issuer Identifier [OpenID.Core], or a URI are
>       examples of things that might be used as "audience" parameter
>       values.  [...]
>
> How does the STS know what type of identifier it is supposed
> to interpret the provided audience value as?
>

The STS will have policy and configuration for the target entities for
which it supports the issuance of tokens to in this flow, even if/when
those entities are different types of things. The STS will have to search
that set of things to find the right one for the given name. In theory I
suppose there's potential ambiguity or even name collision. But in practice
(as it is the STS that ultimately decides the names it supports and can
service) I don't believe there is an actual issue.

>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> The document could perhaps benefit from greater clarity as to whether
> "security token"s refer to inputs, outputs, or both, of the token
> endpoint (for the interactions defined in this specification).
>

I have been aware of the potential need here and endeavored to be clear
about it throughout the document without being overly repetitive or wordy.
I will take another pass through the text and look for opportunities to
further clarity. But if there are specific points in the doc that you
believe need attention, please point them out so I can be sure they get
addressed.

>
> Section 1
>
>                                                             The OAuth
>    2.0 Authorization Framework [RFC6749] and OAuth 2.0 Bearer Tokens
>    [RFC6750] have emerged as popular standards for authorizing third-
>    party applications access to HTTP and RESTful resources.  [...]
>
> nit: possessive "applications'"
>

Will fix.

>
> Section 1.1
>
> This section really jumps in quickly with no lead-in to why we would
> care or transition from the introduction.  I suggest:
>
>   One common use case for an STS (as alluded to in the previous section)
>   is to allow a resource server A to make calls to a backend service C on
>   behalf of the requesting user B.  Depending on the local site policy and
>   authorization infrastructure, it may be desireable for A to use its own
>   credentials to access C along with an annotation of some form that A is
>   acting on behalf of B ("delegation"), or for A to be granted a limited
> access
>   credential to C but that continues to identify B as the authorized
>   entity ("imperesonation").  Delegation and impersonation can be useful
>   concepts in other scenarios involving multiple participants as well.
>

Documents written over time with more than one author sometimes bear the
scars of that process in disjoint transitions, which is the case here I
think.

You're suggestion nicely takes the edge off the transition and provides
context for it. Thanks, I'll add that text to the top of sec 1.1.

> Section 2.1
>
>                                                   For example, [RFC7523]
>    defines client authentication using JSON Web Tokens (JWTs) [JWT].
>
> Please clarify that these are still bearer tokens.
>

Okay.

>
>    The supported methods of client authentication and whether or not to
>    allow unauthenticated or unidentified clients are deployment
>    decisions that are at the discretion of the authorization server.
>
> It seems appropriate to note that omitting client authentication allows
> for a compromised token to be leveraged via an STS into other tokens by
> anyone possessing the compromised token, and thus that client
> authentication allows for additional authorization checks as to which
> entities are permitted to impersonate or receive delegations from other
> entities.
>

I'll add a note that says as much (borrowing heavily from your words,
thanks).

>
>    The client makes a token exchange request to the token endpoint with
>    an extension grant type by 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:
>
> Is there more to say than "just use UTF-8"; any normalization or
> canonicalization issues to consider?
>

Nope, no normalization or canonicalization at this layer.

Note that Adam Roach did raise a DISCUSS around citation for the media type
https://mailarchive.ietf.org/arch/msg/oauth/Q1K-T2VS3wrHW7lx2EiP58b_DYw ,
which might result in a change to the wording here but it's still
x-www-form-urlencoded with UTF-8 as is better described in
https://tools.ietf.org/html/rfc6749#appendix-B

>
>    subject_token
>       REQUIRED.  A security token that represents the identity of the
>       party on behalf of whom the request is being made.  Typically, the
>       subject of this token will be the subject of the security token
>       issued in response to this request.
>
> nit: I think there's a subtle grammar mismatch here, where we start off
> by talking about a/the request and end up with "this request".
>

So changing that last "this request" to say "the request" would fix the
mismatch?

>    In processing the request, the authorization sever MUST validate the
>    subject token as appropriate for the indicated token type and, if the
>    actor token is present, also validate it according to its token type.
>
> I misread this the first time around; I'd suggest something like
> "perform the appropriate validation procedures for the indicated token
> type" (as opposed to just verifying that the presented token is a
> syntactically valid instance of the claimed type).
>

Makes sense, I'll update accordingly.

>
>    In the absence of one-time-use or other semantics specific to the
>    token type, the act of performing a token exchange has no impact on
>    the validity of the subject token or actor token.  Furthermore, the
>    validity of the subject token or actor token have no impact on the
>    validity of the issued token after the exchange has occurred.
>
> Do we really want this strong of a statement?  I suspect that in many
> environments propagating, e.g., expiration time to the exchanged
> credential may be desired.
>

The statement was not in any way intended to prohibit propagating
expiration time (or other criteria) to the exchanged credential. The
statement was added, best I can recall, in response to a question that came
up in a WG chair review asking if the input token(s) would somehow become
invalid once used as input to the exchange. Or if some later expiration or
other invalidation of the input token(s) would somehow invalidate the new
token.  The point of the statement in the doc was to try and say that there
is no inherit linkage effectual relationship between the tokens outside the
exchange event. There could be but that's not a general property of the STS
protocol a would be specific to a particular token type or deployment.

Does that make any more sense? Do you think the wording could/should be
adjusted?

>
> Section 2.2.1
>
>    token_type
> [...]
>       contents of the token itself.  Note that the meaning of this
>       parameter is different from the meaning of the "issued_token_type"
>       parameter, which declares the representation of the issued
>       security token; the term "token type" is typically used with this
>       meaning, as it is in all "*_token_type" parameters in this
>       specification. [...]
>
> Please disambiguate what "typically used with this meaning" means.
> Perhaps it would be even more clear to change this field's name to
> "token_access_token_type" to match the name of the registry?
>

The "token_type" parameter is defined in RFC 6749 for a successful response
from the token endpoint so this document effectively inherits it. The name
is already defined in RFC 6749 and not in scope for this document to
change.
I will update the wording to disambiguate "this meaning" per your request.

> Section 2.3
>
>    The following example demonstrates a hypothetical token exchange in
>    which an OAuth resource server assumes 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 token that it will use to call to
>    a backend service (extra line breaks and indentation in the examples
>    are for display purposes only).
>
> We could maybe add some commas or parentheses to help the reader group
> the various clauses properly.  E.g., it is "(trade an access token (that
> it received in a protected resource request)) for a token...", not
> "trace an access token that it received (in a protected resource request
> for a token)", where parentheses indicate logical grouping.
>

Will try and do some grouping.

>
>     grant_type=urn%3Aietf%3Aparams%3Aoauth%3Agrant-type%3Atoken-exchange
>     &resource=https%3A%2F%2Fbackend.example.com%2Fapi%20
>     &subject_token=accVkjcJyb4BWCxGsndESCJQbdFMogUC5PbRDqceLTC
>     &subject_token_type=
>      urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Aaccess_token
>
>                      Figure 2: Token Exchange Request
> Is there really supposed to be a %20 in the resource query parameter's
> value?
>

Nope. Nice catch. Thank you. I'll remove it.

>
> The token octets in Figures 3 and 4 do not match up, despite the prose
> indicating that they are the same token.
>

Indeed they don't. Look like I missed one token example when updating claim
names. I'll fix that. Thanks for catching that one.

>
> Section 3
>
> Would it be appropriate to note (here or elsewhere) that for non-JWT
> token formats that are a binary format, the URI used for conveying them
> needs to be associated with the semantics of base64 (or otherwise)
> encoding them for usage with OAuth?
>

My thinking had been that it'd be more or less self-evident to the very
small group and type of people who would ever undertake such a thing. But a
brief note to that effect couldn't hurt. I'll add something as such.

>
>                                                 Token Exchange can work
>    with both tokens issued by other parties and tokens from the given
>    authorization server.  [...]
>
> Does "work with" mean "accept as input" or "produce as output" or both?
> For input, as both subject_token and actor_token?
>

Both and yes.

   The following token type identifiers are defined by this
>    specification.  Other URIs MAY be used to indicate other token types.
> I'd also link to the registry here.
>

The aforementioned other URIs may well be in different namespace so won't
ever be in the registry. And that registry also has entries for things
other than token types. So I don't think a link to it here would be
particularly helpful or even appropriate necessarily.

>
> Why is the text about "urn:ietf:params:oauth:token-type:jwt" formatted
> differently than the other URIs listed?
>

The list of the ones defined in this doc is a <list style="hanging"> list
with each URI in the list appearing in a <t hangText="URI:here"> while the
:jwt URI is defined elsewhere in RFC 7519 but relevant enough to warrant
mention in this doc and it is enclosed in a <spanx style="verb"> tag. I
feel like I've seen this style of treatment of literal values with list
items and in paragraph text in other documents so considered it "normal".
Is there a better or more recommended way of doing this kind of thing?

> Section 4.1
>
> Do we want to consider a more self-describing subject identifier scheme,
> akin to
> https://tools.ietf.org/html/draft-ietf-secevent-subject-identifiers ?
>

There's nothing precluding the use of such a scheme (well, except that doc
doesn't actually define a claim so some claim is needed) but the scope of
this document isn't to be that prescriptive in subject identification.

>
> The example in Figure 5 appears to be using the "implicit issuer"
> behavior wherein the "iss" of the actor's "sub" is assumed to be the
> same value as in the containing structure.  I'm not a fan of this type
> of behavior in general, but if it's going to be used, you need to
> document the possibility in some fashion.
>

I'm not a hug fan myself but that's what OpenID Connect did and so it often
rears its head.

I've tried to make examples that will be meaningful to readers and also
somewhat likely to be realistic.

In this section it does say:
"For example, the combination
   of the two claims "iss" and "sub" might be necessary to uniquely
   identify an actor"

And sub in RFC 7519 says:
"The subject value MUST either be scoped to be
   locally unique in the context of the issuer or be globally unique."

>
> I might also consider some language about how "the nested "act" claims
> serve as a history trail that connects the initial request and subject
> through the various delegation steps undertaken before reaching the
> current actor.  In this sense, the current actor is considered to
> include the entire authorization/delegation history, leading naturally
> to the nested structure described here".  (But see also the other ballot
> comment about this potentially leaking information to unauthorized
> parties; it seems a more careful adjustment of the text is in order
> here.)
>

 Okay, I can add something to that effect.

> Section 4.2
>
> Is this really the first time we're defining "scope" as a JWT claim?  I
> would have thought that would be defined long ago...
>

Some things haven't historically happened in OAuth the way one might very
reasonably have expected. And this is one such thing.

>
> Section 4.4
>
> Just to double-check: this is "things that can act as me" (where "me" is
> the subject of the token containing this field), right?

Yes. Honestly, I have a hard time seeing this claim actually being used in
practice. But maybe I'm wrong. And I'm just the editor on this one. But
yes, that's the intended meaning.

The
> parenthetical "May Act For" doesn't really help me decide whether this
> claim represents the source or target of a permitted delegation, so
> maybe "Allowed Impersonators" or similar would be more clear.  Even "act
> as" or "act on behalf of" instead of "act for" would help me, I think.
> [This would have trickle-down effects to later parts of the document as
> well, e.g., the IANA Considerations.]
> (Not that I claim to be a representative population, of course!)
>

On looking at it again, I agree "May Act For" isn't a particularly good
name nor is it helpful in understanding it. I admit to having a hard time
with the language here. But, yeah, "May Act For" isn't very good.

What about "Authorized Actor" in the parenthetical and "Authorized Actor -
the party that is authorized to become the actor" for the Claim Description
in registration?

> It would probably also help greatly to note that when a subject_token is
> presented to the token endpoint in a token exchange request, the
> "may_act" claim in the subject token can be used by the authorization
> service to determine whether the client (or party identified in the
> actor_token) is authorized to engage in the requested delegation [or
> impersonation].
>

Okay, I can add something to that effect.

>
> Section 6
>
> Let me say a bit more here about my perception of the potential privacy
> considerations involved in the use of an error_uri (so we can figure out
> if they are already discussed in a relevant document that we can cite;
> JWT itself doesn't seem to cover this topic).  By sending an error_uri
> instead of an error string, the server is in effect causing the client
> to make an outbound request to a URL of the server's choosing.  If there
> is a proxy between the client and server, this could result in the
> server (and/or a party controlled by the server) learning additional
> information about the client's identity/location.  A malicious server
> could also attempt to construct a URI that, when retrieved by the
> client, performs some unwanted side effect.  Defenses against this
> latter scenario are pretty well known in the web comunity, but we may
> want to be sure that the need for them is mentioned in a discoverable
> place.
>

Thank you for the further explanation. As I wrote earlier, however, the
error_uri response parameter was originally defined in RFC 6749 and any
privacy or security considerations for it are applicable to considerably
more than this document.

> Appendix A.1.1
>
>    In the following token exchange request, a client is requesting a
>    token with impersonation semantics. [...]
>
> What part of the request indicates that impersonation semantics are
> requested?
>

I guess it's not explicitly requesting impersonation semantics per se but
only a subject_token is being supplied in the request so impersonation is
kinda implied as there is no party identified that could be delegated to.

Do you think the wording should be qualified as such or otherwise adjusted?

>
> Is the use of the "jwt" subject_token_type appropriate, given the
> previous discussion about id_token/access_token being generally
> preferred (as conveying more meaning)?
>

The issuer of that token isn't the given AS so it isn't an access_token.
And it doesn't have all the claims required to be an id_token. That leaves
JWT.  And JWT is used a lot in the examples so their claims can also be
seen and an "identity" can be traced through the exchange.

-- 
_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._