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

From: Benjamin Kaduk <[email-masked]>
Date: Fri, 11 Jan 2019 10:13:21 -0600

I also apologize for the slow response (I gave Brian a unicast heads-up
earlier) -- between vacation, the holidays, and a death in a the family I
was away from email for quite some time.

On Tue, Dec 04, 2018 at 02:54:36PM -0700, Brian Campbell wrote:
> 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?

There's two obvious routes -- first, to change the text to use placeholders
like "TBD1" or "the token-exchange URI" (e.g., as opposed to
urn:ietf:params:oauth:grant-type:token-exchange specifically) and request
that IANA allocate the specific suggested values; or
to get IANA to explicitly confirm that
these values can be registered and will be marked as pending until this
document is finalized (to prevent allocation "under our nose" by other
means).  Ekr and I can help mediate any IANA interaction needed for
whatever route we end up taking, if needed.

(Basically, this is a process concern -- the IESG should not give its stamp
of approval to a document in a state that does something we don't want
other people to do, even if the final published RFC will be able to make
these claims correctly.)

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

Before I start trying to tweak text, can you confirm that the actor_token
request parameter is okay to use in both delegation and impersonation
scenarios?

> 
> 
> >
> > 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?

Oh, definitely -- I only asked if there was something existing we could
cheaply reference; this is definitely not the place to be writing this down
from scratch.  Thanks for doing the search!

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

I don't mind having the reference there; it's not really causing problems
and could potentially be helpful.  We should be able to get away with a
generic reference to this class of thing elsewhere and one-sentence
description ("when a proxy or similar mechanism is in place to protect
client privacy, the error_uri mechanism can induce the client to lose some
anonymity by dereferencing a URI pointing to a third party server that can
leak information to the attacker, in a similar fashion as [ref]").  I don't
have a [ref] handy right now, though; I'll need to ask around.

In a pinch we could fallback to analogy to open-redirector issues, though
we differ in which actors are receiving/conveying/acting on untrusted
input, and we can have issues just by making the request as opposed to the
user mis-interpreting the returned resource.  But to reiterate, I'm only
looking for a brief mention that some clients might care and don't need an
exhaustive description.

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

Okay, so at some point we're essentially just doing a lookup based on
audience string, and the type information is attached to the lookup results
(along with everything else needed).

Do you think it makes sense to add a sentence after the non-elided quoted
portion, something like ``However, "audience" values used on a given
authorization server must be unique within that server, to ensure that they
are properly interpreted as the intended type of value.''?  (I'm of course
open to other suggestions, including "just leave it as it is"; I think what
triggered me to comment here is that "both the client and the authorization
server understand" leaves open the possibility that the AS might share one
understanding of a string with one client and a different understanding of
that same string with a second client, since it's only a pairwise condition
but we probably are safer with a global condition.)

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

I made another quick pass, and it is better than I remembered.  So thanks
for the efforts, and sorry for maligning the document!

Maybe 2.2.1's "token_type" description could reiterate "issued security
token" both times that "security token" appears instead of just the second
time, though the context really ought to be enough to make this one clear.
Other than that, the only potential trouble I see is in the introduction
when we get a barrage of the string all at once.  And even that's in
reasonable shape, with the only potential changes I see being in the first
sentence of the second paragraph, something like "capable of validing
security tokens provided to it and issuing new security tokens in
response".

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

Okay, thanks for confirming.

> 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

Sure; I don't expect those changes to introduce any concerns of the nature
I was asking about here.

> 
> >
> >    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?

I think so.

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

Thanks.

> 
> >
> >    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?

That makes perfect sense for what we want to happen, yes.

I wonder if we really want the second sentence to be saying something like
"The exchange is a one-time event and does not create a tight linkage
betwee the input and output tokens, so that (for example) while the expiration
time of the output token may be influenced by that of the input token,
renewal or extension of the input token is not expected to be reflected in
the ouput token's properties.  It may still be appropriate to propagate
token revocation events, though."  (This bit about revocation is perhaps
even more interesting than expiration time, and would seem to be prevented
by the current text.)

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

Okay, thanks.

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

To be clear, I wouldn't mind if you decided to leave it as is.  But thanks
:)

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

