Re: [OAUTH-WG] [apps-discuss] Apps Area review of draft-ietf-oauth-v2-threatmodel-01

[André DeMarre <[e-mail]>]

+1 for keeping the rationale easily accessible in non-normative
security documents. Doing so is great for everyone, implementors and
spec authors alike. Security can be very nuanced, and some
countermeasures are easy to overlook. Also, being transparent with
security rationale encourages people to challenge the countermeasures
that are recommended or built into the spec. If back references help
us accomplish that, we should embrace them.

Regards,
Andre DeMarre

On Sun, Feb 19, 2012 at 9:16 AM, Torsten Lodderstedt
<[e-mail]> wrote:
> Hi Tim,
>
> I just submitted the revised version of the OAuth 2.0 security document
> (http://tools.ietf.org/html/draft-ietf-oauth-v2-threatmodel-02). This
> revision should address the issues you raised in your AppsDir review. We
> especially removed all normative language from the document.
>
> We took the liberty to leave the back references in Section 5. We consider
> this back references to section 4 a valuable information for implementors
> since they justify the particular countermeasure. All to often security
> considerations are given without the corresponding rationales. And without a
> justification, the "unconvinced" implementor may tend to ignore or
> underestimate the respective controls.
>
>
> regards,
> Torsten.
>
> Am 23.01.2012 22:47, schrieb S Moonesamy:
>>
>> 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 mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/oauth
>
> _______________________________________________
> OAuth mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/oauth