Justin, I will look over your comments. But looking at some, i was not looking for an explanation but rather I think the text should explain.
For example the registration access token seemed very mysterious. It took a lot of effort to understand what was going on. Maybe i am seeing this as I am looking at the spec with fresh eyes? Phil On 2013-05-15, at 17:53, "Richer, Justin P." <jric...@mitre.org> wrote: > Phil, many thanks for the extremely thorough review! It is very useful > indeed. > > My comments and responses to each point are inline. > > On May 15, 2013, at 4:30 PM, Phil Hunt <phil.h...@oracle.com> wrote: > >> It seems unfortunate that I have not gotten a chance to get into this level >> of detail much earlier. But then, I guess that's what LC review is for! My >> apologies for not getting many of these concerns to the WG much earlier. >> >> Overall >> ----------- >> I think dynamic registration is a critical part of the OAuth framework now >> that we are starting to consider how individual client applications should >> operate when there is one or more deployments of a particular resource API. >> We've moved from the simple use case of a single API provider like Facebook >> or Flickr and moved on to standards based, open source based, and commercial >> based deployments where there are multiple service endpoints and many >> clients to manage. >> >> The dynamic registration spec is actually dealing with a couple of issues >> that I would like to see made more clear in early part of the spec: >> >> 1. How is a new client application recognized for the first time when >> deployed against a particular SP endpoint? Should clients actually assert >> an application_id UUID that never changes and possibly a version id that >> does change with versions? > > In the general case, why is any recognition required? If you're doing things > as part of a larger protocol ecosystem, like Blue Button+ or a particular > OpenID Connect provider, then you can define semantics for tying together > classes of clients (see below for more discussion on this very point). But in > general, a client is just going to show up as a new instance to the AS and > get issued a new, unique client_id, and that's that. > >> >> 2. How are client credentials managed. Are we assuming client credentials >> have a limited lifetime or rotation policy? > > The intent was that client_secret could be rotated, as indicated by the > expires_at member of the response. If a client's secret expires, it calls the > read operation on the Client Configuration Endpoint (§4.2) to get its new > client_secret. If this is unclear in the current text (which I suspect it may > be after multiple refactorings), then I welcome suggestions and specific text > as to how to make that clear. > >> How does a client authenticate the first time and subsequent times to the >> registration service? > > This is a separate question all together, as it does not involve the > client_secret or client_id at all. Rather, the first time the client shows up > to the registration service, it may either: > - Not authenticate at all (this is the true public, open registration, and > it is recommended that servers do this) > - Authenticate using an OAuth 2.0 token (which ATM means a bearer token). > How the client gets that bearer token and what the bearer token means to the > AS beyond "this client is authorized to register" is out of scope for the > spec, by design. > > Subsequent times that the same registered client (by which we mean the same > instance of a client with a particular client_id) shows up at the > registration endpoint (actually, the Client Configuration Endpoint), it uses > its Registration Access Token that it was issued on initial registration. > This is an OAuth 2.0 Bearer token that is unique to the client instance. > >> >> 3. How is versioning of clients managed? Should each new version of a >> client require a change in client registration including possibly changing >> client_id and authentication credential? I don't have a strong feeling, but >> it is something that implementors should consider. > > This is up to the AS, really, and I don't think that any global policy would > ever fly here. Especially if you consider that the entire notion of "version" > is more fluid these days than it ever has been. I wouldn't mind adding a > discussion in the security considerations if it merits mentioning though, so > that we can help both client developers and server developers institute > reasonably good policy. > >> >> 4. What is the registration access token? Why is is used? What is its >> life-cycle? When is it issued, when is it changed? When is it deleted? > > See the response above above and the definition in §1.2: > > Registration Access Token: An OAuth 2.0 Bearer Token issued by the > Authorization Server through the Client Registration Endpoint which is used > by the Client to authenticate itself during read, update, and delete > operations. This token is associated with a particular Client. > > If this can be clarified, I welcome text suggestions. > >> >> If we distinguish between first time registration of a particular client >> software package, it is possible that somethings like authentication method >> can be negotiate in or out-of-band at that time. Subsequent registrations >> should only have parameters for items that could change per deployment (like >> tos_uri). I think token_endpoint_auth_method is one thing that should not >> be negotiated per instance. > >> When subsequent instances register, I have to ask the question, for example >> when would things like "token_endpoint_auth_method" be negotiated or be >> different for the same client software? Should not all instances use the >> same authentication method. > > > I'm confused by this -- as far as the dynamic registration spec is concerned, > all instances are separate from each other. All parameters change per > instance. And instance, you should keep in mind, is defined as any one copy > of the client code connecting to any new authorization server. That pairing > creates the client_id, and therefore the instance, and therefore the > registration access token, and therefore the registration itself at a > conceptual level. So there is no way other than per-instance for a client to > ask for a particular token_endpoint_auth_method. Where else would the AS find > out about it? And there's no way other than per-instance for the server to > tell the client which token_endpoint_auth_method to use. All instances will > probably ask for the same token_endpoint_auth_method from all auth servers > they talk to, right? And each AS will tell each instance that registers with > it to use a particular auth method. There is no way to tie together different > instances across (or within) auth servers without specifying a significant > amount of other machinery. > > Which is not to say that it's not useful in some circumstances to tie > together different instances of the same code across (or within) auth > servers. This is why, with Blue Button+, we specified a specific token format > for the initial access token that the clients use to call the registration > endpoint the first time, as well as a discovery protocol against a > centralized server that handles pre-registration. All of this machinery is, > in my opinion, a stupendous overkill for the general case, though if some > folks find use for it outside of BB+ then it'd be a good thing to abstract > out and make its own spec that extends the Dyn Reg spec in a fully compatible > way. Furthermore, even in Blue Button+ we specify that all auth servers MUST > also accept open registration, without an initial access token, where the > client simply shows up and says "hey, here are my parameters." The auth > server has no way to know in this case any kind of out-of-band negotiation > for different things. In BB+ we are writing very specific policy guidelines > about how to present the UX and things to the end user for all of these > different cases. But again, all of this is specific to the BB+ use case, and > I don't see value in dragging it in to the registration spec on its own. I > believe it would be far too limiting. > >> >> Finally, there seems to be an inconsistent style approach with >> draft-hardjono-oauth-resource-reg which uses ETags. Should this draft do so >> as well? > > That's an individual submission, not a working group draft. Nobody has, until > now, even mentioned the use of ETags here. UMA (where the resource > registration draft comes from) uses ETags, hence their inclusion there. I > personally don't see their utility here, though, and I wouldn't want to > include a wholly new mechanism this late. > >> >> Specific items: >> --------------------- >> "token_endpoint_auth_method" >> >> There is some confusion as to whether this applies to server or client >> authentication. Suggest renaming parameter to >> "token_endpoint_client_auth_method" > > This is the first I've heard of this particular confusion. I'm fine with > either name but at this stage I don't want to make syntax changes without > very, very compelling reasons to do so. > >> >> * Currently, the API only supports a single value instead of an array. If >> the purpose is to allow the client to know what auth methods it supports, >> this should be an array indicated what methods the client supports - or it >> should not be used. > > I would rather like this to be an array, personally, and brought it up with > the OpenID Connect working group about six months ago. But there it was > decided that a single value was simpler and sufficient for the purpose. The > IETF draft has inherited this decision. I *believe* (though am not 100% > positive) that I brought up this very issue in the WG here but didn't receive > any traction on it, so single it remains. > > Also note that this field, like all others in §2, represents two things: the > client telling the server "I want to use this value for this field", and the > server telling the client "this is the value you have for this field". It's > not exactly a negotiation, more like making a polite request to an absolute > dictator who has the last word anyway. But at least this dictator is nice > enough to tell you what their decision was instead of just decapitating you. > >> >> * There is no authn method for "client_secret_saml" or "private_key_saml". > > Nobody has really asked that these two be included or specified. The > extension mechanism (using an absolute URI) would allow someone else to > define these. Is the definition in the SAML Assertion draft sufficient for > their use? > >> >> There are no profiles referenced or defined for "client_secret_jwt" or >> "private_key_jwt". Neither of the JWT or SAML Bearer drafts referenced cover >> these types of flows. They only cover bearer flows. "client_secret_jwt" and >> "private_key_jwt" seem to have some meaning within OpenID Connect, but I >> note that OIDC does not fully define these either. > > The JWT assertion draft does say how to use a JWT for client authentication, > and the DynReg text differentiates between a client signing said JWT with its > own secret symmetrically vs. a client using its own private key, > asymmetrically. > >> >> There is no authentication method defined for "client_bearer_saml" or >> "client_bearer_jwt" or "client_bearer_ref". Since the bearer specs say this >> is acceptable, the dynamic registration spec should accept these. > > I don't understand this bit -- where are these defined in RFC6750? I don't > even know what client_bearer_ref would refer to. > >> >> A possible suggestion is to remove client_secret_jwt and private_key_jwt and >> define those as register extensions since these profiles are not defined. > > That's possible, but they are in active use already. > >> >> >> About "tos_uri" and "policy_uri" >> >> The distinction between tos_uri and policy_uri is unclear. Can we clarify >> or combine them? > > Terms of service and policy are two different documents. One is something > that a user accepts (generally describing what the user will do), one is a > statement of what the service provider (in this case, the client) will do. > More importantly, we used to have only one, and several people asked for them > to be split. > >> >> Also, aren't these really URIs or are they meant to be URLs? > > There was already discussion about this on the list: The IETF is apparently > trying to deprecate URL in favor of URI in new specs. So in practice they'll > nearly always be URLs, but since all URLs are URIs we're not technically > incorrect in following the new policy. And it makes the IESG less mad at us, > which is a plus. > >> >> About "jwks_uri" >> >> A normative reference for >> http://tools.ietf.org/html/draft-ietf-jose-json-web-key-09 is needed. > > Yes, you're correct, I'll add that. > >> >> Should be URL instead of URI? > > See above. :) > >> >> Section 2.1 >> >> In the table urn:ietf:params:oauth:grant-type:jwt-bearer >> is missing. > > It's there in the copy of -10 I'm reading off of ietf.org right now … ? > >> >> “As such, a server supporting these fields SHOULD take steps to ensure that >> a client cannot register itself into an inconsistent state.” >> >> We may want to define more detailed HTTP error response. E.g. 400 status >> code + defined error code (“invalid_client_metadata”)? > > I'd be fine with defining a more explicit error state in this case. I think > it would help interop for the servers that want to enforce grant-type and > response-type restrictions, but servers that can't or don't care about > restricting grants types and whatnot don't have to do anything special. > >> >> Section 2.2 >> >> May want to add: >> >> When “#” language tag is missing, (e.g. “#en” is missing in “client_name”, >> instead of “client_name#en”) the OAuth server may use interpret the language >> used based on server configuration or heuristics. > > That seems inconsistent with what we already have: > > If any human-readable field is sent without a language tag, parties using it > MUST NOT make any assumptions about the language, character set, or script of > the string value, and the string value MUST be used as-is wherever it is > presented in a user interface. > > Which is to say, treat it as a raw byte-value-string and don't try to get > fancy. > >> >> Section 3 >> >> Existing text: >> >> “In order to support open registration and facilitate wider >> interoperability, the Client Registration Endpoint SHOULD allow initial >> registration requests with no authentication. These requests MAY be >> rate-limited or otherwise limited to prevent a denial-of-service attack on >> the Client Registration Endpoint.” >> >> I would suggest to change the first “SHOULD” to “MAY”. In most cloud >> services, the first client is registered by a human user. Then, other >> clients can be further used to automate other client registration. So, the >> first request would typically come with an authenticated user identity. > > I think the weight of "SHOULD" is appropriate here, because I believe that > turning off open registration at the AS (which is what this is talking about) > really ought to be the exception rather than the rule. > >> >> On the flip side, the earlier paragraph: >> >> “The Client Registration Endpoint MAY accept an initial authorization >> credential in the form of an OAuth 2.0 [RFC6749] access token in order to >> limit registration to only previously authorized parties. The method by >> which this access token is obtained by the registrant is generally >> out-of-band and is out of scope of this specification.” >> >> I tend to think it would be better to change this “MAY” to “SHOULD”. >> >> That access token would carry a human user authenticated identity somehow. >> In some cases, it can be a pure authenticated user assertion token. > > Again, disagree, for the same reasoning as above. > >> >> About section 4.3: >> >> If the client does not exist on this server, the server MUST respond >> with HTTP 401 Unauthorized, and the Registration Access Token used to >> make this request SHOULD be immediately revoked. >> >> If the Client does not exist on this server, shouldn't it return a "404 Not >> Found"? >> >> If revoking the registration access token, is it bound to a single client >> registration? This is not clear. What is the lifecycle around registration >> access token? Only hint is in the Client Information Response in section 5.1. > > The language about the 401 here (and in other nearby sections) is > specifically so that you treat a missing client and a bad registration access > token the same way. You see, returning a 404 here actually indicates things > were in an inconsistent state. Namely, that the registration access token was > still valid but the client that the registration access token was attached to > doesn't exist anymore. The registration access token in meant to be tied to a > client's instance (or registration), so it's actually more sensible to act as > though the registration access token isn't valid anymore. In at least some > implementations (specifically ours at MITRE that's built on SECOAUTH in > Java), you'd never be able to reach the 404 state due to consistency checking > with the inbound token. > > Since the intent of the registration access token is that it's bound to a > single instance, its lifecycle is generally tied to the lifecycle begins at > the issuance of a new client_id with that instance. That token might be > revoked and a new one issued on Read and Update requests to the Client > Configuration Endpoint (and the client needs to be prepared for that -- same > as the client_secret), and it will be revoked when the client is deleted > either with a Delete call to the Client Configuration Endpoint or something > happening out-of-band to kill the client. > > Should we add more explanatory text to the definition in the terminology > section? Elsewhere? I'm very open to concrete suggestions with this, since I > don't think it's as clear as we'd like. > >> >> About section 5.1: >> >> Is registration_access_token unique? Or is it shared by multiple instances? >> If shared, then registration_access_token can't be revoked on delete of >> client. >> >> Suggest rename “expires_at” to “client_secret_expires_at”, >> >> Suggest to rename “issued_at” to “id_issued_at” > > While I do like the suggestion of changing these to client_secret_expires_at > and client_id_issued_at, and I think these are more clear and readable, I > don't want to change the syntax during last call unless there is a clear need > and a clear consensus for doing so. > >> >> Section 7 >> >> When a client_secret expires is it the intent that clients do an update or >> refresh to get a new client secret? > > Yes, the client is supposed to either call the read or update methods on the > client configuration endpoint to get its new secret. As discussed above, I'm > not sure that's as clear as it needs to be, and I welcome suggestions on how > to clarify this. > > > > Again, thanks for the very thorough read through. Have you implemented any of > the spec yet, either as-is or with any of your suggested changes? > > -- Justin
_______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
