Copying websec at Barry's request:
On 8/13/13 2:52 PM, Jari Arkko wrote:
Robert: Thank you very much for your review. This has been very helpful.
Tobias: Thank you for the changes.
(I can see that there could be some discussion about some of the remaining text
changes. Do you Robert think the remaining points are important enough to
warrant me raising them in the IESG discussion? For now I have filed a
No-Objection position.)
Thanks for the nudge Jari. I've continued the conversation below.
I would still like to see some adjustment of the character of the text,
but if I were a balloting AD, I would be putting in comments, not discusses.
Tobias - Thanks for your changes so far. I appreciate that some of what
I'm pushing for may seem subtle.
I've had to handle several documents that might have been
standardization fodder at some point in time but for various reasons
turned out not to be.
The resulting documentation of some of those came out looking to me like
a standards document with an Informational or Independent Submission label
taped to the top. You've done a good job of avoiding that with this
document, and the changes I'm pushing for are to make that aspect even
better (I hope). Thank you (and the WG) for considering them.
Jari
On Aug 12, 2013, at 4:46 AM, Tobias Gondrom <[email protected]> wrote:
Dear Robert,
thank you very much for the review.
I revised the document to version-08 (also including feedback from other
reviews) accordingly including most of your feedback.
Answers inline.
Best regards, Tobias
On 08/08/13 21:00, Robert Sparks wrote:
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.
Apologies, but I would rather keep the three example cases in the
appendix of the document as this will allow better understanding of the
document if references are no longer available.
I don't think they do, actually. If those references are gone, there is not
enough in this document, particularly in those appendices, to inform
someone who didn't already understand clickjacking. To state that more
strongly, I don't think you're helping the type of future reading you are
trying to help, and as a consequence, you may even add to their confusion.
I don't think adding more to this document is the right thing to do.
I really think you should point to references instead. If you don't
trust the
ones you have to be stable for the period of time this document is needed
(which surprises me), then perhaps you should look for something more
stable?
Consider that
clickjacking description references are not IETF, nor other SDOs and
potentially not stable over the full lifecycle of the RFC.
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".
Ok. changed to "definition".
I wasn't specific with which instance of specification I meant (I missed
that there were two).
With the new text (-09), in this paragraph, I would say "this document
describes" instead of
"this definition describes", and "definition of this X-Frame-Options"
rather than "specification
of this X-Frame-Options".
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".
Changed accordingly.
Please note: IMHO I did not feel that the last paragraph of the
introduction reads like standards text. However, I appreciate your
feedback and did modify the last sentence of the last paragraph of the
introduction accordingly to remove the word "conforming".
(I believe the remaining text describes sufficiently that this is
documentation. So I did not want to rewrite the whole paragraph and add
history.)
Thanks for those edits, and the ones you call out below.
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.)
Please note, that section 2.3.2.2. describes the variation in behaviour
between different browsers.
As existing browser implementations may change with regards to whether
they allow _all_ the productions of the ABNF, we intentionally did not
investigate and document all possible outcomes and combinations.
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.
Ok. I changed it to non-2119.
However, the remaining text is appropriate and adds clarity to declare
the scope of the x-frame-options header effects. And implementations and
specifications of x-frame-options out there consider it as part that
plug-ins observe x-frame-options if "these plugins appear essentially as
frames".
Please consider that a plug-in that would "essentially appear as a
frame" but would not be observing x-frame-options would in effect break
the security model of the browser implementation, so I think the
remaining text is appropriate and needed in this case.
This takes you into talking about what new implementations should do,
rather than documenting what's in the wild.
You can avoid that by noting that any existing plug-in implementations
that don't do the right thing here create a problem.
(It would follow that any new ones would have that same problem).
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.
Please consider that even though this is to document existing use
(especially until we get CSP1.1), it's documentation can also help guide
new implementations for the purpose to reduce or avoid divergence of
implementations.
You can have that influence by reporting on what is rather than trying
to write constraints on new things.
This leads to the first sentence and also to the use of
"MAY" in the second sentence, which in fact is not a requirement of the
browser implementation but points out an implementation option.
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
As the comment before, this documentation also helps to reduce or avoid
divergence of implementations. So the "SHOULD" is appropriate and as
intended here.
How about simply saying that any change or divergence to the
implementations will result in (this list of bad things).
That's informative rather than trying to be normative.
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".
Ok. Changed accordingly.
Nit: s/does support only one/only supports one/
Ok. Changed.
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.
Hm. First, as outlined in 2.3.2.2 not all of the browsers support
"Allow-From". And second, the pattern is actually not for the browser
but a way for the server to achieve the specific goal (i.e. of a source
page being framed by more than one web page, even though allow-from
supports only on URI).
To accomodate your feedback and make clear this is not trying to
contrain new development, I changed the word "is recommended" to "can
fulfill that need".
Nit: If Appendix B is kept, please expand CSRF (and consider
re-pointing to a reference)
Ok. Done.
_______________________________________________
Gen-art mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/gen-art
_______________________________________________
Gen-art mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/gen-art