Trusted Types integration

[koto]

We've outlined what we think would be the HTML integration points needed for Trusted Types. This accompanies HTML#3052 describing the integration with HTML.

Approaches

The current draft spec implements the TT checks at the DOM sinks (JS functions) layer, and then discards the type information, such that TT become invisible to most other Web APIs and algorithms. The upside is that TT are simpler to spec & implement, the downside is that the future sinks might be introduced that skip the TT logic, and re-introduce DOM XSS-proneness. Or that we might have missed some existing algorithms that would bypass TT already. There's also a few of bypasses that require some additional custom protections (e.g. this). Let's call that tt-at-sinks approach.

@annevk proposed an alternative approach - to keep the type information intact such that it can be verified by the algorithms running later on (e.g. when script is to be prepared, check that its URL was a TrustedScriptURL. Let's call that tt-at-primitives approach.

TT-at-sinks

• Add text node validation steps to the insert algorithm.
  Note: Perhaps this needs to be changed - the checks might be problematic in step 7 of node insert as they can partially fail, and then step 8 doesn't run. Perhaps hook after insertion but pre execution.

• Changes to Element.setAttribute*. That's to add TT checks for IDL attributes that reflect content attributes with those checks. Currently only explained as a note. This needs some work, but in general the approach is to perform the checks (via relevant global object) that ascertain that TT type checks are performed based, determining the right type for (context object, qualified name) pair. These callouts may be added to set an attribute value algorithm.

• Changes to set an attribute. Callout to TT when the (element, attr qualified name) needs types (e.g. for iframe.srcdoc). The TT checks will attempt to call the default policy on the attribute value, and might abort the algorithm, or change the attr value.

• Changes to Attr value setter; do the TT checks when setting a value of the attribute on an element, if (element + attr name) pair requires TT checks. Callout to TT checks can be implemented either in set an existing attribute value, or in change an attribute from an element algorithm. The latter might make the changes to set an attribute above unneccessary.

TT-at-primitives

The changes required in DOM for this approach would be:

• Add type metadata to Attr and Text nodes. (e.g. allow them to store string or TrustedType).

• Change the signatures of Element.setAttribute* and other affected DOM APIs to also accept a typed value.

• Change Attr value getter to stringify the output; also in https://dom.spec.whatwg.org/#concept-element-attributes-get-value (or perhaps introduce a don't stringify argument?)

• Changes to attribute-related algorithms, similar to ones outlined in TT-at-sinks section. Instead of callouts to TT checks, these would likely assert that a value is of an expected type.

[koto]

I've found another approach to integrate the content attribute checks into the lower-level algorithms - namely into these attribute algorithms:

• change (called by content attr setters, or attr.value setter),
• append (called by clone node, setAttributeNode, attributes.setNamedItem, setAttribute, toggleAttribute, content attr setters)
• remove
• replace (called by setAttributeNode, attributes.setNamedItem)

Similar to attribute change steps, define attribute validate steps in Element
This and other specifications may define the attribute validate steps for elements. The algorithm would be passed element, localName, value and namespace, and would return a string (validatedValue) or throw an error.

In append add optional parameter valueToValidate.
Ascertain that change's value argument does not need to be a string.
In set an attribute value pass input value as valueToValidate in step 4.

In change, append and replace (optionally, also in remove, but that's not needed for TT strictly),
add the following steps at the beginning:

1. If attribute validate steps are not defined, let validatedValue be value, stringified.
2. Otherwise, let validatedValue be the result of runnning attribute validate steps with ... . If that algorithm threw an error, rethrow the error and abort further steps.
3. Set attribute's value to validatedValue.

The attribute validate steps would be defined in other specs (e.g. HTML or CSP), and would define the behavior for sensitive attributes+element pairs. The behavior would be a thin wrapper around Get Trusted Types compliant string.

The result would be that as long as we're dealing with values (e.g. Attr.value setter, or Element.setAttribute(name, value)) the argument could be a Trusted Type, and that would be validated. When dealing with Attr nodes (e.g. Element.setAttributeNode() or NamedNodeMap.setNamedItem()), TT could be enforced via a default policy.

Update: This needs some carve out for parser's create an element for token, which calls append.

This approach solves the bypass via attribute nodes, and also fixes w3c/trusted-types#229.

[koto]

A proof-of concept integration for guarding attribute nodes is in w3c/trusted-types#233.

Preview (Firefox only):
data:text/html,<script>fetch('https://raw.githubusercontent.com/koto/trusted-types/integrations/dist/spec/index.html').then(r=>r.text()).then(t=>document.write(t))</script>#dom-attribute-validate-steps

[koto]

Referring to the proposal of having a slot for URLs and text in a script element (to avoid patching DOM mutation algos), that we discussed offline, I took a look at this, and there's a small set of remaining attribute sinks (inline event handlers + svg scripts + object/embed attrs + srcdoc). These would likely need to get the same treatment, but at least the number of them is manageable. I'll look into it.

[koto]

Alternate take on this, using the slot values for scripts. w3c/trusted-types#236

With this approach, Integration points with DOM disappeared (while the HTML integration gets a bit more involved).
Preview in FF: data:text/html,<script>fetch('https://raw.githubusercontent.com/koto/trusted-types/integrationstake2/dist/spec/index.html').then(r=>r.text()).then(t=>document.write(t))</script>#enfocement-in-scripts

One issue remains in that I think it makes sense for script.setAttribute('src', trustedValue) to work, which reintroduces parts of algorithms proposed earlier in the thread. Thoughts?

[annevk]

It makes more sense to me if types always go through IDL attributes and content attributes remain strings forever.

[koto]

I'm concerned mostly about the migration path for existing codebases that are based on writing content attributes. It's not just a simple replacement from using element attributes to its properties. The attrs do not need to store custom type values, we might only make setAttribute not stringify the 2nd argument (+its namespaced version), which could be picked up by change steps defined in HTML / csp. I already noticed that attribute change steps in HTML return early in https://html.spec.whatwg.org/#event-handler-attributes, before attr value is set in https://dom.spec.whatwg.org/#concept-element-attributes-change) when CSP blocks event handlers. Perhaps this logic could be used in the same way. The only new part would then be just the signature of `setAttribute` function. Edit: Fix typos, added links.

[annevk]

I already noticed that attribute change steps in HTML return early

Those steps returning early does not imply that the calling algorithm returns early too. Or is that not what you meant?

[koto]

I see. Yes, you're right. The value is actually set, it's just that the event object is not created, but the actual content attribute value is set in DOM spec later.

This behavior is a bit similar to what we'd want for TT (fill the right slots based on the value type). So we could define the slots for trusted values in the appropriate elements in HTML (set directly from IDL attributes).

I would argue that we should support setAttribute(name, TrustedXYZ) and setAttribute(name, stringThatWillBeRoutedToDefaultPolicy). Without it, we're placing an unneccessary burden on authors to rewrite applications to not only change the values, but the way they are setting attributes. A big value proposition of TT is that this is rarely needed. We could do it via attribute change steps, if only the change steps didn't get the value stringified. The actual content attribute would remain a string, TT integration would just set a slot value.

[annevk]

I realize this might be hard to answer in the abstract, but in what cases is replacing a setAttribute() call tricky, but passing an object instead of a string through the system straightforward?

[koto]

For example: one only creates the value once, and may use it in various sinks; so TT might only require to change the code that produces the value (say, a function that generates the URLs for dynamic script loads), and not all the places where this data ends up in the DOM.

I can also imagine some situations where there is a difference in behavior between setAttribute and property setting, that might make transition from one to the other more difficult than a simple string replacement. All our existing integrations used both properties and attributes for various reasons (example in React). If TT are agnostic to whether you use content or IDL attributes, the migration must be strictly simpler, no?

[annevk]

The migration would be simpler, but making setAttribute() accept non-strings that have an effect if the this object is a certain element is pretty magical for a generic API.

[koto]

To clarify: are you worried about the call to setAttribute possibly throwing, or about the behavior that happens later with the element? There are side effects when you set a src attribute depending on the actual element (e.g. image gets loaded, or script gets executed, or nothing happens), so in a larger scope attribute assignment is always contextual.

We are proposing the way to query the DOM to see what type is expected for the attribute, to reduce the surprise effect.

[annevk]

Throwing is weird (but probably okay for non-strings). It's more the conceptual change from a string->string map to something else. It's not as invasive as the abandoned tree mutation changes though and if we keep it to setAttribute() and not all ways of manipulating attributes it might be okay. (I'd love it if other folks chimed in.)

[koto]

Throwing is weird (but probably okay for non-strings).

It would most likely throw for strings (expected a type, got a string, default policy doesn't exist or rejects).

+1 for more opinions on the thread. Intitially was thinking roughly that where you're providing the value as a string, than it would make sense to accept a type too (so Attr.value setter as well, but not moving Attrs nodes around via setAttributeNode). But setAttribute is simply quite common, whereas Attr.value is not, so that would be fine. It even makes sense, as setAttribute deals with strings, but Attr is directly trying to mutate the existing node.

[domenic]

I was hoping to weigh in with the desired "more opinions", but I've ended up pretty confused. Would someone be able to summarize what the current question at hand is?

In the meantime, here is my general position:

I believe that setAttribute() should accept trusted type objects; part of the idea of trusted types is that you can stop using the strings throughout your codebase. Saying that authors need to move to properties instead of attributes seems bad.

I also appreciate the simplicity of the model of attributes being a string -> string map, and think that would be nice to keep. Although we have to recognize that in reality it's a list of Attr nodes, not a string -> string map, so the conceptual simplicity is already on shaky ground.

If you combine these it seems like the simplest way to make it work is to normalize the trusted type object to a string as part of the setAttribute() call, which is part of the "TT-at-sinks" program. This could throw. That sounds like a good path to me.

It's also reasonable to add a "trusted" or "came from a trusted type" boolean to the Attr node concept. (Tracked internally, with no exposed API.) Then setAttribute()'s job would be to set that boolean to true when appropriate, and otherwise leave it at its default of false. That's also reasonable, IMO.

[koto]

I was hoping to weigh in with the desired "more opinions", but I've ended up pretty confused. Would someone be able to summarize what the current question at hand is?

Roughly summarizing - when we move the trusted values to slots in elements, and use them instead of content attributes when JS code execution might happen, we need to decide what to do if the values were set using DOM APIs that set content attributes - i.e. how and if to use TT in Javascript APIs from the following list:

• Element.setAttribute(NS)
• Attr.value setter
• element.attributes.setNamedItem(attrNode)

In the meantime, here is my general position:

I believe that setAttribute() should accept trusted type objects; part of the idea of trusted types is that you can stop using the strings throughout your codebase. Saying that authors need to move to properties instead of attributes seems bad.

I also appreciate the simplicity of the model of attributes being a string -> string map, and think that would be nice to keep. Although we have to recognize that in reality it's a list of Attr nodes, not a string -> string map, so the conceptual simplicity is already on shaky ground.

If you combine these it seems like the simplest way to make it work is to normalize the trusted type object to a string as part of the setAttribute() call, which is part of the "TT-at-sinks" program. This could throw. That sounds like a good path to me.

It's also reasonable to add a "trusted" or "came from a trusted type" boolean to the Attr node concept. (Tracked internally, with no exposed API.) Then setAttribute()'s job would be to set that boolean to true when appropriate, and otherwise leave it at its default of false. That's also reasonable, IMO.

FWIW, Chrome's implementation that added TT support for Attr.value and element.attributes.setNamedItem(attrNode) turned out fairly simple. I think it makes sense if at least setAttribute(TT) and Attr.value setter (if the attribute is bound to an element) populated the slot for the element. I don't feel strongly about methods that move attr nodes around or storing trustedness in Attr itself. I think it would be fine if

const s = document.createElement('script');
s.attributes.setNamedItem(someSrcAttr);

succeded, but did not set a slot value, such that TT check would be done in prepare a script later, calling a default policy and likely bailing out.

[annevk]

@domenic I think to some extent you also need to consider appendChild() support for a similar construct ending up being quite complicated (see the various takes on <script>), so if we don't support it there, it's a little weird to support it for setAttribute().

[koto]

We discussed the setAttribute(TT) offline with @annevk, and I believe we can make it work. Now, technically, I think we'd either need the 'attribute validate steps' as described in #789 (comment). The behavior is to be able to accept a matching type early, or throw. The default policy mechanism is not part of the validate steps. Does that sound OK?

[annevk]

Yeah, #805 will make this even easier.

[koto]

Draft integration in #809.