Subject: [OAUTH-WG] WGLC for draft-ietf-oauth-token-exchange-08

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Fri, 2 Jun 2017 15:05:57 -0400

All,

We are starting a WGLC on the Token Exchange document:
https://www.ietf.org/id/draft-ietf-oauth-token-exchange-08

Please, review the document and provide feedback on any issues you see with
the document.

The WGLC will end in two weeks, on June 17, 2017.

Regards,
 Rifaat and Hannes

---

From: Denis <[email-masked]>
Date: Mon, 5 Jun 2017 13:30:28 +0200

Comments on OAuth 2.0 Token Exchange draft-ietf-oauth-token-exchange-08

1. The abstract states:

        "This specification defines a protocol for an HTTP- and JSON- based
        Security Token Service (STS) by defining how to request and obtain
        security tokens from OAuth 2.0 authorization servers, including
        security tokens employing impersonation and delegation".

The problem is that the content of the abstract does not match with the 
content of the document.

The abstract clearly states that all cases of token requests are 
supported, whereas the document mandates
the use of a subject_token parameter which restricts the scope to 
impersonation and delegation.

Currently the text states:

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

The abstract should be changed to reflect the content of the document.

2. The text states on page 4:

        "The scope of this specification is limited to the definition of a
        basic request and response protocol for an STS-style token exchange
        utilizing OAuth 2.0.  Although a few new JWT claims are defined that
        enable delegation semantics to be expressed, the specific syntax,
        semantics and security characteristics of the tokens themselves
    (both
        those presented to the AS and those obtained by the client) are
        explicitly out of scope and no requirements are placed on the trust
        model in which an implementation might be deployed".

These statements are contradictory. One parameter of the request is 
mandatory (i.e. subject_token)
but there is no indication of the kind of treatment which should be done 
with this parameter so that
it will be taken into consideration one way or another in the token that 
will be issued.

This document by itself would be insufficient to allow any kind of 
interoperability.
Conformance to this document would not mean anything.

3. On page 7, the text states:

    "subject_token
       REQUIRED.  A security token that represents the identity of the
       party on behalf of whom the request is being made".

It is understood that one implementation is already using this parameter 
to place in it a security token.
Since this parameter is indicated as REQUIRED, it is not understandable 
why a security token shall necessarily be used.
There are other means for the STS to identify the "party on behalf of 
whom the request is being made".

Please add a rational.

In addition, what the STS will do, can do or should do with this 
parameter is left undefined.

4. On page 7, the text states:

    "subject_token
       REQUIRED.  (...)  Typically, the subject of this token will be
       the subject of the security token issued in response to this
       request".

This sentence is quite hard to understand since the specific syntax, 
semantics and security characteristics of the tokens themselves
are explicitly out of scope. The key point is what the STS should do 
with this parameter: this is left undefined.

5. The text states:

        " (...) Additional
        profiles may provide more detailed requirements around the specific
        nature of the parties and trust involved, such as whether signing
        and/or encryption of tokens is needed or if proof-of-possession
    style
        tokens will be required or issued;

Tokens are always signed. Please modify the sentence accordingly.

6. The following sentence is important and is being noticed.

    The security tokens obtained could be used in a number of contexts,
    the specifics of which are also beyond the scope of this
    specification.

Changing the "could" into a "may" would however be more appropriate.

7. In section  2.1 request, the text defines:

    resource
       OPTIONAL.  Indicates the physical location of the target service
       or resource where the client intends to use the requested security
       token.

and

    audience
       OPTIONAL.  The logical name of the target service where the client
       intends to use the requested security token.

If the resource or the audience parameter is being used, the STS will 
have the ability to know exactly which individual
or entity has accessed which target service and may keep a log of that 
activity. It would be in a position to act as Big Brother.
This should be clearly indicated in a section that is currently missing 
: "7. Privacy Considerations". See a text proposal hereafter.

However, there is indeed the need to restrict the use of tokens to 
specific targets. The key point is that the target service
must be able to recognize itself that the token is indeed targeted to 
it. As an example, a challenge may be requested to
the target service and that challenge may then be placed into a specific 
filed of the token. The target service may then verify
that the value included in the token is the one that has been recently 
provided.

A parameter specifying the type of the control value and the value of 
the control should be added.
This parameter would be called a target_id (tid). It would solve the Big 
Brother case.

8. The Security Considerations section states:

    "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 "scp" claim is suggested to mitigate potential for such abuse, as
    it restricts the contexts in which the delegated rights can be
    exercised".

Section 4.2 defines the "scp" (Scopes) claim in the following way:

    The "scp" claim is an array of strings, each of which represents an
    OAuth scope granted for the issued security token.  Each array entry
    of the claim value is a scope-token, as defined in Section 3.3 of
    OAuth 2.0 [RFC6749].

