Re: [OAUTH-WG] LC Review of draft-ietf-oauth-dyn-reg-12

"Richer, Justin P." <jricher@mitre.org> Tue, 04 June 2013 19:57 UTC

Return-Path: <jricher@mitre.org>
X-Original-To: oauth@ietfa.amsl.com
Delivered-To: oauth@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id E507621F949D for <oauth@ietfa.amsl.com>; Tue, 4 Jun 2013 12:57:30 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -6.226
X-Spam-Level:
X-Spam-Status: No, score=-6.226 tagged_above=-999 required=5 tests=[AWL=0.373, BAYES_00=-2.599, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_MED=-4]
Received: from mail.ietf.org ([12.22.58.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1reZmxaF5B48 for <oauth@ietfa.amsl.com>; Tue, 4 Jun 2013 12:57:25 -0700 (PDT)
Received: from smtpksrv1.mitre.org (smtpksrv1.mitre.org [198.49.146.77]) by ietfa.amsl.com (Postfix) with ESMTP id D1E5A21F9C74 for <oauth@ietf.org>; Tue, 4 Jun 2013 12:45:39 -0700 (PDT)
Received: from smtpksrv1.mitre.org (localhost.localdomain [127.0.0.1]) by localhost (Postfix) with SMTP id 483D61F0289 for <oauth@ietf.org>; Tue, 4 Jun 2013 15:45:39 -0400 (EDT)
Received: from IMCCAS03.MITRE.ORG (imccas03.mitre.org [129.83.29.80]) by smtpksrv1.mitre.org (Postfix) with ESMTP id 3A17C1F031F for <oauth@ietf.org>; Tue, 4 Jun 2013 15:45:39 -0400 (EDT)
Received: from IMCMBX02.MITRE.ORG ([169.254.2.12]) by IMCCAS03.MITRE.ORG ([129.83.29.80]) with mapi id 14.02.0342.003; Tue, 4 Jun 2013 15:45:38 -0400
From: "Richer, Justin P." <jricher@mitre.org>
To: "Anganes, Amanda L" <aanganes@mitre.org>
Thread-Topic: [OAUTH-WG] LC Review of draft-ietf-oauth-dyn-reg-12
Thread-Index: AQHOYUzJ9GUoAoQpNkKnX9uw4Utq9JkmOKCA
Date: Tue, 04 Jun 2013 19:45:37 +0000
Message-ID: <727692BF-DAAF-480F-9B47-0FF1C495E230@mitre.org>
References: <MLQM-20130604143524551-2088@mlite.mitre.org>
In-Reply-To: <MLQM-20130604143524551-2088@mlite.mitre.org>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.146.15.112]
Content-Type: multipart/alternative; boundary="_000_727692BFDAAF480F9B470FF1C495E230mitreorg_"
MIME-Version: 1.0
Cc: "oauth@ietf.org" <oauth@ietf.org>
Subject: Re: [OAUTH-WG] LC Review of draft-ietf-oauth-dyn-reg-12
X-BeenThere: oauth@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: OAUTH WG <oauth.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/oauth>, <mailto:oauth-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/oauth>
List-Post: <mailto:oauth@ietf.org>
List-Help: <mailto:oauth-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/oauth>, <mailto:oauth-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 04 Jun 2013 19:57:31 -0000

Amanda, thanks for the review -- comments inline.

On Jun 4, 2013, at 1:56 PM, "Anganes, Amanda L" <aanganes@mitre.org<mailto:aanganes@mitre.org>> wrote:

[[Apologies if you receive this twice, I accidentally sent this from one of my other email addresses this morning (Outlook seems to have been confused).]]

Hello,

I have reviewed the Dynamic Registration draft and offer some comments below:

After section 1.2, I suggest adding a flow diagram (similar to those in the core OAuth 2.0 spec), showing the interaction of the client/developer with the respective endpoints, and the requests and responses involved. I think this will make the spec easier to read.

I think that this makes a lot of sense, and I'll add one to the next version of the spec. This is what I've got to start with:

       +--------(A)- Initial Access Token
       |
       v
 +-----------+                                      +---------------+
 |           |--(B)- Client Registration Reqeust -->|    Client     |
 |           |                                      | Registration  |
 |           |<-(C)- Client Information Response ---|   Endpoint    |
 |           |                                      +---------------+
 |           |
 |           |                                      +---------------+
 | Client or |--(D)- Read or Update Request ------->|               |
 | Developer |                                      |               |
 |           |<-(E)- Client Information Response ---|    Client     |
 |           |                                      | Configuration |
 |           |                                      |   Endpoint    |
 |           |                                      |               |
 |           |--(F)- Delete Request --------------->|               |
 |           |                                      |               |
 |           |<-(G)- Delete Confirmation -----------|               |
 +-----------+                                      +---------------+

(Apologies if that ascii art doesn't translate through my mail client.)



Inside the 3rd paragraph of section 1.3, there is text buried that talks about client credential rotation (client_secret and Registration Access Token). I think this notion of secret rotation should be called out in its own paragraph or subsection and labeled as such. Suggested text (I'm not sure where it should go):

Section X.X Client Credential Rotation
The Authorization Server may rotate the Client's issued client_secret and/or Registration Access Token. The client_secret MAY be rotated at any time, in which case the Client will likely discover that their secret has expired via attempting and failing to make a request. The Registration Access Token SHOULD only be rotated in response to an update or read request, in which case the new Registration Access Token will be returned in the response back to the Client. The Client can check their current credentials at any time by performing a READ or UPDATE operation at the Client Configuration Endpoint.


This makes sense. I had tried to use §1.3 to talk about this, but I can pull it out as a separate section, like a subsection to 1.3, and I'll incorporate some of your text.

Section 3 Client Registration Endpoint

This section should use the phrase "this endpoint may be open, or it may be an OAuth 2.0 Protected Resource", rather than just stating that it may accept an initial token. I *think* that the choice is an either/or for a given server (ie, a server cannot offer both open and protected registration), but that should be clarified as well.


It's not an either/or choice, you could easily do both on the server and switch your functionality based on whether or not there's an authentication token in the request. There are plenty of OAuth-protected APIs that do this today, where if you call without a token then you get public information, but if you call with a token you get protected information. We want this to follow the same pattern, so I'll work on the language there.

In the 4th paragraph, the final sentence ("As such, the Client Configuration Endpoint MUST…") refers to the Configuration endpoint, not the Registration endpoint. The text is already in Section 4 so it should just be deleted here.

Hm, looks like vestigial text from a very old revision where everything had to go through a single endpoint (and we had an operation parameter).


Section 3.1 Client Registration Request

The lead-ups to the two example requests are phrased oddly. The difference between the two sample requests is not whether the client has a token, it's whether the endpoint is open or protected. Suggest changing them to "For example, if the Client Registration Endpoint supports open registration, the client could send the following request" and "Alternatively, if the endpoint is an OAuth 2.0 protected resource, the client MUST include an OAuth 2.0 Access Token/the Initial Access Token in its request, presented as a Bearer token using the Authorization header according to RFC6749." (My phrasing at the end regarding how to present the AT may be off; point is that it should be called out.)

Calling it out more explicitly as an OAuth 2.0 protected resource here (and elsewhere) will help.


Nits in section 7 Security Considerations:

2nd paragraph, first sentence: "…requests to the *Client* Registration Endpoint"

Yup, my mistake, thanks.


6th paragraph, first sentence: "…the Registration Access Token should not expire…" I think this should be a SHOULD NOT?
Same paragraph, 4th sentence has a non-capitolized "client" that should be "Client" (although, after reading Hannes' review, maybe the capitalized instances should all be lowercased instead).

I also second Hannes' comment that the RFC 2119 language feels off throughout the spec, suggest doing a careful read to check those.


In general, this is an open question I have about where the appropriate use of the 2119 words should go, and I'm hoping for feedback from people with more spec-writing experience with this.

Finally, to address the Initial Access Token / Registration Access Token discussion that has been ongoing:

My initial response after reading this draft (and having followed the discussion) was to say, remove the "Initial Access Token" term completely and instead just clarify the text to say that "the Client Registration Endpoint MAY be an OAuth 2.0 protected resource, but the details of how a given client or developer goes about acquiring an Access Token for use at this endpoint is out-of-scope". I spoke to Justin about this and he pointed out that this term was only added recently, and it was added because of confusion around how the Client Registration Endpoint was defined, and what it means to authenticate to it. I don't think the new name/definition/explanation has helped; but the previous drafts were also missing the "OAuth 2.0 protected resource" language. To be clear, I think this functionality is absolutely necessary, but we need to clarify its explanation

I agree that a new name would be great -- I picked "Initial Access Token" because we needed to call it *something*, and Mike suggested that we call it out in the Terminology section as an explicit term to give us a starting point. I don't like the name as it is myself -- it's too generic, doesn't really tell you what the token's for. Even in offline conversations about this document over the last month, I've taken to just picking a random term to use with my interlocutor in order to refer back to it as a concept. I completely agree that the functionality is essential and we can do a better job of explaining it.

Naming things is hard, and I am very open to suggestions on a new name! Please suggest things!


On the other hand, the Registration Access Token seems very clear to me. I think that term should be called out as a named entity, since it is 'special' - it's not issued by a token endpoint, but by the Client Registration Endpoint.

I see a few options that might help:

1) Perhaps "Initial Access Token" is a bad name. Unfortunately, I think the right names are "Registration Access Token" for accessing the Registration Endpoint, and "Configuration Access Token" for accessing the Configuration Endpoint. This would require changing code, since the configuration token is returned in the Client Registration Response.

"Registration Access Token" is pretty baked in because that's the parameter name (registration_access_token) that's returned from the registration endpoint in the JSON object. I'm not comfortable with changing that at this stage unless there's a very wide support for it from across the working group.


1.a) The examples in Appendix B only use the IAT for Developer authentication/tracking (all clients registered using the same IAT can be traced to the developer that was issued that particular token). Is that the only use case? If there is always a developer in the loop in the protected case, then "Developer Access Token" might be appropriate. This is less generic than the suggestion above, but would not require changing code and might be an improvement over what's there now.

I'm not very comfortable with tying the token explicitly to the developer because that implies that it's some kind of assertion. It might be an assertion but it might just be an OAuth token that's used in the build/deploy process to meter access to the registration endpoint. As Phil has suggested, and I agree, the dynamic registration spec should be silent as to the content and structure of the token, just like RFC6749/6750 is.


2) Perhaps if the spec language were clarified and used the "OAuth 2.0 protected resource" language, the Initial Access Token term could be removed from the document entirely. I don't think the previous drafts got it right, but I think we can do better than those explanations while still avoiding giving a fancy name to something that is *just* an OAuth 2.0 Access Token.

The problem is that I've found that it's a *particular* OAuth 2 access token, and it's distinct from the Registration Access Token. The Initial Access Token might be a JWT, it might be a SAML blob, it might be a hex blob issued locally (as Phil has suggested), and it might not be a bearer token at all (if we can ever finish other token types in the WG). I think we need it to be "an OAuth 2.0 Access Token optionally used for authorized calls to the Registration Endpoint" and leave it at that, but I do still think it needs some kind of name.

Thanks,

 -- Justin


--Amanda


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