Okay.  (I don't have any text suggestions, and as-is is probably fine.)

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

Ah, good points.

> 
> >
> > 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?

Nope, what you did is fine.  I think I managed to forget that I was reading
a list of identifiers *defined by this specification* before I reached the
end of the list :(

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

Okay.

> 
> >
> > 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 guess this could fall into the "globally unique" bucket, that's fair.
(And this was a non-blocking comment anyway...)

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

Okay.  I think the this text has a pretty clear reading, but just wanted to
double-check that I was getting the expected meaning from it (so no change
suggested).

> 
> 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?
> 

I think that's an improvement, thanks.

> 
> > 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?

I could go either way, but if I was adding something, I'd go for a
parenthetical "(with only a subject_token and no actor_token, delegation is
impossible)".

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

Okay, thanks for the clarification.

(And for all the changes!)

-Benjamin

---

From: Mike Jones <[email-masked]>
Date: Fri, 11 Jan 2019 17:33:26 +0000

I would advocate requesting early registration for urn:ietf:params:oauth:grant-type:token-exchange.

				-- Mike

-----Original Message-----
From: Benjamin Kaduk <[email-masked]> 
Sent: Friday, January 11, 2019 8:13 AM
To: Brian Campbell <[email-masked]>
Cc: The IESG <[email-masked]>; oauth <[email-masked]>; [email-masked]; [email-masked]
Subject: Re: [OAUTH-WG] Benjamin Kaduk's Discuss on draft-ietf-oauth-token-exchange-16: (with DISCUSS and COMMENT)

I also apologize for the slow response (I gave Brian a unicast heads-up
earlier) -- between vacation, the holidays, and a death in a the family I was away from email for quite some time.

On Tue, Dec 04, 2018 at 02:54:36PM -0700, Brian Campbell wrote:
> 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.
> 
> > Please refer to 
> > https://www.ietf.org/iesg/statement/discuss-criteria.html
> > "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?

There's two obvious routes -- first, to change the text to use placeholders like "TBD1" or "the token-exchange URI" (e.g., as opposed to urn:ietf:params:oauth:grant-type:token-exchange specifically) and request that IANA allocate the specific suggested values; or to get IANA to explicitly confirm that these values can be registered and will be marked as pending until this document is finalized (to prevent allocation "under our nose" by other means).  Ekr and I can help mediate any IANA interaction needed for whatever route we end up taking, if needed.

(Basically, this is a process concern -- the IESG should not give its stamp of approval to a document in a state that does something we don't want other people to do, even if the final published RFC will be able to make these claims correctly.)

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

Before I start trying to tweak text, can you confirm that the actor_token request parameter is okay to use in both delegation and impersonation scenarios?

> 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?

Oh, definitely -- I only asked if there was something existing we could cheaply reference; this is definitely not the place to be writing this down from scratch.  Thanks for doing the search!

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

I don't mind having the reference there; it's not really causing problems and could potentially be helpful.  We should be able to get away with a generic reference to this class of thing elsewhere and one-sentence description ("when a proxy or similar mechanism is in place to protect client privacy, the error_uri mechanism can induce the client to lose some anonymity by dereferencing a URI pointing to a third party server that can leak information to the attacker, in a similar fashion as [ref]").  I don't have a [ref] handy right now, though; I'll need to ask around.

In a pinch we could fallback to analogy to open-redirector issues, though we differ in which actors are receiving/conveying/acting on untrusted input, and we can have issues just by making the request as opposed to the user mis-interpreting the returned resource.  But to reiterate, I'm only looking for a brief mention that some clients might care and don't need an exhaustive description.

> > How does the STS know what type of identifier it is supposed to 
> > interpret the provided audience value as?
> >
> 
> 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

Okay, so at some point we're essentially just doing a lookup based on audience string, and the type information is attached to the lookup results (along with everything else needed).

Do you think it makes sense to add a sentence after the non-elided quoted portion, something like ``However, "audience" values used on a given authorization server must be unique within that server, to ensure that they are properly interpreted as the intended type of value.''?  (I'm of course open to other suggestions, including "just leave it as it is"; I think what triggered me to comment here is that "both the client and the authorization server understand" leaves open the possibility that the AS might share one understanding of a string with one client and a different understanding of that same string with a second client, since it's only a pairwise condition but we probably are safer with a global condition.)

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

I made another quick pass, and it is better than I remembered.  So thanks for the efforts, and sorry for maligning the document!

Maybe 2.2.1's "token_type" description could reiterate "issued security token" both times that "security token" appears instead of just the second time, though the context really ought to be enough to make this one clear.
Other than that, the only potential trouble I see is in the introduction when we get a barrage of the string all at once.  And even that's in reasonable shape, with the only potential changes I see being in the first sentence of the second paragraph, something like "capable of validing security tokens provided to it and issuing new security tokens in response".

> >   acting on behalf of B ("delegation"), or for A to be granted a 
> > limited access
> 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]
> > 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.
> >
> 

> https://mailarchive.ietf.org/arch/msg/oauth/Q1K-T2VS3wrHW7lx2EiP58b_DY
> w , which might result in a change to the wording here but it's still 

Sure; I don't expect those changes to introduce any concerns of the nature I was asking about here.

> > 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?

I think so.

> > "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).
> >
> 

Thanks.

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

I wonder if we really want the second sentence to be saying something like "The exchange is a one-time event and does not create a tight linkage betwee the input and output tokens, so that (for example) while the expiration time of the output token may be influenced by that of the input token, renewal or extension of the input token is not expected to be reflected in the ouput token's properties.  It may still be appropriate to propagate token revocation events, though."  (This bit about revocation is perhaps even more interesting than expiration time, and would seem to be prevented by the current text.)

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

Okay, thanks.

> > 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.
> >
> 
> >                      Figure 2: Token Exchange Request Is there 
> > really supposed to be a %20 in the resource query parameter's value?
> >
> 

Ah, good points.

Okay.

-Benjamin

---

From: Brian Campbell <[email-masked]>
Date: Sat, 12 Jan 2019 07:31:19 -0700

I would also advocate for requesting early registration of the URIs.

(I'll follow up on the rest of the thread at some point later as time
allows and the usual caveats)

On Fri, Jan 11, 2019, 10:33 AM Mike Jones <[email-masked]
wrote:

> I would advocate requesting early registration for
> urn:ietf:params:oauth:grant-type:token-exchange.
>
>                                 -- Mike
>
> Cc: The IESG <[email-masked]>; oauth <[email-masked]>;
> [email-masked]; [email-masked]
> Subject: Re: [OAUTH-WG] Benjamin Kaduk's Discuss on
> draft-ietf-oauth-token-exchange-16: (with DISCUSS and COMMENT)
>
> There's two obvious routes -- first, to change the text to use
> placeholders like "TBD1" or "the token-exchange URI" (e.g., as opposed to
> that IANA allocate the specific suggested values; or to get IANA to
> explicitly confirm that these values can be registered and will be marked
> as pending until this document is finalized (to prevent allocation "under
> our nose" by other means).  Ekr and I can help mediate any IANA interaction
> needed for whatever route we end up taking, if needed.
>
> (Basically, this is a process concern -- the IESG should not give its
> stamp of approval to a document in a state that does something we don't
> want other people to do, even if the final published RFC will be able to
> make these claims correctly.)
>
> >
> >
> > https://datatracker.ietf.org/doc/draft-ietf-oauth-security-topics/
> would be more appropriate in scope and content?
>
> security tokens provided to it and issuing new security tokens in response".
>
> >
> > >
> > > Section 1
> > >
> > >                                                             The OAuth
> > >                                                   For example,
> [RFC7523]
> > could be but that's not a general property of the STS protocol a would
> be specific to a particular token type or deployment.
> >
> betwee the input and output tokens, so that (for example) while the
> expiration time of the output token may be influenced by that of the input
> token, renewal or extension of the input token is not expected to be
> reflected in the ouput token's properties.  It may still be appropriate to
> propagate token revocation events, though."  (This bit about revocation is
> perhaps even more interesting than expiration time, and would seem to be
> prevented by the current text.)
>
> >
> > >
> > > Section 2.2.1
> > >
> > >    token_type
> > > [...]

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

---

From: Brian Campbell <[email-masked]>
Date: Sat, 19 Jan 2019 06:58:27 -0700

This response is slow but somewhat less slow than those that came before.
So I also apologize again but somewhat less so :)  I do apologize for
sending on a weekend but I just wasn't able to finish and make it to the
"send" button before the end of my Friday.

I've endeavored to continue the exchange inline below where appropriate and
removed portions that needed no further discussion.

On Fri, Jan 11, 2019 at 9:13 AM Benjamin Kaduk <[email-masked]> wrote:

>  <snip>

I'd then ask that the AD(s) initiate and mediate interactions with IANA to
have them explicitly confirm that these values can be registered and have
them somehow marked as pending until the document is finalized.

>  <snip>

Yes, it's certainly okay to use the actor_token in both delegation and
impersonation scenarios. It's necessary for delegation and kinda makes more
sense in that scenario but could still be sent by the client and have the
STS issue a new token with impersonation semantics.

> original

Just to clarify, that draft doesn't currently have anything about
error_uri. However, it seems like a more appropriate place for that in
terms of its scope to have any such guidance. And that, to the extent such
guidance or text is needed, it should go in to that draft (which is a WG
draft in progress) rather than into the token exchange draft.

> have a [ref] handy right now, though; I'll need to ask around.

If you can provide such a [ref], I can add such a sentence to the draft.

But I'm still of the opinion that it would be out of place in this
document.

> looking for a brief mention that some clients might care and don't need an

Perhaps then the sentence you had above but without the "in a similar
fashion as [ref]" part might suffice as the aforementioned brief mention?

> but we probably are safer with a global condition.)

My inclination would be to "just leave it as it is" because well that's the
easiest thing to do but it could also make sense to add something like that
sentence you had.

The idea/intent behind "both the client and the authorization server
understand" was that they both understand the thing and understand the same
way.

> for the efforts, and sorry for maligning the document!

No apology necessary!

> security tokens provided to it and issuing new security tokens in

Thanks for the specifics, I'll make those updates.

> >  <snip> <snip> <snip>
> came
> expiration

Yeah, I kinda think that or something along those lines might be indeed be
better. Propagation of revocation wasn't intended to be prevented. Rather I
was just aiming to say that such propagation wasn't required or even
expected.

> <snip> <snip>

Fair enough, thanks :)

> <snip> <snip> <snip>

Thanks for confirming, I'll make that change.

> >  < snip> <snip>

That works, I'll add that.

> <snip> <snip>