Trusted Types

[mikewest]

Hello TAG!

This is earlier than I'd usually ask for y'all's feedback, but since I'm talking about a type system, it seems reasonable to get some sort of directional feedback from y'all before getting to far ahead of things.

• Name: Trusted Types for DOM Manipulation
• Specification URL: None
• Explainer, Requirements Doc, or Example code: https://github.com/WICG/trusted-types
• Tests: Aiming to eventually land the tests in https://chromium-review.googlesource.com/c/chromium/src/+/559061
• Primary contacts: @mikewest

We'd prefer the TAG provide feedback as (please select one):

• open issues in our Github repo for each point of feedback
• open a single issue in our Github repo for the entire review
• leave review feedback as a comment in this issue and @-notify @mikewest

Again, there's not much concrete in the explainer, and we're pretty far away from having a spec. So now's a great time to give fundamental feedback on the notion of adding this kind of type system to DOM manipulations. We'd love to hear it. :)

[hadleybeeman]

Hi @mikewest! Thanks for sending this! We picked it up at our face-to-face in Nice.

A few comments and questions:

• Why is trustedURL not a subclass of URL? It would be good if these trusted types should fit meaningfully into the type hierarchy. Example code would help us here.
• Overall, we'd like to encourage you to not do this in IDL but instead do it in example code.
• It would be good to see integration with es6 template strings. So that it's possible to come up with a typed output.
• The name implies trusted. Can we name it something a bit more functional, like maybe something like "unserialised types for DOM manipulation"? (yes, we know naming is hard :-) )
• This may result in escaping everything — too much work (also including potential risks of jumping back and forth between escape and override escapes seem risk to be error prone)
• Issues with multiple concatenation, e.g., "mystring" + TrustedHTML.escape('unsafe') + "otherstring" - results in JS string, not typed object.

[koto]

Hey @hadleybeeman, I'm koto, working with Mike on this.

To answer your questions:

Why is trustedURL not a subclass of URL?

No strong reason, these types originate from Google internal implementation, where that was the case. I'm not opposed to subclassing.

Example code vs IDL

In fact, we have a polyfill implementation now in the repository, it can be built with:
$ npm install $ npm run build
or you could check the sources.

Template strings

Yes! I proposed something like that in w3c/trusted-types#26.

Naming

We have reasons why this is named trusted as opposed to e.g. safe, but at this stage names are not that important, though "unserializedTypeForDOMManipulation" will probably not fly ;)

Escaping

We're hoping to come up with an API with safe builders that may make the migration easier, and offering security benefits. This is just an early draft.

Concatenation

That is actually expected, by concatenating safe HTMLs with raw strings you may come up with an unsafe construction (if the strings are user-controlled). I hope to address that in w3c/trusted-types#26, where interpolating user data gets slightly easier, but in general we hope such literal + TrustedHTML contatenations could be automatically refactored, rather than silently accepted at runtime.

[slightlyoff]

Hey @koto:

Thanks for getting back to us! @cynthia noted that the proposal has changed a bit. Are you looking for more feedback now? If not, feel free to close this issue and re-open when you get closer to shipping.

Regards

[koto]

Hey,

There are active changes being planned, and we'll be undergoing some API changes, so I'll close this now as you advised, and reopen when needed. Thanks!

[koto]

@mikewest, can you close this issue? I don't have the permissions to.

[mikewest]

@torgo / @ylafon: Would you mind reopening this review? @koto, et al. have moved the design forward from where we left it in February, and I think it's ready for a more detailed analysis from the platform perspective.

[dbaron]

So one concern I had while the TAG is looking at this today:

Assuming I'm interpreting the explainer correctly... it seems a little bit odd the way policies are identified by URLs, but there isn't really any validation that the URL given has any hard association to the chunk of script that's registered as being the policy identified at that URL. It both:

• seems a little bit like an odd use of URLs, and
• seems to introduce a risk that a policy URL really represents whichever chunk of script wins the race to register that policy (which in turn seems to make the CSP part slightly less useful)

I'm wondering if there's something better here, although I don't immediately see something that doesn't involve a bunch of additional resource fetching (to, say, fetch separate policy scripts each in their own file).

[torgo]

Some discussion again on the use of the term "trusted" and how over-loaded this term is. @dbaron suggested that "policy-checked types" might be better. @lknik suggsted "safe-to-consume". @slightlyoff suggested "validated". In any case, even if you stick with "trusted" I feel the document should explain the scope of the use of the term "trusted" - trusted in what sense - as a specific sub-section of the explainer.

[cynthia]

