Subject: [OAUTH-WG] Barry Leiba's No Objection on draft-ietf-oauth-token-exchange-18: (with COMMENT)

From: Barry Leiba via Datatracker <[email-masked]>
Date: Thu, 18 Jul 2019 14:06:10 -0700

Barry Leiba has entered the following ballot position for
draft-ietf-oauth-token-exchange-18: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

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/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

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

---

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 18 Jul 2019 16:13:01 -0500

Just on one point...

> https://datatracker.ietf.org/doc/draft-ietf-oauth-token-exchange/

I think I was reading this as "if the server can't fulfil the exact request
as given, it returns an error and says "try again slightly differently".

-Ben

---

From: Barry Leiba <[email-masked]>
Date: Thu, 18 Jul 2019 17:15:47 -0400

> as given, it returns an error and says "try again slightly differently".

Yes, I'm sure that's what's meant.  I'm asking that that be clarified,
because I have seen problems in implementation of other specifications
when that wasn't crystal clear.

Barry

---

From: Eve Maler <[email-masked]>
Date: Thu, 18 Jul 2019 19:11:19 -0500

Regarding your “higher importance” comment on Section 1.1 about the impersonation semantic below:

Eve Maler (sent from my iPad) | cell +1 425 345 6756

> On Jul 18, 2019, at 4:06 PM, Barry Leiba via Datatracker <[email-masked]> wrote:
> 

I’ve always been troubled by the same thing. A and B are, in fact, distinguishable (contra “A ... is indistinguishable from B in that context”). What i think is meant by impersonation is that A acts in B’s identity context while using the access it has been delegated, rather than its own context — whatever the extent of this access, great or small.

> _______________________________________________
> OAuth mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/oauth

---

From: Brian Campbell <[email-masked]>
Date: Fri, 19 Jul 2019 08:12:53 -0600

Barry, thanks for the review, ballot position and comments. Please see
inline below for my replies to the latter.

On Thu, Jul 18, 2019 at 3:06 PM Barry Leiba via Datatracker <
[email-masked]> wrote:

> the authors and responsible AD to do the right thing.

I always endeavor to do the right thing.

>   So I’ve divided the

My initial response was going to be "sure, I'll add some bits in sec 1.1
along those lines to clarify that." However, as I look again at that
section for good opportunities to make such additions, I feel like it is
already said that impersonation is controlled. Notably there are these bits
of text:

"or for A to
   be granted a *limited access credential *to C but that continues to
   identify B as the authorized entity ("impersonation")."

"When principal A impersonates principal B, A is given all the rights
   that B has *within some defined rights context*"

So I think it already says that and I'm gonna have to flip it back and ask
if you have concrete suggestions for changes or additions that would say it
more clearly or more to your liking?

> RFC 8174 needs to be normative, along with 2119.

Of course. Not sure how those got separated in the first place but I'll
move 8174 to normative.

> to the end, “A request is successful only when all requested tokens are

Will change to try and avoid that potential trip-up.

> Should “TLS” here have a citation and normative reference?

I didn't include an explicit reference here because TLS is transitively
referenced by other normative references (including 6749 of which this
whole thing is an extension) and TLS is pretty widely recognized even
without citation.

Or maybe it was just an oversight when I added that particular text.

I'm happy to add a citation here but it does raise the question of what the
most appropriate way to cite TLS is right now - 1.3, 1.2, or the BCP or
some combination thereof?

>    a new token that is appropriate to include in a call to a backend

 Will take the suggestion.

