Subject: Re: [OAUTH-WG] AD review of draft-ietf-oauth-threatmodel-05

From: Torsten Lodderstedt <[email-masked]>
Date: Sun, 03 Jun 2012 09:57:24 +0200

Hi Stephen,

thank you very much for your review. Having scanned through your 
comments, I think the document would definitely benefit from 
incorporating them into a new revision. We will start working on it now, 
but I can't tell you how long it will take (due to daytime job load).

best regards,
Torsten.

Am 28.05.2012 20:34, schrieb Stephen Farrell:
>
> Hi all,
>
> I've gotten the publication request for oauth-threatmodel
> so here's my AD review of -05.
>
> Its quite a read (and a good one) but I've a bunch of
> questions. Some of these will need fixing I suspect
> but a lot are ok to fix later after IETF LC, depending
> on whether the authors want to re-spin it before then
> or not. But I'd like to at least see reactions to the
> questions before I start IETF LC.
>
> Although there are many many nits and typos, those
> don't actually make the document unreadable so
> though I'd prefer to see 'em fixed now, I'm ok with
> that happening later if its a problem to get it
> all done now.
>
> If you want to argue for going ahead with IETF LC
> now please do so, but I suspect that this might need
> a revised ID to fix at least a couple of the points
> raised below. If nobody does argue to go ahead now,
> I'll mark it as revised ID needed, but first let's
> see what the answers are to the questions.
>
> Cheers,
> S.
>
>
> (1) s/RFC1750/RFC4086/ is needed as noted in the write-up.
>
> (2) You don't say anything about the probability of
> occurrence of the various threats. I realise that you
> can't be precise but it seems wrong to say nothing.  Would
> it be worth at least saying that that's not done here and
> that readers of this document need to do their own risk
> analysis including that aspect?
>
> (3) Many deployments will use TLS accelerators.  That
> means that TLS isn't fully e2e, and that opens up some
> (mainly) insider attacks or attacks that can be launched
> from within a compromised DMZ, but from outside the server
> applications. Does that need a mention somewhere? (I've
> seen systems like that deployed and a lot could go wrong
> from the inside, so I think this is a real threat.)
>
> (4) Could you use just one of "client identity" or "client
> identifier" consistently? I'd much prefer the latter,
> which has also been the outcome of various discussions on
> this topic elsewhere. For example you say "revocation of
> such an identity" at the end of p13, and that's a
> potential rathole, better to say "revocation of the rights
> associated with a client identifier" or similar I think.
> And similar changes throughout.
>
> (5) 4.4.2.2: Here you recommend native applications should
> use an embedded browser, but earlier you said that was a
> bad idea. I think you need to be consistent or else
> provide more about when its ok to embed and when its not.
> Did I misread it or does that need a change?
>
> (6) 4.4.3.1: This calls for "obfuscation" of passwords in
> logs. I think you ought be stronger there and call for
> strong encryption of passwords wherever they are stored,
> be that logs or DBs or whatever. Would'nt that be reasonable?
>
> (7) 4.6.4: 1st countermeasure: I don't think you mean
> address here (in the sense of an IP address, which is what
> that means) but rather the domain name or FQDN or URL.  In
> any case, you need to say what is meant clearly.  (Also in
> 5.1.5.6 and elsewhere when you use the term "address".)
>
> (8) 4.6.6: You say to use Cache-Control, but don't say
> how.  Is more needed really? Maybe there's a good document
> you could reference for that? (I'm not suggesting you add
> a lecture on caching btw:-)
>
> (9) 5.1.1: needs a reference to RFC 5246 (and better to
> just say TLS and not say SSL, at least for me :-)
>
> (10) 5.1.1: needs a reference to whatever you mean by
> "VPN" since there are many types of VPN. I assume you mean
> an IPsec VPN? But even if not, saying more would be good.
>
> (11) 5.1.2: needs a reference to RFC 5280 and/or RFC 6125
> and/or RFC 2818. Bascially, you need to say how this is
> done (by reference).
>
> (12) 5.1.4.1: Isn't there some good reference you can give
> here? (Having said that, I'd have to go look myself, but
> I'd maybe start with owasp or sans.)
>
> (13) 5.1.4.2.2: if p(collision) should be<=2^(-160) then
> what's the point of saying it ought be<= 2^(-128)? Also
> if sha-1 were perfect, then the 160 bits output would
> really have a collision probability of about 2^(-80) with
> many many tokens, but not 2^(-160). I think you need
> to fix something there.  Perhaps you're really saying that
> all high-entropy secrets should be>=128 bits long and
> constructed with a good (P)RNG? If so, saying so more
> clearly would be better. Not everyone will get the
> collision probability way of saying that even when its
> properly stated. (This text is also repeated, be better if
> you just said this once I think.)
>
> (14) 5.1.5.2: what is a "reasonable duration" - I don't
> think its ok to say that but nothing else. Can't you give
> some "reasonable" examples based on current usage?
>
> (15) 5.1.5.5: needs a reference to SAML assertions with
> the current text.
>
> (16) 5.2.2.3: this describes a refresh token rotation
> scheme that I don't recall being discussed on the list, so
> this is just to check that that rotation scheme, as
> described, doesn't ring any alarms bells for the WG. If
> not, that's fine. And if it was discussed on the list and
> I've forgotten, then sorry about that:-)
>
> (17) 5.2.2.5: Using IMEI's like this has privacy
> implications that I think you ought call out. A single
> sentence and a reference to something that says "be
> careful about privacy here" would be fine I'd say. (And
> you need a reference for "IMEI" and to expand the term.)
>
> (18) 5.2.2.6: needs a reference for X-FRAME-OPTION.
> There's a websec draft about that I think.
>
> (19) 5.2.3.4: what do you mean when you say "for different
> deployments of a client"? That could be one secret per
> install or one secret for all customers of a mobile
> operator and those are radically different. I think you
> need to be clear and hope you mean the former. That's
> almost cleared up in the 3rd paragraph of the section but
> I wanted to just check. Not sure "deployment" is the best
> term there really - what's wrong with "installation"?
>
> (many:-) nits and typos:
>
> 2.3.1: maybe explain "handle-based design" or give a
> reference? (Or maybe just a forward ref to 3.1?)
>
> 2.3.3: It might be worth re-iterating that the term
> "client" in oauth is used differently, e.g.  by copying a
> bit of text from the base spec. I can see folks being
> confused by this otherwise.
>
> 3.1: "is digitally signed" - do you mean it has data
> integrity and origin authentication?  If MACs are commonly
> used (or maybe authenticated encryption), and not
> necessarily signatures, then saying that would be better.
>
> 3.1.2: typo: s/this mechanisms/this mechanism/
>
> 3.1.2: maybe s/Expires_In/expires_in/ to match the case in
> the base spec? I think it'd be better not to capitalise
> this in case it finds its way into someone's code. You
> could also use "Expires In" in the title and then say that
> its "expires_in" in the protocol. Might be worth doing
> something generic to call out when you're talking about
> specific things from the protocol, e.g. use a convention
> like ``expires_in'' or "expires_in" consistently and say
> that in the intro.
>
> 3.4: typo: s/the later/the latter/
>
> 3.4: bullet item 1 isn't really a reason for anything but
> a downside of doing this, at least as written. Maybe this
> needs to be tweaked?
>
> 3.6: expand CSRF on 1st use and maybe give a reference
> CSRF is expanded in 4.4.1.8 which is a good bit later,
> and also in 4.4.2.5 which could presumably be
> shortened a bit by removing the repetition.
>
> 3.7: typo: s/collage associated request/collate associated
> requests/
>
> 3.7: Maybe give a pointer to the definition of "native
> application" in the base spec (Its section 9 of that.)
> 2nd last para of p13 would be a good place for that.
>
> section 4: maybe s/Security Threat Model/Threat Model/
> in the section title - what other kind of threat
> model is there?
>
> section 4: I wondered how we know the threat model
> is "comprehensive"? Perhaps "detailed" is better?
>
> 4.2.1: typo: s/Users/users/g unless you mean something
> special?
>
> 4.2.2: typo: s/a understandable/an understandable/
>
> 4.2.2: "...based on who the client is." is unclear
> and not gramatically nice:-) "...based on the client
> identifier." would seem better.
>
> 4.3.1: typo? s/on transit/in transit/ Or did you
> mean something else? and s/may attempts/may attempt/
>
> 4.3.3: maybe s/Revelation/Disclosure/ to be a tad
> less biblical:-)
>
> 4.3.3: typo? 1st countermeasure reads oddly and
> refers to a different place than other references
> to TLS - is that correct?
>
> 4.3.3: digest auth is nearly the same as sending
> credentials over the wire, TLS client auth based
> on certificates would be a better example, even
> if its not often done.
>
> 4.3.4: 4.3.2 points to 5.1.4.1.2 but this doesn't.
> Maybe it ought to?
>
> 4.3.6: typo s/an authorization servers/an authorization
> server/
>
> 4.3.6: 4.3.5 refers to 5.1.4.2.2 wrt entropy. Is there
> a reason to not do that here too?
>
> 4.4: typo? s/tokens endpoint/token endpoint/ ?
>
> 4.4.1.1: typo: s/by different ways/in different ways/
>
> 4.4.1.1: typo-fix-typo? HTTP has a "Referer" header field,
> but you fixed their typo here - might be better to live
> with the bad spelling, in which case
> s/referrer/referer/g;-)
>
> 4.4.1.1: Is there no better way to reference these
> OASIS docs? More importantly, is there no better (more
> stable) reference than thomasgross.net for the
> PDF you reference? Tidying this up would be good.
>
> 4.4.1.1 and elsewhere: a few times you say "such as
> TLS or SSL," I think it'd be better to just say TLS
> there and do it consistently everwhere. In other
> places (e.g. 4.4.1.5) you say "HTTPS" - again that'd
> be better done consistently.
>
> 4.4.1.1: typo: s/redeem a authorization code/redeem
> an authorizatio code/
>
> 4.4.1.4: "counterfeit a valid client" reads oddly,
> do you mean "pretend to be a valid client"? If so,
> I think that'd be clearer.
>
> 4.4.1.4: "and to prevent unauthorized access to
> them" - I think you might add "via the oauth
> protocol" there since the AS isn't responsible for
> all possible ways of preventing unauthorized access.
>
> 4.4.1.4: typo: s/not neccesserily indicates/doesn't
> necessarily indicate/
>
> 4.4.1.4: typo: s/user should be explained the purpose/
> something better/ :-)
>
> 4.4.1.4: expand/define CAPTCHA on 1st use. Give a
> reference for this too. Especially since you also
> have 5.1.4.2.5, which is maybe the best place for
> the reference.
>
> 4.4.1.4: isn't a PIN code another user authentiation?
> Seems like a bad example of automatic authentication,
> since it isn't automatic if the user has to enter a
> PIN.
>
> 4.4.1.6: Is Facebook a trademark? Maybe better to not
> use that if so?
>
> 4.4.1.7: typo: s/achieve that goal/achieves that goal/
>
> 4.4.1.7: typo: s/victims resources/victim's resources/
>
> 4.4.1.7: typo: s/The attackers gains/The attacker gains/
>
> 4.4.1.7: typo: s/then the target web site/rather than
> the target web site/
>
> 4.4.1.7: "session fixation" could do with a reference
> or definition, and that might be better earlier in
> this section and not just in the countermeasures
> part.
>
> 4.4.1.7: typo: s/kind of attacks/kind of attack/
>
> 4.4.1.8: typo: s/not follow untrusted/to not follow
> untrusted/
>
> 4.4.1.9: maybe add a reference for "iframe"? And
> you say "iFrames" later, better to be consistent.
>
> 4.4.1.9: 1st countermeasure - do you mean end-user
> authorization here or end-user authentication? And
> would it be better to say "system browser" or
> something rather than "external browser"? (I forget
> what phrase you used for that earlier but there
> was something bettter:-)
>
> 4.4.1.9: "javascript framebusting" really needs
> a reference
>
> 4.4.1.10: typo: s/the victims resources/the victim's
> resources/
>
> 4.4.1.10: typo: s/or split the/or splits the/
>
> 4.4.1.10: "corresponding form post requests" isn't
> clear to me - does that need rephrasing or is it
> just me?
>
> 4.4.1.10: this is the first mention of cookies, which
> I found surprising, but that's all;-)
>
> 4.4.1.10: the 2nd "ways to achieve this" bullet is
> a bit unclear - which "It" and whose browser? I
> think this could be clearer.
>
> 4.4.1.10: typo: s/as native app/as a native application/
>
> 4.4.1.10: what does "assume" the threat mean?
>
> 4.4.1.10: typo: s/an user interaction/a user interaction/
>
> 4.4.1.10: typo: s/CAPTCAs, or/CAPTCHAs/ or else get
> rid of the "or" from the last bullet
>
> 4.4.1.10: typo: s/send out of bound/sent out of band/
>
> 4.4.1.10: typo: s/instance message/instant message/
>
> 4.4.1.11: typo? s/directing user(s) browser/directing
> the user's browser/ ?
>
> 4.4.1.11: I don't get the explanation here. Are you
> assuming (but not saying) that generating non-trivial
> entropy is a slow process? (e.g. /dev/random blocking?)
> Its also not clear what "pool" you mean? This probably
> needs a bit of tweaking.
>
> 4.4.1.12: semicomplete.com may not be a sufficiently
> stable reference - can't you find a better one?
>
> 4.4.1.12: typo: s/Defenses such rate limiting/Defenses
> such as rate limiting/
>
> 4.4.1.12: typo: s/an anonymizing systems/an anonymizing
> system/
>
> 4.4.1.12: nicest typo yet! s/annoying system/anonymizing
> system/ :-)
>
> 4.4.1.12: typo? maybe s/iframe/iFrame/ again?
>
> 4.4.1.12: 1st reference to "the CSRF token" here? That
> might warrant a reference of some sort? (Maybe to
> a later section?)
>
> 4.4.1.12: Facebook again.
>
> 4.4.1.12: Signing the code seems like a bit of a hack.  Do
> you really want to recommend this here? I suspect it'd end
> up different if you actually tried it out aiming for an
> interoperable solution. I'd suggest deleting this, or
> maybe make it less specific, e.g. saying if origin
> authentication for authorization codes could be validated
> by clients, then that'd be a countermeasure, but that
> there's no current spec for that. (And doing that might
> just move the DoS to somewhere else depending how you
> did it.)
>
> 4.4.2: typo: s/and It cannot/and it cannot/
>
> 4.4.2.1: Is the countermeasure here TLS? If so, better to
> say so.
>
> 4.4.2.2: requests aren't cached are they but rather
> responses?
>
> 4.4.2.3: typo: s/An malicious/A malicious/
>
> 4.4.2.3: The reference back to 4.4.1.4 isn't very
> clear since a lot of the countermeasures there
> mention authentication. It'd be better to explicitly
> list things here or else if you stick with the
> backwards reference to be more explicit about whic
> exactly apply.
>
> 4.4.2.5: Is this entirely identical to 4.4.1.8? That
> doesn't seem right. If it is, then say so explicitly.
> If not, then say what's different.
>
> 4.4.3: 1st mention of username/password anti-pattern,
> so you could add a reference
>
> 4.4.3: Be good to metion here that passwords are often
> used for>1 service, so this anti-pattern risks whatever
> else is accessible with that credential or an
> algorithmically derivable equivalent (e.g.
> [email-masked]/pwd might easily allow someone to guess
> that the same pwd is used for [email-masked]) and
> then another countermeasure is to educate users to
> not use the same pwd for multiple services (hard as
> that is to really do;-)
>
> 4.4.3.4: again digest auth isn't a good example
> here, but client certs are.
>
> 4.4.3.6: What does "Abandon on grant type..." mean?
> If you mean "don't do this" then say that more
> clearly.
>
> 4.6.2: typo: s/transport security measure/transport
> security measures/ or maybe just say TLS
>
> 4.6.2: typo: s/nounces/nonces/
>
> 4.6.3: 1st countermeasure: maybe s/difficult/infeasible/
> I don't see why "difficult" guessing is hard enough?
>
> 4.6.4: typo: s/a valid access tokens/a valid access token/
>
> 4.6.4: Do you mean the IP address in the 2nd
> countermeasure?  I'm not sure, esp. given the above.
>
> 4.6.4: typo: s/miss the capabilities/lack the capability/
>
> 4.6.6: reference to 2616 is broken
>
> 4.6.6: Should you reference httpbis drafts? Just asking, I
> can see arguments for referencing just 2616 or just
> httpbis or both, given that this'll be read for hopefully
> a while after httpbis is done.
>
> 4.6.7: referrer vs. referer again
>
> 4.6.7: What is an appropriat logging configuration? Can
> you give a reference maybe or is it really that well
> known?
>
> 4.6.7: Reloading the target page again? Are you really
> recommending that?
>
> 5.1: typo: s/consideratios/considerations/
>
> 5.1.3: I think you should note the email notifications
> can be a phishing vector so don't make your emails
> such that lookalike phish messages can easily be
> derived from them.
>
> 5.1.3: Don't you need to say something about getting
> email notifications delivered and not treated as spam?
> Maybe you could recommend dkim here or other ways to
> get better delivery. (Not sure myself, probably best
> to ask someone who operates like this so see if there's
> stuff to be said.)
>
> 5.1.4.1.3: typo: s/not store credential/not store
> credentials/
>
> 5.1.4.1.4: salted hashes don't prevent offline
> dictionary attacks, they just increase the workload
>
> 5.1.5.1.4: would it be worthwhile recommending that
> different client credentials be used in development
> or integration mode vs. when operational? I've seen
> a bunch of times when programmers were given "live"
> API keys when that could've been avoided.
>
> 5.1.4.1.5: I don't get this. If it was only that
> easy:-) I think you need to say which private keys
> are used here and for what for this section to be
> useful.
>
> 5.1.4.2.1: I think you could note that complex password
> policies can also increase the liklihood that users
> re-use passwords or write them down or store them
> somewhere not so good, if e.g. the entropy required
> over all the user's passwords (dozens usually) is
> too great for long-term memory.
>
> 5.1.5.1: typo: s/Basis of/The basis of/
>
> 5.1.5.1: typo: s/sensible service/sensitive service/
> (2nd best typo:-)
>
> 5.1.5.3: typo: s/preciser/more precise/
>
> 5.1.5.3: typo: s/refreshments/refreshes/
>
> 5.1.5.4: 2nd bullet is not a threat but a goal
>
> 5.1.5.4: typo: s/redeem a/redeem an/
>
> 5.1.5.5: what is a "rough" server? Do you mean rogue?
>
> 5.5.5.6: I think you need to clarify what "address"
> means here too.
>
> 5.1.5.9: please clarify if you mean digitally signed
> (asymmetric) or also include MACing here
>
> 5.1.5.10: typo: s/Self-contained/Self-contained tokens/
>
> 5.1.5.10: s/privacy/confidentiality/ ?
>
> 5.1.5.10: this (typically, I'd guess) requires sharing
> symmetric keys between server nodes, so you should
> say that as its a bunch of work.
>
> 5.1.5.11: I don't get why you want to repeat this
> text (and its error:-) better to refer back to
> the earlier section I think.
>
> 5.1.5.12: Another place where a SAML reference would
> be needed.
>
> 5.1.6: 2nd bullet is not a "measure" but a goal. If
> you had something in mind, it doesn't come out from
> that text.
>
> 5.2.2.2: You say the binding should be protected, but
> don't say how. I think you ought.
>
> 5.2.3: typo: s/sub-sequent/subsequent/ but maybe
> better to delete the word
>
> 5.2.3: 2nd bullet - "trustworthiness" has gotta
> be the wrong word there, what did you mean?
>
> 5.2.3: typo: s/statics/statistics/
>
> 5.2.3: typo: s/support achieve objectives/achieve these
> objectives/ ?
>
> 5.2.3: typo: s/by hand of an administrator/something
> better/
>
> 5.2.3.1: "prevents...overestimating" seems wrong, I think
> you mean it reduces the probability of mistakenly treating
> a useless authentication as a good one. (The point is
> that servers don't "overestimate," only people do that.)
>
> 5.2.3.1: "cannot be entirely protected" seems like
> understatement - the reality is any such secret will
> leak out from anything that's any way successful. It
> only protects stuff that *nobody* cares about.
>
> 5.2.3.1: typo: s/trust on/trust/ But I'd rephrase it
> to not use the terms "trust" nor "identity" and then
> it'd be better I think.
>
> 5.2.3.2: typo? s/The authorization may/The authrozation
> server may/ ? Not sure what "issue a cliend id" means
> either. (Same in 5.2.3.3)
>
> 5.2.3.4: typo: s/A authorization server/An authorization
> server/
>
> 5.2.3.5: what are "validated properties"?
>
> 5.2.3.5: what is the 1st bullet list on p57? there's
> no introductory text?
>
> 5.2.3.5: the "it" in "it only works" in the last
> paragraph isn't clear - which "it" do you mean?
>
> 5.2.3.5: saying discovery "gets involved" seems
> wrong - do you mean "is used"? And what is the
> "that" in "that's no longer feasible"?
>
> 5.2.3.6: typo: s/be unintentionally for/unintentionally
> affect/?
>
> 5.2.3.7: typo: s/to distribute client_secret/to
> distribute a client_secret/
>
> 5.2.4.1: Is a "security certificate" a public key
> certificate? If so, saying that is better.
>
> 5.2.4.1: the references to 5.7.2.x are wrong and
> look like you're missing some xref's in the xml
> maybe
>
> 5.2.4.2: you said "address" again, so the usual
> question arises :-)
>
> 5.2.4.3: typo: s/in all situation/in situations/
> (not sure "all" is right so suggest deleting it)
>
> 5.2.4.4: again, be good to say how to protect
> the binding
>
> 5.2.4.5: as for 5.2.4.4 say how binding is done
>
> 5.3.1: typo: s/a associated/an associated/
>
> 5.3.1: "entirely protected" is (again:-) understatement
> see above for a suggestion
>
> 5.3.1: what does "trust on the client's identity" mean?
> Its not clear anyway
>
> 5.3.3: typo: s/operation systems/operating systems/
> (enrire 2nd&  3rd paras could do with re-phrasing)
>
> 5.3.4: typo: s/their level/the level/
>
> 5.3.5: this is too terse - is it really finished? I'd
> say there's a sentence or two missing.
>
> 5.4.2: what does it mean to "encourage resource
> servers" to do something? I guess you mean to encourage
> the people deploying those to pick resource servers
> with that capability and to turn that on.
>
> 5.4.2: not sure if everyone will know what a
> "distinguished name" is, maybe add a reference to
> rfc 5280?
>
> 5.4.2: token-bound secrets would be used to MAC
> the request and not to sign, be better to make that
> clear even if you still call that "signing" here
> (its not really, if we're being pedantic)
>
> 5.4.2: typo: s/This mechanisms/This mechanism/
>
> 5.4.2 and 5.4.3: I forget - where are the protocol
> mechanisms for this defined? Can you give a reference
> or say that its future stuff if there aren't any
> good ones?
>
> 5.5: typo: s/capable to validate/capable of validating/
>
> 8.1: Is the base spec really normative? I'd say you're
> fine with that as informative too.
>
> 8.2: there are a bunch of references missing as per
> the questions above
>
> 8.2: is there no better (more stable) reference than
> portablecontacts.net?
>
>
>
>
>
>
>
>
>
>
> _______________________________________________
> OAuth mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/oauth

---

From: Stephen Farrell <[email-masked]>
Date: Sun, 03 Jun 2012 18:01:37 +0100

> but I can't tell you how long it will take (due to daytime job load).

Sure. With 3 authors and most of the comments only needing trivial
fixes, I'd hope that's not too long a job. (Or, if it is, maybe
ask the chairs to try find more help.)
Cheers,
S.

---

From: Torsten Lodderstedt <[email-masked]>
Date: Wed, 27 Jun 2012 20:03:56 +0200

Hi Stephen,

I just submitted a new revision of the security document incorporating 
your review comments.

Please find answers to your comments below.

Thank you again for your detailed review. You helped us to significantly 
improve the document's quality.

best regards,
Torsten.

> (1) s/RFC1750/RFC4086/ is needed as noted in the write-up.
done
> analysis including that aspect?

That's correct - I added the following paragraph to the introduction:

"Note: This document cannot assess the probability nor the risk 
associated with a particular threat because those aspects strongly 
depend on the particular application and deployment OAuth is used to 
protect. Similar, impacts are given on a rather abstract level. But the 
information given here may serve as a foundation for deployment-specific 
threat models. Implementors may refine and detail the abstract threat 
model in order to account for the specific properties of their 
deployment and to come up with a risk analysis."

> from the inside, so I think this is a real threat.)

I added another paragraph to section 5.1.1:

"Note: this document assumes end-to-end TLS protected connections 
between the respective protocol entities. Deployments deviating from 
this assumption by offloading TLS in between (e.g. on the data center 
edge) must refine this threat model in order to account for the 
additional (mainly insider) threat this may cause."

> And similar changes throughout.

Replaced client identity by client identifier in most places and 
incorporated your proposed text

> Did I misread it or does that need a change?

removed 3rd bullet as native apps should use code flow

We also removed the 1st bullet in 4.4.1.9

> be that logs or DBs or whatever. Would'nt that be reasonable?

This section is about preventing accidential exposure by the client. I 
think encryption is not appropriate here since the password is entered 
in the clear by the user. I added the advice to encrypt credentials as 
alternative means to salted hashes to 5.1.4.1.3.

> 5.1.5.6 and elsewhere when you use the term "address".)

