Re: [webkit-dev] Implementing OffscreenCanvas

2019-10-17 Thread Chris Lord



On 2019-10-17 16:23, Chris Lord wrote:
> So I was just looking at adding a setting, but looks like it's already
> behind a runtime-setting that's controlled by the experimental-features
> flag, so I guess we're ok as it is?

After discussing with Zan, we decided the best thing to do was to split
this setting (which currently controls both ImageBitmap and
OffscreenCanvas) and add a build option for the OffscreenCanvas part -
this way we can retain the current behaviour that affects ImageBitmap
and still allow disabling OffscreenCanvas by default and in the build.
Sequence-wise, this will go in before any OffscreenCanvas-specific
changes go in (i.e. before bug 182686).

--Chris
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Implementing OffscreenCanvas

2019-10-17 Thread Chris Lord


On 2019-10-11 12:13, Ryosuke Niwa wrote:
> On Fri, Oct 11, 2019 at 2:09 AM Chris Lord  wrote:
> 
>> On 2019-10-10 23:15, Ryosuke Niwa wrote:
>>> On Thu, Oct 10, 2019 at 2:07 PM Myles C. Maxfield
>>>  wrote:
>>>
> On Oct 10, 2019, at 12:57 PM, Ryosuke Niwa 
> wrote:
>
> Hi Chris,
>
> I'm excited that you're working on OffscreenCanvas because I
>> think
> it would be a valuable feature

 Me too!!!

> , and I'm confident we can come up with a strategy to limit its
> privacy & security risk as we see fit.
>
> However, many of your patches seem to ignore the fact most of
> WebCore objects aren't thread safe. For example, CSS parser and
> the entire CSS object model aren't designed to used in non-main
> thread. Regardless of how ready Linux ports might be, we can't
> land or enable this feature in WebKit until all thread safety
> issues are sorted out.
>
> Unfortunately, I can't make a time commitment to review & find
> every thread safety issue in your patches. Please work with
> relevant experts and go over your code changes.

 I’d be happy to work with you on this.

