[tracking] Support Trusted Types in Angular#39222
Closed
bjarkler wants to merge 10 commits intoangular:masterfrom
Closed
[tracking] Support Trusted Types in Angular#39222bjarkler wants to merge 10 commits intoangular:masterfrom
bjarkler wants to merge 10 commits intoangular:masterfrom
Conversation
pullapprove
bot
requested review from
IgorMinar,
alxhub,
jelbourn,
mhevery and
petebacondarwin
bjarkler
force-pushed
the
trusted-types-tracking
branch
2 times, most recently
from
f630a9d to
3f9b59c
Compare
Contributor
Author
bjarkler
force-pushed
the
trusted-types-tracking
branch
from
4f431ba to
8912802
Compare
bjarkler
force-pushed
the
trusted-types-tracking
branch
from
8912802 to
a5b8fcc
Compare
Contributor
Author
|
global presubmit for the remaining PRs. |
bjarkler
force-pushed
the
trusted-types-tracking
branch
from
a5b8fcc to
d6d620c
Compare
Sanitizers in Angular currently return strings, which will then eventually make their way down to the DOM, e.g. as the value of an attribute or property. This may cause a Trusted Types violation. As a step towards fixing that, make it possible to return Trusted Types from the SanitizerFn interface, which represents the internal sanitization pipeline. DOM renderer interfaces are also updated to reflect the fact that setAttribute and setAttributeNS must be able to accept Trusted Types.
Make Angular's HTML sanitizer return a TrustedHTML, as its output is trusted not to cause XSS vulnerabilities when used in a context where a browser may parse and evaluate HTML. Also update tests to reflect the new behaviour.
When an application uses a custom sanitizer or one of the bypassSecurityTrust functions, Angular has no way of knowing whether they are implemented in a secure way. (It doesn't even know if they're introduced by the application or by a shady third-party dependency.) Thus using Angular's main Trusted Types policy to bless values coming from these two sources would undermine the security that Trusted Types brings. Instead, introduce a Trusted Types policy called angular#unsafe-bypass specifically for blessing values from these sources. This allows an application to enforce Trusted Types even if their application uses a custom sanitizer or the bypassSecurityTrust functions, knowing that compromises to either of these two sources may lead to arbitrary script execution. In the future Angular will provide a way to implement custom sanitizers in a manner that makes better use of Trusted Types.
Use the bypass-specific Trusted Types policy for automatically upgrade any values from custom sanitizers or the bypassSecurityTrust functions to a Trusted Type. Update tests to reflect the new behavior.
Fix a Trusted Types violation that occurs in Angular applications that use ViewEngine. Promote output from the sanitizer to an appropriate Trusted Type before it is assigned to an attribute or property, using Angular's unsafe-bypass policy. This differs slightly from the corresponding fix for Ivy, in that Angular's main policy is not used to promote output from Angular's native sanitizer. This is for technical reasons; ViewEngine (defined in the core package) does not have access to the native sanitizer (defined in the platform-browser package) and thus has no way to detect it. This is considered good enough to unblock Trusted Types adoption in applications using ViewEngine, considering that ViewEngine is being turned down in favor of Ivy.
Fix a Trusted Types violation that occurs in Angular applications that use ViewEngine. Promote constant values of properties and attributes to an appropriate Trusted Type directly in the compiled source code, before they are passed to an instruction that assigns this value to the given property or attribute. This re-uses the conversion functions introduced in the corresponding fix for Ivy.
Add an implementation of a SanitizerFn that promotes strings directly to an appropriate Trusted Type without any sanitization. This is meant to be used for sanitizing constant strings coming directly from an Angular template, and are thus trusted.
When an attribute has a constant value that is marked for translation (i18n-attrName), a special variable is created in the `consts` array of the compiled template. Since this is not a constant, it is not promoted to a Trusted Type by angular#39211, and can thus cause Trusted Types violations. The same applies to constant attributes parsed out of (potentially translated) i18n ICU messages. Use the newly introduced `SanitizerFn` to promote these constants directly to appropriate Trusted Types. Tree shakability is not a concern here since Trusted Types are already required for i18n to work (angular#39208).
Introduce a Trusted Types policy for use by Angular's JIT compiler named angular#unsafe-jit. As the compiler turns arbitrary untrusted strings into executable code at runtime, using Angular's main Trusted Types policy does not seem appropriate, unless it can be ensured that the provided strings are indeed trusted. Until then, this JIT policy can be allowed by applications that rely on the JIT compiler but want to enforce Trusted Types, knowing that a compromise of the JIT compiler can lead to arbitrary script execution. In particular, this is required for enabling Trusted Types in Angular unit tests, since they make use of the JIT compiler. Also export the internal Trusted Types definitions from the core package so that they can be used in the compiler package.
The JIT compiler uses the Function constructor to compile arbitrary strings into executable code at runtime, which causes Trusted Types violations. To address this, JitEvaluator is instead made to use the Trusted Types compatible Function constructor introduced by Angular's Trusted Types policy for JIT.
bjarkler
force-pushed
the
trusted-types-tracking
branch
from
d6d620c to
655b2d6
Compare
mhevery
reviewed
Nov 16, 2020
| abstract removeStyle(el: any, style: string, flags?: RendererStyleFlags2): void; | ||
| abstract selectRootElement(selectorOrNode: string | any, preserveContent?: boolean): any; | ||
| abstract setAttribute(el: any, name: string, value: string, namespace?: string | null): void; | ||
| abstract setAttribute(el: any, name: string, value: string | TrustedHTML | TrustedScript | TrustedScriptURL, namespace?: string | null): void; |
Contributor
There was a problem hiding this comment.
We should not need to update our public API by bringing in Trusted* Type into it.
Contributor
Author
There was a problem hiding this comment.
This was part of the draft PR for ViewEngine support (#39286). We decided to focus on Ivy instead of spending time on fixing this and other issues.
Contributor
Author
|
Closing as most of the PRs covered have been merged. Any remaining work will be discussed in individual PRs. |
14 tasks
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
pullapprove
bot
added
area: compiler
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Do not merge!
This is a tracking PR for initial support of Trusted Types in Angular. It consists of the following PRs:
It also contains a commit that updates goldens for size constraints.
See the individual PRs for details.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Does this PR introduce a breaking change?
Other information
This is part of an ongoing effort to add support for Trusted Types to Angular.