replaced address in most cases by URL and in some place by FDQN

> a lecture on caching btw:-)

Added text from core spec describing usage of Cache-Control and Pragma 
response header fields

> just say TLS and not say SSL, at least for me :-)

done
> an IPsec VPN? But even if not, saying more would be good.

Extended description and added reference to RFC 4301.
> and/or RFC 2818. Bascially, you need to say how this is

Added reference to RFC 2818 since it seems to be closest to the problem

> I'd maybe start with owasp or sans.)

added reference to OWASP

> (13) 5.1.4.2.2: if p(collision) should be <=2^(-160) then
> what's the point of saying it ought be <= 2^(-128)? Also
> all high-entropy secrets should be >=128 bits long and

modified text as follows

"When creating secrets not intended for usage by human users (e.g. 
client secrets or token handles), the authorization server should 
include a reasonable level of entropy in order to mitigate the risk of 
guessing attacks. The token value should be >=128 bits long and 
constructed from a cryptographically strong random or pseudo-random 
number sequence (see [RFC4086] for best current practice) generated by 
the Authorization Server."

removed 5.1.5.11. (redundant text) and updated references accordingly

> some "reasonable" examples based on current usage?

added examples and explanation of factors determining reasonable 
expiration time

"Tokens should generally expire after a reasonable duration. This 
complements and strengthens other security measures (such as signatures) 
and reduces the impact of all kinds of token leaks. Depending on the 
risk associated with a token leakage, tokens may expire after a few 
minutes (e.g. for payment transactions) or stay valid for hours (e.g. 
read access to contacts).

