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: > 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. > > 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. > done >> (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? > > 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." > >> (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.) > > 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." > >> (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. > > Replaced client identity by client identifier in most places and > incorporated your proposed text > >> (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? > > removed 3rd bullet as native apps should use code flow > > We also removed the 1st bullet in 4.4.1.9 > >> (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? > > 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. > >> (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".) > > replaced address in most cases by URL and in some place by FDQN > >> (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:-) > > Added text from core spec describing usage of Cache-Control and Pragma > response header fields > >> (9) 5.1.1: needs a reference to RFC 5246 (and better to >> just say TLS and not say SSL, at least for me :-) > > done >> (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. > > Extended description and added reference to RFC 4301. >> (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). > > Added reference to RFC 2818 since it seems to be closest to the problem > >> (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.) > > 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 >> 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.) > > 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 > >> (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? > > 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 >> the current text. > > done - also added reference to RFC4120 in section 3.1 >> (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:-) > > 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 > >> (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.) > > added note and reference to respective 3gpp spec > >> (18) 5.2.2.6: needs a reference for X-FRAME-OPTION. >> There's a websec draft about that I think. > > added reference to > http://tools.ietf.org/html/draft-gondrom-x-frame-options/ >> (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"? > > nothing is wrong with "installation" :-) > > replaced deployment by installation and partially re-phrased section > >> (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?) > > added 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. > > copied a sentence from base spec and extended description > >> 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. > > we mean data integrity and origin authentication - added some text to > explain this > >> 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. > > Renamed section to "Limited access token limetime" and changed wording > to explicitely distinct between concept and protocol parameter. > >> 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? > > tweaked it >> 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. > > expanded CSRF a bit, added forward reference to 4.4.1.8 and shortened > 4.4.2.5 > >> 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. > > added pointer > >> section 4: maybe s/Security Threat Model/Threat Model/ >> in the section title - what other kind of threat >> model is there? > > changed to Threat Model > >> section 4: I wondered how we know the 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.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:-) > > changed to "Revelation of client credentials during transmission" > >> 4.3.3: typo? 1st countermeasure reads oddly and >> refers to a different place than other references >> to TLS - is that correct? > > changed to standard wording > >> 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. > > 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. >> Maybe it ought to? > > Sorry, don't understand this comment. Are you saying 5.1.4.1.2 should > point back to 4.3.2? > >> 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? > > 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. > >> 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;-) > > ok :-) > >> 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. > > added references to OASIS documents and stable reference to 3rd document > in proceedings of Computer Security Applications Conference > >> 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. > > 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.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/ :-) > > 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. " > > >> 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. > > 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." > > >> 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. > > see above >> 4.4.1.6: Is Facebook a trademark? Maybe better to not >> use that if so? > > Changed Facebook reference section to: > (e.g. as in the implementation of "Login" button to a third-party social > network site) > > >> 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. > > 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." > > >> 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:-) > > 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 >> a reference > added >> 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/ > > considerably re-worded this section > >> 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? > > unfortunately not >> 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. >> j...@example.com/pwd might easily allow someone to guess >> that the same pwd is used forjoe.u...@example.net) 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. > > Changed to "Consider not to use grant type "password"" > >> 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 >> OAuth@ietf.org >> https://www.ietf.org/mailman/listinfo/oauth > _______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth