Hi Hannes, thanks for the review. Comments inline.

On Jan 12, 2015, at 10:10 AM, Hannes Tschofenig <hannes.tschofe...@gmx.net> 
wrote:

> Hi Justin, Hi all,
> 
> I re-read the latest version of the dynamic client registration
> management document and I still have a few issues with the document.
> 
> Let me start with the document structure.
> 
> When you look at the ToC you will notice that there is something wrong.
> 
>   2.  Client Configuration Endpoint . . . . . . . . . . . . . . . .   5
>     2.1.  Client Read Request . . . . . . . . . . . . . . . . . . .   6
>     2.2.  Client Update Request . . . . . . . . . . . . . . . . . .   6
>     2.3.  Client Delete Request . . . . . . . . . . . . . . . . . .   9
>   3.  Responses . . . . . . . . . . . . . . . . . . . . . . . . . .  10
>     3.1.  Client Information Response . . . . . . . . . . . . . . .  10
>       
> Figure 1 describes a number of request and responses, namely
> * Client Registration Request and Client Information Response
> * Read or Update Request and Client Information Response
> * Delete Request and Delete Confirmation
> 
> Section 3 only covers the "Client Information Response".
> 
> It would of course been cleaner to have a response that matches the
> request. This avoids confusion. But that's where we are right now.
> 
> Currently, most of the response message content is actually described in
> Section 2 rather than in Section 3.
> 
> You may want to think about re-structure the text in the following way.
> This would also cause you to re-think a lot of the data duplication issue.
> 
> For example, an update request requires the client to re-submit all data
> and then, in the response, the authorization server has to submit a lot
> of the data back to the client again (even if it does not change it).
> That does not sound right to me.
> 
> 
>   2.  Client Configuration Endpoint
>     2.1. Client Registration Request and Client Information Response
>     2.2. Client Read Request and Client Information Response
>     2.3. Client Update Request and Client Information Response
>     2.4. Client Delete Request and Delete Confirmation
>               
> 

This suggestion sounds much more confusing than what's already there. Section 3 
describes the extensions to the client information response (as defined in 
DynReg core) that are common to all the create, read, and update requests. It 
describes the content of the response, while the information in section 2 
describes the HTTP particulars that are going to be different depending on what 
action you're taking. What you're suggesting would duplicate a large amount of 
information unnecessarily.

> Appendix B.  Registration Tokens and Client Credentials
> 
>   Throughout the course of the dynamic registration protocol, there are
>   three different classes of credentials in play, each with different
>   properties and targets.
> 
>   o  The initial access token is optionally used by the client or
>      developer at the registration endpoint.
> 
> s/client or developer/client

As in the comments on the core Dynamic Registration protocol, "client or 
developer" is correct.

> 
>         This is an OAuth 2.0
>      token that is used to authorize the initial client registration
>      request.  The content, structure, generation, and validation of
>      this token are out of scope for this specification.  The
>      authorization server can use this token to verify that the
>      presenter is allowed to dynamically register new clients.  This
>      token may be shared among multiple instances of a client to allow
>      them to each register separately, thereby letting the
>      authorization server use this token to tie multiple instances of
>      registered clients (each with their own distinct client
>      identifier) back to the party to whom the initial access token was
>      issued, usually an application developer.  This token should be
>      used only at the client registration endpoint.
> 
> [hannes] Is this correct? "should only be used at the client
> registration endpoint"
> It would be better to say that this must only be used at the client
> registration endpoint since that was the purpose for creating it.
> 

We could instead say "This token is intended to be used only at the client 
registration endpoint." 

>   o  The registration access token is used by the client or developer
>      at the client configuration endpoint and represents the holder's
>      authorization to manage the registration of a client.
> 
> s/client or developer/client

Again, no, it's correct as-is.

> 
>         This is an
>      OAuth 2.0 bearer token that is issued from the client registration
>      endpoint in response to a client registration request and is
>      returned in a client information response.  The registration
>      access token is uniquely bound to the client identifier and is
>      required to be presented with all calls to the client
>      configuration endpoint.  The registration access token should be
>      protected and should not be shared between instances of a client
>      (otherwise, one instance could change or delete registration
>      values for all instances of the client).
> 
> [hannes] The registration access token must be confidentiality protected
> in transit and also protected against unauthorized access when stored
> locally on the client.
> 

That's correct, it's an OAuth bearer token and inherits the protections needed 
thereupon.

> In what cases do you envision the registration access token to be shared
> among client instances?

Only if somebody made a mistake? That's why we say not to do it. Think of it 
this way: if the developer uses dynamic registration and then bakes the 
registration token into the client and then ships copies of said client to a 
thousand different users, that registration token isn't much of a secret 
anymore.

> 
>         The registration access
>      token can be rotated through the use of the client update method
>      on the client configuration endpoint.  The registration access
>      token should be used only at the client configuration endpoint.
> 
> [hannes] Why 'should' and not 'must'? Again, we create this registration
> access token specifically for the use with the client configuration
> endpoint.     