Section 3.3 from RFC 6749 defines the Access Token Scope as:

    "The authorization and token endpoints allow the client to specify the
    scope of the access request using the "scope" request parameter.  In
    turn, the authorization server uses the "scope" response parameter to
    inform the client of the scope of the access token issued.
    The value of the scope parameter is expressed as a list of space-
    delimited, case-sensitive strings.  The strings are defined by the
    authorization server".

Section 5.4.1 Registry Contents defines scp as:

    o  Claim Name: "scp"
    o  Claim Description: Scope Values
    o  Change Controller: IESG
    o  Specification Document(s): Section 4.2 of [[ this specification ]]

Since the "strings are defined by the authorization servers", what a 
scope may mean is subject to multiple interpretations.
The current definition of scp is insufficient to allow any kind of 
interoperability, now or in the future.

It is thus unclear how the use of the "scp" claim might mitigate the risk.

9. This document is currently targeted to become a Standards Track document.

RFC 6410 recognizes two maturity levels:

    - the First Maturity Level: Proposed Standard
    - the Second Maturity Level: Internet Standard

It is not believed that currently it is possible to construct two 
independent interoperating implementations
looking at this document only. Unless more guidance is provided, this 
document should be targeted to "Experimental".

10. Text proposal for a new section "7. Privacy Considerations".

    7. Privacy Considerations

    7.1. Resource and audience parameters

    The use of any these two parameters allow the STS to know which
    target servers are being accessed by any party making a token
    request.  Any STS is then able to log token requests.  This is not
    a problem if the resource owner and the target server are collocated,
    but this document is not restricted to that case.

    For the other cases, it should be noticed that the STS will be in
    position to act as Big Brother.  When privacy is a concern, the use
    of these parameters is deprecated and the use of a "tid" parameter
    is recommended.

Denis

> Please, review the document and provide feedback on any issues you see 
> with the document.
>
> _______________________________________________
> OAuth mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/oauth

---

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Mon, 26 Jun 2017 08:40:39 -0400

Hi (as individual),

I have reviewed this version of the document and I have the following
comments/questions:

Section 2.1, page 8, last paragraph:

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

Would the validity of the new issued token be impacted later on by the
validity of the subject or actor tokens?

Section 2.2.2, page 10, second paragraph:

  "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 MAY be used in the error
   response."

Can you please elaborate on why the above text is using "MAY" for the use
of "invalid_target" in this case?

Section 4.1, page 14, second paragraph:

  "However, claims within the "act" claim pertain only to the identity
   of the actor and are not relevant to the validity of the containing
   JWT in the same manner as the top-level claims.  Consequently, claims
   such as "exp", "nbf", and "aud" are not meaningful when used within
   an "act" claim, and therefore should not be used."

If the "exp", "nbf", and "aud" claims are not meaningful inside the "act"
claim, why is the sentence stating that it "should not be used"?
Would it not be more appropriate to state that it "must not be used"
instead?

Regards,
 Rifaat

---

From: Brian Campbell <[email-masked]>
Date: Fri, 30 Jun 2017 14:08:23 -0600

Thanks for the review, Rifaat. Replies are inline below...

> validity of the subject or actor tokens?

No, the intent is that the tokens presented for exchange need to be valid
at the time of exchange but after that the validity of the issued token is
decoupled from, and has no dependency on, the subject or actor tokens.

Do you feel that the doc should state this more explicitly? If so, a
sentence like this could be added following the text you quoted,
"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."

> of "invalid_target" in this case?
To be honest, I don't recall exactly why I went with "MAY" there. And on
seeing your question and reading it again, that feels like it should be
stronger than "MAY".

Should that "MAY" be changed to a "SHOULD"? Or even a "MUST"?

> Would it not be more appropriate to state that it "must not be used"
My thinking here is that saying, 'such as "exp", "nbf", and "aud" claims'
is more of a general statement of guidance rather than a fully inclusive of
list of claims that aren't meaningful inside the 'act' claim. And a full
list isn't really feasible given that new claims can be defined in the
future.  So the use of "should" seemed more appropriate in that context
rather than "must" or any RFC 2119 words. We can discuss changing that
somehow, if you and/or other WG members think a change is needed? But that
was my line of reasoning behind the current text.

Regards,

-- 
*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: Fri, 30 Jun 2017 15:35:09 -0600

Thanks for the review, Denis. And I apologize for the slow reply - I've had
a lot of different things on my plate recently. And, quite frankly, I was
sort of hoping one of the co-authors on the document might respond to your
comments. But hope only goes so far. So replies are inline below...

> The abstract should be changed to reflect the content of the document.

I don't see a discrepancy between the content of the abstract and the
content of the document.

