[OAUTH-WG] WGLC review of draft-ietf-oauth-security-topics-13

Guido Schmitz <g.schmitz@gtrs.de> Fri, 22 November 2019 17:01 UTC

Return-Path: <g.schmitz@gtrs.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 EAEB91200EB for <oauth@ietfa.amsl.com>; Fri, 22 Nov 2019 09:01:05 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.897
X-Spam-Level:
X-Spam-Status: No, score=-1.897 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_NONE=0.001, URIBL_BLOCKED=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 75J3iQYMDocl for <oauth@ietfa.amsl.com>; Fri, 22 Nov 2019 09:01:03 -0800 (PST)
Received: from mail.gtrs.de (mail.gtrs.de [94.23.167.223]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id CC93712004C for <oauth@ietf.org>; Fri, 22 Nov 2019 09:01:02 -0800 (PST)
Received: from [129.69.182.104] (pool4-104.sec.uni-stuttgart.de [129.69.182.104]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.gtrs.de (Postfix) with ESMTPSA id D37011C0198 for <oauth@ietf.org>; Fri, 22 Nov 2019 16:45:30 +0000 (UTC)
To: "oauth@ietf.org" <oauth@ietf.org>
From: Guido Schmitz <g.schmitz@gtrs.de>
Openpgp: preference=signencrypt
Autocrypt: addr=g.schmitz@gtrs.de; prefer-encrypt=mutual; keydata= mQINBExwZ1UBEACrRLUyt0jbab7/nCjULVkApY3mu+pLUfekl+yfw9XfUHFrmFonu4zYAFu8 dduseM9m0AG5b50i7L/dyJjgMlOx/WqoDycdIh37l3u6rU7EY7UY1w88pywOcUt64NW9uQnz 5wQU/UMC/JuFfGKirVTmtVY9kedfrcry8pJ5IPxmKxxf/LA42Qos/DSOzZpJYkeUM4z9l73t GB8C+WrMjGEviyQfQhlx0Ad1MbDF9ZavTSAKnRtX599BCl2p33l4iIq5ebh7Gq19LHACd3c8 SGLNnt8TqRXUo1HPE8ug05hagzc+85eAW37tjesFpkKQB4miXv6IB7tW8We128H3gxbWXzZb kktAHocZkAxoDPIPuxW5h3X4kotDb7aQsDM/Rbnc/YhVHd0L9EHnHkFbL1MJa+LXP0eCwq5D Ud51rtE5Ktmzhc0AZ0AYEDQNSyD4KwiRM9idMjM2DmNRvjyTL7VJj3NdVX+HkeVRhwkI6eCI U7I65yNdAKPfxSEhK1RKjXDzhEr3IZwZMjzwaaTt77sjIZan5UclTKP3tEYu0VknbDAusClB 9omg4jq8qM94lqSrHDVpps4xmMTW3T+Hwl6kHTsvLbz2q/QROItletxdfeys0y0jeD1pJ7Uu bQmUS8YeJa10PLNgUWvho+6gng5lmYJppmjXLcO4oL0v8TQ2mQARAQABtCtHdWlkbyBUaW0g Um9tYW4gU2NobWl0eiA8Zy5zY2htaXR6QGd0cnMuZGU+iQI7BBMBAgAlAhsDBgsJCAcDAgYV CAIJCgsEFgIDAQIeAQIXgAUCTHBpZAIZAQAKCRAgw0pxFWieqbHXEACPKQS/0Cmq2kur8FLL g3YlehjORZDxcnF3XzC6qkD0iJu5xmwMxNgwGElD6ezRWRkNhc4C0G0YmG1IKB3qbpRLVMke ApjkNcoKkffvEACReUxNSzaNhRvzWdK8fBLs4DFqudV5iBC0kmm6DjzUccNXMjbFn7smYVbR MIsZ74hNUkpn44g6oB3myR5s8VdQdKiQJmP9qdwNtfDI81HBK1jU9dJSS11M2pxmhklYZ8us mjkNEFi9qy3Prx1vlyio/78TP0xwTkIRe48Kg6oMdXKWFjXq/K2iOm2d0rRzvpF8C3mE8MWP YsGD65XHlhtJ0bAJw+F5xVhvzFkRPOPwjmgZ2RKfChchTb/L5mD9+2kor0dWiFGNCs0Dxa5T j0PLd0ExYzZi6GRs+Yvz3moSiN92Rlvoasgebp6pUM1kfTsh9ln7IMWUZW7UeQDymT2HHYfx Gc/qiBU5u2kT7qzAfN6mJkDYQZthzirETcnHlElX6YZQRR2gqov8Px5miMaIGxW514vwCTvD 7Lh8jnTrxDYCCkwItscBMNxNeySYYMQoDnfPHeMGnEa0NoykRaZDAYXBrDduM4E6fJoRhlM/ RLi3XvxtkDNMejHXWq5Z3znULfFVy/OUPanoOC8Qd15YQMA4kVJ4u+Wr2h5dpwrIvB6O3zpE yhgES3bXkqo7KHHjTLkCDQRMcGdVARAAts2esuf7nLVY2PBKMw8J9eN5qo6u+5O4rq5v2vn0 iDRKvOO3pttLx3bCMCFBuWqiXN0LGssAfYa2UXQZx4UhGMVa/Bz2W0k+DkAZjBDTqEDmdBl8 pNVTWNeHf196natvO6xWKL5UQed9LfOsDnwRxlt47f/h+dbaw6AoVKeQ1UuQGH/xOHjdbjBv 15CzxQtHOj30DuqTLdVbM65u2bF+MwGAEC8NGexiQmYmns/UoKbkmXgZc5mm8w4M1O5xI7MI I5YT4HIPPRPzUFL347Mt5nRpuW3H8JIiBCv8YBlTMzTS1IslP0+yERsS8SBRM3U34hH3867Y WwQ2gfKq5Hmycmj3H1X/fBD/kosOKxRsMhEl9vEc1n2FYvb/tQc5vKULR/ZQW4aZWGRMKbni GP65lTF9FxWSyaddNZi/RqBZnYiWc6Z+M5Qr/KSrOl2h7IE9Hhz4ZAupVam49NGuptf8RBmv NyCWrGXvR09/sSbPw+unkR4xHMQDDNI9HlkuiiVf1i0s/16jGyHVbZG8bUlSWOsN2CTR3bXO Pa5a4osEb/DAhGZrgz88/6fNNv4oWPpAv1NoVfXiwtoFn2fIus8RxL7+TR2IhUOzUZ8HLPTl gZAHLIui8dTrF2PyawUNaaELujmMQP/X/sSiUHO7BqsUBFhSDmT3sDkqPYqDBmhMXL8AEQEA AYkCHwQYAQIACQUCTHBnVQIbDAAKCRAgw0pxFWieqSoLD/kBN3aIqikEdTR3p2newQ+Kzyvx xTt1PSrrDWWZ9+hdqNJSJSv85cq9nJOpJLSCsBpapZCR5OXOPbuvUtmfKpZo90zbeGPHVuvL 1i4qrV+YTvSsFR8ORkfzu6Jyh+MYSeWTKfSW53Re/TupJV/0PcNmOw5XufdHSsS1kdF+A53v cR1oXBXwBAlu4wdvPejjE3ON4l4Ge2GHyhoTXj6iOYOwcebHHJ0BMg2py5IWoII1HM0dfT9r hcOvlc2Wegy4I6+tFnWdiWXAzYpHcxiWdyRIy3ElQj+sQisQqEvJaIUYW8ul3I5pUuKhlKbQ qwA9DA5pYwMcwiRvHslc8dOWK1XUF03xRrNq4mA6Un+mFyteHlDDKfXE9695KcxgZsE93kNf 9ij+zU02/65xellirmk8s/GXoBxj7mhV/CJ9XRC8ZVg2ZzGP/UtyS+5mW6KmbJk5o/Lv/R3n S/fBQE5yevffwhFu5b/Zo8B/l1MQ7DMSLG4y423ciOZE2yYiwYmlddalhrnYVnQ7+9BFnDAl ORUf7ffJlp3YHDfi7+ytMiDjiWbqKzf7hFcybptYpVdm0WS+9mLVKrfYhLAuaK2M7xZSvzZP T4Fe4GGgZELOyx8qGiiBMDRVpLCV2uze4mW7ARr4VcSWN2HbB6pzGmgxF4qwJJUzxhc8wklS IEKDvXjlUw==
Message-ID: <a017889e-f50a-3e34-f8e5-477a56518051@gtrs.de>
Date: Fri, 22 Nov 2019 18:00:55 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/oauth/ypfU4wNkvtbQRO66Xos3f898cQQ>
Subject: [OAUTH-WG] WGLC review of draft-ietf-oauth-security-topics-13
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: Fri, 22 Nov 2019 17:01:06 -0000

Hi,

All of my comments on oauth-security-topics-13 are
remarks/questions/suggestions for clarification in the document, i.e., I
do not have any fundamental objections. Overall, the draft is, in my
opinion, in good shape to be published and as already discussed, open
points can be updated later. I think that it is important that the
document should become a BCP as soon as possible.

* Section 2, Attacker Model: While A1 and A2 are quite clear, A3-A5 seem
to be extensions for A1 and A2 respectively, e.g., we could have
attackers like (A1+A3) or (A2+A4+A5). A reader not fully familiar with
this concept might misunderstand this classification. A sentence to
clarify this would be good.

* Section 3.1, Second Paragraph: The reference [owasp] is quite coarse
as it points to the entry page of the OWASP organization. I suggest
pointing to the OWASP cheat sheet on open redirects at
https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html.

* Section 3.1, Third Paragraph, Section 4.7, and other places throughout
the document: (Please excuse that the following might be a bit
nitpicking.) The term CSRF might be misleading as we are not talking
about "classical" CSRF, where a party is tricked into (implicitly)
believing that a request was caused by its own origin (from a web
browser), but was actually sent cross-origin. Here, we are talking about
requests which (by protocol) are typically intended to be cross-origin.
The point here is not that the request is cross-origin, but that it
originates from an unintended (cross-)origin. Maybe you could mention
that we are talking about an advanced kind of CSRF in the
double-redirection context.

* Section 3.1.2: What about SPAs? Either they use authorization code
grant with the help of some server or use CORS to access the token
endpoint. The token endpoint, however, then needs to support CORS. The
obvious question is then, what is the CORS policy of the token endpoint?
It is probably fine to allow all origins (if this policy only applies to
the token ep itself and not other eps at the same origin) or do I miss
something?

* Section 4.1.3, "fix fragments": The source [fb_fragments] only says
that Facebook adds the fragment #_=_ to some URLs. There is no
explanation or reasoning in the source (as well as in this document) at
all that this technique strips the fragment from redirects. (Mike Jones
also commented on this.)

* Section 4.2.4, Bullet Point "referrer header": Adding the
rel="noreferrer" attribute to links does not protect against leakage via
third-party content. Referrer Policy is the only effective
countermeasure of the mentioned two as it can be used to completely
suppress the Referer header or at least strip it to the origin. Should
we give a concrete example for Referrer Policies? For example, the
header "Referrer-Policy: no-referrer" in the response completely
suppresses the Referer header in all requests originating from the
resulting document.  Also, this measure is easier to implement than
adding rel="noreferrer" to each link. (BTW, there is also an excess
quotation mark in this paragraph.)

* Section 4.3.2: You mention postmessage communication (I think Hans
also pointed to this). Afaik Google uses postmessage in their client
library. Is there any document describing this technique?

* Section 4.4.1: The attack description is based on a very concrete
example. Some details, however, are not relevant for the attack to
succeed. In Step 3, the exact response code could be amended with an "e.g."

* Section 4.5: Add pointers (A1) and (A2) to web or network attacker in
last sentence.

* Section 4.5.3, "Code-bound State": This requires that state is fresh
for each flow. You should mention or even emphasize this.

* Section 4.5.3, "Per-instance client id/secret": This essentially says
that native apps or anything that might be able to obtain and store
client id/secret (e.g., using dynamic registration) could do this. Web
SPAs could do this as well (e.g., using Web Storage). The last sentence
of this paragraph is somehow confusing.

* Section 4.5.3, Second Paragraph after bullet points, "Note on
pre-warmed secrets": What is the context of this note? Does the attacker
start a flow on some device, extract secrets, and then pass this device
to the victim who then completes the OAuth flow? (Mike Jones also
remarked that "pre-warmed secrets" is not explained)

* Section 4.8.1.1, metadata parameter "resource_servers": This is
supposed to be an extension to RFC8414, right? (Also mentioned by Mike
Jones)

* Section 4.8.1.1, return parameter "access_token_resource_server": This
is supposed to be an extension of the token endpoint (as defined in
RFC6749), right? I suggest adding a pointer to its definition.

-Guido