We've historically been light handed about telling people where to use their 
tokens. It's possible that some people want to issue tokens that do multiple 
things beyond the registration endpoint, but that's the kind of thing we need 
more deployment experience around. I suggest we use the same text suggested 
above instead of a normative-sounding phrase: "This token is intended to be 
used only at the client registration endpoint." Will that work?

>       
>   o  The client credentials (such as "client_secret") are optional
>      depending on the type of client and are used to retrieve OAuth
>      tokens.
> 
> [Hannes] Proposed text: "The use of client credentials (such as
> "client_secret") is optional
>      and depends on the type of client."
> 
>         Client credentials are most often bound to particular
>      instances of a client and should not be shared between instances.
>      Note that since not all types of clients have client credentials,
>      they cannot be used to manage client registrations at the client
>      configuration endpoint.  The client credentials can be rotated
>      through the use of the client update method on the client
>      configuration endpoint.  The client credentials cannot be used for
>      authentication at the client registration endpoint or at the
>      client configuration endpoint.
> 
> [hannes] Why do you mean that they cannot be used for authentication?
> What happens if a client uses a client credential in the interaction
> with these endpoints?

It won't work to authenticate the client, if they're following the protocol. 
That's what the registration access token is for.

> 
> B.1.  Credential Rotation
> 
>   The authorization server MAY rotate the client's registration access
>   token and/or client credentials (such as a "client_secret")
>   throughout the lifetime of the client in order to minimize the risk
>   of leakage of these credentials.
> 
> [Hannes] Proposed text: "The authorization server may be configured to
> issue new registration access
>   tokens and/or client credentials (such as a "client_secret") to the
> client
>   throughout the lifetime of the client instance to limit the lifetime
> of the credentials.
>   This may help to minimize impact of exposed credentials."
> 

I'm fine with this suggested text, thanks!

> [Hannes] Note also that the registration access token is a mandatory
> field in the client information response and so it appears that this
> rotation in some sense happens with every request/response unless the
> authorization server returns the same info all the time (which would be
> very wasteful).

It *could* happen every time, yes. Clients doing the management API need to be 
prepared for that.

> 
>   The client can discover that these
>   values have changed by reading the client information response
>   returned from either a read or update request to the client
>   configuration endpoint.
> 
> [Hannes] Proposed Text: "The authorization server conveys new new
> registration access
>   tokens and/or client credentials to the client in the client
> information response of
>   either a read or update request to the client configuration endpoint."
> 

I'm Ok with this suggested text, thanks!

>   The client's current registration access
>   token and client credentials (if applicable) MUST be included in this
>   response.
> 
> [Hannes] Shouldn't that read "The client's current registration access
>   token and client credentials (if applicable) MUST be included in the
> request."
> 

No, it's talking about the Client Information Response coming back from the 
server. I'll amend the text to say "client information response" here so it's 
clear.

>   The registration access token SHOULD be rotated only in response to
>   an update request to the client configuration endpoint, at which
>   point the new registration access token is returned to the client and
>   the old registration access token SHOULD be discarded by both
>   parties.
> 
> [Hannes] We talked about the second SHOULD statement here on the mailing
> list before. I thought I convinced you to either turn it into a MUST or
> to tell the reader when to discard and it when it should not be discarded.
> 
> If you additionally take the text in Section 2.2 into account the entire
> story is inconsistent. Here is the text:
> 
> " If the
>   authorization server includes a new client secret and/or registration
>   access token in its response, the client MUST immediately discard its
>   previous client secret and/or registration access token.  The value
>   of the "client_id" MUST NOT change from the initial registration
>   response.
> "
> 

It's not inconsistent -- the client MUST throw out the token, and the server 
SHOULD throw out the token if it can. There are plenty of OAuth implementations 
that use stateless tokens that can't be revoked actively but only expire. In 
those cases, you've got two active tokens during the expiration window, and 
that's a security/performance tradeoff made by the server implementors. What we 
really need is a new normative term: MUST IF POSSIBLE. Suggestions on how to do 
that?

> 
> Also the first SHOULD statement raises the question why it is there. Of
> course, it makes no sense to rotate the credential with a delete
> request. Is this what you are trying to say? I don't see why the
> registration access token cannot be rotated in response to a read request.

We're trying to say "don't rotate the client's registration token or secret 
without giving them a way to recover", using either update or read. Otherwise, 
the client won't be able to call the configuration endpoint because its 
credential for accessing said endpoint will be gone (which is the paragraph 
just below here, that you copy below). 

I also believe that paragraph is supposed to say "update or read", and not just 
"update", so I'll fix that inconsistency.

> 
>   If the registration access token were to expire or be
>   rotated outside of such requests, the client or developer might be
>   locked out of managing the client's configuration.
> 
>   Methods by which the client can request credential rotation are
>   outside the scope of this document.
> 
> [Hannes] Proposed text: "Note that the authorization server decides
> about frequency of credential rotation and not the client."

Thanks, I'll add that text to the end.

 -- Justin

> 
> Ciao
> Hannes
> 
> 
> 
> _______________________________________________
> OAuth mailing list
> OAuth@ietf.org
> https://www.ietf.org/mailman/listinfo/oauth

_______________________________________________
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth

Reply via email to