SRI: Remove the restriction to HTTPS documents and resources.

SRI: Remove the restriction to HTTPS documents and resources.

Based on discussion at https://groups.google.com/a/chromium.org/d/msg/security-dev/hJwYZOOPaH4/jb6E28o-_W0J

BUG=355467
R=jww@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=190537

[Mike West, 2014-12-14 08:00:23]

mkwst@chromium.org changed reviewers:
+ jschuh@chromium.org, jww@chromium.org, lcamtuf@google.com, palmer@chromium.org

[Mike West, 2014-12-14 08:00:24]

Hello, people who expressed opinions on the thread. WDYT? :)

-mike

[palmer, 2014-12-15 20:21:57]

So, what would the outcome of this CL be? I'm not familiar with this code and
can't immediately tell.

HTTPS pages could include HTTP resources but with integrity protection? My
objection to that is that we should not create affordances for mixed content. We
should be past this point and should be working toward defeating traffic
analysis, not improving it.

Or, would the outcome be that HTTP pages could include HTTP or HTTPS resources,
but with integrity "protection"? There would be no real protection.

[Mike West, 2014-12-15 20:25:55]

On 2014/12/15 20:21:57, Chromium Palmer wrote:
> HTTPS pages could include HTTP resources but with integrity protection? My
> objection to that is that we should not create affordances for mixed content.
We
> should be past this point and should be working toward defeating traffic
> analysis, not improving it.

Those requests would never hit SRI, as they'd be blocked beforehand by mixed
content. This change wouldn't have any effect on mixed content checking at all.

> Or, would the outcome be that HTTP pages could include HTTP or HTTPS
resources,
> but with integrity "protection"? There would be no real protection.

This is the impact of the change: as discussed on the list, I agree with you
theoretically, and disagree with you practically. :)

[palmer, 2014-12-15 21:25:57]

I am not an OWNER and you don't have to listen to me. But for the record, I am
against landing this CL.

[jww, 2014-12-16 03:36:35]

I'd like to hold off on landing this for a little bit as we certainly didn't
come to consensus on the list, and I don't know what the Right Answer is yet.

https://codereview.chromium.org/803773002/diff/1/Source/core/frame/Subresourc...
File Source/core/frame/SubresourceIntegrity.cpp (left):

https://codereview.chromium.org/803773002/diff/1/Source/core/frame/Subresourc...
Source/core/frame/SubresourceIntegrity.cpp:98: UseCounter::count(document,
UseCounter::SRIElementWithIntegrityAttributeAndInsecureOrigin);
If this CL eventually lands, we should deprecate these use counters.

[Mike West, 2014-12-16 05:22:21]

On 2014/12/16 03:36:35, jww wrote:
> I'd like to hold off on landing this for a little bit as we certainly didn't
> come to consensus on the list, and I don't know what the Right Answer is yet.

More discussion is fine; I see the conversation splintering a bit into distinct
camps that seem to fundamentally disagree with each other, so I think we're just
going to have to make a decision at some point, but that point doesn't need to
be right now.

[jww, 2014-12-18 18:44:26]

I very much hesitate to write this, but lgtm (with a nit). I am convinced that
we will not come to a consensus, and this seems like an unsatisfying change that
most closely matches the most opinions. On a personal level, while I disagree
with this, Mike has also convinced me that SRI is just another form of content
policy, and if we're not going to require CSP over HTTPS-only, it doesn't make
sense to do it here, either.

[palmer, 2014-12-22 21:35:53]

Not LGTM.

[Mike West, 2015-01-19 14:07:54]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2015-01-19 14:08:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/803773002/40001

[commit-bot: I haz the power, 2015-01-19 14:08:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-19 14:08:54]

A disapproval has been posted by following reviewers: palmer@chromium.org.
Please make sure to get positive review before another CQ attempt.

[Mike West, 2015-02-19 11:54:08]

On 2015/01/19 at 14:08:54, commit-bot wrote:
> A disapproval has been posted by following reviewers: palmer@chromium.org.
> Please make sure to get positive review before another CQ attempt.

Based on Chris' response at
https://groups.google.com/a/chromium.org/d/msg/security-dev/hJwYZOOPaH4/KuFWV...,
I don't think he intended to block this from landing, but wanted to be on record
as very strongly against it. Based on the rest of the thread, I'm comfortable
claiming that there was a rough consensus that this was worth exposing to HTTP
origins as well as HTTPS origins.

I'll land this manually if the bots are happy.

[jww, 2015-02-19 21:27:30]

sgtm

On Thu Feb 19 2015 at 3:54:09 AM <mkwst@chromium.org> wrote:

> On 2015/01/19 at 14:08:54, commit-bot wrote:
> > A disapproval has been posted by following reviewers:
> palmer@chromium.org.
> > Please make sure to get positive review before another CQ attempt.
>
> Based on Chris' response at
> https://groups.google.com/a/chromium.org/d/msg/security-
> dev/hJwYZOOPaH4/KuFWVAYkPP4J,
> I don't think he intended to block this from landing, but wanted to be on
> record
> as very strongly against it. Based on the rest of the thread, I'm
> comfortable
> claiming that there was a rough consensus that this was worth exposing to
> HTTP
> origins as well as HTTPS origins.
>
> I'll land this manually if the bots are happy.
>
> https://codereview.chromium.org/803773002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[blink-reviews, 2015-02-19 22:17:58]

Correct; fire away. Sorry!
On Feb 19, 2015 03:54, <mkwst@chromium.org> wrote:

> On 2015/01/19 at 14:08:54, commit-bot wrote:
>
>> A disapproval has been posted by following reviewers: palmer@chromium.org
>> .
>> Please make sure to get positive review before another CQ attempt.
>>
>
> Based on Chris' response at
> https://groups.google.com/a/chromium.org/d/msg/security-
> dev/hJwYZOOPaH4/KuFWVAYkPP4J,
> I don't think he intended to block this from landing, but wanted to be on
> record
> as very strongly against it. Based on the rest of the thread, I'm
> comfortable
> claiming that there was a rough consensus that this was worth exposing to
> HTTP
> origins as well as HTTPS origins.
>
> I'll land this manually if the bots are happy.
>
> https://codereview.chromium.org/803773002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Mike West, 2015-02-20 07:08:34]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2015-02-20 07:09:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/803773002/60001

[commit-bot: I haz the power, 2015-02-20 07:09:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-20 07:09:32]

A disapproval has been posted by following reviewers: palmer@chromium.org.
Please make sure to get positive review before another CQ attempt.

[Mike West, 2015-02-20 07:17:58]

On 2015/02/20 at 07:09:32, commit-bot wrote:
> A disapproval has been posted by following reviewers: palmer@chromium.org.
> Please make sure to get positive review before another CQ attempt.

Note to future self: TBR doesn't override. :) Manual it is.

[Mike West, 2015-02-20 07:30:34]

Committed patchset #4 (id:60001) manually as 190537 (presubmit successful).