> There should be hyphens in “request-and-reaponse protocol” (compound

Okay, request-and-response it will be.

> “must not be used” or both “should not be used” (or perhaps both “are not

Makes sense, I'll make them the same.  And I think I'll go with “are not
used".

-- 
_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: Barry Leiba <[email-masked]>
Date: Fri, 19 Jul 2019 10:31:44 -0400

>> and I trust the authors and responsible AD to do the right thing.
>

You do; hence, the trust.  :-)
And thanks for the quick responses.

> My initial response was going to be "sure, I'll add some bits in sec 1.1 along those lines to clarify
> that." However, as I look again at that section for good opportunities to make such additions, I feel
> like it is already said that impersonation is controlled.
...
> So I think it already says that and I'm gonna have to flip it back and ask if you have concrete
> suggestions for changes or additions that would say it more clearly or more to your liking?

It is mentioned, true, and that might be enough.  But given that Eve
also replied that she would like more here, let me suggest something,
the use of which is entirely optional -- take it, don't take it,
modify it, riff on it, ignore it completely, as you think best.  What
do you think about changing the last sentence of the paragraph?: "For
all intents and purposes, when A is impersonating B, A is B within the
rights context authorized by the token, which could be limited in
scope or time, or by a one-time-use restriction."

> I didn't include an explicit reference here because TLS is transitively referenced by other
> normative references (including 6749 of which this whole thing is an extension) and TLS
> is pretty widely recognized even without citation.
...
> I'm happy to add a citation here but it does raise the question of what the most appropriate
> way to cite TLS is right now - 1.3, 1.2, or the BCP or some combination thereof?

I wondered the same thing, and you're also right that it might not
need a reference in this document.  I only even flagged it because
it's the subject of a MUST.  I'll leave it to the Sec ADs (who
obviously didn't flag it themselves, so maybe they agree that it's not
necessary).

Barry

---

From: Brian Campbell <[email-masked]>
Date: Fri, 19 Jul 2019 10:05:57 -0600

> You do; hence, the trust.  :-)

I do appreciate that, thank you.

> And thanks for the quick responses.

I try. To varying degrees of success.

> along those lines to clarify

Sure, I think that or some slight modification thereof can work just fine.
I'll do that and get it and the rest of these changes published when the
I-D submission embargo is lifted for Montreal.

> referenced by other

I'm gonna just leave it as-is then, unless I hear otherwise from the Sec
ADs.

---

From: Benjamin Kaduk <[email-masked]>
Date: Sat, 20 Jul 2019 23:24:50 -0500

> I'm gonna just leave it as-is then, unless I hear otherwise from the Sec

I'll throw it out there that "TLS" is marked as "well-known" at
https://www.rfc-editor.org/materials/abbrev.expansion.txt ...

-Ben

---

From: Benjamin Kaduk <[email-masked]>
Date: Sat, 20 Jul 2019 23:28:41 -0500

> I-D submission embargo is lifted for Montreal.

My brain is apparntly storming and not sleeping.  Another option for
consideration, is to have two sentences:

For all intents and purposes, when A is impersonating B, A is B within the
rights context authorized by the token.  A's ability to impersonate B could
be limited in scope or time, or even with a one-time-use restriction,
whether via the contents of the token or an out-of-band mechanism.

-Ben

---

From: Brian Campbell <[email-masked]>
Date: Sun, 21 Jul 2019 06:14:32 -0600

That works for me.

On Sat, Jul 20, 2019 at 10:28 PM Benjamin Kaduk <[email-masked]> wrote:

> > > >> 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
> > > > that." However, as I look again at that section for good
> opportunities
> > > to make such additions, I feel
> > > > So I think it already says that and I'm gonna have to flip it back
> and
> > > ask if you have concrete
> > > > suggestions for changes or additions that would say it more clearly
> or
> > > more to your liking?
> > >

---

From: Brian Campbell <[email-masked]>
Date: Sun, 21 Jul 2019 09:43:19 -0600

https://tools.ietf.org/html/draft-ietf-oauth-token-exchange-19 has been
published with the updates discussed in this thread.

On Sun, Jul 21, 2019 at 6:14 AM Brian Campbell <[email-masked]>
wrote:

>> > > >> missing is pointing out that impersonation here is still
>> controlled,
>> > > that “A is
>> > > >> Second, there is accountability: audit information still shows
>> that the
>> > > token

---

From: Barry Leiba <[email-masked]>
Date: Sun, 21 Jul 2019 12:31:04 -0400

Thanks, Brian!

Barry

On Sun, Jul 21, 2019 at 11:43 AM Brian Campbell
<[email-masked]> wrote:
>
> https://tools.ietf.org/html/draft-ietf-oauth-token-exchange-19 has been published with the updates discussed in this thread.
>
> On Sun, Jul 21, 2019 at 6:14 AM Brian Campbell <[email-masked]> wrote:
>>
>> That works for me.
>>
> 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: John Bradley <[email-masked]>
Date: Sun, 21 Jul 2019 13:18:35 -0400

Thanks

On Sun, Jul 21, 2019, 12:31 PM Barry Leiba <[email-masked]> wrote:

> >>> > > >> 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).
> >>> > > > So I think it already says that and I'm gonna have to flip it
> back and
> >>> > > ask if you have concrete
> >>> > > > suggestions for changes or additions that would say it more
> clearly or
> >>> > > more to your liking?
> >>> > >
> > 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.
>