Support a truly report-only and debuggable default policy

[koto]

Currently, errors thrown from a default policy createXYZ functions are surfaced as CSP violations, but (since there is no return value from the policy), also cause the assignment to fail (see Chrome implementation)., i.e.:

• in report only mode an empty string is used instead of the original payload.
• in enforcing mode the assigment throws.

Additionally, all regular (e.g. ReferenceError) errors in a default policy are treated as TT CSP violations, and as such are not propagated to the application, and do not reach the global error handler.

That behavior is surprising, as one would expect the report-only CSP mode to be just that. When using default policies in report-only mode, authors have to implement a custom report-only reporting, and assure they don't throw - not to break existing code.

[koto]

This issue is not simple to solve.

As the policies may have side-effects, or change a global state, it's impossible to have them "speculatively execute". If they run, they should run fully.

They also should definitely run in a report-only mode - otherwise there's no way of testing them (and practically, there's no way to adopt Trusted Types on existing applications without a report-only mode trial first).

I would also argue that the default policy return value should be respected. E.g if the value was changed, the new value should be used in an application (otherwise switching to enforcing mode would introduce an observable change in application behavior, which is surprising).

That said, currently there's 2 practical problems:

• regular errors in a TT default policy code are only collected in CSP violation reports.
• there's no easy way for a default policy to cause a report-only violation in a way that allows the original input value to be used in the sink.

One idea we're thinking of to solve this issue is introducing a new JS error type e.g. InvalidStringAtSinkError. If the default policy throws that error, he following logic applies:

• CSP violation is triggered
• If the mode is report-only, the input value to the policy createXYZ function is used.
• Otherwise, throw an error.

If the policy throws any other error, the algorithm bails out early, and this error is surfaced to the application. This allows the authors to detect & fix the regular error conditions in their default policies in separation from TT issues.

So, the following code:

trustedTypes.createPolicy('default', {
createHTML: (input) => {
 if (input.match(/\</)) {
  throw new InvalidStringAtSinkError(); // I don't know how to sanitize HTML, but i'm not expecting it either
 }
 return input + SUFFIX; // // SUFFIX is not declared
}
});

// Will cause a violation, but assign <img> in report-only mode
// Will cause a violation and throw in enforcing mode
el.innerHTML = '<img>'; 
// Will throw a ReferenceError before assignment happens
el.innerHTML = 'foo'

Spec wise, this requires small changes to https://wicg.github.io/trusted-types/dist/spec/#get-trusted-type-compliant-string-algorithm.

@vrana @engelsdamien

[koto]

One caveat, not strictly related to the issue is that CSP violation reports are not the best tool for debugging the issues, as their collection is delayed, and they have very limited data available. Which means, that during the adoption period, the authors may actually prefer regular error reporting capabilities, and use CSP violation reports only to ascertian there are no violations left before switching to the enforcing mode.

[otherdaniel]

Hm. I take it the key is to ensure that report-only is a danger-free way to test out deployment. So how about this:

When a default policy is called, and when the default policy fails to execute, do the "usual" CSP processing (console message + report-to, if enabled), but suppress the exception in JavaScript and proceed with the assignment of the original value?

[koto]

Do you mean in report-only? If yes, than this looks like what we're proposing exactly. If you mean that this should also happen in enforcing mode - I don't think so, there needs to be a way to bail out in a default policy in a way that's safe. Our proposal addresses that. Essentially, you throw a specific error and its processing and propagation depends on the mode.

…

On Tue, Aug 27, 2019, 11:18 Daniel ***@***.***> wrote: Hm. I take it the key is to ensure that report-only is a danger-free way to test out deployment. So how about this: When a default policy is called, and when the default policy fails to execute, do the "usual" CSP processing (console message + report-to, if enabled), but suppress the exception in JavaScript and proceed with the assignment of the original value? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#209>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAA7JK47VRXXHYAQETB7MSTQGTWPLANCNFSM4IOUNRQA> .

[otherdaniel]

Yes, I meant in report-only.

I'm not sure what the specific error adds. Wouldn't any exception do?

[koto]

Those two issues are slightly orthogonal. The specific error object is meant to address debuggability problem:

If the policy throws any other error, the algorithm bails out early, and this error is surfaced to the application. This allows the authors to detect & fix the regular error conditions in their default policies in separation from TT issues.

However the first issue (no true report-only default policy is possible now) is more important I think.

[koto]

The default policy createXYZ function may result in two different kinds of error conditions.

One that signals to CSP/TT that its input value is not OK to convert to a trusted type. This issue suggests to generate the violation for this, and use the input value anyway iff the mode is report-only.

Second that is just a result of a programming error in the function or its dependencies (e.g. ReferenceError). It feels natural to propagate these to the application, which would also have the side effect of stopping the assignment. If we don't, than it's hard to detect these errors - on the CSP collecting server there are no stack traces, and the data available there is limited for privacy reasons.
These errors make it possible for the default policy to break production in report-only mode, but so do any other errors, which suggests that we should rather allow the authors to be able to debug these issue using their regular tools.

[koto]

After reconsideration, it feels like instead of introducing a new error type (that would trigger a violation, have mode-dependent behavior and not be propagated to the JS application), default policy should just return a special (i.e. not string) value that is interpreted like that.

How about false? It makes it similar to how event handlers behave (return false implies Event.preventDefault()).