The expiration time is determined by a couple of factors, including:

- risk associated to a token leakage
- duration of the underlying access grant,
- duration until the modification of an access grant should take effect, and
- time required for an attacker to guess or produce valid token."

> (15) 5.1.5.5: needs a reference to SAML assertions with

done - also added reference to RFC4120 in section 3.1
> I've forgotten, then sorry about that:-)

It has been discussed, the first time with the introduction of the 
option to return a new referesh token value in response to a refresh 
token grant request. A more recent discussion can be found here: 
http://www.ietf.org/mail-archive/web/oauth/current/msg06974.html

> you need a reference for "IMEI" and to expand the term.)

added note and reference to respective 3gpp spec

> There's a websec draft about that I think.

added reference to 
http://tools.ietf.org/html/draft-gondrom-x-frame-options/
> term there really - what's wrong with "installation"?

nothing is wrong with "installation" :-)

replaced deployment by installation and partially re-phrased section

> reference? (Or maybe just a forward ref to 3.1?)

added ref to 3.1
> confused by this otherwise.

copied a sentence from base spec and extended description

> necessarily signatures, then saying that would be better.

we mean data integrity and origin authentication - added some text to 
explain this

> like ``expires_in'' or "expires_in" consistently and say

Renamed section to "Limited access token limetime" and changed wording 
to explicitely distinct between concept and protocol parameter.