The functionality is currently exposed only to Window on the draft spec, is there is a reason for this? I would imagine this could be really useful in a service worker, which would require it to under WindowOrWorkerGlobalScope. (Seems like a simple fix, but if there is a clear reason for this decision, we'd like to know.)

Additionally, this feels like it would be really useful for server side JS runtimes as well, so making it implementable by node.js (for example) would be really useful.

[mikewest]

Hello, friendly TAGgers!

It's unclear to me what the status is on y'all's end. :) We think we're converging on something that's reasonable, and it would be a great time to give us feedback if you have an opinion about the shape of the API we're ending up with. :)

Thanks!

[mikewest]

Dearest friends and neighbors,

We still think we're converging on something reasonable, and are getting reasonable feedback from developers who are trying out Chrome's current implementation. If y'all have thoughts or concerns, it would still be a lovely time to express them!

Thanks!

/cc @torgo

[annevk]

As was a problem with CSP, I'm worried with trying to address this problem based on callsites, rather than the more fundamental algorithms that are responsible for performing a certain action that we want to avoid exposing to non-privileged code.

That is, I think a more principled model would modify "navigate" to take a "trusted" value and based on some policy error on non-"trusted" values, and then adapt callsites as appropriate to be able to pass it "trusted" values. (I'm also a little worried that the current approach leaves certain attack vectors unexplored. Are malicious target values problematic? Is changing <form method> not problematic? I.e., should all parameters to "navigate" effectively be "trusted" values? Why only the URLs?

(Also, window.location is protected, but MathML links are not? With a system that protects navigation (and similar such algorithms) at source, you don't really have to worry about this, at least not until someone wants to use MathML links with it.)

[koto]

Thanks, Anne!

As was a problem with CSP, I'm worried with trying to address this problem based on callsites, rather than the more fundamental algorithms that are responsible for performing a certain action that we want to avoid exposing to non-privileged code.

That is, I think a more principled model would modify "navigate" to take a "trusted" value and based on some policy error on non-"trusted" values, and then adapt callsites as appropriate to be able to pass it "trusted" values.

We're not opposed to adding additional assertions at navigation time (or when a script is prepared), but what we feel really strongly about is throwing the errors (also?) at assignment time. For one - this can be polyfilled, but more importantly - this allows for statically (as much as it's possible with JS) identifying places in the code where sinks are used in a non-compliant way. Security bug becomes a programming error, and one can debug it early where it happens, instead of only being left with SecurityPolicyViolationEvents and CSP reports later with not enough context to identify the issue.

I'll identify which alorithms would be a good target for the additional checks and update the spec with the proposed alterations.

I'm also a little worried that the current approach leaves certain attack vectors unexplored. Are malicious target values problematic? Is changing <form method> not problematic? I.e., should all parameters to "navigate" effectively be "trusted" values? Why only the URLs?

We are targetting DOM XSS, content exfiltration or confinement is explicitly not in scope (mostly because this feels like a much more difficult to achieve with the sink-based approach only due to the existence of various other side-channels for the data).

(Also, window.location is protected, but MathML links are not? With a system that protects navigation (and similar such algorithms) at source, you don't really have to worry about this, at least not until someone wants to use MathML links with it.)

All link based vectors stop being risky once we simply disable javascript: URLs (and optionally reenable them selectively at navigation time via efault polocy). This is what we're proposing in the update to the API (we'll likely discuss this at lentgh at TPAC). After those updates, the only URLs that are relevant are URLs of scripts and object/embeds.

[lknik]

Seems it's shipping in Chrome soon?

[alice]

Hi @mikewest et al,

We're coming back to this, finally (apologies!).

We had a read through the intent to ship thread, and noted that the intent was paused in November to take a look at:

• DOM node manipulation and the <script> tag.
• Extensibility
• Developer support / Support by Frameworks

We couldn't find issues for the first and third of those topics; could you possibly give us a pointer?

We also noticed that there was a recent update to that thread specifically calling out the Extensibility issue as having been addressed, as well as the default policy issue (was that in addition to the above three issues, or a rephrasing of one of them?)

Was the Extensibility issue resolved to the satisfaction of the Mozilla folks who raised concerns? (@annevk may know more here.)

What is the status of the other issue or two from the original list?

And, is the current version of the explainer a good place for us to go to get an overview of the current state of the proposal?

[otherdaniel]

Thanks for taking another look at TT! :)

Regarding intent-to-ship & 3 issues: It's certainly our intent to have addressed the concerns.

• Extensibility: I think this is done. The original concern was correct, in that extending the design (e.g. to cover CSS) was risky. With the require-trusted-types-for directive it's straightforward for the page author to indicate what scope they intended for the current version of their page.

• DOM node manipulation & friends + "default policy": This is the same issue. Or actually, it's an extended version of the original issue. When we presented & discussed our first attempt at fixing this, we discovered that the issue runs a bit deeper than we had initially understood. We think we have now fixed this (in the spec; the implementation is still being finished), by checking the <script> text against a shadow slot, and by more precisely spec-ing and implementing when the "default policy" runs. (The last comment explicitly lists the 4 items that address the original DOM node manipulation issue.)

