Re: Mixed Content ED Comments

Wow, Glenn. Thanks for the thorough review! I've landed the following
set of changes, and left lots of comments
inline.

https://github.com/w3c/webappsec/compare/139755e25f0e35c10af3c458a8be800a92d4197e...master

On Fri, Jul 4, 2014 at 12:47 AM, Glenn Adams <glenn@skynav.com> wrote:
>
> The following comments apply to the 02 July 2014 ED.
>
> 1. Suggest changing "JavaScript" to "ECMAScript" throughout, or making it a more generic "Script"

Sure. I've changed JavaScript to "script" throughout, except for
"JavaScript global environment" as that's a specific term defined
elsewhere. I don't think we need to bring in ECMA-262 as a normative
reference.

> 2. Under 2.1, suggest changing the first bullet under "mixed content" to read "... and the origin of the JavaScript global environment into which it is loaded is a secure context." This way, both definitions of mixed content are defined in terms of comparing properties of two origins.

"Secure context" is defined in a way that requires not only the origin
itself, but the origins of its ancestors. I don't think this section
needs to change.

> 3. Under 2.1, link to CA/Browser Forum's Baseline Requirements

Done.

> Under 2.1, definition of potentially secure origin, the Note has language "will take place" that sounds rather normative (even though it doesn't use an RFC2119 keyword).

I'm happy to change this, but in general I don't have a problem with
statements of fact that don't use RFC2119 keywords. The ordering is
defined in Fetch, this note just makes the logic visible here as well.

> Under 2.1, definition of potentially secure origin, suggest changing in Note from "should simplify assumptions of" to "simplifies assumptions about".

Running with "simplifies determination of".

> Under 2.1, definition of deprecated TLS-protection, the language "resources from that origin" is ambiguous with respect to how many (or the nature of the) resources that satisfy a condition for labeling that origin as insecure due to being weakly TLS-protected or using deprecated TLS-protection.

Dropped the phrase for clarity. We're talking about a single resource
in this definition.

> Under 2.2, change label "plugin" to "plugin document".

I don't think we need the definition here at all; it's only used once
in the document. I've added a normative [HTML5] link there.

> Under 2.2, definition of "TLS-protected", the link Section 5.2 of "Web Security Context: User Interface Guidelines", should have the link to [WSC-UI] placed immediately after this reference instead of at the end of the paragraph.

Sure.

> Under 2.2, definition of "TLS-protected", 1st paragraph, suggest removing the informal "In short".

Sure.

> Under 2.2, definition of "TLS-protected", the phrases "strong protection", "self-signed certificate", "weakly authenticates", and "weak cipher suite" are not well defined nor refer to an external definition (even though one might expect readers to infer definitions).

I think there's enough clarity in the referenced WSC-UI spec to make
these terms clear. If others disagree, I'll take another pass at the
paragraph.

> Under 2.2, definition of "TLS-protected", the Note containing the language "strongly recommended" is problematic because it is attempting to make what would normally be considered a normative recommendation, i.e., a normative SHOULD, but is doing so in the context of an informative note. If there is going to be a recommendation as such, then suggest it should be made a normative recommendation and use SHOULD.

Happy to make this SHOULD; it seemed a bit outside the scope of the
document, but it matches Chrome's behavior. Moved down to Section 4.1.

> Under 2.2, definition of "TLS-protected", the Note uses the term interstitial, which does not refer to a definition nor is it a commonly understood English language term as used in this special fashion.

How about "confirmation screen"?

> Under 2.2, definition of globally unique identifier, suggest changing "Section 4 of the Origin specification" to read "RFC6454 §4" to better correspond with the reference "[RFC6454]" at the end of the definition.

Done.

> Under 2.2, definition of globally unique identifier, suggest moving "Note that URLs ..." into an actual Note.

Done.

> Under 2.2, definitions of fetch, request, response, suggest changing "Fetch living standard" to "Fetch specification". Also, a plan will be needed to move this work to a W3C REC track specification.

The reference is currently to the document named "Fetch living
standard"; I think the name is accurate for the moment.

Regarding the bigger question, I've raised what to do with Fetch a
number of times in the WG; the W3C is going to have to work that out.
I'd suggest that forking the spec is a bad idea and that we should
just point to the WHATWG document, but that's someone else's call.

> Under 2.2, definition of "JavaScript global environment", the link "Section 2.2.2" does not point at the HTML5 specification, but at this definition itself.

Hrm. That's not helpful. :)

> Sections entitled Conformance, References, Index, and Issues Index need to be labeled as numbered or lettered appendices.

Why?

> Under Normative References, the reference to HTML5 is out of date, and needs to be updated to at least the current "latest" edition, namely http://www.w3.org/TR/html5/ or http://www.w3.org/html/wg/drafts/html/CR/.

The link in the doc is current according to
http://dev.w3.org/csswg/biblio.ref. If it should point somewhere else,
that file needs to be updated. Not entirely sure who to file a bug
against for that. Probably Tab:
https://github.com/tabatkins/bikeshed/issues/188 :)

> Under 3, Categories of Content, this entire section reads like a back-of-napkin draft of a white paper; it is clearly couched in non-specification language of a non-normative nature; suggest it be labeled non-normative until it is completely rewritten.

There are certainly normative components of this section, so I
disagree with the suggestion to mark it as non-normative. That said, I
agree wholeheartedly with the suggestion that Brian made in an earlier
thread to reduce the emphasis on today's categorizations: I intend to
drop most of the discussion of categorization, and simply list the
content types that are impractical to block by default on today's web,
and the intent to block everything as soon as practical.

> Under 3, Categories of Content, the term navigational request context is not defined.

It's defined right there. :) I'll clarify.

> Under 3.1, Active Content, the characterization of "active content" is highly questionable; in particular, style sheets, SVG documents that don't contain script, XSL transformations, and HTML Manifests definitely do not satisfy the common understanding of active content as being executable code, e.g., see NIST's Guidelines on Active Content and Mobile Code, as well as the earlier definition of active object content in DASE Part 1 §3.3, and prior use with ActiveX; furthermore, the claim at the top of "completely control the activity on a webpage and exfiltrate data at will" is not true for many of these types.

That claim is specifically with regard to script and style, which
certainly do have those capabilities. The broader "active" definition
covers content that "can in some way directly manipulate the resource
with which a user is interacting", which XSLT and manifests certainly
can (manifests define CSP, for instance).

But, again, detailed discussion of which resource falls into which
category is probably not worthwhile. I intend to recommend that we
block all mixed content by default, with the exception of those items
currently in "optionally blockable passive".

> Under 3.2.2, please explain the risk presented by subtitles and captions loaded via <track> elements. I would question categorizing this content as blockable as opposed to optionally blockable.

Passive monitoring of content is bad for users. Subtitles and captions
directly reveal what media is being viewed.

This implies, of course, that we should also block the media itself.
We'll get there.

> Under 4.1, step 2, change "Requests for" to "Responses to requests for".

Indeed.

> Under 4.1, step 4, the language "MAY be modified" is entirely too vague with respect to what modifications are permitted or recommended.

Any. All. It's intentionally vague, as it's intended to allow user
agents flexibility to experiment with ways of reducing the risk of
loading insecure content that's too widespread to block outright.
There are two examples in the step that are meant to clarify; I hope
that's enough.

> Under 4.1, in the second step 1 (about private/public origins), the restraint on generating network traffic may be insufficient to handle file: and localhost origins.

I would consider the loopback interface as "network", but you're
probably right; this could be clearer.

> Under 4.2, items 1 through 3 need citations to specific sections of XHR, SSE, and WEBSOCK.

Each links directly to the relevant method. What else would you like
to see? Just "[WEBSOCKETS]" links to the normative references?

> Under 4.2, item 2, typo s/provessing/processing/.

Good eye!

> Under 4.3, 1st paragraph, combine first paragraph and second paragraph to use a single MUST NOT instead of combining "MUST adhere to" followed by "MUST NOT".

Hrm. Ok, yeah. I can go back to this structure if/when we add more
requirements. I was assuming we'd come up with more. :)

> Under 4.3, need citation and reference for "EV status" link.

Linked to [CAB].

> Under 4.4, the first Note should be removed. W3C specs don't make recommendations to end users.

Why not? That seems fairly arbitrary. It's clearly not normative, but
saying "Hey, this would be a good thing to do if your user agent
allows it" seems perfectly reasonable to me.

> Under 4.4, the second Note must not use the RFC2119 keyword phrase SHOULD NOT. Further, the language of this note if overly informal.

It uses RFC6919's "REALLY SHOULD NOT". :) Also: it's a note. It's
meant to be informal, explaining the rationale for the MAY-level
recommendation above.

> Under 5.1, the use of the term environment is problematic because in the algorithm steps, the environment is said to be TLS-protected or not, however the definition of TLS-protected applies to a resource (from an origin) and not an environment. It feels like this algorithm should be called "Is origin secure?". See also comment 2 above.

Hrm. I don't see this as problematic: the environment in which the
resource is loaded is either secure or not. For the purposes of mixed
content, we only care whether a resource is loaded from an insecure
origin if the resource is being loaded into an insecure environment.

How would you suggest clarifying these two concepts and their relationship?

> Under 5.1, 1st paragraph, change "can determine if" to "determines if".

Sure.

> Under 5.1, 1st paragraph, remove the logically redundant ", which returns true if environment is a secure context, and false otherwise". It is redundant since the following steps say as much.

Yes. It's redundant, but it gives the algorithm a clear signature with
meaning. I don't think the redundancy is harmful, anymore than `bool
isRedundant() { return false; }`'s "redundancy" is harmful. The
function _says_ return `false`, but knowing that it's supposed to
return a `bool` is helpful.

> Under 5.1, the phrase "If a document is framed" is not well defined, i.e., what is meant by "is framed"?

Changed to "is a nested browsing context".

> Further, this paragraph seems overly informal: is it intended to be non-normative? If so, then perhaps it should go into a Note.

I can change it to a note, I suppose. It's simply meant to explain and
justify the algorithm above.

> Under 5.1, Example 4, the term "frames" needs a definition.

Really? This seems quite clear in context. Are there cases where the
term would be unclear?

> Under 5.1, Example 5, the term "framed data URL" needs a definition.

Again, this seems clear enough in context for an example.

> Under 5.2, 1st paragraph, the term "should" appears. Is it intended that this be a RFC2119 "SHOULD"?
> Under 5.2, 1st paragraph, what is meant by "entirely"? Does it imply that it may instead "partially block requests"?

Dropped the clause; the document's intent is made clear above. In an
ideal case, all mixed content is completely blocked.

> Under 5.2, 1st paragraph, it seems unusual to say what the Fetch specification will do with this algorithm.

Dropped "will"; the Fetch spec does now hook into this algorithm, so
we'll just state that fact for clarity.

> Overall, this 1st paragraph is overly informal and sounds like non-normative content.

Is informality bad? I'm happy to make it a note. *shrug*

> Under 5.2, 2nd paragraph, change "can determine whether" to "determines whether".

Alright.

> Under 5.2, 2nd paragraph, the fragment "whether the should proceed" seems to want to be read as "whether the request should proceed". Also need to add a COLON at end of this paragraph.

Indeed. Thanks for noticing the typos.

> Under 5.2, step 6 may need tweaking regarding the term environment; i.e., is it the environment that is secure or the origin associated with the environment that is secure? See also step 7 which talks about the origin being a priori insecure. There seems to be a general confusion around environment or origin when making statements about secure or insecure. See also step 8 which talks about origin being potentially secure.

Given the rationale above, I think it's correct to talk about the
environment as being secure, not its origin.

Step 7 and 8 are referring to the origin of the resource being
requested. Step 6 to the security of the environment in which that
resource is being requested. This isn't unintentional. I'm happy to
make it more clear if you have suggestions.

> Under 5.3, 1st paragraph is overly informal "we still might want to", etc. Needs rewriting and potentially labeling as non-normative.

It's describing the intended usage of the algorithm. I suppose it
could be a note.

Again, I don't see why informality is bad. I would like folks to be
able to read the spec and understand why algorithms are they way they
are. Formal language isn't required for that purpose, and is often
contrary to it.

> Under 5.3, 2nd paragraph, change "can determine what" to "determines what".

Ok.

> Under 5.3, step 3, same comment about environment vs origin secure confusion as described in comment 43 above.

Same response.

> Under 6, change "The Fetch algorithm MUST be updated as follows" to read "The Fetch algorithm is modified as follows". W3C specs don't place requirements on other spec writers, but on interpretations or implementations.

I need to revamp this section anyway, now that Anne has made the
changes described here. Thanks for the reminder.

> Under 7, 1st paragraph, rewrite to read simply as follows "The WebSocket() constructor algorithm [WEBSOCKETS] is modified as follows:" followed by the first bullet "Replace Step 2 ...", then introduce a new paragraph "The WebSocket Connection algorithm [WSP] is modified as follows:" followed by the second bullet "After step 5 ...".

Reasonable, thanks.

> Under 8, Acknowledgements, this material is background material and not the usual acknowledgements of individuals contributing to the specification. Suggest moving it into the introduction and adding a list of individual contributors to this work.

Alright.

> Under unnumbered "Conformance", suggest moving this material to a numbered section before Section 4 User Agent Requirements, or to be the first sub-heading under Section 4.

Why?

> Under unnumbered "Conformance", what requirements listed in this specifications apply to servers? If none, then remove "conformant server".

Ah, boilerplate. I don't think we have any distinct server-side
requirements, except for the implicit requirements not to serve weakly
TLS-protected resources.

> Under Normative References, there may be a better citation than [CSS-2010] to refer to stylesheets, particularly since that document is rather out of date.

Yes indeed. Changed to [CSS21].

> Under Normative References, no reference is (yet) made to ECMA-262 (but see comment 1 above).

It's referenced under the first bullet of "active content" to describe
"scripts". I think we can probably drop it entirely, to be honest.

-mike

Received on Friday, 4 July 2014 09:31:18 UTC