And by “6790” below I obviously mean RFC6749. (Note to self, don’t write working group emails this early in the morning.)
— Justin On Nov 26, 2014, at 8:31 AM, Justin Richer <jric...@mit.edu> wrote: > > On Nov 26, 2014, at 2:35 AM, Hannes Tschofenig <hannes.tschofe...@gmx.net> > wrote: > >> Hi Justin, >> >> thanks for your quick response. A few remarks below. >> >> On 11/24/2014 10:11 PM, Richer, Justin P. wrote: >>> Hannes, thank you for the review. Answers inline. >>> >>> On Nov 24, 2014, at 3:09 PM, Hannes Tschofenig >>> <hannes.tschofe...@gmx.net> wrote: >>> >>>> Hi Justin, Mike, John, Maciej, >>>> >>>> as part of my shepherd write-up I carefully read through >>>> draft-ietf-oauth-dyn-reg-management-05. Overall the document is in >>>> good shape but I have a few minor clarification questions that >>>> should be resolved before the document goes to the IESG. >>>> >>>> In Section 1.4. "Registration Tokens and Client Credentials" you >>>> explain the different credential types and I was wondering why you >>>> aren't issuing client_secrets to all clients instead of introducing >>>> another credential, the registration access token. While you try to >>>> explain the reasons those appear somewhat constructed. For example, >>>> you say that "not all types of clients have client credentials" but >>>> you are the author of the specification and you can essentially >>>> decide what to do. The registration access token is essentially the >>>> same as the client credential since it is "is uniquely bound to the >>>> client identifier". >>>> >>>> I believe we have discussed this issue in the past but I don't >>>> remember what the reasoning was. Can you describe what your design >>>> rational was (and add it to the document)? >>> >>> That's exactly the question this section is trying to answer, and if >>> it can be made clearer then please suggest some text that will help >>> that purpose. The rationale for the registration access token is very >>> simple: we want to have a means to call the management endpoint in a >>> protected manner, and treating the management endpoint as an OAuth >>> Protected Resource makes a lot of sense. RFC6750 gives us the means >>> to make an authorized call to an API endpoint, so we're re-using that >>> instead of trying to invent some other mechanism. Shouldn't we be >>> helping the world get away from API passwords? >>> >>> And it's very true that not every client is going to have a client >>> secret to use at the token endpoint -- some will have no use of one >>> (public and implicit clients) and some will be using TLS or >>> asymmetric keys (JWT assertions, for instance) or other mechanisms to >>> authenticate that don't involve the secret. Instead of forcing these >>> clients to use a client secret for one new endpoint, or forcing that >>> endpoint to potentially support the infinite number of client >>> authentication mechanisms, we decided to just use OAuth. These are >>> OAuth clients, remember -- they will *all* have the ability to send >>> OAuth tokens to a protected resource already built in. >>> >> >> Here is the point I don't quite get: the registration access token is >> essentially the same as a client secret. You as a specification author >> essentially decides whether clients will get a client secret or a >> registration access token. It is under your control. >> >> To be more specific, here is what you could have done: >> >> C -> S: Client Registration Request >> C <- S: Client Information Response (with new client secret) >> >> C -> S: Read or Update Request (with client id/client secret) >> C <- S: Client Information Response >> >> Instead, you currently have this exchange: >> >> C -> S: Client Registration Request >> C <- S: Client Information Response (with new reg. access token) >> >> C -> S: Read or Update Request (with client id + reg. access token) >> C <- S: Client Information Response >> >> Do you see the similarity that I see? > > For clients that use a client secret to authenticate to the token endpoint, > your point makes sense, but for clients who would not otherwise have a client > secret, it doesn’t. You end up with a confusing state where you’re giving > somebody a “client_secret” which is defined in 6790 to be used at the token > endpoint but then telling them not to use it at the token endpoint (where > they use something else or do’t use it at all) but to use it at this new > endpoint instead. Can you see the confusion point here? I think it’s only > going to get worse as we get more clients that use auth mechanisms that don’t > depend on a shared secret (public key, TLS, etc.). > > Instead we’re giving people an OAuth access token to access a protected API, > something that OAuth clients ought to know how to do anyway. This mechanism > also gives us an opportunity to potentially use the new PoP tokens (once > they’re actually defined) in place of the bearer credential that’s defined > today. > >> >>>> >>>> In Section 1.4.1. "Credential Rotation" you write: >>>> >>>> " 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. " >>>> >>>> You might want to add reasons why the authorization server may want >>>> to take this step. >>> >>> OK, but there's nothing normative here in my view. It's basically up >>> to the auth server to say "well let's make a new credential". Can you >>> suggest text that would clarify this? >>> >> >> What about making the spec simpler by not allowing credential rotation? >> Rekying is a useful concept under two circumstances, namely >> * when they provide a performance improvement (such as it is the case >> with session resumption in TLS where you can do an abbreviated handshake >> without all the heavy public key crypto) >> * when the entrophy of the key is exhausted, which typically happens if >> you exceed a number of data packets sent under a given key. Often this >> is tied to the way how freshness is ensured and the need to avoid >> encrypting data with the same initialization vector/nonce twice. >> >> Neither of these two cases seem to apply here (IMHO). > > > Rekeying the client is useful in a whole lot more cases than these two, and > most of them boil down to “Something seems fishy”. I think if anything we > need to eventually figure out how to do *more* secret rotation, including a > proactive mechanism started by the client (as Brian has mentioned, among > others). > >> >> >>>> >>>> There are also a bit too many SHOULDs in the document. Every time >>>> you write SHOULD you have to keep in mind to tell the reader what >>>> happens in the other case. For example, in Section Section 1.4.1 >>>> you write: >>>> >>>> " 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. " >>>> >>>> Here the question arises whether you are using the SHOULD to >>>> indicate that the authorization server does not always need to >>>> rotate the registration access token and if he does then is must >>>> only happen in response to an update request or does it indicate >>>> that the authorization server could also rotate it in response to >>>> other calls? >>> >>> It's more that the server SHOULD NOT throw out a registration access >>> token that a client still thinks is good without some way to >>> communicate the new token to the client. Think of it this way, you've >>> got the token, the server decides to chuck it on you -- what do you >>> do? You can try calling your READ or UPDATE operations to get the new >>> one but no dice, your token is bad. So what we're saying here is not >>> to throw out the client's only means of finding out if its keys are >>> good anymore. >> >> I think I got confused with the description of the state management (as >> described in the document). There is some fuzziness. >> >> Here I would prefer to have either no rekeying (which would make the >> issue go away) or a very deterministic way of rekeying. >> >> I prefer the former. > > I’m not a fan of enforcing permanent credentials on the world. And we have > the same construct with refresh tokens today, for what it’s worth. > >> >>> >>>> >>>> Also, in the second line one would wonder why the old registration >>>> access token isn't mandatorily discarded. If there are good >>>> reasons, then tell the reader. >>> >>> We've tried to put requirements on the server discarding tokens in >>> the past, but there was significant pushback from providers who use >>> non-revocable time-limited tokens. Maybe they won't be doing that >>> here, for the reason above. >> >> >> I wouldn't reflect that in the document. Of course, you can never be >> sure that the implementation really discards any state but for the >> purpose of the protocol state machine it is gone. >> >>> >>>> Furthermore, the text in Section 2.2 seems to contract this >>>> statement: " 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. " >>> >>> This is a requirement on the client, not the server, so it's not >>> bound by the token lifetime restrictions above. We're telling the >>> client to stop using a secret or token after it's been given a new >>> one, that's fine. >> >> OK >> >>> >>>> In Section 2.2 you write: " However, since read operations are >>>> intended to be idempotent, the read request itself SHOULD NOT cause >>>> changes to the client's registered metadata values. " >>>> >>>> I am wondering whether this SHOULD NOT statement adds a lot of >>>> value since you allow the request to change metadata values. >>> >>> I think you're right, since the metadata values might have changed >>> through outside forces since the client last read, and all the >>> clients need to be able to deal with that. We can remove that line. >> >> OK >> >>> >>>> You also write the security consideration section: " the >>>> registration access token MAY be rotated when the developer or >>>> client does a read or update operation on the client's client >>>> configuration endpoint. " >>>> >>>> This means that the content of the registration access token may >>>> also change with a read operation. >>> >>> That's correct. >>> >>>> Terminology: >>>> >>>> Sometimes you write "Client Information Response" and sometimes >>>> "client information response" The same with "Authorization Server" >>>> and "authorization server" >>> >>> They're all supposed to be lower cased, as is the style in RFC6749. I >>> tried to bump everything down in a previous edit but it looks like I >>> missed some. >>> >>>> Typo: >>>> >>>> " Some values in the response, including the "client_secret" and >>>> r"egistration_access_token", MAY be ^^^^^^^^^^^^^^^^^^^^^^^^^^^ >>>> different from those in the initial registration response. " >>> >>> Thanks, noted! >>> >>>> >>>> In Section 2.4 "Client Delete Request" you write: >>>> >>>> " The authorization server SHOULD immediately invalidate all >>>> existing authorization grants and currently-active tokens >>>> associated with this client. " >>>> >>>> Under what circumstances wouldn't the authorization invalidate all >>>> grants and active tokens? >>> >>> When it's using a non-revocable stateless token and it can't >>> physically do that. Too bad 2119 doesn't have MUST IF POSSIBLE or >>> equivalent. >> >> Maybe it would be good to add this information. >> >> It might also be worthwhile whether this notion of a non-vocable >> stateless token actually exists in this context since we are really >> talking about the same infrastructure here. This registration management >> mechanism is really very central to the authorization server (unlike >> some other access token mechanisms where we talk about the resource >> server, which may be in a different administrative domain even). > > Why doesn’t the existing SHOULD cover this? > >> >>> >>>> You might also want to say what tokens you are talking about since >>>> there are at least the following tokens around: - access tokens - >>>> refresh tokens - registration access tokens - initial access token >>> >>> OK, we can add that. >>> >>>> >>>> " If the server does not support the delete method, the server >>>> MUST respond with an HTTP 405 Not Supported. " >>>> >>>> Why aren't you demanding that the server must support this method? >>>> This would essentially mean that there would be some cases where >>>> deregistration isn't possible. Of course, there may be the question >>>> how important deregistration actually is if the registration >>>> automatically expires. >>> >>> Because delete is not always an easy operation to implement. The >>> client should be able to call the endpoint with the DELETE verb and >>> at least know if it's allowed to do that or not. >> >> Hmmm. I didn't know that the delete method is difficult to implement. > > Depends on your infrastructure and how things get propagated between > components. > >> >>> >>>> >>>> You write: " 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 and someone sends a request with a >>>> random registration access token I am not sure what exactly you >>>> want to revoke here. >>> >>> It's not the case of a random token, it's the case of a client having >>> been deleted but using an otherwise valid access token. If the >>> token's no good, you don't get this far -- you return a 401 as >>> defined in RFC6750. >> >> I guess it might make sense to add this information. > > It’s already in there, in the paragraph just before the one you quoted: > > If the registration access token used to make this request is not > valid, the server MUST respond with an error as described in OAuth > Bearer Token Usage [RFC6750]. > > >> >>> >>>> >>>> In Section 3.1. "Client Information Response" you state the new >>>> elements that are returned to the client. While the client_secret >>>> has an expires_at field the registration_access_token doesn't. Does >>>> the expiry value of the client_secret_expires_at automatically >>>> indicate the lifetime of the registration access token? I think >>>> so. But what happens if the client_secret is not issued? To make it >>>> more complicated you write the following text in the security >>>> consideration section: >>>> >>>> " While the client secret can expire, the registration access >>>> token SHOULD NOT expire while a client is still actively >>>> registered. " >>> >>> There isn't a separate expiration for the registration access token >>> because it's not supposed to unceremoniously expire on a client. It >>> should be good until it gets rotated out on a future read/update >>> operation or the client's registration is no good anymore. >> >> I think it might be good to have a small section that explains how state >> management works. > > Can you suggest text for this beyond the paragraph that’s already there? > >>> >>>> >>>> The IANA consideration section is empty. Wouldn't it be necessary >>>> to register 'registration_access_token' and >>>> 'registration_client_uri' into the OAuth Dynamic Registration >>>> Client Metadata Registry? >>> >>> No, these are not client metadata. The client can not send these in a >>> registration request, so they don't need to be in there. >> >> Really? >> >> I thought that the IANA registry created with Section 5.1 of >> http://tools.ietf.org/html/draft-ietf-oauth-dyn-reg-20 was meant to be >> used with the Client Registration Request and the Client Registration >> Response exchange. The 'registration_access_token' and >> 'registration_client_uri' parameters are used in the response. >> >> Looking again at draft-ietf-oauth-dyn-reg-20 I noticed an inconsistency: >> The protocol interaction should either be >> >> >> C -> AS: Client Registration Request >> C <- AS: Client Registration Response >> >> OR >> >> C -> AS: Client Registration Request >> C <- AS: Client Registration Error Response >> >> Currently, sometimes the term "Client Registration Response" or "Client >> Information Response" is used. We need to fix this since it spills over >> to the management API document as well. > > Client Information Response and Client Error Response are sub-classes of > Client Registration Response. If that’s not clear from the current document, > please suggest new wording to make it clearer. > >> >> Also, I noticed that we say that the server MUST support TLS 1.2 RFC >> 5246 and/or TLS 1.0. We definitely cannot say TLS 1.0 anymore. > > Kathleen made a similar comment on dyn-reg and suggested text that we’ll > incorporate (unless there are objections from others on the list). > >> >> In Figure 1 it might be useful to indicate that exchanges A, B, C, and D >> are inherited from the dynamic client registration document and only >> step D is enhanced with additional parameters, as described in Section >> 3.1. Furthermore, I wonder whether it would make sense to somehow >> indicate in the figure that the endpoints are actually part of the >> Authorization Server. > > While they usually are the same in practice, these endpoints might not be > part of the Authorization Server — they might be part of a separate (but > related) service that handles objects of various kinds within a security > domain. No reason to tie them together unnecessarily. > > — Justin > > >> >> Ciao >> Hannes >> >>> >>> -- Justin >>> >> >> _______________________________________________ >> 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
signature.asc
Description: Message signed with OpenPGP using GPGMail
_______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
