Hi, Please find my feedback on the first 10 pages below.
Hans. Overall: - grammar in the first sections: there's a lot of comma-separated sentences that could/should be reworked by a native speaker - perhaps readers guidance pointing developers straight to section 3. as Torsten said on the call would be useful - as also discussed on the call, perhaps it would be good to spell out how this BCP relates to the original OAuth 2.0 security considerations in the RFC(s) P3 1. Introduction Usage of OAuth vs. OAuth 2.0 across the document: perhaps explain that in the remainder OAuth refers to OAuth 2.0. First bullet: - CSRF and referrer header are being mentioned without explaining the abbreviation, context or reference. And btw, it is the “referer” header… - “defense in depth” -> “in depth defense” ? Third bullet: - I believe those higher security use cases were considered from the start, but they were not acted upon before (e.g. MAC spec was left abandoned). P4 first paragraph, in the middle: “This way the same..” -> “In this way the same…” “serves as a frontend…” -> “serve as a frontend…” “in a multi-tenancy.” -> “in multi-tenant environment.” In general there are a lot of sentences broken up by comma’s that could be rearranged and do without the comma’s. 1.1 the whole section is comprised of comma-split sentences that could be re-arranged. 2. In general: shouldn’t the OAuth 2.0 terms be capitalized everywhere e.g. resource owner -> Resource Owner authorization server -> Authorization Server Etc. ? - “was laid out that described…” -> “was laid out that describes…” - “at an authorization server (AS)…” -> “at the authorization server (AS)…” P5: A1: - first sentence ends the list with “etc.” but I don’t feel an enumeration of attacks can be left up to the reader to complete. - “be achieved through many ways” -> “be achieved in many ways” A3 and A4 could very conveniently be bundled together, isn’t it, by specifying “read but not write the authorization request and/or response”. P6: 2. The last paragraph of 2. is hand-wavy, almost scaring the implementor but not giving him the guidance to act properly. I’m not sure this helps, especially the usage of MUST will make implementers feel uncomfortable because it does not tell them exactly what they MUST do. Something like “an implementer SHOULD always consider other attack vectors specific to their environment” may work better? 3.1 End of first paragraph “It also helps to detect mix-up attacks.” lacks an explanation or a reference to a section where this is explained. As a matter of fact I don’t immediately get why this would help to mitigate mix-up attacks. P7: 2nd paragraph: I don’t think [I-D.bradley-oauth-jwt-encoded-state] is going anywhere and – even as a co-author - I’m not sure the reference should be included here. Last paragraph of 3.1: “AS which redirect a request…” -> “ASes which redirect a request…” or “An AS that redirects a request…” P8: 3rd paragraph: I guess it was discussed in length but I would like to see “clients SHOULD NOT use the implicit grant” change into “clients MUST NOT use the implicit grant”, as done for ROPC further down. This is a security BCP and implementers complying with it must really not support Implicit any longer IMHO. It is still an option for implementers to not follow this BCP and “just” do OAuth 2.0… or implement sender constrained tokens. The SHOULD gives people a way out and an excuse that we should no longer allow. P9: 3.3 missing text about not using access tokens for user authentication (again?) plus reference to oauth.net/articles/authentication, A thing I still often see in deployments and still a source of confusion for developers and deployers -- hans.zandb...@zmartzone.eu ZmartZone IAM - www.zmartzone.eu
_______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