> For example, it's never safe to an object that's RefCounted in
> multiple threads because RefCounted isn't thread safe. One would
> have to use ThreadSafeRefCounted. It's never safe to use
> AtomString from one another in another because AtomString has a
> pool of strings per thread. For that matter, it's never safe to
> use a single String object from two or more threads because
>> String
> itself is RefCounted and isn't thread safe. It's not not okay to
> do readonly access to basic container types like Vector,
>> HashMap,
> etc... because they don't guarantee atomic update of internal
>> data
> structures and doing so can result in UAF.
>>
>> To the best of my knowledge, I've taken this into account (it's hard
>> to
>> see this without applying the whole patch series, as later patches
>> assume the changes made with respect to thread-safety in the earlier
>> patches). There are of course bound to be things I've missed - and
>> I'm
>> also open to taking a different strategy on certain things too, I'm
>> fairly new to the WebKit codebase (well, I'm assuming having worked
>> on
>> it around 10 years ago doesn't apply too much anymore :)) and I
>> imagine
>> I've taken some naive approaches in places that someone more
>> experienced
>> would be able to correct me on.
> 
> I think you want Antti's input most here since many of the classes
> you're touching in CSS is maintained by Antti these days.
> 
> The thing that worries me most about this feature is the thread
> safety. Historically, we had many thread safety issues regarding
> Timer, RefCounted objects, WeakPtr, etc... Chris (Dumez) and I have
> been addressing some of those issues more systematically (e.g.
> https://trac.webkit.org/r241499 & https://trac.webkit.org/r248113)
> because we seem to keep making same mistakes across the codebase. The
> key here is to really make which objects are used concurrently or in
> non-main threads obvious to whomever reading the code in the future.
> 
> I can think of a few ways to do that. For one, we should be adding
> lots of thread safety assertions. Assertions are better than comments
> because they're obvious to readers and they warn us whenever they get
> out of date or we make mistakes. In fact, I discourage adding comments
> about thread safety; if you find yourself doing that, then it's time
> to refactor code such that the code is obviously multi-threaded or
> concurrent by the virtue of the way things are structured or make it
> not matter because the code is naturally thread safe. Just as an
> example, one of your patches made CSSValuePool's member functions
> thread safe but it left the rest of code still access
> CSSValuePool::singleton() object. This is problematic because
> singleton objects are usually shared across threads. A better approach
> is to have each caller explicitly get a specific instance of
> CSSValuePool from some other object; e.g. CSSParser's member variable.
> This will make it so that the code that needs to run concurrently
> would not rely on singleton objects, and whatever new code which gets
> introduced to CSSValuePool or CSSParser would naturally be thread safe
> so long as the author follows the same convention. Finally, we can add
> an assertion in CSSValuePool::singleton() to make sure it's only
> called in the thread. This way, anyone who makes a mistake of calling
> this singleton function in CSSParser, or elsewhere which can be used
> by the offscreen canvas code in worker would hit this assertion.

Just an update on this, CSS parsing largely happens via static
functions, so there's no one place to add a CSSValuePool member variable
- however, I did add it where applicable and added alternative functions
to all of the CSS parsing static functions that end up in us

Re: [webkit-dev] Implementing OffscreenCanvas

2019-10-11 Thread Carlos Alberto Lopez Perez
On 11/10/2019 11:09, Chris Lord wrote:
> This sounds good to me, I'll file a bug and write a patch for this. I
> assume there are ways of enabling features when tests are run? While a
> user (or a developer) using WebKit wouldn't want this feature to be
> enabled, I think it should be enabled for (some) test runs as soon as
> possible to give us an indication of any issues that exist at the
> moment.

This can be achieved with a run-time flag that is off by default but enabled 
for the layout tests.
See TestController::resetPreferencesToConsistentValues() in 
Tools/WebKitTestRunner/TestController.cpp



signature.asc
Description: OpenPGP digital signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Implementing OffscreenCanvas

2019-10-11 Thread Ryosuke Niwa
On Fri, Oct 11, 2019 at 2:09 AM Chris Lord  wrote:

>
> On 2019-10-10 23:15, Ryosuke Niwa wrote:
> > On Thu, Oct 10, 2019 at 2:07 PM Myles C. Maxfield
> >  wrote:
> >
> >>> On Oct 10, 2019, at 12:57 PM, Ryosuke Niwa 
> >>> wrote:
> >>>
> >>> Hi Chris,
> >>>
> >>> I'm excited that you're working on OffscreenCanvas because I think
> >>> it would be a valuable feature
> >>
> >> Me too!!!
> >>
> >>> , and I'm confident we can come up with a strategy to limit its
> >>> privacy & security risk as we see fit.
> >>>
> >>> However, many of your patches seem to ignore the fact most of
> >>> WebCore objects aren't thread safe. For example, CSS parser and
> >>> the entire CSS object model aren't designed to used in non-main
> >>> thread. Regardless of how ready Linux ports might be, we can't
> >>> land or enable this feature in WebKit until all thread safety
> >>> issues are sorted out.
> >>>
> >>> Unfortunately, I can't make a time commitment to review & find
> >>> every thread safety issue in your patches. Please work with
> >>> relevant experts and go over your code changes.
> >>
> >> I’d be happy to work with you on this.
> >>
> >>> For example, it's never safe to an object that's RefCounted in
> >>> multiple threads because RefCounted isn't thread safe. One would
> >>> have to use ThreadSafeRefCounted. It's never safe to use
> >>> AtomString from one another in another because AtomString has a
> >>> pool of strings per thread. For that matter, it's never safe to
> >>> use a single String object from two or more threads because String
> >>> itself is RefCounted and isn't thread safe. It's not not okay to
> >>> do readonly access to basic container types like Vector, HashMap,
> >>> etc... because they don't guarantee atomic update of internal data
> >>> structures and doing so can result in UAF.
>
> To the best of my knowledge, I've taken this into account (it's hard to
> see this without applying the whole patch series, as later patches
> assume the changes made with respect to thread-safety in the earlier
> patches). There are of course bound to be things I've missed - and I'm
> also open to taking a different strategy on certain things too, I'm
> fairly new to the WebKit codebase (well, I'm assuming having worked on
> it around 10 years ago doesn't apply too much anymore :)) and I imagine
> I've taken some naive approaches in places that someone more experienced
> would be able to correct me on.


I think you want Antti's input most here since many of the classes you're
touching in CSS is maintained by Antti these days.

The thing that worries me most about this feature is the thread
safety. Historically, we had many thread safety issues regarding Timer,
RefCounted objects, WeakPtr, etc... Chris (Dumez) and I have been
addressing some of those issues more systematically (e.g.
https://trac.webkit.org/r241499 & https://trac.webkit.org/r248113) because
we seem to keep making same mistakes across the codebase. The key here is
to really make which objects are used concurrently or in non-main threads
obvious to whomever reading the code in the future.

I can think of a few ways to do that. For one, we should be adding lots of
thread safety assertions. Assertions are better than comments because
they're obvious to readers and they warn us whenever they get out of date
or we make mistakes. In fact, I discourage adding comments about thread
safety; if you find yourself doing that, then it's time to refactor code
such that the code is obviously multi-threaded or concurrent by the virtue
of the way things are structured or make it not matter because the code is
naturally thread safe. Just as an example, one of your patches made
CSSValuePool's member functions thread safe but it left the rest of code
still access CSSValuePool::singleton() object. This is problematic because
singleton objects are usually shared across threads. A better approach is
to have each caller explicitly get a specific instance of CSSValuePool from
some other object; e.g. CSSParser's member variable. This will make it so
that the code that needs to run concurrently would not rely on singleton
objects, and whatever new code which gets introduced to CSSValuePool or
CSSParser would naturally be thread safe so long as the author follows the
same convention. Finally, we can add an assertion
in CSSValuePool::singleton() to make sure it's only called in the thread.
This way, anyone who makes a mistake of calling this singleton function in
CSSParser, or elsewhere which can be used by the offscreen canvas code in
worker would hit this assertion.

>>> I think the hardest part of this project is validating that
> >>> enabling this feature wouldn't introduce dozens of new thread
> >>> safety issues and thereby security vulnerabilities.
> >>
> >> Sounds like this this is a good candidate for a feature flag.
> >
> > Yeah, in fact, this should really have a compile time flag in addition
> > to runtime flag, and it should be default off until we've done enough
> > fu

Re: [webkit-dev] Implementing OffscreenCanvas

2019-10-11 Thread Chris Lord


On 2019-10-10 23:15, Ryosuke Niwa wrote:
> On Thu, Oct 10, 2019 at 2:07 PM Myles C. Maxfield
>  wrote:
> 
>>> On Oct 10, 2019, at 12:57 PM, Ryosuke Niwa 
>>> wrote:
>>>
>>> Hi Chris,
>>>
>>> I'm excited that you're working on OffscreenCanvas because I think
>>> it would be a valuable feature
>>
>> Me too!!!
>>
>>> , and I'm confident we can come up with a strategy to limit its
>>> privacy & security risk as we see fit.
>>>
>>> However, many of your patches seem to ignore the fact most of
>>> WebCore objects aren't thread safe. For example, CSS parser and
>>> the entire CSS object model aren't designed to used in non-main
>>> thread. Regardless of how ready Linux ports might be, we can't
>>> land or enable this feature in WebKit until all thread safety
>>> issues are sorted out.
>>>
>>> Unfortunately, I can't make a time commitment to review & find
>>> every thread safety issue in your patches. Please work with
>>> relevant experts and go over your code changes.
>>
>> I’d be happy to work with you on this.
>>
>>> For example, it's never safe to an object that's RefCounted in
>>> multiple threads because RefCounted isn't thread safe. One would
>>> have to use ThreadSafeRefCounted. It's never safe to use
>>> AtomString from one another in another because AtomString has a
>>> pool of strings per thread. For that matter, it's never safe to
>>> use a single String object from two or more threads because String
>>> itself is RefCounted and isn't thread safe. It's not not okay to
>>> do readonly access to basic container types like Vector, HashMap,
>>> etc... because they don't guarantee atomic update of internal data
>>> structures and doing so can result in UAF.

To the best of my knowledge, I've taken this into account (it's hard to
see this without applying the whole patch series, as later patches
assume the changes made with respect to thread-safety in the earlier
patches). There are of course bound to be things I've missed - and I'm
also open to taking a different strategy on certain things too, I'm
fairly new to the WebKit codebase (well, I'm assuming having worked on
it around 10 years ago doesn't apply too much anymore :)) and I imagine
I've taken some naive approaches in places that someone more experienced
would be able to correct me on.

>>> I think the hardest part of this project is validating that
>>> enabling this feature wouldn't introduce dozens of new thread
>>> safety issues and thereby security vulnerabilities.
>>
>> Sounds like this this is a good candidate for a feature flag.
> 
> Yeah, in fact, this should really have a compile time flag in addition
> to runtime flag, and it should be default off until we've done enough
> fuzzing. The thread unsafe code can be turned into an attack gadget if
> it exists at all in production binary.

This sounds good to me, I'll file a bug and write a patch for this. I
assume there are ways of enabling features when tests are run? While a
user (or a developer) using WebKit wouldn't want this feature to be
enabled, I think it should be enabled for (some) test runs as soon as
possible to give us an indication of any issues that exist at the
moment.


>> On Thu, Oct 10, 2019 at 4:23 AM Chris Lord  wrote:
>>
>> I've spent the last month or so 'finishing' the implementation of
>> OffscreenCanvas[1], based on Žan Doberšek's work from a year
>> ago[2].
>> OffscreenCanvas is an API for being able to use canvas drawing
>> without a
>> visible canvas, and from within Workers. It's supported by Blink and
>> has
>> partial support in Gecko.
>>
>> It's at the point now where I'd consider it a finished draft - it is
>> almost fully implemented and passes the majority of relevant tests
>> in a
>> debug build without crashing, but has some areas that need
>> completion on
>> other platforms (async drawing on non-Linux) and some missing parts
>> (Web
>> Inspector, ImageBitmapRenderingContext). It almost certainly needs
>> reworking in places.
>>
>> My work is on GitHub[3] - I'd like to solicit reviews and comment.
>> Some
>> of the bugs hanging off [2] have patches that need review and I
>> think
>> are near ready to being landable as the foundation of this work. It
>> is
>> broadly split up like so:
>>
>> - Refactor to move functionality from HTMLCanvasElement to
>> CanvasBase
>> - Refactor to not unnecessarily require HTMLCanvasElement in places
>> - Implement OffscreenCanvas functionality
>> - Make font loading/styling usable from a Worker and without a
>> Document
>> - Implement AnimationFrameProvider on DedicatedWorkerGlobalScope
>> - Implement asynchronous drawing updates on placeholder canvases
>>
>> I expect the font-related stuff to be the most contentious, and my
>> AnimationFrameProvider implementation may be too trivial (but might
>> be
>> ok for a first go?)
>>
>> All feedback appreciated. Best regards,
>>
>> Chris
>>
>> [1]
>>
> https://html.spec.whatwg.org/multipage/canvas.html#the-offscreencanvas-interface
>> [2] https://bugs.webkit.org/show_bug.cgi?id=1837

Re: [webkit-dev] Implementing OffscreenCanvas

2019-10-10 Thread Ryosuke Niwa
On Thu, Oct 10, 2019 at 2:07 PM Myles C. Maxfield 
wrote:

>
>
> On Oct 10, 2019, at 12:57 PM, Ryosuke Niwa  wrote:
>
> Hi Chris,
>
> I'm excited that you're working on OffscreenCanvas because I think it
> would be a valuable feature
>
>
> Me too!!!
>
> , and I'm confident we can come up with a strategy to limit its privacy &
> security risk as we see fit.
>
> However, many of your patches seem to ignore the fact most of WebCore
> objects aren't thread safe. For example, CSS parser and the entire CSS
> object model aren't designed to used in non-main thread. Regardless of how
> ready Linux ports might be, we can't land or enable this feature in WebKit
> until all thread safety issues are sorted out.
>
> Unfortunately, I can't make a time commitment to review & find every
> thread safety issue in your patches. Please work with relevant experts and
> go over your code changes.
>
>
> I’d be happy to work with you on this.
>
>
> For example, it's never safe to an object that's RefCounted in multiple
> threads because RefCounted isn't thread safe. One would have to use
> ThreadSafeRefCounted. It's never safe to use AtomString from one another in
> another because AtomString has a pool of strings per thread. For that
> matter, it's never safe to use a single String object from two or more
> threads because String itself is RefCounted and isn't thread safe. It's not
> not okay to do readonly access to basic container types like Vector,
> HashMap, etc... because they don't guarantee atomic update of internal data
> structures and doing so can result in UAF.
>
> I think the hardest part of this project is validating that enabling this
> feature wouldn't introduce dozens of new thread safety issues and thereby
> security vulnerabilities.
>
>
> Sounds like this this is a good candidate for a feature flag.
>

Yeah, in fact, this should really have a compile time flag in addition to
runtime flag, and it should be default off until we've done enough fuzzing.
The thread unsafe code can be turned into an attack gadget if it exists at
all in production binary.

- R. Niwa

On Thu, Oct 10, 2019 at 4:23 AM Chris Lord  wrote:
>
>>
>> I've spent the last month or so 'finishing' the implementation of
>> OffscreenCanvas[1], based on Žan Doberšek's work from a year ago[2].
>> OffscreenCanvas is an API for being able to use canvas drawing without a
>> visible canvas, and from within Workers. It's supported by Blink and has
>> partial support in Gecko.
>>
>> It's at the point now where I'd consider it a finished draft - it is
>> almost fully implemented and passes the majority of relevant tests in a
>> debug build without crashing, but has some areas that need completion on
>> other platforms (async drawing on non-Linux) and some missing parts (Web
>> Inspector, ImageBitmapRenderingContext). It almost certainly needs
>> reworking in places.
>>
>> My work is on GitHub[3] - I'd like to solicit reviews and comment. Some
>> of the bugs hanging off [2] have patches that need review and I think
>> are near ready to being landable as the foundation of this work. It is
>> broadly split up like so:
>>
>> - Refactor to move functionality from HTMLCanvasElement to CanvasBase
>> - Refactor to not unnecessarily require HTMLCanvasElement in places
>> - Implement OffscreenCanvas functionality
>> - Make font loading/styling usable from a Worker and without a Document
>> - Implement AnimationFrameProvider on DedicatedWorkerGlobalScope
>> - Implement asynchronous drawing updates on placeholder canvases
>>
>> I expect the font-related stuff to be the most contentious, and my
>> AnimationFrameProvider implementation may be too trivial (but might be
>> ok for a first go?)
>>
>> All feedback appreciated. Best regards,
>>
>> Chris
>>
>> [1]
>>
>> https://html.spec.whatwg.org/multipage/canvas.html#the-offscreencanvas-interface
>> [2] https://bugs.webkit.org/show_bug.cgi?id=183720
>> [3] https://github.com/Cwiiis/webkit/tree/offscreen-canvas
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Implementing OffscreenCanvas

2019-10-10 Thread Myles C. Maxfield


> On Oct 10, 2019, at 12:57 PM, Ryosuke Niwa  wrote:
> 
> Hi Chris,
> 
> I'm excited that you're working on OffscreenCanvas because I think it would 
> be a valuable feature

Me too!!!

> , and I'm confident we can come up with a strategy to limit its privacy & 
> security risk as we see fit.
> 
> However, many of your patches seem to ignore the fact most of WebCore objects 
> aren't thread safe. For example, CSS parser and the entire CSS object model 
> aren't designed to used in non-main thread. Regardless of how ready Linux 
> ports might be, we can't land or enable this feature in WebKit until all 
> thread safety issues are sorted out.
> 
> Unfortunately, I can't make a time commitment to review & find every thread 
> safety issue in your patches. Please work with relevant experts and go over 
> your code changes.

I’d be happy to work with you on this.

> 
> For example, it's never safe to an object that's RefCounted in multiple 
> threads because RefCounted isn't thread safe. One would have to use 
> ThreadSafeRefCounted. It's never safe to use AtomString from one another in 
> another because AtomString has a pool of strings per thread. For that matter, 
> it's never safe to use a single String object from two or more threads 
> because String itself is RefCounted and isn't thread safe. It's not not okay 
> to do readonly access to basic container types like Vector, HashMap, etc... 
> because they don't guarantee atomic update of internal data structures and 
> doing so can result in UAF.
> 
> I think the hardest part of this project is validating that enabling this 
> feature wouldn't introduce dozens of new thread safety issues and thereby 
> security vulnerabilities.

Sounds like this this is a good candidate for a feature flag.

> 
> - R. Niwa
> 
> On Thu, Oct 10, 2019 at 4:23 AM Chris Lord  > wrote:
> 
> I've spent the last month or so 'finishing' the implementation of
> OffscreenCanvas[1], based on Žan Doberšek's work from a year ago[2].
> OffscreenCanvas is an API for being able to use canvas drawing without a
> visible canvas, and from within Workers. It's supported by Blink and has
> partial support in Gecko.
> 
> It's at the point now where I'd consider it a finished draft - it is
> almost fully implemented and passes the majority of relevant tests in a
> debug build without crashing, but has some areas that need completion on
> other platforms (async drawing on non-Linux) and some missing parts (Web
> Inspector, ImageBitmapRenderingContext). It almost certainly needs
> reworking in places.
> 
> My work is on GitHub[3] - I'd like to solicit reviews and comment. Some
> of the bugs hanging off [2] have patches that need review and I think
> are near ready to being landable as the foundation of this work. It is
> broadly split up like so:
> 
> - Refactor to move functionality from HTMLCanvasElement to CanvasBase
> - Refactor to not unnecessarily require HTMLCanvasElement in places
> - Implement OffscreenCanvas functionality
> - Make font loading/styling usable from a Worker and without a Document
> - Implement AnimationFrameProvider on DedicatedWorkerGlobalScope
> - Implement asynchronous drawing updates on placeholder canvases
> 
> I expect the font-related stuff to be the most contentious, and my
> AnimationFrameProvider implementation may be too trivial (but might be
> ok for a first go?)
> 
> All feedback appreciated. Best regards,
> 
> Chris
> 
> [1]
> https://html.spec.whatwg.org/multipage/canvas.html#the-offscreencanvas-interface
>  
> 
> [2] https://bugs.webkit.org/show_bug.cgi?id=183720 
> 
> [3] https://github.com/Cwiiis/webkit/tree/offscreen-canvas 
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org 
> https://lists.webkit.org/mailman/listinfo/webkit-dev 
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org 
> https://lists.webkit.org/mailman/listinfo/webkit-dev 
> 
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Implementing OffscreenCanvas

2019-10-10 Thread Ryosuke Niwa
Hi Chris,

I'm excited that you're working on OffscreenCanvas because I think it would
be a valuable feature, and I'm confident we can come up with a strategy to
limit its privacy & security risk as we see fit.

However, many of your patches seem to ignore the fact most of WebCore
objects aren't thread safe. For example, CSS parser and the entire CSS
object model aren't designed to used in non-main thread. Regardless of how
ready Linux ports might be, we can't land or enable this feature in WebKit
until all thread safety issues are sorted out.

Unfortunately, I can't make a time commitment to review & find every thread
safety issue in your patches. Please work with relevant experts and go over
your code changes.

For example, it's never safe to an object that's RefCounted in multiple
threads because RefCounted isn't thread safe. One would have to use
ThreadSafeRefCounted. It's never safe to use AtomString from one another in
another because AtomString has a pool of strings per thread. For that
matter, it's never safe to use a single String object from two or more
threads because String itself is RefCounted and isn't thread safe. It's not
not okay to do readonly access to basic container types like Vector,
HashMap, etc... because they don't guarantee atomic update of internal data
structures and doing so can result in UAF.

I think the hardest part of this project is validating that enabling this
feature wouldn't introduce dozens of new thread safety issues and thereby
security vulnerabilities.

- R. Niwa

On Thu, Oct 10, 2019 at 4:23 AM Chris Lord  wrote:

>
> I've spent the last month or so 'finishing' the implementation of
> OffscreenCanvas[1], based on Žan Doberšek's work from a year ago[2].
> OffscreenCanvas is an API for being able to use canvas drawing without a
> visible canvas, and from within Workers. It's supported by Blink and has
> partial support in Gecko.
>
> It's at the point now where I'd consider it a finished draft - it is
> almost fully implemented and passes the majority of relevant tests in a
> debug build without crashing, but has some areas that need completion on
> other platforms (async drawing on non-Linux) and some missing parts (Web
> Inspector, ImageBitmapRenderingContext). It almost certainly needs
> reworking in places.
>
> My work is on GitHub[3] - I'd like to solicit reviews and comment. Some
> of the bugs hanging off [2] have patches that need review and I think
> are near ready to being landable as the foundation of this work. It is
> broadly split up like so:
>
> - Refactor to move functionality from HTMLCanvasElement to CanvasBase
> - Refactor to not unnecessarily require HTMLCanvasElement in places
> - Implement OffscreenCanvas functionality
> - Make font loading/styling usable from a Worker and without a Document
> - Implement AnimationFrameProvider on DedicatedWorkerGlobalScope
> - Implement asynchronous drawing updates on placeholder canvases
>
> I expect the font-related stuff to be the most contentious, and my
> AnimationFrameProvider implementation may be too trivial (but might be
> ok for a first go?)
>
> All feedback appreciated. Best regards,
>
> Chris
>
> [1]
>
> https://html.spec.whatwg.org/multipage/canvas.html#the-offscreencanvas-interface
> [2] https://bugs.webkit.org/show_bug.cgi?id=183720
> [3] https://github.com/Cwiiis/webkit/tree/offscreen-canvas
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Implementing OffscreenCanvas

2019-10-10 Thread Maciej Stachowiak


> On Oct 10, 2019, at 10:18 AM, John Wilander  wrote:
> 
>> On Oct 10, 2019, at 9:42 AM, Maciej Stachowiak  wrote:
>> 
>> For clarity, it’s already possible to render to a regular canvas offscreen. 
>> The  can be hidden using any of the techniques that can make any 
>> other canvas invisible. Name notwithstanding, OffscreenCanvas is mainly 
>> about being able to render from a Worker, not about enabling rendering 
>> offscreen.
>> 
>> Thus, I would not expect it to make it easier to invisibly fingerprint using 
>> canvas.
> 
> My thinking here is that viable mitigations for device fingerprinting could 
> be requiring a Canvas to be visible on screen or even for it to get user 
> interaction before it’ll get access to hardware acceleration. Such 
> restrictions would make it harder for tracking scripts to get away with GPU 
> fingerprinting that the site owner never wanted or accepted. If we allow 
> offscreen Canvas in workers, site owners will stay in the dark and we can’t 
> tie user interaction to it.

That is neat thinking. If a mitigation like this was effective, and we did it 
across the board, that would mean OffscreenCanvas is basically always disabled.

But I’m not sure it’s viable. A few concerns:
- It’s notoriously hard to tell if an element is really visible onscreen.
- Canvas backing store size is independent of its layout size, so even if we 
could solve the visibility problem, a rather small canvas could still get all 
the fingerprinting bits. (And the minimum size can’t be too low.)
- Rendering offscreen for multiple buffering or to to implement sprites is a 
legit use case and necessary for games.
- Rendering a canvas that’s currently scrolled below the fold is valuable. 
Otherwise you get a scrolling glitch later, or show the user blank content.
- Even software rendering is fingerprintable.
- Drawing itself isn’t even the fingerprinting step, it’s the readback.

I think canvas fingerprinting mitigations will have to be more along the lines 
of identifying the true source of the script attempting to perform readback and 
returning fake values, injecting noise (either into drawing or into readback), 
or things along these lines. It might also be possible to specifically block 
readback from invisible/offscreen canvases (as long as copying directly to 
another canvas is still allowed).

Thus, on reflection, I still do not think OffscreenCanvas hurts our ability to 
do fingerprinting mitigations.

But it is very good to consider tracking and fingerprinting risk for every 
feature, so I’m glad you brought this up!

Regards,
Maciej


> 
>   Regards, John
> 
>>> On Oct 10, 2019, at 9:32 AM, Chris Lord  wrote:
>>> 
>>> Hi John,
>>> 
>>> I don't know what the current state is of counter-measures for such an
>>> attack, but I don't immediately imagine OffscreenCanvas would make them
>>> more effective. The patch series doesn't add any new rendering paths, so
>>> whatever was possible before will likely still be possible and whatever
>>> wasn't will hopefully still not be possible. That said, I'll look into
>>> this and discuss it with some people that will know better than me and
>>> try to get a better picture.
>>> 
>>> Thanks,
>>> 
>>> Chris
>>> 
>>> On 2019-10-10 17:32, John Wilander wrote:
 Hi Chris!
 
 Canvas is a very popular GPU fingerprinting vector and allowing it
 offscreen sounds like a more convenient way to perform such an attack
 on user privacy. Do you know if Blink or Gecko have elaborated on
 this? What is your assessment?
 
 Given the cross-engine effort to fight device fingerprinting and
 WebKit and Gecko’s recently published tracking prevention policies, we
 should do a threat analysis of this feature.
 
 Regards, John
 
> On Oct 10, 2019, at 4:24 AM, Chris Lord  wrote:
> 
> Hi all,
> 
> I've spent the last month or so 'finishing' the implementation of
> OffscreenCanvas[1], based on Žan Doberšek's work from a year ago[2].
> OffscreenCanvas is an API for being able to use canvas drawing without a
> visible canvas, and from within Workers. It's supported by Blink and has
> partial support in Gecko.
> 
> It's at the point now where I'd consider it a finished draft - it is
> almost fully implemented and passes the majority of relevant tests in a
> debug build without crashing, but has some areas that need completion on
> other platforms (async drawing on non-Linux) and some missing parts (Web
> Inspector, ImageBitmapRenderingContext). It almost certainly needs
> reworking in places.
> 
> My work is on GitHub[3] - I'd like to solicit reviews and comment. Some
> of the bugs hanging off [2] have patches that need review and I think
> are near ready to being landable as the foundation of this work. It is
> broadly split up like so:
> 
> - Refactor to move functionality from HTMLCanvasElement to CanvasBase
> - Refactor to not unnecessaril

Re: [webkit-dev] Implementing OffscreenCanvas

2019-10-10 Thread John Wilander
> On Oct 10, 2019, at 9:42 AM, Maciej Stachowiak  wrote:
> 
> For clarity, it’s already possible to render to a regular canvas offscreen. 
> The  can be hidden using any of the techniques that can make any 
> other canvas invisible. Name notwithstanding, OffscreenCanvas is mainly about 
> being able to render from a Worker, not about enabling rendering offscreen.
> 
> Thus, I would not expect it to make it easier to invisibly fingerprint using 
> canvas.

My thinking here is that viable mitigations for device fingerprinting could be 
requiring a Canvas to be visible on screen or even for it to get user 
interaction before it’ll get access to hardware acceleration. Such restrictions 
would make it harder for tracking scripts to get away with GPU fingerprinting 
that the site owner never wanted or accepted. If we allow offscreen Canvas in 
workers, site owners will stay in the dark and we can’t tie user interaction to 
it.

   Regards, John

>> On Oct 10, 2019, at 9:32 AM, Chris Lord  wrote:
>> 
>> Hi John,
>> 
>> I don't know what the current state is of counter-measures for such an
>> attack, but I don't immediately imagine OffscreenCanvas would make them
>> more effective. The patch series doesn't add any new rendering paths, so
>> whatever was possible before will likely still be possible and whatever
>> wasn't will hopefully still not be possible. That said, I'll look into
>> this and discuss it with some people that will know better than me and
>> try to get a better picture.
>> 
>> Thanks,
>> 
>> Chris
>> 
>> On 2019-10-10 17:32, John Wilander wrote:
>>> Hi Chris!
>>> 
>>> Canvas is a very popular GPU fingerprinting vector and allowing it
>>> offscreen sounds like a more convenient way to perform such an attack
>>> on user privacy. Do you know if Blink or Gecko have elaborated on
>>> this? What is your assessment?
>>> 
>>> Given the cross-engine effort to fight device fingerprinting and
>>> WebKit and Gecko’s recently published tracking prevention policies, we
>>> should do a threat analysis of this feature.
>>> 
>>>  Regards, John
>>> 
 On Oct 10, 2019, at 4:24 AM, Chris Lord  wrote:
 
 Hi all,
 
 I've spent the last month or so 'finishing' the implementation of
 OffscreenCanvas[1], based on Žan Doberšek's work from a year ago[2].
 OffscreenCanvas is an API for being able to use canvas drawing without a
 visible canvas, and from within Workers. It's supported by Blink and has
 partial support in Gecko.
 
 It's at the point now where I'd consider it a finished draft - it is
 almost fully implemented and passes the majority of relevant tests in a
 debug build without crashing, but has some areas that need completion on
 other platforms (async drawing on non-Linux) and some missing parts (Web
 Inspector, ImageBitmapRenderingContext). It almost certainly needs
 reworking in places.
 
 My work is on GitHub[3] - I'd like to solicit reviews and comment. Some
 of the bugs hanging off [2] have patches that need review and I think
 are near ready to being landable as the foundation of this work. It is
 broadly split up like so:
 
 - Refactor to move functionality from HTMLCanvasElement to CanvasBase
 - Refactor to not unnecessarily require HTMLCanvasElement in places
 - Implement OffscreenCanvas functionality
 - Make font loading/styling usable from a Worker and without a Document
 - Implement AnimationFrameProvider on DedicatedWorkerGlobalScope
 - Implement asynchronous drawing updates on placeholder canvases
 
 I expect the font-related stuff to be the most contentious, and my
 AnimationFrameProvider implementation may be too trivial (but might be
 ok for a first go?)
 
 All feedback appreciated. Best regards,
 
 Chris
 
 [1]
 https://html.spec.whatwg.org/multipage/canvas.html#the-offscreencanvas-interface
 [2] https://bugs.webkit.org/show_bug.cgi?id=183720
 [3] https://github.com/Cwiiis/webkit/tree/offscreen-canvas
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 https://lists.webkit.org/mailman/listinfo/webkit-dev
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Implementing OffscreenCanvas

2019-10-10 Thread Maciej Stachowiak

For clarity, it’s already possible to render to a regular canvas offscreen. The 
 can be hidden using any of the techniques that can make any other 
canvas invisible. Name notwithstanding, OffscreenCanvas is mainly about being 
able to render from a Worker, not about enabling rendering offscreen.

Thus, I would not expect it to make it easier to invisibly fingerprint using 
canvas.

> On Oct 10, 2019, at 9:32 AM, Chris Lord  wrote:
> 
> Hi John,
> 
> I don't know what the current state is of counter-measures for such an
> attack, but I don't immediately imagine OffscreenCanvas would make them
> more effective. The patch series doesn't add any new rendering paths, so
> whatever was possible before will likely still be possible and whatever
> wasn't will hopefully still not be possible. That said, I'll look into
> this and discuss it with some people that will know better than me and
> try to get a better picture.
> 
> Thanks,
> 
> Chris
> 
> On 2019-10-10 17:32, John Wilander wrote:
>> Hi Chris!
>> 
>> Canvas is a very popular GPU fingerprinting vector and allowing it
>> offscreen sounds like a more convenient way to perform such an attack
>> on user privacy. Do you know if Blink or Gecko have elaborated on
>> this? What is your assessment?
>> 
>> Given the cross-engine effort to fight device fingerprinting and
>> WebKit and Gecko’s recently published tracking prevention policies, we
>> should do a threat analysis of this feature.
>> 
>>   Regards, John
>> 
>>> On Oct 10, 2019, at 4:24 AM, Chris Lord  wrote:
>>> 
>>> Hi all,
>>> 
>>> I've spent the last month or so 'finishing' the implementation of
>>> OffscreenCanvas[1], based on Žan Doberšek's work from a year ago[2].
>>> OffscreenCanvas is an API for being able to use canvas drawing without a
>>> visible canvas, and from within Workers. It's supported by Blink and has
>>> partial support in Gecko.
>>> 
>>> It's at the point now where I'd consider it a finished draft - it is
>>> almost fully implemented and passes the majority of relevant tests in a
>>> debug build without crashing, but has some areas that need completion on
>>> other platforms (async drawing on non-Linux) and some missing parts (Web
>>> Inspector, ImageBitmapRenderingContext). It almost certainly needs
>>> reworking in places.
>>> 
>>> My work is on GitHub[3] - I'd like to solicit reviews and comment. Some
>>> of the bugs hanging off [2] have patches that need review and I think
>>> are near ready to being landable as the foundation of this work. It is
>>> broadly split up like so:
>>> 
>>> - Refactor to move functionality from HTMLCanvasElement to CanvasBase
>>> - Refactor to not unnecessarily require HTMLCanvasElement in places
>>> - Implement OffscreenCanvas functionality
>>> - Make font loading/styling usable from a Worker and without a Document
>>> - Implement AnimationFrameProvider on DedicatedWorkerGlobalScope
>>> - Implement asynchronous drawing updates on placeholder canvases
>>> 
>>> I expect the font-related stuff to be the most contentious, and my
>>> AnimationFrameProvider implementation may be too trivial (but might be
>>> ok for a first go?)
>>> 
>>> All feedback appreciated. Best regards,
>>> 
>>> Chris
>>> 
>>> [1]
>>> https://html.spec.whatwg.org/multipage/canvas.html#the-offscreencanvas-interface
>>> [2] https://bugs.webkit.org/show_bug.cgi?id=183720
>>> [3] https://github.com/Cwiiis/webkit/tree/offscreen-canvas
>>> ___
>>> webkit-dev mailing list
>>> webkit-dev@lists.webkit.org
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Implementing OffscreenCanvas

2019-10-10 Thread Chris Lord
Hi John,

I don't know what the current state is of counter-measures for such an
attack, but I don't immediately imagine OffscreenCanvas would make them
more effective. The patch series doesn't add any new rendering paths, so
whatever was possible before will likely still be possible and whatever
wasn't will hopefully still not be possible. That said, I'll look into
this and discuss it with some people that will know better than me and
try to get a better picture.

Thanks,

Chris

On 2019-10-10 17:32, John Wilander wrote:
> Hi Chris!
> 
> Canvas is a very popular GPU fingerprinting vector and allowing it
> offscreen sounds like a more convenient way to perform such an attack
> on user privacy. Do you know if Blink or Gecko have elaborated on
> this? What is your assessment?
> 
> Given the cross-engine effort to fight device fingerprinting and
> WebKit and Gecko’s recently published tracking prevention policies, we
> should do a threat analysis of this feature.
> 
>Regards, John
> 
>> On Oct 10, 2019, at 4:24 AM, Chris Lord  wrote:
>>
>> Hi all,
>>
>> I've spent the last month or so 'finishing' the implementation of
>> OffscreenCanvas[1], based on Žan Doberšek's work from a year ago[2].
>> OffscreenCanvas is an API for being able to use canvas drawing without a
>> visible canvas, and from within Workers. It's supported by Blink and has
>> partial support in Gecko.
>>
>> It's at the point now where I'd consider it a finished draft - it is
>> almost fully implemented and passes the majority of relevant tests in a
>> debug build without crashing, but has some areas that need completion on
>> other platforms (async drawing on non-Linux) and some missing parts (Web
>> Inspector, ImageBitmapRenderingContext). It almost certainly needs
>> reworking in places.
>>
>> My work is on GitHub[3] - I'd like to solicit reviews and comment. Some
>> of the bugs hanging off [2] have patches that need review and I think
>> are near ready to being landable as the foundation of this work. It is
>> broadly split up like so:
>>
>> - Refactor to move functionality from HTMLCanvasElement to CanvasBase
>> - Refactor to not unnecessarily require HTMLCanvasElement in places
>> - Implement OffscreenCanvas functionality
>> - Make font loading/styling usable from a Worker and without a Document
>> - Implement AnimationFrameProvider on DedicatedWorkerGlobalScope
>> - Implement asynchronous drawing updates on placeholder canvases
>>
>> I expect the font-related stuff to be the most contentious, and my
>> AnimationFrameProvider implementation may be too trivial (but might be
>> ok for a first go?)
>>
>> All feedback appreciated. Best regards,
>>
>> Chris
>>
>> [1]
>> https://html.spec.whatwg.org/multipage/canvas.html#the-offscreencanvas-interface
>> [2] https://bugs.webkit.org/show_bug.cgi?id=183720
>> [3] https://github.com/Cwiiis/webkit/tree/offscreen-canvas
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Implementing OffscreenCanvas

2019-10-10 Thread John Wilander
Hi Chris!

Canvas is a very popular GPU fingerprinting vector and allowing it offscreen 
sounds like a more convenient way to perform such an attack on user privacy. Do 
you know if Blink or Gecko have elaborated on this? What is your assessment?

Given the cross-engine effort to fight device fingerprinting and WebKit and 
Gecko’s recently published tracking prevention policies, we should do a threat 
analysis of this feature.

   Regards, John

> On Oct 10, 2019, at 4:24 AM, Chris Lord  wrote:
> 
> Hi all,
> 
> I've spent the last month or so 'finishing' the implementation of
> OffscreenCanvas[1], based on Žan Doberšek's work from a year ago[2].
> OffscreenCanvas is an API for being able to use canvas drawing without a
> visible canvas, and from within Workers. It's supported by Blink and has
> partial support in Gecko.
> 
> It's at the point now where I'd consider it a finished draft - it is
> almost fully implemented and passes the majority of relevant tests in a
> debug build without crashing, but has some areas that need completion on
> other platforms (async drawing on non-Linux) and some missing parts (Web
> Inspector, ImageBitmapRenderingContext). It almost certainly needs
> reworking in places.
> 
> My work is on GitHub[3] - I'd like to solicit reviews and comment. Some
> of the bugs hanging off [2] have patches that need review and I think
> are near ready to being landable as the foundation of this work. It is
> broadly split up like so:
> 
> - Refactor to move functionality from HTMLCanvasElement to CanvasBase
> - Refactor to not unnecessarily require HTMLCanvasElement in places
> - Implement OffscreenCanvas functionality
> - Make font loading/styling usable from a Worker and without a Document
> - Implement AnimationFrameProvider on DedicatedWorkerGlobalScope
> - Implement asynchronous drawing updates on placeholder canvases
> 
> I expect the font-related stuff to be the most contentious, and my
> AnimationFrameProvider implementation may be too trivial (but might be
> ok for a first go?)
> 
> All feedback appreciated. Best regards,
> 
> Chris
> 
> [1]
> https://html.spec.whatwg.org/multipage/canvas.html#the-offscreencanvas-interface
> [2] https://bugs.webkit.org/show_bug.cgi?id=183720
> [3] https://github.com/Cwiiis/webkit/tree/offscreen-canvas
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev