Subject: Re: [OAUTH-WG] Working Group Last Call on "Symmetric Proof of Possession for the OAuth Authorization Code Grant"

From: Nat Sakimura <[email-masked]>
Date: Wed, 3 Sep 2014 02:38:34 +0900

Responses inline:

2014-08-29 10:00 GMT+09:00 Mike Jones <[email-masked]>:

>  Here's some feedback on the document.
>
>
>
> First, while I believe that the document is a good first working group
> draft and this specification is important, it is not ready for last call,
> since there are open issues called out in the document that have not been
> discussed by the working group and aspects of the specification are
> incomplete.  I’ll discuss those below.  There are also significant
> ambiguities in the document at present that could lead to non-interoperable
> implementations.  I believe that it would be more appropriate to bring the
> document to WGLC once these significant issues (and those that may be
> raised by other reviewers) have been addressed.
>
>
>
> 2.  Terminology – I would list “code challenge” before “code verifier”
> both because it is used first and because it’s alphabetically first.
>
The "code verifier" is listed before "code challenge" as the later uses the
former in its definition.

>
>
> 2.1 code verifier – Is this an octet sequence to be sent as-is (with the
> requirement for %-escaping octets representing non-url-safe characters) or
> as the base64url encoding of the octet sequence?
>
Previously, it was agreed to be octet sequence and it is to be sent as-is.
The clarification will be added.

>
>
> 2.2 code challenge – Same question as above.  Then I would just say that
> the code challenge value is a function of the code verifier value and
> discuss the possible functions in its own section or subsection.
>
Same as above.
It is an open question whether we want to preserver the extensibility in
the transformation at this time.
If we decide not, then this spec will be further simplified and this point
will become subsumed.

>
>
> NOTE 1:  This should be discussed in the section about the transformation
> function.  Also, say here what criteria people might use to choose function.
>
>
>
> NOTE 2:  Also fold this into the transformation function section.
>
>
>
> 3.2 Client registers its desired code challenge algorithm – A means of
> registering this algorithm using OAuth Dynamic Client Registration should
> be defined.  Use of this method should be optional.  Any metadata values
> defines should be registered in the appropriate registry.
>

As stated above, if we do not need extensibility, we do not need this
section.
My sense is that we do not. See my response to Hannes' detailed review.

>
>
> 3.4 Client sends the code challenge – Is the code challenge octet sequence
> value sent or the base64url encoding of it?
>
As above.

>
>
> 3.6  Client sends the code and the secret  – Is the code verifier octet
> sequence value sent or the base64url encoding of it?
>
As above.

>
>
> 4.1  OAuth Parameters Registry – The change controller should be “IESG”.
>
Yes.

>
>
> 5.  Security Considerations – The implications of choosing different kinds
> of transformation functions should be discussed.
>
Will become unnecessary if we throw the extensibility away at this time.

>
>
> I would recommend running the HTML output of xml2rfc through a grammar and
> spelling checker, as numerous grammar nits would be caught by doing so.
>

Thanks for the advise.

Best,

Nat

>
>
>                                                                 -- Mike
>
>
>
> -----Original Message-----
> From: OAuth [mailto:[email-masked]] On Behalf Of Hannes Tschofenig
> Sent: Wednesday, August 27, 2014 8:45 AM
> To: [email-masked]
> Subject: Re: [OAUTH-WG] Working Group Last Call on "Symmetric Proof of
> Possession for the OAuth Authorization Code Grant"
>
>
>
> Based on the reaction from a few I thought I should add a few words about
> this working group last call.
>
>
>
> There is no requirement to wait a specific timeframe after a document
> became a WG item to issue a working group last call.
>
>
>
> In this specific case, the document was around for a while and I didn't
> see a reason for not-finishing it as soon as possible.
>
>
>
> Additionally, since the document deals with a security vulnerability that
> is being exploited today I thought it might make sense to get the attention
> from the group to review it.
>
>
>
> Finally, it is also a fairly "simple" document (if there is something as
> simple in this working group).
>
>
>
> Ciao
>
> Hannes
>
>
>
> On 08/26/2014 09:32 PM, Hannes Tschofenig wrote:
>
> > Hi all,
>
> >
>
> > This is a Last Call for comments on the "Symmetric Proof of Possession
>
> > for the OAuth Authorization Code Grant" specification.
>
> >
>
> > The document can be found here:
>
> > http://datatracker.ietf.org/doc/draft-ietf-oauth-spop/
>
> >
>
> > Please have your comments in no later than September 9th.
>
> >
>
> > Ciao
>
> > Hannes & Derek
>
> >
>
> >
>
> >
>
> > _______________________________________________
>
> > OAuth mailing list
>
> > [email-masked]
>
> > https://www.ietf.org/mailman/listinfo/oauth
>
> >
>
>
>
> _______________________________________________
> OAuth mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/oauth
>
>

