-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
When farbling canvas, farble across all channels #12069
Comments
cc: @pes10k |
Howdy @Thorin-Oakenpants , nice to run into you again :) If you're in anyway not 100% sure that its not a security issue, feel free to email me at [email protected] and we can discuss more there. Otherwise, feel free to add your findings / concerns here and we can sort things out here. Thanks! |
@pes10k email sent: feel free to close this (or not) |
Just to keep things transparent, the concern is about how reversable canvas fingerprinting protections are (answer, not very hard to reverse!). Specifically, whether
I'm going to update this issue accordingly:
Thanks again to @Thorin-Oakenpants for reporting! |
Just to be clear: item 2 is out of scope, and was asked as an off-topic generic overall design question: @pes10k 's information in the email helps explain some of those decisions - thanks! I still think there can be changes there, but not in this ticket |
Sorry, apologies @Thorin-Oakenpants , didn't mean to put words in your mouth or misstate your comments. Thank you for clarifying! |
^^ no worries: I see OP has been edited: "but it is probably detectable given a determined attacker" <- just to make that crystal clear - it is 100% detectable (with the right script), not probably |
randomizing can also be completely bypassed (completely meaning zero channels are randomized) in iframes and currently offscreen worker doesn't seem to be covered: @pes10k email me if you want details, or you may already be aware of these |
@Thorin-Oakenpants hmm, the iframes example is very surprising, we have tests, but we may have missed something! Same with workers! So, if there are further edge cases we need to cover, that would be very welcome! You can see our manual test cases here too, fwiw (there are of course automated tests we have in CI too), though you'll need to be in nightly to see everything we expect to be covered. There are somethings tested in these tests that are not yet implemented, but most of it is. https://dev-pages.brave.software/farbling.html TL;DR; Yes, more details / sunlight you can offer, the more thats appreciated! |
Closing this out because we now also determine which channel to farble depending on on the seed, which will prevent the attacker from knowing which channels are unmodified and which are farbled. Thanks for the issue @Thorin-Oakenpants ! |
@Thorin-Oakenpants pointed out some techniques sites could use to determine which channel is being farbled in the current implementation (thanks!). After discussing, I'm convinced we should do as the issue suggested, and farble across all channels. I'll follow up with a possible PR. Thanks! |
for QA, there are new, per channel tests on https://dev-pages.brave.software/farbling.html |
Verified
Verified that the values per channel (∟ red channel, etc.) do all of the following:
Shields up, Shields down,
Shields up, restart,
Shields down, restart, In addition to the screenshots, I've captured the values, below; SU = Shields up, SD = Shields down
|
Verification passed on OnePlus 6T with Android 10 running 1.22.51 x64 build
Verification passed on Samsung Tab A with Android 10 running 1.22.51 x64 build
|
Updated Issue (@pes10k edit)
Currently the canvas farbling only effects one channel. That channel is determined by the per-eTLD+1, per-session farbling seed, so the channel isn't guessable, but it is probably detectable given a determined attacker.
It might make reversing the canvas farbling slightly more difficult if the farbling was done across all channels, which would make it a bit more difficult for an attacker.
Original Issue
There is a flaw in canvas randomizing. It's not a security issue. Do you want to discuss it here, or via email, or in a private repo. I'm not talking about the per-origin, per-execution to protect the random seed. It should be a simple fix
The text was updated successfully, but these errors were encountered: