[OAUTH-WG] Comments on draft-ietf-oauth-v2-1-00 (The OAuth 2.1 Authorization Framework)
Karsten Meyer zu Selhausen <karsten.meyerzuselhausen@hackmanit.de> Thu, 06 August 2020 10:48 UTC
Return-Path: <karsten.meyerzuselhausen@hackmanit.de>
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 8038E3A101E for <oauth@ietfa.amsl.com>; Thu, 6 Aug 2020 03:48:42 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.898
X-Spam-Level:
X-Spam-Status: No, score=-1.898 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-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=hackmanit-de.20150623.gappssmtp.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 Y12cc-9WNglM for <oauth@ietfa.amsl.com>; Thu, 6 Aug 2020 03:48:39 -0700 (PDT)
Received: from mail-ed1-x52b.google.com (mail-ed1-x52b.google.com [IPv6:2a00:1450:4864:20::52b]) (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 CB6693A101B for <oauth@ietf.org>; Thu, 6 Aug 2020 03:48:36 -0700 (PDT)
Received: by mail-ed1-x52b.google.com with SMTP id v22so24015314edy.0 for <oauth@ietf.org>; Thu, 06 Aug 2020 03:48:36 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hackmanit-de.20150623.gappssmtp.com; s=20150623; h=cc:from:subject:autocrypt:to:message-id:date:user-agent :mime-version:content-language; bh=Cz11XR0sdbBgSgtPiYn4AvTL+aCJEPfhBDp7/E9gqIA=; b=VkJctib3fZoyyWy67YB/AKOD4BBdSHbNg0rev62xT6TIuL9c3CFnwHfRT2dKgM/fqZ yDvwt/VLrOAiNlV9a8j55vl5QBuIGxraiwH096peRE5g+Vxr0u+QtSQvjY7xlsVvGUb0 MZpRBDFaBuv74iOoKyddWDbpuvItb6+ocS6cY7k0fQSeRvwRVlyMvRhRgoggl9BIYGSy zo96xWBYkQMl5OV2SBNCZ9RaA4wxF1wlyH6Cusy01dQxirnE7maIuGC2U/QdX8/N5E/b cMXTNtsx3Y+pRJBheLGckwHloWjLDGOIVV3k7A7u6LX0Oni7HE9bmS7Z2kuWXKHbUBDk YzrA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:cc:from:subject:autocrypt:to:message-id:date :user-agent:mime-version:content-language; bh=Cz11XR0sdbBgSgtPiYn4AvTL+aCJEPfhBDp7/E9gqIA=; b=py4nN7Goqzf32QGK4lYh/xgjgLLlY26Y53WtgyJ6xIw0zrrEkTzycITGskxDqfkU6k 53S+z+Hkqt8ziDmmcT92fHlREdoKGMzDTXqFUGlhx8kf3oEdg/2FpfnoCx2WCaAiEzYX u2qC5tK1B6gagoCUBNrAycTZCgJTlmUeJX6u6LPQdntOH+5d0Alerh5shcCq+yKtNnVZ Oi7eD3sptlEbDqexk43sZZCtJfjcDdmNene0HEWhc23SbhaHhS60HJlC0VOLiOiq0l7Z f4t7PzeLuQDMTGozfFdY0ETI1NYTopUO6LndAedgd7gjBD9lb++wLUY5na9D2kkpQsKw IIDw==
X-Gm-Message-State: AOAM531iBHBeuE6N9DRT3QAj7gsTRR0DJTS11fAUZgXzcUIz4zFKeuGq Fu7sqAjO8oxqjWEq6OWxcsn6EPOHZm0=
X-Google-Smtp-Source: ABdhPJxeheUKNCQbWlibV+bpuuegUs2vKx8BeH2gfY1E7PYSyoOIutTduTbiEgK0d5RorGoN1vKm7Q==
X-Received: by 2002:aa7:d9d7:: with SMTP id v23mr3628770eds.112.1596710914731; Thu, 06 Aug 2020 03:48:34 -0700 (PDT)
Received: from [192.168.178.22] (b2b-37-24-87-133.unitymedia.biz. [37.24.87.133]) by smtp.gmail.com with ESMTPSA id e8sm3138925edy.68.2020.08.06.03.48.33 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 06 Aug 2020 03:48:33 -0700 (PDT)
Cc: Christian Mainka <christian.mainka@hackmanit.de>
From: Karsten Meyer zu Selhausen <karsten.meyerzuselhausen@hackmanit.de>
Autocrypt: addr=karsten.meyerzuselhausen@hackmanit.de; keydata= mQINBFh1IBMBEADV73c10lB7zeFy6/ezLFzOBp8z6Zy1zUyIrf6RoBk1GQWREcGEGeaL90Pj F5plZeASVJdsEYnYXdgcIPE0tlBq6al6OYoWtH/VbFPWEPLVhA3rL1iXVJveD3J40OzSYP8N G7bla3zQ2+TXOB3iDPPsHZUdHCLASkIIWQK6+fE1C2epAdPtnsLsb++1d080jfXXwgyUUh4y bimcy9Jg5oZ4QMwnSq3Y+x38PNb+nTgjDi1X/89/WsNd7Bdh4Zvw3CAuc/W58CFaDjb7liUD YRoAp6ysnjPKEUSnAnMpgaiXJc1gFoL+ahdKJ3D9XTn28NTjUrvOkVidsuKbyxnXP9I6BO6i 2jzjrH6TEAfIYMjZlYTyPZTt271SW5iAHYwvPZWlqQTBT2P/d4gHl0To5b4e+UXxjQgxqUyi QIcxh3Ris21Kx4lKQKDXYWiwNTZzx8AdqrcxCWfK+MRpFyk0B+4uDMm7Apm5ZWwDKN/JnVsJ yokkkrrHs/elRCUGtN9NyhJQf3VnE87862Pej8PVvQJr3uVnoNX2yieTvJZftIOBG1b9ta6Z BcYyn3un1rSn7lBPg+RSnPemposVorQpjGwT+Dhg13Bpv5q0JfSc//js/nB6A4iq5YssdtQ7 35QBWLLaF1oCxalvrQVDD4Sh06eAUQsga9xeE0yv7sxqdsozdwARAQABtEJLYXJzdGVuIE1l eWVyIHp1IFNlbGhhdXNlbiA8a2Fyc3Rlbi5tZXllcnp1c2VsaGF1c2VuQGhhY2ttYW5pdC5k ZT6JAj8EEwEIACkFAlh1IBMCGyMFCQlmAYAHCwkIBwMCAQYVCAIJCgsEFgIDAQIeAQIXgAAK CRBFNcDn2xbxSK7sEAC5hk0VQHo2+fMV3b4TgSt4qSPLz6EnWwoqcEzUGYHErQXy7tCENpqS rsZsFphpgvWo1tcQdpyQTFm0dry4ASJD78lEiYC/8Hedp0fIaJTGwxrSLpRxV/Wb+iqkbgz8 /Qydl3QyupSqznSHQMd0uhvzHLxoYvHAIKy52gCK0T9gmxcCIh7UEjDfm+kqHp+oU4sbNe+2 ZEtJLuCKW+amNyqnHXr7ehAIaYmTdKOEcUb2UM7Yzp9g4kSkg1GbPlAn6yjyAqJ96sobKFXX S3rkXksRTxkGKW278Nrs4UBO+OIu32kIXCM2m3fKaUK777jAQu1e8sdj2nL0sPWQvMikZRx6 0dy+wVuH8gGHZsd7rW201Sv5pAhSAK4l58GS3xSLId6smXCend9Vu+tcYA+Bb+45943LmoPA PrdIUeI+zC9pjGwm+x+jFiCxbChqAiJF7RyYv9crziEYnTQ70gHGNOTOTIS5t0ufc9D4wD4O IkkrPQYg3KcAqP2Kyj1uHcqdk7XEhV1fdTXdeEt1e7auWPh0d3Fo+BTtiGXfNMuORArE0El6 ky8eUOqZEJ8rYpEGDLt0JFkJM5AhX4PrQWekjaMhQ5yl/+M+Ss0V0JkImagSgWdvUn1+eAs0 zEuVxTc6ON69mIyMalQ5d4ofvPnKr3GNVmEiXAVDMGUZHoeabfgSBbkCDQRYdSATARAAsp2V mr3N7iNND8+M/OyA/OwcDQ6utZh+m4TnKsOVdiNLGpu2U3/2Qg3yrbjic2dWx1CsS6VH2/oO 1e/a4FlxA93wFv/OZjiUjHtEvdIJeHWlCvWOUlMsqyGDc3Q75fNjFw6DGKkiOu9lZaBs6naS BmkvAMGjV5bNKLyIL5j7Im1pCdZ2lCjD7eVwR3RQQKobTmu916htX8g1cB9yFmquu37X+ZBl A4GLJi63Kw0L2r8i8iO1NqDLOfT8IeNkOroEm3SDAuEApGAubKLSPBJ1khQ7kDhpdfzSYKUF tiIHpGWVOImDjqf4JIcF7OIdRPQfFPlwoPnsyBAS8znQJvmqbbMowgFZe3UMLAN78CETZHGM OLBPB873oWyZ07Ar4v/SL5/aD+FRj2VnYEcGwt0HMmMyaN6ed8Udj4OTNZ7ceZA1Tw8/lZuI KCamj0XfJIK6376RCGnqjsEfS65P1KWZXfWphCKWp2c7uWKtau1q8pgiVRoBSAmjvfXRrIvK LhhQyNOiCUDKrvEWpoeq9y5GTrY27ncLov8nSR/SUPOw5HwJmzdFjhOF9XAOtiND/QRH886O IohdlnUu668mwLCmL2ROe7XWcTkFQWLDg+5b0bC9dgfL+HHpWGUdQPG3CCyPG5LfDmnmuXkE eU1kSD27kFe1kM6pfqpCydJW66DuwoMAEQEAAYkCJQQYAQgADwUCWHUgEwIbDAUJCWYBgAAK CRBFNcDn2xbxSAAbEACeIsfrsq2tlyigZv+bwkiVP1oKtWfXN1e3K3lDOBqPJaPXWFOopq/1 9osk58PFtVEaDlYPlN/NP6Jq5nTTC8QyLG3swAdo4ZJXWEg1NTRu8ddYUvZWuRHWRghaq7qh eW5lVPqilCndSG7bkDPU/Vyd93nPKnKTKKs/Nd7ePneWA0JQohEg5gO/GU0v5SN3YfTxG1LV Cxu3HHHFodDLK4KITSYmt1+g0WCADeclwm5+L5lIvgKQvcIpjpMGNK1wj2E3exsLlgo/ZEyS AslOPXyQw2yfYLrcfGpvWa3e+AvU7eLVBgihskpibJg53yw31B0CXAJBbjg7AsxR8UE5pl6h 2gTjN2t++GvqefGtw/bPvx2RzFsorh1/RYaFgcaFyefghmpi55iiIhgEOiSIct0LoYl3cmH8 DGYKhSskpSDgfE41Esk/P2odeax9SmJuv4mnqkiGFPpTwCfUka2k0mCpBDpfTdECWUFhreGS qFbrvJDZRBiyaVyCjOvOc0v6Z0/iIRgHWTjITpqaQh69kqAtt9GQWV6i3THnpHFlIC2ecvdc YCagneZdoLEHCS8Nois/uDbp5qZwZcF5zKMI+T7u6Qf8EGdvxCB1fp0Sdlmeto0c6/gnFUix 4J/tozBwSXSg7JCxTrUdnJtcQAJzosOUZTVO/ZZR/n0+904kud6o3w==
To: oauth@ietf.org
Message-ID: <a5c9ee0e-5b41-c4d3-ebff-d4a7985b663f@hackmanit.de>
Date: Thu, 06 Aug 2020 12:48:31 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0
MIME-Version: 1.0
Content-Type: multipart/alternative; boundary="------------C632C9223B4FCCE2A57344C0"
Content-Language: en-US
Archived-At: <https://mailarchive.ietf.org/arch/msg/oauth/eu3c9t99rqVHiqcsef4svwCjPrc>
Subject: [OAUTH-WG] Comments on draft-ietf-oauth-v2-1-00 (The OAuth 2.1 Authorization Framework)
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: Thu, 06 Aug 2020 10:48:43 -0000
Hi all, great to see that the OAuth 2.1 draft was adopted by the WG. We appreciate the efforts to create a single document which contains the current state of the art in terms of OAuth and all security best practices very much. It will make it a lot easier for developers and others to get started with OAuth. Christian and I reviewed the current draft (https://www.ietf.org/id/draft-ietf-oauth-v2-1-00.html) and would like give some feedback. You can find our comments below; they are sorted based on the section they affect. I divided the comments into two parts: The first one consists of suggestions and improvements to generally clarify the draft and increase the security of deployments implementing OAuth 2.1. The second part contains typos and other minor mistakes. There should be no need for further discussion on the list regarding these comments. We are happy to hear your thoughts on our suggestions. Best regards, Karsten Comments: Part 1: Suggestions and Improvements Section 1. Introduction - 1.4: "The string is opaque to the client, but depending on the authorization server, may be parseable by the resource server." - What does opaque mean in this context? The client is technically able to decode an AT if it is, for example, a JWT. - For refresh tokens the wording is not as strong: "The string is **usually** opaque to the client." (see section 1.5) - 1.5: In step 2 and 8 the AS "authenticates the client" but in step 7 the client provides "client authentication if it has been issued credentials". Should the possibility of public clients and credentialed clients (which cannot be authenticated) not be taken into account in step 2 and 8, as well? - 1.7 (and in general): HTTP status code 303 shoudl be used instead of 302 in this section and generally in all examples due to the recommendation to use 303 in section 9.7.2. Section 2. Client Registration - 2.3 (and in general): Is "client authentication" the right term for credentialed clients? We think the term "authentication" is confusing because their credentials should be considered to be public. Therefore, the AS can not be sure about their identity when they present their client credentials. - 2.3.1: An AS MUST NOT accept requests with ambiguous client information (both HTTP Basic Authentication and client_id/client_secret body parameters). - Different application logics could extract and use a different client_id from the request otherwise. For example, the HTTP header is validated to pass client authentication but the client_id parameter is used to check if code/RT was issued for this client. - Should also be considered in 4.1.3. - Implicitly included in 5.2 Error Response: ""invalid_request": The request ... includes multiple credentials, utilizes more than one mechanism for authenticating the client …" Section 3. Protocol Endpoints - 3.1.2: "The authorization server MUST compare the two URIs using simple string comparison as defined in [RFC3986], Section 6.2.1." We think the term "two URIs" is ambiguous and suggest to name the parameters here directly. - 3.1.2.4: "If an authorization request fails validation due to a missing, invalid, or mismatching redirect URI, the authorization server SHOULD inform the resource owner of the error and MUST NOT automatically redirect the user-agent to the invalid redirect URI." We think the term "missing" is inaccurate because it might indicate that a redirect URI should always be present. Maybe the paragraph could be changed to something like "If an authorization request fails validation due to **an invalid, a mismatching, or a missing (if it was required**) redirect URI, the authorization server SHOULD inform the resource owner of the error and MUST NOT automatically redirect the user-agent to the invalid redirect URI.". Section 4. Obtaining Authorization - 4.1.2.1: ""invalid_request": The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once" Is this only relevant for the parameters defined in this RFC or also for extensions? For example, RFC8707 (Resource Indicators) allows to use multiple "resource" parameters in the authorization request ("Multiple resource parameters MAY be used to indicate that the requested token is intended to be used at multiple resources.", https://www.rfc-editor.org/rfc/rfc8707.html#section-2-2.2). - 4.2.2: Combine "The client MUST authenticate with the authorization server as described in Section 3.2.1." and "The authorization server MUST authenticate the client." below the example. The second sentence looks oddly placed below the example. Section 6. Refreshing an Access Token - 6.1: Is token binding supported by any major browser? AFAIK it has been removed recently, so it could be removed here as well. Section 7. Accessing Protected Resources - 7.2.1: The RS MUST NOT accept resource requests which use more than one method to transmit the access token. This should be stated more clearly here and not be included only in 7.2.3 Error Codes "invalid_request". - 7.4.2: - "As a further defense against token disclosure, the client MUST validate the TLS certificate chain when making requests to protected resources, including checking the Certificate Revocation List (CRL) [RFC5280]." The paragraph should also mention OCSP [RFC 6960] as an alternative to CRLs. - "Bearer tokens MUST NOT be stored in cookies that can be sent in the clear". Maybe state explicitly that only cookies with secure flag are allowed? - 7.4.3.4: Maybe add recommended cookie flags (secure, httpOnly, sameSite) with note "or equivalent measures" (to ensure that future cookie flags / alternatives are allowed). This is also discussed in this thread: https://mailarchive.ietf.org/arch/msg/oauth/f18vynrMXC4dj4tqck7SZIl4xp4/ Section 9. Security Considerations - 9.2: "Authorization servers MUST require clients to register" should be clarified to "Authorization servers MUST require **native app** clients to register" although this is already present in the headline of the section. - 9.7: Explicitly state that code_challenge / nonce must be bound to user agent, as well? - 9.15: Adjust text according to CSRF part in 9.7. - 9.21: Remove the section as the only present recommendation is redundant and already covered in 9.6. Appendix: - Appendix C: - Sort Extensions either alphabetically or depending on the RFC number (drafts below the RFCs). - Should RFC7592 (Dynamic Client Management) really be included as an "well-established extension"? It is only categorized as "experimental" RFC. General Comments: - Unify the order of client types: In 2.1 and 9 it is web application, browser-based application, native application, but the main section about native applications (10) is in front of the main section about browser-based applications (11). This should also be taken into account when sections are added to 9, 9.1, or 9.3. Currently 9.1.1, 9.2, and 9.3.1 are about native applications and sections about browser-based applications should be added in front of them (if there will be sections specifically about browser-based applications). - Is there any statement that it is allowed to add additional parameters in the authorization/token request/response? Part 2: Minor Mistakes (typos etc.) Section 1. Introduction - 1.5: "If the authorization server issues a refresh token, it is included when issuing an access token (i.e., step (4) in Figure 2)." -> "If the authorization server issues a refresh token, it is included when issuing an access token (i.e., step (**2**) in Figure 2)." Section 4. Obtaining Authorization - 4.1.2: Note "(with extra line breaks for display purposes only)" is missing in front of example. - 4.2.2: Note "(with extra line breaks for display purposes only)" present but there are no extra line breaks. The note is not present in 4.1.2.1, 4.1.4, and 4.2.3, for example. Section 5. Issuing an Access Token - 5.2: "The provided authorization grant (e.g., authorization code, resource owner credentials)" -> "The provided authorization grant (e.g., authorization code, **client** credentials)" Section 6. Refreshing an Access Token - 6: Note "(with extra line breaks for display purposes only)" present but there are no extra line breaks. Section 7. Accessing Protected Resources - 7.2.2: Note "(with extra line breaks for display purposes only)" is missing in front of last example. Section 9. Security Considerations - 9.1.1: "public native app**s** clients" -> "public native app clients" - 9.4.2: Missing reference to "(#pop_tokens)". - 9.5: Missing reference to "(#refresh_token_protection)". - 9.7: - Missing references to "(#insufficient_uri_validation)", "(#open_redirector_on_client)", "(#csrf_countermeasures)", and "(#mix_up)" (twice). - "Clients MUST store the authorization server they sent an authorization request to and bind this information to the user agent and check that the authorization request was received from the correct authorization server." -> "Clients MUST store the authorization server they sent an authorization request to and bind this information to the user agent and check that the authorization **response** was received from the correct authorization server." - 9.7.2 (and in general): Replace "user agent" (used 18 times) with "user-agent" (used 64 times). - 9.8: Missing reference to "(#secmodel)". - 9.18.1: Missing reference to "(#redir_uri_open_redir)". - 9.21: Missing reference to "(#client_impersonating)". -- Karsten Meyer zu Selhausen IT Security Consultant Phone: +49 (0)234 / 54456499 Web: https://hackmanit.de | IT Security Consulting, Penetration Testing, Security Training Unsere nächste Live Online-Schulung zur Sicherheit von OAuth und OpenID Connect am 24.09 + 25.09: https://hackmanit.de/de/schulungen/109-live-online-schulung-single-sign-on-sicherheit-oauth-openid-connect-am-24-und-25-09-2020 Hackmanit GmbH Universitätsstraße 60 (Exzenterhaus) 44789 Bochum Registergericht: Amtsgericht Bochum, HRB 14896 Geschäftsführer: Prof. Dr. Jörg Schwenk, Prof. Dr. Juraj Somorovsky, Dr. Christian Mainka, Dr. Marcus Niemietz
- [OAUTH-WG] Comments on draft-ietf-oauth-v2-1-00 (… Karsten Meyer zu Selhausen
- Re: [OAUTH-WG] Comments on draft-ietf-oauth-v2-1-… Dick Hardt