-- 
Nat Sakimura (=nat)
Chairman, OpenID Foundation
http://nat.sakimura.org/
@_nat_en

---

From: Brian Campbell <[email-masked]>
Date: Tue, 2 Sep 2014 11:49:29 -0600

On #1, I know some have pushed for having the transformation options so I
don't know if dropping it will work. But, if not removed entirely, the
transformation stuff could definitely be deemphasized in favor of what will
be the most common case of the client sending a random string value on the
authorization request and then sending the same value on the token request.

On #2, there are already implementations and deployments of this so I'd
request that the parameter names not be changed.

On #3, I agree that the title is confusing/misleading. Perhaps, "Request
Correlation for the OAuth Authorization Code Grant" or something like that?

On #4, on one hand you are right. On the other hand, this is the OAuth WG
and this draft is addressing a very specific security issue in OAuth
deployments. Having it be OAuth-centric seems right given the circumstances.

On Thu, Aug 28, 2014 at 11:03 PM, Manger, James <
[email-masked]> wrote:

> Couple of comments on draft-ietf-oauth-spop-00:
>
> The draft defines a nice little mechanism to link two requests: one from
> app-to-browser-to-server, the other from app-to-server.
>
> 1.
> The spec defines the "bearer token" version of linking the requests: pick
> a random value and send it in both requests. The spec repeatedly hints that
> other "transformations" are possible (and even mentions one in a note), but
> it doesn't define enough to make these other ones interoperable.
> I suggest just specifying the bearer version and dropping all the other
> text.
> If we want another transform standardized later then we write another spec
> (which can use its own field names).
>
> 2.
> Linking requests is orthogonal to whether or not the requests include a
> field called "code". I suggest changing the labels "code_challenge" and
> "code_verifier" to drop the "code" reference. Perhaps replace both with
> "session_id" ("sid" for short?).
>
> 3.
> The spec is titled "Symmetric Proof of Possession ..." but defines a
> bearer mechanism, which you cannot really classify as proof-of-possession.
> Suggestion: change the title.
>
> 4.
> The text is totally OAuth-centric, though the mechanism is not really
> limited to this case. It would be much nicer to describe the mechanism more
> generically (eg app running on a user's computer wanting to link two
> requests made to a server over different channels). The abstract (and the
> start of the introduction) should be comprehensible without having to know
> what the phrase "OAuth 2.0 public client" means. There would still be some
> OAuth-specific sections describing how the mechanism applies to the code
> flow (and to register a field in the IANA OAuth registry).
>
>
> --
> James Manger
>

---

From: Nat Sakimura <[email-masked]>
Date: Wed, 3 Sep 2014 03:32:24 +0900

Hi James and Brian,

First, I apologize for taking a long time to respond to James.

My responses inline:

> authorization request and then sending the same value on the token request.

Indeed, there have been some lengthy discussions around it, so I have not
removed them.
However, if the WG feels that it is ok to leave them out, I am fine with
it.
It would impact the name of the spec though, as it will not be PoP anymore.
The justification for using PoP in the name is that in non-degenerate case,
it is a PoP.

> request that the parameter names not be changed.

Noted.

> Correlation for the OAuth Authorization Code Grant" or something like that?

See #1.

> deployments. Having it be OAuth-centric seems right given the circumstances.

IMHO, this is a scoping issue. The current draft is scoped to solve a
specific problem of OAuth public clients.
More generic way can obviously be envisaged, but I am not sure if it is
something to be worked on within OAuth WG.

Nat

>> If we want another transform standardized later then we write another
>> spec (which can use its own field names).
>>
>> 2.

---

From: John Bradley <[email-masked]>
Date: Tue, 2 Sep 2014 16:17:20 -0400

I don't think the inclusion of a MAC transform to protect the request is necessary for it to be called proof of possession.

The other way to protect the request is with a signed/encrypted request object.  That is heaver weight than the HMAC transform.

I may have come up with the trick of sending the hmac in the request and the key as the proof, so am a bit partial to it.

