[OAUTH-WG] A token review of draft-parecki-oauth-v2-1-01
Brian Campbell <bcampbell@pingidentity.com> Tue, 21 April 2020 22:49 UTC
Return-Path: <bcampbell@pingidentity.com>
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 9BBF13A0D34 for <oauth@ietfa.amsl.com>; Tue, 21 Apr 2020 15:49:03 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.098
X-Spam-Level:
X-Spam-Status: No, score=-2.098 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=pingidentity.com
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 CqdWiNBVCLVR for <oauth@ietfa.amsl.com>; Tue, 21 Apr 2020 15:48:59 -0700 (PDT)
Received: from mail-lj1-x230.google.com (mail-lj1-x230.google.com [IPv6:2a00:1450:4864:20::230]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id A19593A0D32 for <oauth@ietf.org>; Tue, 21 Apr 2020 15:48:58 -0700 (PDT)
Received: by mail-lj1-x230.google.com with SMTP id u15so251970ljd.3 for <oauth@ietf.org>; Tue, 21 Apr 2020 15:48:58 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pingidentity.com; s=google; h=mime-version:from:date:message-id:subject:to; bh=AQOaCxwllQXUZvzOKl5BOI9CVJOr+/7BtxlHitnITII=; b=VFK2WnfehgFuLq9A3ezspprKj+1hTIAzaJ6UCNFF9k6Uj2S7MWvAtOoLItC1TDYy5/ r8eDDQKteBsqDoBATF3/8GlNuD0V4F8m4tVI5BzMa+dtYHrDCKNLvNKZeEeJaBHd6XEL xuRylongihyLBt0X2m7NSi+r5Xod2InCUfbvNFOsCMYegBtButW0l1rjPFv88iDyxrxl ALdnNrwAVY444rvkUJlDPC0OefiYKdcbvjD0Z+HIsmGjnSmcRS+FDUAQKWpjFa0eSgxV XbSs4ctwHUiKh7BK1YbtyRZKDFAHoYpm3PyWrbkBMPyDufDTZIbR2ZcHpO4TKYbP1rTC 16WA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to; bh=AQOaCxwllQXUZvzOKl5BOI9CVJOr+/7BtxlHitnITII=; b=QPKqZZqN4A0qfbh/9vV2MXgNWUI/seBvaJYFWHLd6YTAa+mfCNjd5LaPBpgYRZQeh7 W0EH7hG4riLk0FN3LFjBivf2ECKRXIloGierEq34ESPblxNMBurM/QpUTw8YuaisazY6 vFujWC+UcgIXN8R3rsBhBmSpswO8NWZF09uIVd1t/WTYniUMy34M8sjqPBVwNHCXflHB 99RdLaTL24vwmxus621NmtKKxIN1E5QbBVrBnGU3zsduI/5mIegMq47RefurxeDTnbzT 08vs8OyMXggfO1299smLE+I5XLsudDaTH9hG+m7LUEVtYp68zWZhiV4luDqPZRshDosL wWnQ==
X-Gm-Message-State: AGi0PubERc854iH2pCXmCd9yGrko+YB/gzrYeTooHHJkdLL8aWVWowuJ vTjlkwr1paOmfuXousi1nAURijr80+irLowL/T35zrYOA3KFOB94n4Nfyeoq9zOwCo6EGe631Ti f1mNOclmtlu5JmVtJEOY=
X-Google-Smtp-Source: APiQypJAe9Z/4yJa77mk1nDrdGI2r28OlDBkkOP7fQeVFXsVENrmNvuExR9OWurfjdSW+/MH6a/uhBrGbOKkF5l6JtM=
X-Received: by 2002:a2e:b8c1:: with SMTP id s1mr15017354ljp.0.1587509335834; Tue, 21 Apr 2020 15:48:55 -0700 (PDT)
MIME-Version: 1.0
From: Brian Campbell <bcampbell@pingidentity.com>
Date: Tue, 21 Apr 2020 16:48:29 -0600
Message-ID: <CA+k3eCSawja4LZ4qwOPqFEBYkwfUbwh9mZgeT7Jgs4PS8p1VSw@mail.gmail.com>
To: oauth <oauth@ietf.org>
Content-Type: multipart/alternative; boundary="0000000000000a578a05a3d4d2fa"
Archived-At: <https://mailarchive.ietf.org/arch/msg/oauth/5hCWu0tIRk11jnW6_M87Rx7S560>
Subject: [OAUTH-WG] A token review of draft-parecki-oauth-v2-1-01
X-BeenThere: oauth@ietf.org
X-Mailman-Version: 2.1.29
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: <https://mailarchive.ietf.org/arch/browse/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, 21 Apr 2020 22:49:04 -0000
Been working on this on and off for a while now (it's not exactly short at 80+ pages, various other priorities, etc.) but wanted to share my thoughts from an initial review of the OAuth 2.1 draft before the interim next week where it is on the agenda https://datatracker.ietf.org/doc/agenda-interim-2020-oauth-06-oauth-01/. So for better or worse, here's that review: Abstract: "replaces and obsoletes the OAuth 2.0 Authorization Framework described in RFC 6749." I think "replaces" is probably unnecessary here and, to be pedantic, is arguably inaccurate because published RFCs don't ever go away or get replaced. Probably should also consider using the official "obsoletes" attribute marker too for 6749 https://tools.ietf.org/html/rfc7991#section-2.45.8 and probably also "updates"/"obsoletes" for others based on the scope of the rest of the document (not sure how this even works with a BCP or if you can or would want to update or obsolete a BCP) if this work proceeds. That scope could be better described in the abstract too as discussed somewhat in the thead https://mailarchive.ietf.org/arch/msg/oauth/Ne4Q9erPP7SpC5051sSy6XnLDv0/ and also the 1st paragraph of https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-12. What is and isn't in scope is another larger question that I"m not even sure how to ask. What's included vs. what's referenced? Should this doc be incorporating bits of BCP 212 - OAuth 2.0 for Native Apps? Seems kinda antithetical to what a BCP is supposed to be but it's also a bit hard to tell how much was used. I mean, what happens if/when the BCP is updated? And that kinda begs the question of if it should also incorporate parts of or even replace the browser based apps draft? I guess that's a TBD circa page 68. There was talk about the device grant being in scope but I'm not seeing it (not saying it should or shouldn't be there but it was talked about). I dunno exactly but those are some scope questions that come to mind. Speaking of obsoleting, I do want to ensure that existing extensions and profiles of RFC 6749 that use legitimate extension points still present and unchanged in OAuth 2.1 aren't inadvertently impacted by this effort. I'm not sure how that should work in practice but want to be aware of it as/if this work progresses. https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-1 "is designed for use with HTTP ([RFC2616])." I was momentarily proud of myself for knowing that RFC 2616 is obsolete but then remembered that the nits tool can automate the check for other obsolete or problematic references https://tools.ietf.org/idnits?url=https://tools.ietf.org/id/draft-parecki-oauth-v2-1-01.txt so take a look at those issues. Probably should also check the errata of any documents this one intends to update/obsolete or just borrowed a lot of text from to see if anything should be applied. https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-1.1 "The interaction between the authorization server and resource server is beyond the scope of this specification." Don't want to try and change the scope but perhaps a mention that things like RFC 7662 Token Introspection and self-contained tokens a la JWT exist here (or sec 1.4 or 7 or...) would be worthwhile. https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-1.5 " Steps (3), (4), (5), and (6) are outside the scope of this specification, as described in Section 7." But Section 7 incorporates parts of RFC 6750 so being out of scope isn't really true. https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-1.8 Not too sure about this section in this document but seems that it should at least reflect the fact that some things like RFC 8414 Authorization Server Metadata do now exist. https://tools.ietf.org/html/rfc6749?#section-1.9 All the cool drafts are doing BCP 14 [RFC2119] [RFC8174] these days. https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-2 Mentioning the existence of RFC 7591 Dynamic Client Registration here seems appropriate. And I think it's be super useful to say that even when RFC 7591 isn't being used directly, it (and registered extensions to it https://www.iana.org/assignments/oauth-parameters/oauth-parameters.xhtml#client-metadata) imply a common general data model for clients that's pretty widely used and useful even with so called static registration that "typically involve end-user interaction with an HTML registration form." https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-2.2 "Authorization servers SHOULD NOT allow clients to influence their "client_id" or "sub" value or any other claim if that can cause confusion with a genuine resource owner." This text taken from draft-ietf-oauth-security-topic is out of context and somewhere between meaningless and confusing here in this document. Removing it or maybe something like this might be more appropriate: "Authorization servers SHOULD NOT allow clients to influence their "client_id" value in such a way that it might cause confusion with the identifier of a genuine resource owner during subsequent protocol interactions." https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-2.3.1 "The client MAY omit the [client_secret] parameter if the client secret is an empty string." Let's remove that sentence. As Michael Peck noted in his review, it doesn't even make sense in the context of this section describing authentication of confidential clients using a password/client secret. And that sentence has been the source of some other problems. More detail is in this thread https://mailarchive.ietf.org/arch/msg/oauth/D1-FrLrSeWrg9M_ca9dbkYeruc4/ https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-2.3.2 "Other Authorization Methods" should be "Other *Authentication* Methods" It's probably worthwhile to note or reference the other client authentication methods that have been specified since this text was written in 6749 and/or point to the OAuth Token Endpoint Authentication Methods registry https://www.iana.org/assignments/oauth-parameters/oauth-parameters.xhtml#token-endpoint-auth-method for them and also maybe mention that, despite the Token Endpoint in the name, they are general methods of client auth to the AS when making direct requests. https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-3.1 "The means through which the client obtains the location of the authorization endpoint are beyond the scope of this specification, but the location is typically provided in the service documentation." Maybe add something like "or in the authorization server's metadata document [RFC8414]." "Request and response parameters MUST NOT be included more than once." please clarify that sentence to say: "Request and response parameters defined by this specification MUST NOT be included more than once." https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-3.1.2 " The authorization server MUST compare the two URIs using simple string comparison as defined in [RFC3986], Section 6.2.1." There's absolutely no context here for understanding what two URIs are being compared. Mandating full redirect_uri comparison is maybe more appropriate in 4.1.1 with the redirect_uri parameter somewhere. And 3.1.2.3. and 3.1.2.2 needs some attention with respect to this too. But also an exception (for the port part) to full redirect_uri comparison is needed for loopback redirect_uris on native clients as in https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-9.6.1 and https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-9.2 and https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-10.3.3 https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-3.1.2.1 As Michael Peck noted in his review, it's probably okay now to just mandate TLS for HTTP redirect URIs. Although custom scheme or loopback redirect URIs for native apps wouldn't use or require TLS. https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-3.1.2.2 and https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-3.1.2.3 Seem to still allow for registration of partial redirection URIs. Which isn't gonna work with an exact match requirement. https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-3.2 "Request and response parameters MUST NOT be included more than once." please clarify that sentence to say: "Request and response parameters defined by this specification MUST NOT be included more than once." " The means through which the client obtains the location of the token endpoint are beyond the scope of this specification, but the location is typically provided in the service documentation." Maybe add something like "or in the authorization server's metadata document [RFC8414]." https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-3.2.1 "Client authentication is critical when an authorization code is transmitted to the redirection endpoint over an insecure channel or when the redirection URI has not been registered in full." Isn't full registration of redirection URI now required (other than maybe the port for native apps) by virtue of requiring a full comparison be done when validating the authz request? And other than a native app using the loopback, maybe it's time to move to require that redirection URIs be accessed via secure channel? https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-4.1.1.2 "Clients are permitted to use "plain" only if they cannot support "S256" for some technical reason and know via out-of-band configuration that the server supports "plain"." With code_challenge_methods_supported from Authorization Server Metadata / RFC 8414 it doesn't have to be out-of-band anymore. https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-4.1.2 " Typically, the "code_challenge" and "code_challenge_method" values are stored in encrypted form in the "code" itself but could alternatively " 'Typically' - really? https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-4.1.3 " * ensure that the "redirect_uri" parameter is present if the "redirect_uri" parameter was included in the initial authorization request as described in Section 4.1.1, and if included ensure that their values are identical." The reference should be to 4.1.1.3. https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-4.1.4 " An example successful response: HTTP/1.1 200 OK Content-Type: application/json;charset=UTF-8 Cache-Control: no-store Pragma: no-cache" Pretty sure application/json shouldn't have a charset (see note at end of section https://tools.ietf.org/html/rfc8259#section-11) and I've long thought that "Pragma: no-cache" shouldn't be there (see https://mailarchive.ietf.org/arch/msg/oauth/9DdkE2P0RrUZMeZAbdf3NrMfy0w/ ) Note that these apply to most of the example responses in the draft. https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-4.3 " specifying the grant type using an absolute URI (defined by the authorization server) as the value of the "grant_type" parameter" The words in the parenthetical have led to questions in AD review of documents making use of the grant type extension point (see https://mozphab-ietf.devsvcdev.mozaws.net/D4278#inline-1581) and I think "understood by the authorization server" might be better phrasing or even removing the parenthetical all together. RFC7522 is mentioned here as an example extension grant but maybe worth also including mention of others like RFC7523, RFC8628, RFC8693, and maybe even non IETF ones. https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-6 " * _Sender-constrained refresh tokens:_ the authorization server cryptographically binds the refresh token to a certain client instance by utilizing [I-D.ietf-oauth-token-binding] or [RFC8705]." Given the relative immaturity of ways to do this, maybe something more open ended would be appropriate? This reads like token-binding or MTLS are the only ways allowed. I'd think wording that would allow for DPoP or some yet-to-be-defined method would be better here. Also maybe drop the token-binding reference all together (it's long expired and doesn't look like that's gonna change). https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-7.4.4 The SHOULDs/RECOMMENDEDs in this section seem a little overzealous and/or too specific. https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-9.4 Seems to be a lot of overlap and duplication between 9.4. Access Tokens (under 9. Security Considerations) and section 7.4. Access Token Security Considerations, which could/should maybe be reconciled. "(#pop_tokens)" hints that some text was copied from elsewhere but the markdown references weren't fixed/updated https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-9.5 "(#refresh_token_protection)" same thing about markdown references not fixed/updated https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-9.6 "(#insufficient_uri_validation)", "(#mix_up)", "(#open_redirector_on_client)","(#csrf_countermeasures)", again "(#mix_up)", and "(#redirect_307)" https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-9.7 "Attacker A4 in (#secmodel)" "The use of PKCE is RECOMMENDED to this end." PKCE is required elsewhere so this doesn't seem quite right. Similar comments about text ijn https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-9.14 that talks as though PKCE might not be there. https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-9.17.1 "(#redir_uri_open_redir)" https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-9.20 "(#client_impersonating)" https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#section-12 " * Redirect URIs must be compared using exact string matching as per Section 4.1.3 of [I-D.ietf-oauth-security-topics]" Should that maybe be qualified to cover dynamic ports on ephemeral local http servers used for redirect URI with native clients? BTW, does [I-D.ietf-oauth-security-topics] need to make a similar allowance? https://tools.ietf.org/html/draft-parecki-oauth-v2-1-01#appendix-C "TBD" Given the potentially high visibility of an OAuth 2.1 effort, I think it'd be worthwhile to list organizational affiliations of individuals here in the acknowledgements along with their names. Something like what was done in the first part of https://tools.ietf.org/html/draft-ietf-oauth-access-token-jwt-06#appendix-A. This can help with visibility and justification of (sometimes not insignificant) time spent on the work by non-authors/editors. -- _CONFIDENTIALITY NOTICE: This email may contain confidential and privileged material for the sole use of the intended recipient(s). Any review, use, distribution or disclosure by others is strictly prohibited. If you have received this communication in error, please notify the sender immediately by e-mail and delete the message and any file attachments from your computer. Thank you._
- [OAUTH-WG] A token review of draft-parecki-oauth-… Brian Campbell
- Re: [OAUTH-WG] A token review of draft-parecki-oa… Dick Hardt
- Re: [OAUTH-WG] A token review of draft-parecki-oa… Brian Campbell
- Re: [OAUTH-WG] A token review of draft-parecki-oa… Aaron Parecki
- Re: [OAUTH-WG] A token review of draft-parecki-oa… Brian Campbell