What text or changes would you suggest in the abstract?

>    semantics and security characteristics of the tokens themselves (both

The token exchange framework promotes interoperability in the form of
common patterns and parameters that can be supported in libraries,
products, and services. It facilitates deployments like this one
https://help.salesforce.com/articleView?id=remoteaccess_oaut
h_asset_token_flow.htm or this one https://developer.box.com/docs
/getting-started-with-new-box-view, for example.

> There are other means for the STS to identify the "party on behalf of whom
> the request is being made".
>
> Please add a rational.
>

I don't understand what kind of rational you are looking for here. Can you
suggest some specific text for the document that would address the concern
you have?

>
> In addition, what the STS will do, can do or should do with this parameter
> is left undefined.
>
>
> are explicitly out of scope. The key point is what the STS should do with
> this parameter: this is left undefined.
>

I don't view the sentence as difficult to understand.  Maybe that's because
I wrote it?  But it is true that typically it is the case that the subject
of the inbound token will be the subject of the issued token. I don't know
how else to say it. Please offer suggested text, if you believe there's a
way to say it that is easier to understand.

>    and/or encryption of tokens is needed or if proof-of-possession style

A token might well be cryptographically secure random sequence of
characters that reference data that can be looked up by the appropriate
parties. Or a token might be an AEAD symmetrically encrypted JWT. So no,
tokens are not always signed.

> Changing the "could" into a "may" would however be more appropriate.

Agreed, I'll make that change.

> If the resource or the audience parameter is being used, the STS will have
> the ability to know exactly which individual
> This should be clearly indicated in a section that is currently missing :
> "7. Privacy Considerations". See a text proposal hereafter.
>
> must be able to recognize itself that the token is indeed targeted to it.
> As an example, a challenge may be requested to
> A parameter specifying the type of the control value and the value of the
> control should be added.

That is both beyond the scope of this document and potentiality applicable
to a broader context of use. I'd suggest you write an individual draft for
the concept, if you want to pursue it.

> Since the "strings are defined by the authorization servers", what a scope
> may mean is subject to multiple interpretations.

Scoping the token restricts what the token can be used for which limits the
impact of a compromised or misused token. The scope values are interpreted
by the parties involved just as with regular OAuth.

> document should be targeted to "Experimental".

I completely disagree. And would also note that the the document's intended
status has been stable and unquestioned by this WG for several years.

>    of these parameters is deprecated and the use of a "tid" parameter
I will add a privacy considerations with mention of being able to track the
target systems intended to be accessed by the party requesting a token.

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

---

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Fri, 30 Jun 2017 17:59:58 -0400

Thanks Brian.

See my replies inline...

On Fri, Jun 30, 2017 at 4:08 PM, Brian Campbell <[email-masked]>
wrote:

> On Mon, Jun 26, 2017 at 6:40 AM, Rifaat Shekh-Yusef <[email-masked]
> > wrote:
>
>> Hi (as individual),
>>
Yeah, your proposed text looks good to me. It is better to explicitly state
that rather than leave it open to different interpretations.

> Should that "MAY" be changed to a "SHOULD"? Or even a "MUST"?

It seems to me that at least "SHOULD" is warranted here.
Anybody has a strong opinion on this?

> was my line of reasoning behind the current text.
How about something along the line of the following to replace the last
sentence above:

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

Regards,
 Rifaat

>
>
>
>>
>>
>>
> Regards,
>>  Rifaat
>>
>>
>>
>>
>>
>> On Fri, Jun 2, 2017 at 3:05 PM, Rifaat Shekh-Yusef <[email-masked]
>> > wrote:
>>
>>> All,
>>>
> *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: Fri, 30 Jun 2017 16:52:32 -0600

Okay, thanks Rifaat. I'll make those changes.

On Jun 30, 2017 3:59 PM, "Rifaat Shekh-Yusef" <[email-masked]> wrote:

> Thanks Brian.
>
> See my replies inline...
>
>
> On Fri, Jun 30, 2017 at 4:08 PM, Brian Campbell <
> [email-masked]> wrote:
>
>> On Mon, Jun 26, 2017 at 6:40 AM, Rifaat Shekh-Yusef <
>> [email-masked]> wrote:
>>
>>> Hi (as individual),
>>>
> Yeah, your proposed text looks good to me. It is better to explicitly
> state that rather than leave it open to different interpretations.
>
>
>
>>
>>
>>> Can you please elaborate on why the above text is using "MAY" for the
>>> use of "invalid_target" in this case?
>>>
>>>
>>> On Fri, Jun 2, 2017 at 3:05 PM, Rifaat Shekh-Yusef <
>>> [email-masked]> wrote:
>>>
>>>> All,
>>>>