Thanks! LGTM2
On Fri, Oct 1, 2021 at 9:11 AM Antonio Sartori <antoniosart...@chromium.org> wrote: > > > On Fri, Oct 1, 2021 at 5:09 PM Chris Harrelson <chris...@chromium.org> > wrote: > >> One more requirement from my perspective, and one question below. >> >> > Thanks. Replies inline. > > >> On Fri, Oct 1, 2021 at 2:50 AM Yoav Weiss <yoavwe...@chromium.org> wrote: >> >>> *LGTM1* >>> >>> Thank you for doing this analysis! 2/550K is significantly less than 80% >>> :P >>> >>> I believe that puts us at ~0.00036%, and the actual number of affected >>> sites (as opposed to workers) is an order of magnitude lower. Also, there's >>> no particular reason to believe HA is unrepresentative for this particular >>> change. (as I wouldn't expect main page workers to vary vastly from workers >>> in other pages) >>> >>> That makes me agree with your assessment of this being low risk. >>> >>> On Fri, Oct 1, 2021 at 10:12 AM Antonio Sartori < >>> antoniosart...@chromium.org> wrote: >>> >>>> Sorry, of course. I just forgot. >>>> >>>> On Fri, Oct 1, 2021 at 9:30 AM Yoav Weiss <yoavwe...@chromium.org> >>>> wrote: >>>> >>>>> Thanks!! >>>>> >>>>> On Fri, Oct 1, 2021 at 9:23 AM Antonio Sartori < >>>>> antoniosart...@chromium.org> wrote: >>>>> >>>>>> I did some more research in httparchive. By filtering out only >>>>>> interesting CSPs delivered by workers and comparing them with their owner >>>>>> document's CSP, I was left with only two entries (out of the total >>>>>> 457,780 >>>>>> worker requests) which might actually break. The details are in this >>>>>> document >>>>>> <https://docs.google.com/document/d/16_TiDx3MOE7uO6vMhE6qE1jN78ZzzvUsaqcV0Cb3G0E/edit?usp=sharing>. >>>>>> >>>>>> >>>>> >>>>> Could the doc be made public? >>>>> >>>>> >>>>>> Could this give us confidence that content won't break, even without >>>>>> adding user counters in the code? >>>>>> >>>>>> Regarding Safari, I did some more experiments and checked their code >>>>>> <https://github.com/WebKit/WebKit/blob/5f99dfc43d30d3af1cabbf24d111acbff994808b/Source/WebCore/workers/DedicatedWorkerGlobalScope.cpp#L60>. >>>>>> I would now say confidently that they do implement this. >>>>>> >>>>>> Thanks! >>>>>> >>>>>> On Wed, Sep 29, 2021 at 5:05 PM Yoav Weiss <yoavwe...@chromium.org> >>>>>> wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>> On Wed, Sep 29, 2021 at 4:16 PM Antonio Sartori < >>>>>>> antoniosart...@chromium.org> wrote: >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Wed, Sep 29, 2021 at 2:50 PM Yoav Weiss <yoavwe...@chromium.org> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Wed, Sep 29, 2021 at 2:14 PM Antonio Sartori < >>>>>>>>> antoniosart...@chromium.org> wrote: >>>>>>>>> >>>>>>>>>> From a quick search through httparchive (data from >>>>>>>>>> httparchive.requests.2021_07_01_desktop) it looks like out >>>>>>>>>> of 549,668 outgoing requests for dedicated workers, 457,780 of them >>>>>>>>>> returned a Content-Security-Policy header (hence ~80%). As a >>>>>>>>>> comparison, >>>>>>>>>> from the same data it looks like ~15% of document requests return a >>>>>>>>>> Content-Security-Policy. >>>>>>>>>> >>>>>>>>> >>>>>>>>> That's a lot! >>>>>>>>> >>>>>>>>> >>>>>>>>>> >>>>>>>>>> Better data could be gathered by building some use counters in >>>>>>>>>> chromium's code. >>>>>>>>>> >>>>>>>>> >>>>>>>>> Unless there's particular urgency here, I think use-counters would >>>>>>>>> make sense here. That can give us confidence that content won't break >>>>>>>>> as a >>>>>>>>> result of this change in CSP rule application. >>>>>>>>> >>>>>>>> >>>>>>>> Just to double-check that I understand correctly: For giving us >>>>>>>> confidence that content won't break, we would have to build a >>>>>>>> mechanism to >>>>>>>> check workers' response CSPs (without enforcing them) and report via >>>>>>>> use >>>>>>>> counters in case a resource which is currently allowed would have been >>>>>>>> blocked. This is what I was sketching below, and requires quite some >>>>>>>> implementation work, which I am not sure when we would be able to >>>>>>>> prioritize. >>>>>>>> >>>>>>> >>>>>>> Maybe I'm missing the mark, but wouldn't that implementation work be >>>>>>> something that's on path to what you'd need to implement anyway for >>>>>>> this? >>>>>>> I certainly have no interest in making you do extra work, but with >>>>>>> 80% (!!) of workers currently having CSP headers, I'm concerned that >>>>>>> mismatches between the worker headers and the main document headers >>>>>>> exist, >>>>>>> which can result in broken content. >>>>>>> I take your point that if such discrepancies exist, they are >>>>>>> currently reflected in broken content in e.g. Firefox. At the same time, >>>>>>> I'd be more comfortable with taking a decision here based on a bit more >>>>>>> data. >>>>>>> >>>>>>> >>>>>>>> I don't think there is a particular urgency, although this is a >>>>>>>> bugfix and I believe this is a big security improvement, since it >>>>>>>> allows >>>>>>>> servers to have different policies on their main page and on the >>>>>>>> worker (so >>>>>>>> for example an application which needs eval inside a worker would not >>>>>>>> need >>>>>>>> to enable eval in the main document). >>>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>>> >>>>>>>>>> I have not looked into how often that would remove restrictions >>>>>>>>>> from existing web contents. I don't see an easy way to do it without >>>>>>>>>> adding >>>>>>>>>> code to chrome to check both the inherited and the response CSP for >>>>>>>>>> each >>>>>>>>>> outgoing request from a worker and report when the two checks >>>>>>>>>> mismatch. It >>>>>>>>>> is surely doable but I have the impression it might be overkilled in >>>>>>>>>> this >>>>>>>>>> case. >>>>>>>>>> >>>>>>>>>> Since the proposed behaviour is already implemented by Firefox >>>>>>>>>> (and it seems also by Safari), I believe the probability of this >>>>>>>>>> breaking >>>>>>>>>> something to be fairly low (developer would have noticed their >>>>>>>>>> websites not >>>>>>>>>> working on Firefox/Safari already). >>>>>>>>>> >>>>>>>>>> On Wed, Sep 29, 2021 at 1:21 PM Yoav Weiss < >>>>>>>>>> yoavwe...@chromium.org> wrote: >>>>>>>>>> >>>>>>>>>>> Have you looked into the compatibility implications of changing >>>>>>>>>>> behavior here? How often would that remove restrictions from >>>>>>>>>>> existing web >>>>>>>>>>> content? How often do dedicated workers currently get CSP headers >>>>>>>>>>> which >>>>>>>>>>> will now be applied? >>>>>>>>>>> >>>>>>>>>>> On Mon, Sep 27, 2021 at 12:50 PM Antonio Sartori < >>>>>>>>>>> antoniosart...@chromium.org> wrote: >>>>>>>>>>> >>>>>>>>>>>> Contact emailsantoniosart...@chromium.org >>>>>>>>>>>> >>>>>>>>>>>> Specification >>>>>>>>>>>> https://html.spec.whatwg.org/#initialize-worker-policy-container >>>>>>>>>>>> >>>>>>>>>>>> https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy#csp_in_workers >>>>>>>>>>>> >>>>>>>>>>>> Summary >>>>>>>>>>>> >>>>>>>>>>>> Dedicated workers should be governed by the Content Security >>>>>>>>>>>> Policy delivered in their script response headers. Chrome >>>>>>>>>>>> incorrectly used >>>>>>>>>>>> to instead apply the Content Security Policy of the owner >>>>>>>>>>>> document. We >>>>>>>>>>>> would like to change chrome's behaviour to adhere to what is >>>>>>>>>>>> specified. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> For background, see the discussion on the github issue where >>>>>>>>>>>> this was agreed: >>>>>>>>>>>> https://github.com/w3c/webappsec-csp/issues/336 >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Blink componentBlink>SecurityFeature>ContentSecurityPolicy >>>>>>>>>>>> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3ESecurityFeature%3EContentSecurityPolicy> >>>>>>>>>>>> >>>>>>>>>>>> TAG review >>>>>>>>>>>> >>>>>>>>>>>> TAG review statusNot applicable >>>>>>>>>>>> >>>>>>>>>>> >> I agree that a TAG review is not applicable in this case, because the >> behavior is already in an agreed-upon specification and one browser is >> already shipping the behavior. >> >> Am I correct that the specification in HTML covers all the behaviors in >> this intent? >> > > Yes, that is correct. > > >> >> >>>>>>>>>>>> >>>>>>>>>>>> Risks >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Interoperability and Compatibility >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Gecko: Shipped/Shipping See also the discussion on the issue >>>>>>>>>>>> https://github.com/w3c/webappsec-csp/issues/336 >>>>>>>>>>>> >>>>>>>>>>>> WebKit: N/A >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> Why N/A? >>>>>>>>> >>>>>>>> >>>>>>>> Because I am not totally sure. While Firefox argued publicly that >>>>>>>> this is how they believe CSPs for workers should work and this is what >>>>>>>> they >>>>>>>> implement, there was no signal from WebKit AFAIK. From some manual >>>>>>>> testing, >>>>>>>> it looks to me like WebKit implements this. Unfortunately our WPTs >>>>>>>> here are >>>>>>>> not so great, since many of them time out on Safari ( >>>>>>>> https://wpt.fyi/results/content-security-policy/inside-worker?label=experimental&label=master&aligned >>>>>>>> ). >>>>>>>> >>>>>>> >>>>>>> Would be good to ask for their official opinion: >>>>>>> https://bit.ly/blink-signals >>>>>>> >>>>>> >> >> Please ask for a signal from WebKit. >> > > I have asked for a signal starting this thread: > https://lists.webkit.org/pipermail/webkit-dev/2021-October/031999.html > >> >> >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> >>>>>>>>>>>> Web developers: Positive ( >>>>>>>>>>>> https://bugs.chromium.org/p/chromium/issues/detail?id=1012640) >>>>>>>>>>>> This has been reported as a bug to chrome. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Debuggability >>>>>>>>>>>> >>>>>>>>>>>> Warnings regarding Content Security Policy are and will >>>>>>>>>>>> continue to be reported in the devtools console. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Is this feature fully tested by web-platform-tests >>>>>>>>>>>> <https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_platform_tests.md> >>>>>>>>>>>> ?Yes >>>>>>>>>>>> >>>>>>>>>>>> Flag name >>>>>>>>>>>> >>>>>>>>>>>> Requires code in //chrome?False >>>>>>>>>>>> >>>>>>>>>>>> Tracking bug >>>>>>>>>>>> https://bugs.chromium.org/p/chromium/issues/detail?id=1253267 >>>>>>>>>>>> >>>>>>>>>>>> Estimated milestones >>>>>>>>>>>> >>>>>>>>>>>> No milestones specified >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Link to entry on the Chrome Platform Status >>>>>>>>>>>> https://chromestatus.com/feature/5715844005888000 >>>>>>>>>>>> >>>>>>>>>>>> This intent message was generated by Chrome Platform Status >>>>>>>>>>>> <https://www.chromestatus.com/>. >>>>>>>>>>>> >>>>>>>>>>>> -- >>>>>>>>>>>> You received this message because you are subscribed to the >>>>>>>>>>>> Google Groups "blink-dev" group. >>>>>>>>>>>> To unsubscribe from this group and stop receiving emails from >>>>>>>>>>>> it, send an email to blink-dev+unsubscr...@chromium.org. >>>>>>>>>>>> To view this discussion on the web visit >>>>>>>>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOzWxF5EX2mHofXHLK_V7VTQ5v%3DPcunu_BiF%2BzFJQTFy9DSwTQ%40mail.gmail.com >>>>>>>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOzWxF5EX2mHofXHLK_V7VTQ5v%3DPcunu_BiF%2BzFJQTFy9DSwTQ%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>>>>>>>> . >>>>>>>>>>>> >>>>>>>>>>> -- >>> You received this message because you are subscribed to the Google >>> Groups "blink-dev" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to blink-dev+unsubscr...@chromium.org. >>> To view this discussion on the web visit >>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfX7OJhyuHziAv4s66%2B_wwK3GWKXB1Mb5fmcR3B4EhsqhA%40mail.gmail.com >>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfX7OJhyuHziAv4s66%2B_wwK3GWKXB1Mb5fmcR3B4EhsqhA%40mail.gmail.com?utm_medium=email&utm_source=footer> >>> . >>> >> -- > You received this message because you are subscribed to the Google Groups > "blink-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to blink-dev+unsubscr...@chromium.org. > To view this discussion on the web visit > https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOzWxF75vpsv5nQmA_Tj29nOg1h4ZPmtKb9Fw_aba7Gpfo5hLw%40mail.gmail.com > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOzWxF75vpsv5nQmA_Tj29nOg1h4ZPmtKb9Fw_aba7Gpfo5hLw%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > -- You received this message because you are subscribed to the Google Groups "blink-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscr...@chromium.org. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOMQ%2Bw9-G_G4mXA9%2BhagWoSf1OkGz%2BjsmoS7hpeL8f1_1p0Jfw%40mail.gmail.com.