> a downside of doing this, at least as written. Maybe this

tweaked it
> shortened a bit by removing the repetition.

expanded CSRF a bit, added forward reference to 4.4.1.8 and shortened 
4.4.2.5

> 2nd last para of p13 would be a good place for that.

added pointer

> in the section title - what other kind of threat

changed to Threat Model

> is "comprehensive"? Perhaps "detailed" is better?

We are rather confident because we created the threat model a systematic 
way and had it reviewed by a lot of security folks

> 4.3.3: maybe s/Revelation/Disclosure/ to be a tad

changed to "Revelation of client credentials during transmission"

> to TLS - is that correct?

changed to standard wording

> on certificates would be a better example, even

Isn't TLS client authentication always combined with TLS protected 
communication? So this would merly be an additional and not alternative 
mechanism.

> 4.3.4: 4.3.2 points to 5.1.4.1.2 but this doesn't.

Sorry, don't understand this comment. Are you saying 5.1.4.1.2 should 
point back to 4.3.2?

> a reason to not do that here too?

this section discussed a potential threat on dynamic client 
registration. Wen decided to remove the whole section as dynamic client 
registration is subject of current charter and respective security 
considerations should delt with in this context.

> with the bad spelling, in which case

ok :-)

> PDF you reference? Tidying this up would be good.

