[OAUTH-WG] [apps-discuss] Apps Area review of draft-ietf-oauth-v2-threatmodel-01
S Moonesamy <sm+ietf@elandsys.com> Mon, 23 January 2012 23:40 UTC
Return-Path: <sm@elandsys.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 A5A4C21F86B6 for <oauth@ietfa.amsl.com>; Mon, 23 Jan 2012 15:40:57 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -101.465
X-Spam-Level:
X-Spam-Status: No, score=-101.465 tagged_above=-999 required=5 tests=[AWL=-1.166, BAYES_00=-2.599, MANGLED_LIST=2.3, USER_IN_WHITELIST=-100]
Received: from mail.ietf.org ([12.22.58.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id jCllB9iOVYSN for <oauth@ietfa.amsl.com>; Mon, 23 Jan 2012 15:40:54 -0800 (PST)
Received: from mx.ipv6.elandsys.com (mx.ipv6.elandsys.com [IPv6:2001:470:f329:1::1]) by ietfa.amsl.com (Postfix) with ESMTP id 99D9621F867B for <oauth@ietf.org>; Mon, 23 Jan 2012 15:40:54 -0800 (PST)
Received: from SUBMAN.elandsys.com ([41.136.234.130]) (authenticated bits=0) by mx.elandsys.com (8.14.5/8.14.5) with ESMTP id q0NNebXa012758 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 23 Jan 2012 15:40:49 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=opendkim.org; s=mail2010; t=1327362052; i=@elandsys.com; bh=lCqBVooJ016BSRsUXp/+3S9c+4Ev8cP+WOY0dLE6mdI=; h=Message-Id:Date:To:From:Subject:Cc:Mime-Version:Content-Type; b=TJkGBmiC7w8nXONHfSgMt5c3flAaz/9/101eVTNQND0zlukskXlJhB2kXAexRo0UI OoXjeG4fqmMR/Zm1y/PKNC0OoG791HY+XwNlk8SNhx274ZUNsY0cV+9ZGJmLPl+PkB HyX4lLxoRap4RJfjaERzk81aM7nlv5VeNxlf4tUc=
DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=elandsys.com; s=mail; t=1327362052; i=@elandsys.com; bh=lCqBVooJ016BSRsUXp/+3S9c+4Ev8cP+WOY0dLE6mdI=; h=Message-Id:Date:To:From:Subject:Cc:Mime-Version:Content-Type; b=xCjhV6+pjGGNfPC8YWJ1Aeaun8dcsF1+i82m900v2G4/4pRG+MStBB9e0riYdN3W+ uBQnS1oMpQ6hloROmB4UHcTANkB8c+C1tA0eiMWm9kh8XuULbgUkvkVTuvVJ/E3i6c cI7Xg95Lttri8HXekEW2Anjf4ysgPkDF87SuWyfw=
Message-Id: <6.2.5.6.2.20120123134319.0a76c338@resistor.net>
X-Mailer: QUALCOMM Windows Eudora Version 6.2.5.6
Date: Mon, 23 Jan 2012 13:47:33 -0800
To: draft-ietf-oauth-v2-threatmodel.all@tools.ietf.org
From: S Moonesamy <sm+ietf@elandsys.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; format="flowed"
Cc: Tim Bray <tbray@textuality.com>, oauth@ietf.org
Subject: [OAUTH-WG] [apps-discuss] Apps Area review of draft-ietf-oauth-v2-threatmodel-01
X-BeenThere: oauth@ietf.org
X-Mailman-Version: 2.1.12
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: <http://www.ietf.org/mail-archive/web/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: Mon, 23 Jan 2012 23:40:57 -0000
The following is the AppsDir review performed by Tim Bray. It would be appreciated if a reply is sent to Tim Bray with a copy to the apps-discuss mailing list. I have been selected as the Applications Area Directorate reviewer for this draft (for background on appsdir, please see http://trac.tools.ietf.org/area/app/trac/wiki/ApplicationsAreaDirectorate) Please resolve these comments along with any other Last Call comments you may receive. Please wait for direction from your document shepherd or AD before posting a new version of the draft. Document: draft-ietf-oauth-v2-threatmodel-01 Title: OAuth 2.0 Threat Model and Security Considerations Reviewer: Tim Bray Review Date: Jan 23, 2012 Summary: This needs some more editorial work, but is basically sound. It's not clear, though, whether it wants to be an Informational RFC or not; the use of RFC2119 language needs special attention. I think a few of the "minor issues" are worthy of a little bit more work in another draft. Major Issues: The use of 2119 MUST/SHOULD/etc doesn't seem fully thought through. I normally wouldn't expect a "threat model" to include normative text. The basic idea would be to say "Here is an enumeration of the threats, and here are the tools available to OAUTH2 implementors to meet them." I was impressed by the enumeration, which seemed very complete and well thought through. But the usage of 2119, which makes statements normative, seems inconsistent. I can think of 2 ways to address this: 1. Remove all the 2119 words, so this document isn't normative, and publish it as an Informational RFC 2. Go through and clean up the 2119 language so it's used consistently; then this becomes a normative document. This is going to affect the references to this document from other I-Ds in the OAuth suite, which are currently in last call. Here are all the section-numbered notes enumerating my issues around 2119, as I encountered them: Section 2.3, I'm a little confused about the use of RFC2119 MAY in a threat analysis. When you say "The following data elements MAY be stored or accessible...", Is this saying that "The OAuth2 RFC says that the following data elements MAY be..." or is it saying something else. I don't think there's anything seriously wrong here, but a bit more explanation might be in order. I note a comparative absence of 2119-ese in section 5 describing countermeasures, where one would expect to find it. Also in 4.3.1, first bullet point, there's "Authorization servers MUST..." Also in: 4.4.1.1, 4.4.1.6, 4.4.1.12, 4.6.*, 5.1.4.1.5, 5.1.5.11 Related: "SHALL"?! in 4.6.3 Adding to the concern, there is use of lower-case "must"; note 2nd & 3rd bullet points in 4.4.3, which use "MUST" and "must" respectively. Minor Issues: 4.1.2 first attack: It says "An attack may obtain the refresh tokens issued to a web server client." This needs to be clearer... a "Web server client" can be a browser or a native app. Do you mean, "the refresh tokens issued by the web server to multiple clients?" 4.1.2 last attack. In the case where a device is cloned, wouldn't "Utilize device lock to prevent unauthorized device access" still be a countermeasure? In many devices, such cloning would carry along the user's device-lock settings. 4.4.1.4 2nd bullet. The explanation of why this wouldn't work for native clients wasn't comprehensible to me. I'm suspicious of any such claims because I can emulate most things a browser can do in a mobile client. Perhaps this would be obvious to someone who is an OAuth2 implementor. 4.4.1.9 I think where it says "iFrame" it might mean "WebView", i.e. a Web Browser control embedded in the native app. If that's not what it means, I don't understand what it's saying. If this is true, then the second bullet point is probably wrong. 4.6.6 1st bullet. I'm not convinced that the Cache-Control header will *ensure* that a client protects information properly. Should say something like "minimize the risk that authenticated content is not protected" 5.* The enumeration, for some but not all of the countermeasures in this section, of the threats against which this is a countermeasure, reduces readability and, unless it's generated automatically from the underlying source, is redundant information, which is unlikely to be consistent between sections 4 and 5, and adds difficulty to maintenance of this document without adding much value. I'd just wipe all these bullet lists out. If it's generated automatically it's less damaging, but still reduces readability. In the current draft, this is there for some countermeasures but absent for others. Another good reason to just take it out. 5.2.2.5 Device identifiers are very tricky. It's correct that IMEI is not adequate, but there are ways to do it without SMS. For more, see http://android-developers.blogspot.com/2011/03/identifying-app-installations.html 5.3.4 Surely a little more could be said about device lock. On a typical modern phone, "device lock" options include PINs, passwords, "face recognition" and so on. These are *not* equal in their level of security they provide. Nits: Formatting is lousy. There are notations, including ** and _whatever_ that I'm not familiar with in the RFC context. Section 1.0: s/in-built into/built into/ 2.1, last bullet point: "An example could by a..." s/by/be/ 2.2, 1st bullet point s/eaves drop/eavesdrop/ 2.3, 1st para, s/treat/threat/ 2.3.1, last bullet, "per authorization process". Adjectival phrases should be hyphenated: "per-authorization process" 2.3.3, last bullet, ditto 3.1, 1st para, "all kinds of tokens" should be "many kinds of tokens" 3.1, 2nd para, should be ; not , after "within the authorization server" s/protected/protect/ s/different system/different systems/ 3.4 1st para, s/intermediary/intermediate/ list item 1. s/short-living/short-lived/ 3.5 s/malicious client/malicious clients/ 3.7 top of page 12, what is the underscore notation _client_id_ mean? I'm not familiar with this in the RFC context. 1st bullet point: s/token/token's/ 2nd bullet point, multiple issues, 1st sentence should be " the initial authorization and issuance of a token by an end-user to a particular client, and subsequent requests by this client to obtain tokens without user consent (automatic processing of repeated authorization) halfway down page 13, s/insures/ensures/ s/validates the clients/validates the client's/ 4. first sentence, s/this sections/this section/ 4.1.2 first para, the last sentence is confusing. How about: "Before enumerating the threats, here are some generally applicable countermeasures:" 4.2.4 2nd bullet s/could not be/can not be/ 4.3.3 1st bullet, capitalized phrase "Confidentiality of Requests" - I assume that's supposed to be a hyperlink to one of the 5.* sections? 4.4.1.1 last bullet, s/referee/referrer/ - also, should note that the referrer header may contain an Authorization code in a ?a=b style argument 4.4.1.2 first bullet, "can be employed" is inconsistent with style of rest of doc 4.4.1.3 first 2 bullets have un-labeled links. 4.4.1.4 1st bullet s/authentication/authenticate/ 4.4.1.4 2nd bullet s/mean/means/ 4.4.1.7 2nd bullet s/tokens/token's/ 4.4.1.10, 2nd para, s/requisiete/requisite/ s/embbed/embed/ 4.4.1.10, 3rd bullet, s/aibility/ability/ 4.4.1.10, toward bottom of page 30, s/e.t.c./etc./ 4.4.1.12 I think the href to semicomplete.com needs to be turned into an IETF-style reference 4.4.2 " since HTTP user agents do not send fragments server requests." What you mean to say is "Since HTTP user agents do not send the fragment part of URIs to HTTP servers." 4.4.2.2 s/browser/browser's/ 5.1.4.1.3 s/consider to not store/refrain from storing/ 5.* s/may consider to $(verb)/may consider $(verb)ing/ 5.1.6 Needs some sort of sentence structure 5.3.2 Needs some sort of sentence structure; or is this intended just to be a title, with 5.3.3 etc nested under it?
- [OAUTH-WG] [apps-discuss] Apps Area review of dra… S Moonesamy
- Re: [OAUTH-WG] [apps-discuss] Apps Area review of… Michael Thomas
- Re: [OAUTH-WG] [apps-discuss] Apps Area review of… Justin Richer
- Re: [OAUTH-WG] [apps-discuss] Apps Area review of… Michael Thomas
- Re: [OAUTH-WG] [apps-discuss] Apps Area review of… Torsten Lodderstedt
- Re: [OAUTH-WG] [apps-discuss] Apps Area review of… John Bradley
- Re: [OAUTH-WG] [apps-discuss] Apps Area review of… Torsten Lodderstedt
- Re: [OAUTH-WG] [apps-discuss] Apps Area review of… Torsten Lodderstedt
- Re: [OAUTH-WG] [apps-discuss] Apps Area review of… André DeMarre
- Re: [OAUTH-WG] [apps-discuss] Apps Area review of… Igor Faynberg
- Re: [OAUTH-WG] [apps-discuss] Apps Area review of… Peter Wolanin
- Re: [OAUTH-WG] [apps-discuss] Apps Area review of… Mark Mcgloin