Last Call Review of draft-ietf-websec-x-frame-options-07
review-ietf-websec-x-frame-options-07-genart-lc-sparks-2013-08-08-00

I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at

<

http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-websec-x-frame-options-07
Reviewer: Robert Sparks
Review Date: 2013-08-08
IETF LC End Date: 2013-08-12
IESG Telechat date: 2013-08-15



Summary: This draft is basically ready for publication, but has nits 


that should be fixed before publication.






This document is intended to act as a informational reference describing 


what's already deployed, and not to act as a standard for new 


implementations to follow. It's obvious that some care has been taken to 


keep that tone in the text, but there are places where it still looks 


like a proposed standard. Please make it even more clear who the 


audience for this document is, and look for places to reinforce that 


this is documenting what _is_, as opposed to trying to influence what 


should be.






The introduction of this version of the document does a good job of 


describing what a clickjack attack is. Appendix B is not really adding 


to that description. I suggest that the two usecases it call out be 


reduced to a couple of sentences in the introduction as examples, and a 


reference to a more in-depth treatment of scenarios like them be added 


for those looking for more information. Section B.3 is different, and 


doesn't add to the understanding of this document. Again, I suggest 


pointing to somewhere else that provides an in-depth treatment. 


(Hopefully, that source would discuss the nested frame issue the 


security considerations section points out in the context of the this 


specific configuration page). [FRAME-BUSTING] might be enough, but 


pointers to something even more rich would certainly be helpful.




Here are some suggestions in document order:



In the Abstract, I suggest replace "specification" with either 


"definition" as the introduction uses, or with "the syntax and behavior 


observed in current deployments".






The last paragraph of the introduction reads like standards text. Please 


consider rewriting it with a tone of history and current fact. Perhaps 


starting with "In existing implementations, ", and replacing the last 


sentence with something like "The policy this HTTP header declares is 


enforced by the browser implementations documented here".






Since this is documenting an existing deployed header (not standardizing 


it), it would be useful to comment whether the existing known 


implementations allow all the productions of the ABNF described here to 


be used. For instance, would they all honor X-FRAME-OPTIONS: deny? 


(While thinking about this, focus on who this document is for.)






The last sentence of 2.3.1 as written is an attempt to be a standard, 


and is an odd use of 2119. I suggest rewriting it as a warning about 


what happens with existing plugins that ignore the policy carrid in the 


header. That keeps the message aligned with documenting what exists.






The first sentence of 2.3.2 is not adding anything to the document. I 


suggest deleting it. The second sentence is placing an implementation 


requirement again. Please consider rewriting as an observation of what 


existing things do, calling out the ramifications of the ones that 


behave differently.






Similarly, the first paragraphs of Section 2.3.2.1 is written to affect 


new implementation, not document existing implementation. They could be 


re-tuned as above. The third paragraph is good history






The last sentence of 2.3.2.2 should not say "replace this document" - 


this document is informational documentation about existing deployments. 


CSP1.1 won't Obsolete or Replace this document. Rather it should say 


something like "it is expected that newer implementations will use it 


rather than the mechanisms documented here".




Nit: s/does support only one/only supports one/



Do the known implementations follow the pattern described in 2.3.2.3? 


Saying something would help tie this to being information about the 


known deployment rather than trying to constrain new development.






Nit: If Appendix B is kept, please expand CSRF (and consider re-pointing 


to a reference)