added references to OASIS documents and stable reference to 3rd document 
in proceedings of Computer Security Applications Conference

> be better done consistently.

removed SSL - would you expect us to replace HTTPS by TLS? We used HTTPS 
for the more specific case of HTTP+TLS

> 4.4.1.4: typo: s/user should be explained the purpose/

tried my best:

"In this context, the authorization server should explain to the 
end-user the purpose, scope, and duration of the authorization the 
client asked for. "

> have 5.1.4.2.5, which is maybe the best place for

Changed counter measure paragraph from:
"If the authorization server automatically authenticates the end-
       user, it may nevertheless require some user input in order to
       prevent screen scraping.  Examples are CAPTCHAs or user-specific
       secrets like PIN codes."
  to:
"If the authorization server automatically authenticates the end-user, it may nevertheless require some user input in order to prevent screen scraping. Examples are CAPTCHAs (Completely Automated Public Turing test to tell Computers and Humans Apart) or other multi-factor authentication techniques such as random questions, token code generators, etc."

> since it isn't automatic if the user has to enter a

see above
> 4.4.1.6: Is Facebook a trademark? Maybe better to not

Changed Facebook reference section to:
(e.g. as in the implementation of "Login" button to a third-party social network site)

> this section and not just in the countermeasures

changed

"The authorization server may also enforce the usage and validation of pre-registered redirect URIs (see Section 5.2.3.5).  This will allow for an early recognition of session fixation attempts."
to:
"The authorization server may also enforce the usage and validation of pre-registered redirect URIs (see Section 5.2.3.5).  This will allow for an early recognition of authorization code disclosure to counterfeit clients."

