Re: [MIX] Initial feedback on Mixed Content

On Tue, Nov 18, 2014 at 6:53 AM, Mike West <mkwst@google.com> wrote:
> I trimmed things up a little bit today:
> https://github.com/w3c/webappsec/compare/96b18716431c7107e6744a7f04f6d1e298a3201f...master
> WDYT?
>
> I doubt it addresses all of your concerns, but it's what I had time for
> today. :)

Actually, it looks really good.

I have some nits:

> Abstract
>
> This specification describes how a user agent should handle
> rendering and execution of content loaded over unencrypted
> or unauthenticated connections in the context of an encrypted
> and authenticated document."

Actually, the specification describes how a user agent *must fetch or
must not fetch content*, not so much how it is rendered or executed.
The effects on rendering and execution are side effects of the
fetching/non-fetching. I think it is good to be precise about this.
(Also in CSP.)

> Status of this document

Skipped (boilerplate)

> Introduction

Skipped (non-normative)

>  2.1. Terms defined by this specification

In this section, there are some definitions of terms that are
redefined in a more precise way later in the specification. This is
confusing. Is it a W3C requirement to have the "Terms defined by this
specification" section? If not, I suggest removing it. Otherwise, I
suggest at least providing a link to the normative definition in each
of the definitions here, like was done in section 2.2 (Terms defined
elsewhere)

> 2.2 Terms defined elsewhere

Skipped (laziness)

> 3. Content Categories

> For instance, a survey in 2013 notes that blocking mixed
> content would break around ~43% of secure websites in
> one way or another [DANGEROUS-MIX].

That survey actually says that 43% of secure websites in their study
have mixed-content vulnerabilities. It does NOT say that blocking
mixed content would break the website. The implementation experience
of Firefox (and other browsers, but I'm most familiar with Firefox's
experience) showed that many websites were actually *improved* when
mixed content ads and other things were blocked. Consequently, at a
minimum, I suggest using a more neutral word than "break," such as
"affect." More generally, I don't think that statistic is useful for
making decisions; in fact, I think it is very misleading in
overstating the problem. Plus, the study is just plain out of date,
due to the effects of Firefox and Chrome improving their mixed content
blocking, and due to the Snowden effect. So, I suggest just removing
the sentence completely.

> Draconian blocking policies applied to some types of mixed
> content are (for the moment) infeasible.

This is something of a tautology. If they were feasible, you wouldn't
want to label them "Draconian." I suggestion you just drop the
sentence, because the surrounding text already makes the same point.

> With that in mind, we here split mixed content into two
> categories defined in the following two sections [...]

I suggest you cut "defined in the following two sections".

> 3.1 Optionally-blockable Content

> A resource is considered optionally-blockable content [...]

Remove "considered". This is the definition of the term, so it should
be worded more definitively.

> [...] This could be because mixed usage of the resource type is
> sufficiently high, or because the resource is very clearly
> low-risk in and of itself.

I suggest "Mixed usage of the resource type is high and it is low-risk
in and of itself." In particular, s/or/and/ and avoid "very clearly";
I think the clarify is much more debatable than the overall judgement
that they are low-risk.

I think the paragraph following this one can be merged into this one.

> 3.2 Blockable Content
>
> Any resource that isn’t optionally-blockable is considered blockable content.

Remove "considered". This is the definition of the term, so it should
be worded more definitively.

> In short, every request context that is not optionally-blockable
> is a blockable request context

Remove "In short,". It implies that the statement is a not-quite-true
approximation, but I think it is actually definitive.

> 4. Insecure Content in Secure Contexts
>
> When fetching resources, the mixed content checks described
> in the algorithms below should be inserted at the top of the
> Fetch algorithm to block network traffic to a priori insecure
> origins, and at the bottom of the algorithm, to block responses
> from insecure origins.

This is ambiguous and potentially misleading. In particular, it could
be interpreted to mean that all the algorithms get inserted at the
top, and all the algorithms get inserted at the bottom. It seems like
later text restates the same things in a more
precise/correct/unambiguous way, so I think you should just remove
this paragraph.

This section states where algorithm 4.2 and algorithm 4.3 are inserted
into the HTML specification, but it doesn't specify where algorithm
4.1 is to be inserted. I think a similar statement for algorithm 4.1
would be very useful, if not required.

Note that I'm not sure any browser other than Chrome implements the
logic for "deprecated TLS-protection." I guess that is why that term
is defined to be "vendor-specific." But, effectively, all this really
means is that the implementation may choose to block a fetch for any
implementation-defined reason. Does this really belong in this
specification? It seems like it belongs in Fetch. In particular, this
technically covers things like Firefox's nsIContentPolicy-based
extensions (ad blockers, no script), which have nothing to do with
mixed content. This makes me think that the stuff regarding
"deprecated TLS-protection" can be removed too. Or, rather, perhaps it
should be deferred until deprecated TLS-protection is defined.

My understanding is you are planning to make changes (just removing
section 5? or more?) to the rest of the document, so I'll stop here
until you've done so.

Cheers,
Brian

Received on Friday, 21 November 2014 22:48:30 UTC