On the other hand I don't know that the incremental value is that much.

It may be better to skip that and add a way for a client to specify it's public key (JWK) as part of it's authorization request to the AS. 

I see that more as part of the full OAuth PoP spec for access tokens.

That way the access tokens would also be bound to the client.   

That would let us keep SPOP for code as simple as possible.

John B.
On Sep 2, 2014, at 2:32 PM, Nat Sakimura <[email-masked]> wrote:

> On #1, I know some have pushed for having the transformation options so I don't know if dropping it will work. But, if not removed entirely, the transformation stuff could definitely be deemphasized in favor of what will be the most common case of the client sending a random string value on the authorization request and then sending the same value on the token request.
> 
> Indeed, there have been some lengthy discussions around it, so I have not removed them. 
> However, if the WG feels that it is ok to leave them out, I am fine with it. 
> The justification for using PoP in the name is that in non-degenerate case, it is a PoP. 
>  
> 
> On #2, there are already implementations and deployments of this so I'd request that the parameter names not be changed.
> 
> Noted. 
>  
> 
> On #3, I agree that the title is confusing/misleading. Perhaps, "Request Correlation for the OAuth Authorization Code Grant" or something like that?
> 
> See #1. 
>  
> 
> On #4, on one hand you are right. On the other hand, this is the OAuth WG and this draft is addressing a very specific security issue in OAuth deployments. Having it be OAuth-centric seems right given the circumstances.
> 
> IMHO, this is a scoping issue. The current draft is scoped to solve a specific problem of OAuth public clients. 
> More generic way can obviously be envisaged, but I am not sure if it is something to be worked on within OAuth WG. 
> 
> Nat
>  
> 
> 
> 
> 
> On Thu, Aug 28, 2014 at 11:03 PM, Manger, James <[email-masked]> wrote:
> The draft defines a nice little mechanism to link two requests: one from app-to-browser-to-server, the other from app-to-server.
> 
> 1.
> The spec defines the "bearer token" version of linking the requests: pick a random value and send it in both requests. The spec repeatedly hints that other "transformations" are possible (and even mentions one in a note), but it doesn't define enough to make these other ones interoperable.
> I suggest just specifying the bearer version and dropping all the other text.
> If we want another transform standardized later then we write another spec (which can use its own field names).
> 
> 2.
> Linking requests is orthogonal to whether or not the requests include a field called "code". I suggest changing the labels "code_challenge" and "code_verifier" to drop the "code" reference. Perhaps replace both with "session_id" ("sid" for short?).
> 
> 3.
> The spec is titled "Symmetric Proof of Possession ..." but defines a bearer mechanism, which you cannot really classify as proof-of-possession. Suggestion: change the title.
> 
> 4.
> The text is totally OAuth-centric, though the mechanism is not really limited to this case. It would be much nicer to describe the mechanism more generically (eg app running on a user's computer wanting to link two requests made to a server over different channels). The abstract (and the start of the introduction) should be comprehensible without having to know what the phrase "OAuth 2.0 public client" means. There would still be some OAuth-specific sections describing how the mechanism applies to the code flow (and to register a field in the IANA OAuth registry).
> 
> 
> --
> James Manger
>

---

From: Nat Sakimura <[email-masked]>
Date: Wed, 3 Sep 2014 10:13:32 +0900

I support the use of public key. As I remember, our discussion started
there.
I still believe this is something that is needed to be standardized.

However, for spop use case, we have determined that is overkill and best
left for another draft.

It looks like there is a strong support in the thread to droop the
transform.
I have not heard any strong desire to keep the extensibility.

If any of you do, please voice it now.
Otherwise, I will drop the transform related clauses from the next rev.

Nat

2014-09-03 5:17 GMT+09:00 John Bradley <[email-masked]>:

> I don't think the inclusion of a MAC transform to protect the request is
> necessary for it to be called proof of possession.
>
> The other way to protect the request is with a signed/encrypted request
> object.  That is heaver weight than the HMAC transform.
>
> I may have come up with the trick of sending the hmac in the request and
> the key as the proof, so am a bit partial to it.
>
> It may be better to skip that and add a way for a client to specify it's
> public key (JWK) as part of it's authorization request to the AS.
>
>>> The spec defines the "bearer token" version of linking the requests:
>>> pick a random value and send it in both requests. The spec repeatedly hints
>>> that other "transformations" are possible (and even mentions one in a
>>> note), but it doesn't define enough to make these other ones interoperable.