> what phrase you used for that earlier but there

We mean "authorization"

We came to the conclusion that it does not matter in which browser the 
page is loaded - removed 1st bullet

> 4.4.1.9: "javascript framebusting" really needs
added
> 4.4.1.10: typo: s/instance message/instant message/

considerably re-worded this section

> stable reference - can't you find a better one?

unfortunately not
> used for >1 service, so this anti-pattern risks whatever
> that the same pwd is used [email-masked]) and

Changed to "Consider not to use grant type "password""

> (enrire 2nd & 3rd paras could do with re-phrasing)
>

---

From: Stephen Farrell <[email-masked]>
Date: Thu, 28 Jun 2012 01:17:10 +0100

Hiya,

Great. I've asked for IETF LC to start. I'll go through
the changes below during that in case there's some more
follow up.

Thanks,
S.

On 06/27/2012 07:03 PM, Torsten Lodderstedt wrote:
> "If the authorization server automatically authenticates the end-user,
> it may nevertheless require some user input in order to prevent screen
> scraping. Examples are CAPTCHAs (Completely Automated Public Turing test
> to tell Computers and Humans Apart) or other multi-factor authentication
> techniques such as random questions, token code generators, etc."
> 
> 
> "The authorization server may also enforce the usage and validation of
> pre-registered redirect URIs (see Section 5.2.3.5).  This will allow for
> an early recognition of session fixation attempts."
> to:
> "The authorization server may also enforce the usage and validation of
> pre-registered redirect URIs (see Section 5.2.3.5).  This will allow for
> an early recognition of authorization code disclosure to counterfeit
> clients."
> 
>

---

From: Phil Hunt <[email-masked]>
Date: Wed, 27 Jun 2012 20:21:54 -0700

Stephen

One more threat has come up in the core spec that is being looked at. As Torsten is heading out on vacation I have agreed to augment if needed with any supplementary text to the threat model.

Will keep you informed. 

Phil