• Dev support: We've run an origin trial and are experimenting with TT within Google. We have also received feedback from external parties, although they don't seem too keen on making public statements in this phase. So this is arguably still open. However it seems that we have largely exhausted the feedback potential for experimental features: Developers will put only modest amount of work into an unreleased feature. We will get more & more qualified feedback only after making this more widely available.

Explainer: The broad strokes of the explainer are correct, but it doesn't reflect recent changes to the spec. We need to update it & add more detail.

[mikewest]

Hey folks! As an FYI: there is an intent to ship thread ongoing on blink-dev@. IMO, the right thing to do is to get this API in front of developers, and iterate on it as we learn from their feedback. If y'all believe there are things we really ought to change before shipping it, please do weigh in. We value your input.

[alice]

@plinss and I are looking at this at our face to face.

Firstly: is the explainer up to date now?

Secondly: I would like to encourage @dbaron and @cynthia to take another look before we close this - perhaps if the explainer needs another round of edits, you could let us know when it is up to date?

[koto]

Firstly: is the explainer up to date now?

Yes.

There's some minor details we're working on w.r.t. the shape of the API. The most relevant ones would be:

• Restrict to secure contexts w3c/trusted-types#259,
• Consider allowing creating a policy via a constructor. w3c/trusted-types#258,
• Document.write and such as names are misleading w3c/trusted-types#260,
• Consider removing getPolicyNames() w3c/trusted-types#235,
• Alternative Options for Default Policy. w3c/trusted-types#248

We're also discussing the exact integration with other specs, e.g.:

• Added a definition of {{StringContext}} extended attribute. whatwg/webidl#841,
• Add attribute validate steps. whatwg/dom#809,

That said, the explainer correctly describes the problem we want to address and the solution we propose. Further details are in the spec draft.

[plinss]

I'm also wondering why assign trusted type objects to the existing API surface that expects strings? Would it make more sense to introduce new API surface for content injection that only takes objects, whether they are trusted type like things or other higher-level objects, and then deprecate the string based apis (or turn them off in given contexts). Seems like this would make feature detection and polyfills simpler as well as giving extra options for more control over content injection.

[mikewest]

Hey, Peter! Thanks for the feedback!

The main reason we ended up reusing .innerHTML rather than introducing .innerTrustedHTML or similar is deployment cost. As currently specified, Trusted Types can be rolled out to an existing application in a piecemeal fashion in a way that works in both browsers that support the mechanism and those that don't. Changing the DOM entry points themselves would require substantial rewriting and branching at every point at which the DOM is modified; @koto will have more detail here, but my understanding is that most(?) application have significantly fewer points of string creation and/or sanitization than points of usage. Auditing and adjusting the former is a tractable task, branching for the latter would be more difficult.

[otherdaniel]

On separate 'trusted' entry points: It's a big thing for library support: In the current model, every trusted-types valid program is also non-trusted-types valid program. (With exception to actual policy creation, which can be guarded by a single if-statement).That makes it possible for programs, libraries, and frameworks to transition to Trusted Types support without leaving their non-TT users behind.

With separate entry points, libraries would be forced to maintain two versions (either two actual versions, or effectively two versions by branching on every affected DOM call). And every program that wishes to use TT would be pushed deeper into dependency hell, since now they'd have to transition every dependency and dependency-of-dependency to TT first, or to somehow manually enable them with via default policies or patch them elsehow. In a world were 100% self-contained, dependency-less programs have become a rarity, that's a pretty big deal.

[dbaron]

@sangwhan and I are looking at this in a breakout.

One comment is that after reading the explainer and parts of the spec, it's not clear to me why require-trusted-types-for 'script' is named what it is. It seems like an odd name to me for a directive that's about requiring something for javascript URLs. Though the Adopting Trusted Types section seems to imply that it's about something much broader than just the handling of javascript URLs.

Beyond that, I don't have anything to say immediately, but it really took me half an hour to load this whole thing into my head again, and I'd probably need another read-through to come up with more interesting thoughts on it.

[dbaron]

So I think given that @alice and @torgo were suggesting we consider closing it, and I think we think we're OK with closing this as well. I'm not sure that we've gone through everything in detail, but I also think we're reasonably comfortable with this approach at a high level even if we haven't dug into all of the details.

I'd note that I found the explainer a bit confusing, particularly for understanding how the CSP integration works (see previous comment). But this seems like it's probably straightforward to fix, and it's certainly understandable that an explainer for something that's changed so much over time might get a little confused at points.

[koto]

Thanks! For the record, require-trusted-types-for 'script' controls all enforcement for DOM XSS prevention (the only sink group we're tagetting right now), not just javacript: navigation. Point taken though, I'll try to clarify that in the explainer and review the spec to make that clear.