MV3: Error on CSP that declares remote hosts or eval

[wagnerand]

Since MV3 in Firefox disallows executing remotely hosted code as well as inline code, there is not much of a reason for AMO to accept such submissions.

The linter should throw an error for a CSP that declares a sript-src or default-src using a remote website or unsafe-eval.

@rpl could you comment if #3007 needs to be included here as well? @Rob--W could you double-check the requirement above to make sure I didn't forget anything?

┆Issue is synchronized with this Jira Task

[Rob--W]

• script-src and worker-src only accepts 'self', 'none' and 'wasm-unsafe-eval'.
• script-src-elem only accepts 'self', 'none' (#3007)
• If script-src is not specified, default-src must be specified and not contain remote resources

We should already be validating all of the above, but the difference is that we only emit warnings. We should emit errors instead, at least for MV3.

Note that even if we accept the insecure CSP declarations, that the presence of an insecure script directive results in a ban of the whole CSP, and the default directive script-src 'self' is used instead.

[willdurand]

We should already be validating all of the above, but the difference is that we only emit warnings. We should emit errors instead, at least for MV3.

@Rob--W with GA approaching, it'd be good to know if we need to do anything here, especially if it involves converting warnings into errors. Could you clarify what's needed to resolve this issue please?

[Rob--W]

We are currently still reporting warnings. We should be reporting errors if the CSP does not adhere to the format described in my previous comment.

Note that Firefox already enforces these requirements, so extensions with remote sources in their CSP won't be able to run remote code. But the presence of invalid CSP causes Firefox to reject the whole CSP string altogether, which may potentially include any stricter directives that the extension had set. Therefore there is still value in ensuring that we return errors for these cases.

[willdurand]

@Rob--W would you be ok with starting a PR for that?

[Rob--W]

I reviewed the existing CSP validation, and found several bugs (#4575, #4576, #4577, #4578). I already have patches for these (and #4518), and will submit PRs for them.

I did all of that, because a fully correct CSP validator is a prerequisite before we can reject submissions.

After doing all of that, I do have a question though... In Manifest V3, it does not matter what the developer puts in the CSP. They cannot make it stricter than we want. Therefore an informational warning would suffice. The main argument in favor of a hard error that I can think of is that we haven't started signing MV3 extensions yet, and that the presence of insecure (but ignored) CSP directives can be misleading to whoever reads the manifest.json file.

I actually think that there may be a stronger case for making it an error in MV2 and just a warning in MV3. But that's only feasible if we are willing to reject MV2 extensions that use 'unsafe-eval' and such.

On the linter part, the least that we can do in relation to this issue is improving the accuracy of the emitted warning. I have posted a detailed write-up at #4579.

[wagnerand]

I actually think that there may be a stronger case for making it an error in MV2 and just a warning in MV3. But that's only feasible if we are willing to reject MV2 extensions that use 'unsafe-eval' and such.

I would like to make it an error in MV2 as well, but it would break too many extensions all of a sudden. We could do it, but we'd have to communicate to developers and give significant lead time, at which point we might not be too far from deprecating MV2 altogether.