Re: [Ace] Review of draft-ietf-ace-oauth-authz-07 by Mike Jones
Ludwig Seitz <ludwig.seitz@ri.se> Mon, 09 October 2017 11:47 UTC
Return-Path: <ludwig.seitz@ri.se>
X-Original-To: ace@ietfa.amsl.com
Delivered-To: ace@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id E2945134517 for <ace@ietfa.amsl.com>; Mon, 9 Oct 2017 04:47:59 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -5.401
X-Spam-Level:
X-Spam-Status: No, score=-5.401 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-2.8, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 60hx-h-R3x0u for <ace@ietfa.amsl.com>; Mon, 9 Oct 2017 04:47:55 -0700 (PDT)
Received: from se-out2.mx-wecloud.net (se-out2.mx-wecloud.net [89.221.255.177]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 22741134518 for <ace@ietf.org>; Mon, 9 Oct 2017 04:47:47 -0700 (PDT)
Received: from sp-mail-2.sp.se (unknown [194.218.146.197]) by se-out2.mx-wecloud.net (Postfix) with ESMTPS id 95081221234; Mon, 9 Oct 2017 11:47:45 +0000 (UTC)
Received: from [192.168.0.166] (10.116.0.226) by sp-mail-2.sp.se (10.100.0.162) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.669.32; Mon, 9 Oct 2017 13:47:45 +0200
To: Mike Jones <Michael.Jones@microsoft.com>, "ace@ietf.org" <ace@ietf.org>
References: <CY4PR21MB0504D057E5B47ED166F35083F57D0@CY4PR21MB0504.namprd21.prod.outlook.com>
From: Ludwig Seitz <ludwig.seitz@ri.se>
Message-ID: <70d9981f-3e4d-7389-5281-938618f1d3c1@ri.se>
Date: Mon, 09 Oct 2017 13:47:44 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0
MIME-Version: 1.0
In-Reply-To: <CY4PR21MB0504D057E5B47ED166F35083F57D0@CY4PR21MB0504.namprd21.prod.outlook.com>
Content-Type: text/plain; charset="windows-1252"; format="flowed"
Content-Language: en-US
Content-Transfer-Encoding: 8bit
X-Originating-IP: [10.116.0.226]
X-ClientProxiedBy: sp-mail-2.sp.se (10.100.0.162) To sp-mail-2.sp.se (10.100.0.162)
X-CMAE-Score: 0
X-CMAE-Analysis: v=2.2 cv=KqQ94SeN c=1 sm=1 tr=0 a=L5DDne6A+dD0FbDkt2Fblw==:117 a=L5DDne6A+dD0FbDkt2Fblw==:17 a=sZ8rJzgPlrQA:10 a=N659UExz7-8A:10 a=02M-m0pO-4AA:10 a=48vgC7mUAAAA:8 a=NEAV23lmAAAA:8 a=I0CVDw5ZAAAA:8 a=TXYz5_4QjbeewaRuK0gA:9 a=U0PlRQbKV-T0Kf_F:21 a=LUaYOS0AC8H4-iiq:21 a=pILNOxqGKmIA:10 a=w1C3t2QeGrPiZgrLijVG:22 a=YdXdGVBxRxTCRzIkH2Jn:22
X-Virus-Scanned: clamav-milter 0.99.2 at MailSecurity
X-Virus-Status: Clean
X-MailSecurity-Status: 0
X-Scanned-By: WeCloud MailSecurity
X-MailSecurity-Score: 0
Archived-At: <https://mailarchive.ietf.org/arch/msg/ace/d7AiRUv87sSsA9T5jbGgUE6cdtA>
Subject: Re: [Ace] Review of draft-ietf-ace-oauth-authz-07 by Mike Jones
X-BeenThere: ace@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: "Authentication and Authorization for Constrained Environments \(ace\)" <ace.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ace>, <mailto:ace-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/ace/>
List-Post: <mailto:ace@ietf.org>
List-Help: <mailto:ace-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ace>, <mailto:ace-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 09 Oct 2017 11:48:00 -0000
Hello Mike, thank you very much for your detailed review. There are two general points I'd like to address here to avoid repeating myself in the detailed comments below. 1.) You strongly recommend against using the client credentials grant. I get the feeling we have fundamentally different understandings of how that grant is used. For example I'm puzzled by your statement: "The client should never have the user’s credentials". This sounds more like the "Resource Owner Password Credentials" grant. I was under the impression that with the client credentials grant the client would be issued (or not issued) an access token solely based on its own credentials, thus it would _not_ have access to the user's credentials. This was exactly the reason I chose this grant type over those involving user generated grants. Machine-to-machine communication is probably the most common scenario for ACE, and I don't quite see how the manual user interactions for providing grants fit well into such a setting. The text in the OAuth spec seems to support my point of view: "Client credentials are used as an authorization grant typically when the client [...] is requesting access to protected resources based on an authorization previously arranged with the authorization server." ( see https://tools.ietf.org/html/rfc6749#section-1.3.4 ) 2.) Your main criticism is that we have made extensions to OAuth mandatory and implicit. I fully agree with the second part of your criticism, the parts of the ACE framework that are extensions to OAuth should be more clearly marked as such, and I will update the draft accordingly. I will also update Appendix A to give more concrete design justifications for each of these extensions. However I strongly disagree with your suggestion to make all these extensions optional. ACE is intended to make OAuth "constrained friendly", if we don't make any requirements, how is this supposed to work? Any implementation of plain OAuth could then just claim conformance with the ACE framework, which is definitely wrong! My suggestion would be to make the use of CBOR as data format mandatory, as well as the use of the abbreviated request/response parameters, claim names, and error codes. I however agree with your suggestion to make the use of CoAP optional (since you correctly commented that there are many other communication protocols that may use ACE). I would still use it as the baseline example in the framework, and leave it to the profiles that do not use CoAP to define mappings of e.g. the RESTful method, response and error codes. Please find detailed comments inline. /Ludwig On 2017-10-02 11:11, Mike Jones wrote: > Having read the spec cover-to-cover, it surprised me how different this > is from OAuth, despite the statements that it is based on OAuth. My > highest-priority request is to make all the extensions to OAuth defined > in this document optional, so that profiles could use actual OAuth, > which currently doesn’t seem to be possible. Much of this draft written > in a way that huge OAuth extensions are obliquely assumed when > describing other features, without any clear definitions or > justifications for them when they are introduced in passing. These > things need to be teased apart, with references to sections actually > defining and justifying each extension, rather than assuming that many > of them exist in passing in other sections. > See above > Detailed substantive comments on specific document text follow. > Editorial nits are communicated in the accompanying pull request > https://github.com/LudwigSeitz/ace-oauth/pull/112. > Done > Title > > The current title “Authentication and Authorization for Constrained > Environments (ACE)” doesn’t match the scope of what’s in the spec, as > the spec is only about resource authorization but not authentication. > Please change the title to “Resource Authorization Framework for > Constrained Environments” or something similar. (Authentication would > require defining the equivalent of an OpenID Connect ID Token to convey > authentication information, but this is entirely absent from the > specification.) > > 1. Introduction > > The introduction defines what it means by “authorization” by doesn’t say > what it is referring to by “authentication”. This provides more > evidence that “authentication” does not belong in the title or > abstract. Please remove the passing references to the underspecified > concept “authentication” from the introduction and the abstract. (Note > that while “OAuth client authentication” is a limited form of > authentication, its use as a mechanism in the spec does not justify > saying that the spec is defining authentication for its use cases in any > general sense of the term.) > When I included 'Authentication' in the title I was thinking about device authentication, and the fact that the AS facilitates establishing authentication credentials between C and RS in this draft. I take it from your comments that you don't think that this is authentication enough. > 2. Terminology > > The “/” syntax for endpoint names, such as “/token” is misleading, as it > seems to imply that these endpoints use particular pathnames. Please > replace all uses of “/endpoint-name” with just “endpoint-name”. >Already done in the editor's copy > 2. Terminology > > References are needed for the first uses of many of the terms and > abbreviations used. For instance, you’re using abbreviations like “AS”, > “RS”, and “IoT” without saying what they refer to. > I will re-check the terminology section for missing abbreviations. > 2. Terminology > > Please change all uses of the word “introspect” to “introspection” when > referring to the introspection endpoint. > Done. > 2. Terminology > > You introduce an “authz-info” endpoint in the terminology section, when > this isn’t commonly understood. You need a section reference following > the first use of this concept. > Ok, will do. > 3.1 OAuth 2.0 – token and introspect endpoints > > This section is the first of many places in which the text is written in > a way that assumes that introspection will be implemented and used. > This needs to be corrected throughout the document, appropriately > qualifying descriptions of introspection saying that some profiles may > use it and others will not. For instance, the draft says “The token > introspection endpoint, /introspect, is used by the RS when requesting > additional information regarding a received access token.” Please > change this so something more like “In some profiles, a token > introspection endpoint may be present and used by the RS to request > additional information about a received access token.” You should also > clearly state that introspection is unnecessary and adds overhead that > can be avoided when the resource server understands the access token > format, such as when it is a CWT. > Agree, I will change the text to clarify that introspection is always optional. > > 3.1 OAuth 2.0 – Proof of Possession Tokens > > The draft uses the term “proof of possession token” to denote an access > token supporting proof of possession. Note however, that there are many > other kinds of proof of possession tokens other than just access > tokens. Please change all uses of the term “proof of possession token” > to “proof of possession access token” or “access token supporting proof > of possession”. Similarly, change all uses of the term “PoP token” to > “PoP access token”. I was hoping to avoid such Wagnerian word monsters, but I see your point. Unless someone has a good suggestion to make the name of these things shorter and still convey the meaning, I will implement that change. > > 3.1 OAuth 2.0 – Scopes and Permissions > > This section contains another example of an extension to OAuth being > assumed, when for many profiles it won’t be necessary. The draft says > “In turn, the AS may use the scope response parameter to inform the > client of the scope of the access token issued.” This assumes that a > “scope” response will be added to all participating OAuth > implementations, without justifying this addition to the standard. At > most, this section should refer to another section which defines an > optional “scope” response parameter and describes the circumstances in > which profiles would and would not need it. I don't quite see what you mean here. Scope is used in the same way it is used in the OAuth 2.0 spec, i.e. it is optional, as the words "may use the scope response parameter" indicate. > > 3.1 OAuth 2.0 – Scopes and Permissions > > Likewise, the draft says “As the client could be a constrained device as > well, this specification uses CBOR encoded messages for CoAP”. This is > one of many places in the draft that it’s assumed that CBOR and COAP > will be used rather than JSON and HTTP. The framework draft should > explicitly leave these choices up to profiles. > See general answer. > 4. Protocol Interactions > > Please do not in any way endorse using the Client Credentials grant – > which defeats the purpose of even having OAuth. The client should never > have the user’s credentials! At most, you should say that while the > Client Credentials grant may be used for some scenarios, it has known > security and privacy flaws and its use is deprecated. > See general answer. > 4. Protocol Interactions > > The sentence “In such a case the resource owner or another person on his > or her behalf have arranged with the authorization server out-of-band, > which is often accomplished using a commissioning tool.” needs > clarification. What has been arranged? > Right, that sentence has been mangled. Will fix. > Figure 1: Basic Protocol Flow > > This is yet another place where a major OAuth extension is assumed, > without clear justification. The diagram adds “+ RS Information” to the > authorization server response to the client, when this isn’t a normal > part of OAuth and not defined. At least, please indicate that this is > optional and refer to a section that defines and justifies this optional > extension, rather than assuming in passing that it’s been added. >This wouldn't qualify as a "major extension" in my world, the protocol is just returning a few additional parameters to enable proof-of-possession and the use of different communication/communication-security profiles. Note that e.g. draft-ietf-oauth-pop-key-distribution also returns additional parameters ('key') together with the access token response. We just gave those additional parameters a name to be able to refer to them collectively. > Figure 1: Basic Protocol Flow > > Please rework this diagram to illustrate that introspection is optional > (and wastes resources by performing unnecessary communication). > Agreed. > 4. Protocol Interactions – Access Token Response > > This also assumes the existence of “RS Information”. At least, say that > “Some profiles may choose to return information about the resource > servers” and reference a section describing this optional feature, > rather than write the text in a way that assumes its existence. > See above. > 4. Protocol Interactions – Access Token Response > > This also assumes the existence of profile information in the response. > This would normally be implicit. Again, please do not write the draft > in a way that assumes the presence of extensions that are often not needed. > Indeed it makes sense to make the profile information an optional element in cases where it is implicitly known. I will modify the text to clarify this. > 4. Protocol Interactions – Resource Request > > The two-part request described in the third paragraph is not OAuth, nor > is there any justification for adding additional steps. Likewise, the > language in paragraph 4 about “comparing the claims in the access token > with the resource request” is a highly specific extension that is not > normal OAuth. Please refactor this description to clearly delineate > what’s OAuth and what are optional extensions – referencing specific > sections that define these extensions. > The justification for this was discussed at the interim meeting between IETF 94 and 95 (https://datatracker.ietf.org/meeting/interim-2016-ace-01/materials/slides-interim-2016-ace-1-2/), I will provide a summary in Appendix A to clarify why this was designed that way. > 4. Protocol Interactions – Token Introspection Request > > Please rework this section to describe that introspection is optional > (and wastes resources by performing unnecessary communication). > Describe how things are simpler and more efficient when the resource > directly understands the contents of the access token. > See comment above. > 4. Protocol Interactions – Token Introspection Response > > This section adds yet another OAuth extension on the fly – the “client > token” – again with no justification or clear definition. Please remove > the oblique “client token” reference here. > We designed the client token in order to solve use cases where the client is constrained and has no direct means to communicate with the AS at the time of the resource access. If you have a better suggestion on how to solve this, that is more in line with OAuth, I'd be grateful to hear about it. > 5. Framework – Proof-of-Possession > > After “The binding is provided by the "cnf" claim” add references to > [RFC 7800] and [draft-ietf-ace-cwt-proof-of-possession]. > Ok > 5. Framework – Proof-of-Possession > > The text currently includes “update a token”. Shouldn’t this actually > be “get a new token”, since an existing access token can’t be updated? > Right. Will clarify the text. > 5.1 Authorization Grants > > This section includes the phrase “client credentials grant is > recommended”. Because of the security and privacy problems with the > client credentials grant, please rework this section to ensure that > implementers and profile writers understand that the use of client > credentials grant is never recommended, including describing why that is > the case. > See general comment. > 5.1 Authorization Grants > > Reference [RFC 6749] after you say “see the OAuth 2.0 framework”. > Ok > 5.2 Client Credentials > > After you say that “the OAuth framework defines one client credential > type”, you should also describe that RFC 7522 and RFC 7523 define > additional kinds of client credentials that may be used. > I'm not sure how relevant these credential types are for constrained environments. For example RFC 7522 refers to SAML, which generally is way too verbose for constrained environments. I guess we could refer to RFC 7523 but with the caveat that CBOR/CWT is the preferred/mandatory format in ACE. > 5.4 The Authorize Endpoint > > Change all occurrences of “authorize endpoint” to “authorization endpoint”. > Ok > 5.5 The Token Endpoint > > Rather than saying that “this framework extends”, say that the framework > defines optional extensions and reference their definitions and > justifications. > See general discussion. I'm OK with providing more concrete design justifications in Appendix A. > 5.5 The Token Endpoint > > Change “this framework defines encodings using CoAP and CBOR, as a > substitute for HTTP and JSON” in a way that allows profiles to make > either choice. See general comment. > > 5.5.1 Client-to-AS Request > > This section is another place in which this specification is making > assumptions about choices that rightfully belong to profiles. It starts > with “The client sends a CoAP POST request”. Profiles may choose to use > CoAP or they may choose to use HTTP. Please reword this so that it says > something more like “The client sends a CoAP or HTTP POST request, > depending upon the profile”. Please likewise generalize the description > of the framework so that it’s clear that the choice between CoAP or HTTP > is up to the profile. Heck, Bluetooth Smart is another transport that’s > possible, which this spec shouldn’t preclude! > I'm Ok with some language that allows profiles to specify other transport protocols (like Bluetooth low energy), but I'd like to keep CoAP as the baseline, to be able to specify request and response codes in some meaningful way. It would then be up to the profile that uses another transport protocol to define mappings. > 5.5.1 Client-to-AS Request > > This is again written in such a way that it assumes that the additional > parameters will be implemented and used, which won’t be necessary for > some profiles. It starts “In addition to these parameters, this > framework defines the following parameters”. Qualify this by saying > that some profiles may choose to implement and use the following OAuth > extension parameters. > These parameters (aud and cnf) are already optional, I don't quite understand what more you are suggesting, could you clarify? > 5.5.1 Client-to-AS Request – aud > > The OAuth Token Exchange spec uses the “audience” parameter for this > functionality. See > https://tools.ietf.org/html/draft-ietf-oauth-token-exchange-09#section-2.1. > Please reference this spec and use the same parameter name. > Is there a difference between the semantics of the 'aud' claim from JWT and the 'audience' parameter from this draft? I'd rather use semantics that are defined in a stable RFC, than referring to work in progress. > 5.5.1 Client-to-AS Request – aud > > The spec says that “If a client submits a request for an access token > without specifying an "aud" parameter, and the AS does not have a > default "aud" value for this client, then the AS MUST respond with an > error message”. This violates the OAuth principle that implementations > must ignore parameters that they do not understand. > https://tools.ietf.org/html/rfc6749#section-3.1 says “The authorization > server MUST ignore unrecognized request parameters”. Rework this to say > that it’s perfectly legitimate for participants to ignore the “aud” > parameter when they don’t implement or understand it. > How would a regular OAuth 2.0 AS react to an access token request for which it doesn't know which audience it is supposed to address? I think that this must be either implicit or given by the 'aud' parameter as suggested in https://tools.ietf.org/html/draft-ietf-oauth-pop-key-distribution-03#section-3. > 5.5.2 AS-to-Client Response > > Once again, extensions to OAuth are implicitly required – in this case > the “profile” and “cnf” parameters. Reword to make these choices > available to profiles. I see your point for the profile parameter, given that it could be implicit. I have my doubts though that 'cnf' could ever be implicit, do you have any case in mind (that uses proof-of-possession) where that would be the case? > > 5.5.2 AS-to-Client Response – profile > > This certainly shouldn’t be required, as the profile knowledge will be > implicit in most OAuth deployments. Very rare indeed will the cases in > which participants will support multiple profiles. > Ok > 5.5.2 AS-to-Client Response > > The spec says that “Figure 5 summarizes the parameters that may be part > of the RS Information”. However the term “RS information” is > undefined. Instead, change this sentence to say “Figure 5 summarizes > the parameters that may be part of the token response”. > The term "RS Information" is defined in section 4, bullet point B. Since that is obviously too well hidden. I will move that definition to the terminology section. > 5.5.2 AS-to-Client Response – Figure 6 > > I suggest removing “profile” from the example, as this is unnecessary > protocol bloat. If we remove it, we should add some explanatory text that states that the profile is implicit in that case. > > 5.5.3 Error Response > > The spec says “The error codes MAY be abbreviated using the codes > specified in table Figure 7”. Again, whether to do this is a profile > choice. Making it optional only makes all implementations bigger > because they have to support both choices. Reword to say that profiles > will specify whether error codes are abbreviated in this manner or not. > See general comment. > 5.5.3 Error Response – Figure 7 > > These values should be in a registry – possibly called something like > “OAuth error code CBOR value mappings”. This registry should reference > the values registered in the OAuth Extensions Error Registry at > https://www.iana.org/assignments/oauth-parameters/oauth-parameters.xhtml#extensions-error. > Correct. Will fix. > 5.5.4.1 Audience > > Reference the audience parameter in > https://tools.ietf.org/html/draft-ietf-oauth-token-exchange-09#section-2.1 > and indicate that whether it is used a profile-specific choice. > See discussion above (5.5.1 Client-to-AS Request – aud) > 5.5.4.1 Audience > > The spec says that “It should be encoded as CBOR text string”. Again, > reword this to say that it’s a profile choice whether to use CBOR or > standard OAuth. > See general comment. > 5.5.4.2 Grant Type > > Rather than saying “MAY”, say that the profile specifies the > representation that it used. > I would actually rather make the abbreviated representations mandatory for the reasons given in my general comment. > 5.5.4.2 Figure 8 > > These values should be in a registry – possibly called something like > “OAuth grant type CBOR value mappings”. This registry should reference > the sections defining these values in RFC 6749 and the grant type values > defined by RFC 7522 and RFC 7523. > Correct. Will fix. > 5.5.4.3 Token Type > > Reference draft-ietf-oauth-pop-key-distribution for the definition of > the “pop” token_type. > I was under the impression that this draft was dead. Is it really acceptable to reference this type of work in a normative way? I've given that draft a paragraph in the Acknowledgment section. > 5.5.4.3 Token Type > > When you say that “The values in the ‘token_type’ parameter MUST be CBOR > text strings”, you’re again implicitly making choices that belong to > profiles. Please generalize this description. > > See general comment. > 5.5.4.4 Profile > > Whether “profile” is even needed is up to the profile. Typically it > will be implicit, since implementations will use a single profile. > Please revise accordingly. > Agree. > 5.5.4.5 Confirmation > > Please replace the definition of “cnf” for CBOR here with references to > RFC 7800 and draft-ietf-ace-cwt-proof-of-possession. > Note that we use "cnf" here as a request and response parameter. I do refer to draft-ietf-ace-cwt-proof-of-possession, but that draft only defines "cnf" as a CWT claim, so I still feel the need to specify its semantically equivalent use as a req/resp parameter. > 5.5.5 Mapping parameters to CBOR > > It looks to me like these values are intended to align with those > registered in the CBOR Web Token (CWT) Claims registry initially > populated by > https://tools.ietf.org/html/draft-ietf-ace-cbor-web-token-08#section-9.1.2. Correct > If so, the spec should make this relationship explicit. Will do. > However, it > would be inappropriate to use the rare single-byte CBOR integer values > for application-specific claim keys. I would suggest that the claim > identifiers for client_id through refresh_token and profile start at 256 > (a two-byte CBOR value) and go up from there. In that case, I suspect > they could be successfully registered in the CWT Claims registry – which > I think you want. (“cnf” will already be registered by > draft-ietf-ace-cwt-proof-of-possession.) > Why would we want to artificially increase the size of the framework parameters coming from OAuth? (like e.g. code, access_token etc) This feels counter-productive, they are by no means application-specific, these are used in basic OAuth 2.0. We should do a case-by-case evaluation. Also as for registering those into the CWT claims registry, I feel that would not be a good fit, since we are using those as request/response parameters in the different OAuth/ACE protocol messages, while CWT registers claim values. I see a mismatch here don't you? > 5.6 The Introspect Endpoint > > First, change introspect to introspection. And like other places, state > that it is up to the profile whether to use introspection at all, and if > so, whether to use standard introspection syntax or the CoAP/CBOR version. > Agree with the first part, disagree with allowing other encodings than CBOR. > 5.6.2 AS-to-RS Response – Figure 15 > > Remove profile and client_token from this example, since both are pretty > esoteric and not likely to be used in the common case. > As to whether the example should include optional parameters, I have no strong opinion, although their absence in examples makes it harder to understand their intended use. > 5.6.4 Client Token > > I agree with Hannes’ review of this feature: “/The "Client Token" is > somewhat experimental and not on par with the rest of the document in > terms of maturity and alignment with OAuth. I would prefer this > functionality to be covered in a separate document, if someone still > cares about it. While OAuth has seen a lot of formal analysis this > feature obviously hasn't./” Please remove this from the specification > and if you still believe in it, place it in a separate document for > independent consideration by the working group. > See comment above (4. Protocol Interactions – Token Introspection Response) > 5.6.4 Client Token > > The spec says “The client is pre-configured with a generic, long-term > access token when it is commissioned.” This hardly seems secure – > especially since secrets can often be extracted from deployed software. > I don't see how this is less secure than a client that doesn't have a long-term token, but instead the credentials/grants that allow it to retrieve fresh tokens from the AS. These credentials can just as easily be extracted and misused if the client is stolen. > 5.6.4 Client Token > > The spec says “The RS then performs token introspection to learn what > access this token grants.” Again, this is unnecessary if the resource > understands the claims in the access token. The text is probably not well written in that section. The token is intended to just be a reference, not a full CWT. Thus it would not contain any claims that the RS can process independently. I will try to clarify this. > > 5.6.5 Mapping Introspection Parameters to CBOR > > As in my related comment on 5.5.5, it looks to me like these values are > intended to align with those registered in the CBOR Web Token (CWT) > Claims registry initially populated by > https://tools.ietf.org/html/draft-ietf-ace-cbor-web-token-08#section-9.1.2. > If so, the spec should make this relationship explicit. However, it > would be inappropriate to use the rare single-byte CBOR integer values > for application-specific claim keys. I would suggest that the claim > identifiers for client_id through refresh_token and profile start at 256 > (a two-byte CBOR value) and go up from there. In that case, I suspect > they could be successfully registered in the CWT Claims registry – which > I think you want. (“cnf” will already be registered by > draft-ietf-ace-cwt-proof-of-possession.) > Same comments as before apply (5.5.5 Mapping parameters to CBOR) > 5.7 The Access Token > > Reference [RFC 7800] and [draft-ietf-ace-cwt-proof-of-possession] when > describing the use of “cnf”. > The text does reference RFC 7800, I failed to update this section after we moved the 'cnf' specification to draft-ietf-ace-cwt-proof-of-possession. Will fix. > 5.7.1 The Authorization Information Endpoint > > Please describe the relationship between this endpoint and the resource > endpoint used by RFC 6750. > I can't find a reference to a resource endpoint in RFC 6750. Are you sure this is the right reference? > 5.7.1 The Authorization Information Endpoint > > Again, please clarify that support for this OAuth extension, the use of > CoAP, and the use of introspection are all choices up to particular > profiles. > This will just yield us a lot of incompatibility. Note that profiles are free to define additional methods of token transfer. > 5.7.1 The Authorization Information Endpoint > > The spec says “The RS MUST be prepared to store more than one token for > each client, and MUST apply the combined permissions granted by all > applicable, valid tokens to client requests.” This seems really overly > specific for a framework spec. Simple systems will likely only have one > token per client, let alone not supporting complex permission > combination logic! Please remove this statement. > Ok > 5.7.2 Token Expiration > > Rather than saying “CWT/JWT” expand this to “CWT or JWT” for readability. > I'm actually considering to remove JWT entirely instead. See general discussion. > 6. Security Considerations > > Add a reference upon first use of the term “AEAD”. Ok > > 7. Privacy Considerations > > The spec says “The latter may reveal the client's identity.” What is > meant by the client’s identity? The identity of the specific device running the client application. > What kinds of information does this > entail and what are the privacy risks associated with it? How does the > client’s identity relate to the identities of people who may be > associated with the system in some way? The user owning that device might be linkable to the device identity. Therefore that user's privacy might be at stake. It seems this paragraph needs clarification. > > 8.1 OAuth Introspection Response Parameter Registration > > Every place an IANA registration currently says “this document”, please > change it to “Section x.y.z of this document” (using the appropriate > <ref target=”x.y.z”/> tag for the section that defines the value. > Ok > 8.1 OAuth Introspection Response Parameter Registration > > “aud” is already registered at > https://www.iana.org/assignments/oauth-parameters/oauth-parameters.xhtml#token-introspection-response. > Right, will remove. > 8.4 Token Type Mappings > > The name “Token Type Mappings” registry is too generic. Please change > it to “OAuth Token Type CBOR Mappings” or something similar. > Ok > 8.4.1 Registration Template > > As was pointed out in comments on earlier versions of the CWT spec, the > range 1 to 65536 makes no sense. Please consider using the same > treatment of value ranges as CWT does (which themselves derived from the > COSE usage of value ranges). Do this consistently every place that “1 to > 65536” occurs in the spec. > Ok > 8.5 CBOR Web Token Claims > > Consider using the “scp” claim defined at > https://tools.ietf.org/html/draft-ietf-oauth-token-exchange-09#section-4.2 > rather than “scope”. If you don’t, at least say why you are introducing > a different claim to convey the same information. > It is my understanding that "scp" is defined as an array of scopes, which in turn are a space delimited list of strings. I thus felt that "scp" was unnecessarily introducing a doubly nested list of scope-tokens, which is the reason why I choose not to use it. > 8.5 CBOR Web Token Claims > > “cnf” is already being registered by draft-ietf-ace-cwt-proof-of-possession. > Correct, this should already be removed in the editor's draft. > 8.6 ACE Profile Registry > > The name “ACE Profile Registry” registry is too generic. Please change > it to “ACE OAuth Profile Registry” or something similar. > Ok > 8.7 OAuth Parameter Mappings Registry > > The name “OAuth Parameter Mappings” registry is too generic. Please > change it to “OAuth CBOR Parameter Mappings” or something similar. > Ok > 8.7.2 Initial Registry Contents > > Per my earlier comments, these values should actually reference the CWT > Claims registry and application-specific values such as “client_id”, > etc. should not use the scarce single-byte value range. > See previous comments (5.5.5 Mapping parameters to CBOR). > 8.8.1 Registration Template > > Registrations for the Introspection Endpoint CBOR Mappings registry > should contain a field that lists the corresponding value registered in > the OAuth Token Introspection Response registry at > https://www.iana.org/assignments/oauth-parameters/oauth-parameters.xhtml#token-introspection-response. > Ok > 8.10 CWT Confirmation Methods Registry > > Delete this section, as it has been moved to > draft-ietf-ace-cwt-proof-of-possession. > Ok > Appendix A. Design Justification – Communication Constraints > > Add that power saving may be another reason for communication constraints. > I plan to completely rewrite that section to provide detailed justifications for the different changes to OAuth 2.0 we made. I will make sure to include some text on power consumption. > Appendix B. Roles and Responsibilities – Authorization Server > > Add that enabling discovery of the AS’s capabilities via metadata is > likely in scope. Reference draft-ietf-oauth-discovery. > Ok > Appendix B. Roles and Responsibilities – Client > > What do the parenthetical letters such as “(A)” refer to? Why are “(D)” > and “(E)” missing? > They refer the steps in figure 1. I will add some text to clarify this. > Appendix C. Requirements on Profiles > > You should be much more clear in this section that choices of JSON vs. > CBOR, JWT vs. CWT, HTTP versus COaP, PoP versus Token Binding, > Introspection or not, authz-info endpoint or not, etc. must be made by > profiles. See general comments. > Appendix E. Deployment Examples – Figure 20 > > Use the “scp” claim defined at > https://tools.ietf.org/html/draft-ietf-oauth-token-exchange-09#section-4.2 > rather than “scope”. > See previous comment on "scp" vs "scope" (8.5 CBOR Web Token Claims) > Appendix E.2 Introspection Aided Token Validation > > It makes no sense to assume that the client is not able to access the AS > at the time of the access request but can access the introspection > endpoint, since the introspection endpoint is part of the AS. Please > remove this section or revise accordingly. > The text here seems to be prone to misunderstandings. It is the RS not the client that is capable of making introspection requests in this scenario. I will clarify the text. > Appendix E.2 Introspection Aided Token Validation – Figure 23 > > Please revise the example to not use Client Credentials, because of the > security and privacy problems associated with this grant type. > See general comments. > *Surprising Omission of Token Binding!* > > The specification should describe the ability to use Token Bound access > tokens, rather than PoP access tokens, as this will be substantially > simpler for most implementations. Please reference > draft-ietf-oauth-token-binding and describe how to apply it to this > specification. My cursory examination of the relevant drafts seems to indicate that token binding follows the same general principles as proof of possession access tokens (i.e. embedding some binding information into the token, which is equivalent to the 'cnf' claim defined by RFC 7800). Can you explain the advantages that token binding would have over proof-of-possession? -- Ludwig Seitz, PhD Security Lab, RISE SICS Phone +46(0)70-349 92 51
- [Ace] Review of draft-ietf-ace-oauth-authz-07 by … Mike Jones
- Re: [Ace] Review of draft-ietf-ace-oauth-authz-07… Ludwig Seitz
- Re: [Ace] Review of draft-ietf-ace-oauth-authz-07… Anthony Nadalin
- Re: [Ace] Review of draft-ietf-ace-oauth-authz-07… Ludwig Seitz