[OAUTH-WG] FW: New Version Notification for draft-sakimura-oauth-rjwtprof-06.txt

["Kepeng Li" <[e-mail]>]

Hi Mike,

Thanks for your review. It is very helpful, and I also forward it to the
whole list.

We will make a update when the submission window opens.

Kind Regards
Kepeng

发件人:  Mike Jones <[e-mail]>
日期:  Wednesday, 21 October, 2015 1:07 am
至:  Li Kepeng <[e-mail]>
抄送:  Nat Sakimura <[e-mail]>
主题:  RE: New Version Notification for draft-sakimura-oauth-rjwtprof-06.txt

Hi Kepeng,
 
Thanks for the pointer to the current version of the document.  Review
comments follow…
 
Abstract – Remove the reference, since they aren’t allowed in IETF
abstracts.
 
Introduction.  I would replace this text:
   While Proof-Of-Possession Semantics for JSON Web Tokens
   (JWTs) [POPS 
<https://tools.ietf.org/html/draft-sakimura-oauth-rjwtprof-06#ref-POPS> ]
touches briefly on the Sender Constraint, it is only
   one paragraph within a introductory text and does not discuss it in
   detail.  Instead, it devotes much of the discussion to the Key
   Confirmation method.  It also is making the usage of such token
   against the resource server out of scope.
with something closer to this:
While Proof-of-Possession Key Semantics for JSON Web Tokens (JWTs) [POPS]
specifies how to use key confirmation with JWTs, it does not specify how to
use a sender constraint.
And then maybe merge this paragraph with the following one, which starts
“This discussion draft”…
 
I have no idea what the last sentence above was referring to.  I’d just
delete it.
 
Everyplace you say “Proof-Of-Possession Semantics for JSON Web Tokens
(JWTs)” you should change this to the current title “Proof-of-Possession Key
Semantics for JSON Web Tokens (JWTs)”.
 
I would delete the term “Authorized Presenter” and instead refer to the
“Presenter” term in [POPS] by reference.  The term “Authorized Presenter”
has a bad history and tends to cause more confusion than clarity when it
appears.  When you want to talk about the legitimate presenter, I would just
use a phrase such as “intended presenter” or “legitimate presenter”.  (I
would refer to an unauthorized presenter in your discussion as “the
attacker”.)
 
s/use-case/use case/
 
Delete this sentence, since Sender Constraint isn’t in scope for POPS, and
the sentence makes it sound like it is:
   Key Confirmation mechanism is
   described in OAuth 2.0 Proof-Of-Possession Semantics for JSON Web
   Tokens (JWTs) [POPS
<https://tools.ietf.org/html/draft-sakimura-oauth-rjwtprof-06#ref-POPS> ]
specification in detail, but Sender Constraint
   mechanism is not explained in detail.
 
Justification – Some places in the draft you talk about “.well-known/jwks”
(plural) and other places you talk about “.well-known/jwk” (singular).  I
suspect that multiple keys have to be published in the general case in order
to enable key rotation, with the right key selected by the “kid”.  See
http://openid.net/specs/openid-connect-core-1_0.html#RotateSigKeys for a
description of how keys are rotated in OpenID Connect.  This spec should do
the same or something similar.
 
Sender Constraint Representation – I would not overload the “azp” claim
specified in http://openid.net/specs/openid-connect-core-1_0.html#IDToken
with a different meaning.  There’s a widespread agreement in the OpenID
Connect working group that “azp” is confusing – see
https://bitbucket.org/openid/connect/issues/973/core-2-3137-azp-claim-unders
pecified-and.  John Bradley has also stated that the use you’re making of it
is different than Google’s usage (which is the only party using “azp” that
I’m aware of), and should use a different claim name.  Rather, I’d suggest
adding a new parameter name under the “cnf” claim defined by POPS to express
the presenter – possible “prsnt”.  So the syntax would be:
 
    “cnf”: {“prsnt”: “presenter identifier”}
 
and when a Key ID is needed to select from among multiple keys, the syntax
would be:
 
    “cnf”: {“prsnt”: “presenter identifier”, “kid”: “key identifier”}
 
This syntax would be more harmonized with POPS, and is the kind of extension
to “cnf” that were anticipated and enabled by the JWT Confirmation Methods
Registry in Section 6.2 of POPS.
 
Client Authentication – In (1), why is a “HEAD” request listed as a
possibility?  Why not just “GET”?
 
In (2), where is the “named” Authorization header value defined?  Please
provide a reference where you first use it.  (Oh – I just realized when I
got to 7.1 that you are defining the scheme in this specification.  I
couldn’t tell for sure until then.  I would have a separate section that’s
just about defining “named” and how it works before you use it.)
 
In (3), do you mean that the nonce is signed using the JWS Compact
Serialization, with the nonce being the JWS Payload value?  What key is used
for signing?
 
In (4), I’d rename “at” to “access_token” for consistency with other specs
and rename “s” to “signature”.
 
In (5), I’d cite RFC 7591 when you refer to Dynamic Client Registration.
Also, people won’t like the “may have been”.  You should probably define one
or a few preferred ways of doing this.
 
In (7), you need to say how the resource server verifies the access token.
What steps does it take to do this, for instance?
 
6.1 URI Client ID – As I wrote earlier, this has to be a JWK Set, with the
key being selected by “kid”, to enable key rotation.
 
6.3 Metadata about the authorization server is its discovery information.
For instance, OpenID Connect publishes this information as described at
http://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata
and the following sections.  I would describe this as “authorization server
discovery information” and probably add an informative reference to OpenID
Connect Discovery.
 
10.2 Informative References – PKCE and Introspection is now both RFCs.
Please add URLs to all of the references like those in the Normative
References.  If you use references like these, you get them automatically:
      <?rfc 
include="http://xml2rfc.tools.ietf.org/public/rfc/bibxml3/reference.I-D.draf
t-ietf-oauth-pop-architecture-03.xml" ?>
Or you can include an explicit target= value in the <reference> element,
such as in this reference:
      <reference anchor="IANA.JWT.Claims"
target="http://www.iana.org/assignments/jwt">
        <front>
          <title>JSON Web Token Claims</title>
          <author>
            <organization>IANA</organization>
          </author>
          <date/>
        </front>
      </reference>
 
I hope you find these review comments to be useful.
 
                                                                Best wishes,
                                                                -- Mike