Wrap trusted-types policy creation in a try/catch

[Zlatkovsky]

In our use-case of the ms-rest-js library, we have a large existing application (complete with caching, service-workers, etc) that we dynamically load additional JavaScript code into. The additional JS is what brings in the ms-rest-js.

The application has Trusted Types enabled (e.g., CSP rule 'trusted-types LibraryA LibraryB etc). But, interestingly, its require-trusted-types-for 'script' is currently under "report only" (Content-Security-Policy-Report-Only). In those cases, a call to trustedTypes.createPolicy will fail because the policy isn't on the allowed-list, even though skipping the createPolicy would be OK. So in effect, https://github.com/Azure/ms-rest-js/commit/531aea9bfbbe6ef6c06984ab06f99775b90923ca introduced a regression.

We are working on extending the allow-list of our application to include @azure/ms-rest-js#xml.browser, but that change takes time. In the meantime, we'd like to wrap the policy-creation in a try/catch. There is no security risk to it, since for host applications that DO have strict enforcement of trusted-types, the code will simply fail when the dangerous sink is used (e.g., when doing parseFromString). And likewise, wrapping in try/catch and doing nothing on catch is OK, because the code already deals with the possibility of the trustedTypes API not being available on the browser.

[jeremymeng]

But, interestingly, its require-trusted-types-for 'script' is currently under "report only" (Content-Security-Policy-Report-Only). In those cases, a call to trustedTypes.createPolicy will fail because the policy isn't on the allowed-list, even though skipping the createPolicy would be OK

If createPolicy is skipped, there would be issues reported for our usages of parseFromString?

[Zlatkovsky]

If createPolicy is skipped, there would be issues reported for our usages of parseFromString?

@jeremymeng: It depends on whether the host application has require-trusted-types-for 'script' on or off.

If it's on: yes, the createPolicy would have been bypassed via the "catch", and parseFromString would be the one to fail. So we'd basically just be shifting the error from not being able to create a policy, to failing during the invocation of a dangerous-sink API (where the policy would have been used). This is OK from a security perspective, since the browser still enforces the security.

If it's off (or report-only), which is what our case was: parseFromString would continue to work just fine once the createPolicy is bypassed -- whereas otherwise the code immediately breaks and nothing else executes.

[jeremymeng]

If it's off (or report-only),

Does "report-only" mean Content-Security-Policy-Report-Only: require-trusted-types-for 'script';? Under this setting, createPolicy is bypassed, would usage of parseFromString be reported?

[Zlatkovsky]

Does "report-only" mean Content-Security-Policy-Report-Only: require-trusted-types-for 'script';? Under this setting, createPolicy is bypassed, would usage of parseFromString be reported?

Yes. With the try/catch change proposed in the PR, the createPolicy would fail and thereby not create a policy. Then, if the host application has Content-Security-Policy-Report-Only: require-trusted-types-for 'script'; (which is exactly the setup we had), the browser would see that someone is trying to call parseFromString without having the string having been first sanitized by a policy, and hence would flag it with a warning in the console and a report to the CSP report URL.

Screenshot showing how it would look on the Console; only the second message is relevant for this case:

[Zlatkovsky]

@jeremymeng , gentle ping.

[jeremymeng]

@Zlatkovsky sorry things move slower now that US is in the Thanksgiving holiday week. I will ping the Teams team again next week to get their Okay.

[jeremymeng]

@Zlatkovsky v2.6.4 is now on npmjs.