Add style attr sanitizer

[g-k]

As discussed in #248, we'd like to move away from the regex based CSS sanitizer but don't necessarily have a maintained CSS parser with Python 2 support to switch to (also some users don't necessarily want full CSS parsing).

Changes in this PR:

• add a css_style_attr_sanitizer param/arg to bleach Cleaner and BleachSanitizerFilter to make it possible to override sanitize_css without changing Cleaner.clean
• update the arbitrary style goal

We'd then cut a major release that:

• drops python 2 support Drop support for EOL Python versions <3.6 #520
• replaces old regex based stuff with tinycss2 redo sanitize_css to use a css parser #248

r? @jdufresne

cc @willkg @peterbe

[willkg]

I only set the date here if it's been released. Otherwise it gets confusing in the documentation as to what was released and what's in the master branch that hasn't been released, yet.

So for this, I'd use "In development" or something along those lines.

[willkg]

I only set the date here for releases. When it's in development, I leave this as the empty string.

[willkg]

This sentence doesn't parse in my head. Do you mean something like this?

The argument css_style_attr_sanitizer in bleach.sanitizer.Cleaner can be used ..

[willkg]

I'd rename this to my_sanitize_css to make it clearer it's the overridden one rather than the Cleaner instance method.

[willkg]

This is much more useful than what we had before. 👏

[willkg]

I like this, but I wonder if the ongoing "Bleach does it right by default but then lets users do whatever with warnings of dire but vague consequences" architecture changes we're making should also be accompanied by something that can help users know when their overrides create bad situations.

Could we create a test harness that people could pass a cleaner to and it helps them figure out if they've got vulnerabilities?

Maybe we can base it on the OWASP-derived test cases we've got now? (https://github.com/mozilla/bleach/tree/master/tests/data) Maybe we can base it on the website harness we've got? Maybe there's a page safety site out there already we could use?

I'm just wondering out loud. There's no way users on average understand the problem domain enough to know a bad decision when they're overriding Bleach thing.

[g-k]

Thanks for the review willkg!

I wonder if the ongoing "Bleach does it right by default but then lets

users do whatever with warnings of dire but vague consequences" architecture changes we're making should also be accompanied by something that can help users know when their overrides create bad situations.

Could we create a test harness that people could pass a cleaner to and it

helps them figure out if they've got vulnerabilities?

Maybe we can base it on the OWASP-derived test cases we've got now? (

https://github.com/mozilla/bleach/tree/master/tests/data) Maybe we can base it on the website harness we've got? Maybe there's a page safety site out there already we could use?

I'm just wondering out loud. There's no way users on average understand

the problem domain enough to know a bad decision when they're overriding Bleach thing. Yeah, library consumers shouldn't need to know details of what threats bleach is trying to protect against. I think there are two things we can do: 1. take a cue from https://cryptography.io/en/latest/#layout and split the API into a safe and potentially a more stable "recipes" layer (just bleach.clean, bleach.linkify, and maybe some other top-level functions taking a limited subset of args) and a more dangerous "hazmat" layer with all the knobs and buttons to tweak and override things. Security bugs against the recipes layer would be higher severity. 2. create an evaluator (like various CSP ones) or config generator (like the Moz SSL config generator) for people using the recipes or hazmat layer for consumers to provide additional context (where they're using bleach output, who's entering the data and how much they trust them, what other controls are in place, etc.) and to know whether their bleach config is safe.

…

On Fri, May 22, 2020 at 11:54 AM Will Kahn-Greene ***@***.***> wrote: I like this, but I wonder if the ongoing "Bleach does it right by default but then lets users do whatever with warnings of dire but vague consequences" architecture changes we're making should also be accompanied by something that can help users know when their overrides create bad situations. Could we create a test harness that people could pass a cleaner to and it helps them figure out if they've got vulnerabilities? Maybe we can base it on the OWASP-derived test cases we've got now? ( https://github.com/mozilla/bleach/tree/master/tests/data) Maybe we can base it on the website harness we've got? Maybe there's a page safety site out there already we could use? I'm just wondering out loud. There's no way users on average understand the problem domain enough to know a bad decision when they're overriding Bleach thing. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#535 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABXKLOZBRAY3J36F2GGG4DRS2N5DANCNFSM4NBWVK3A> .

[willkg]

I don't think we want to make these changes.

In another PR, @g-k mentioned reevaluating what it is we're protecting against with CSS sanitization and moving forward from there.

I do like the idea of:

take a cue from https://cryptography.io/en/latest/#layout and split the
API into a safe and potentially a more stable "recipes" layer (just
bleach.clean, bleach.linkify, and maybe some other top-level functions
taking a limited subset of args) and a more dangerous "hazmat" layer with
all the knobs and buttons to tweak and override things. Security bugs
against the recipes layer would be higher severity.

However, that's a big backwards-incompatible project, so I won't have time for that any time soon.

I wrote up issue #633 to cover redoing css handling.