-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Trusted Types] Counter Proposal #1
Comments
Following up from mozilla/standards-positions#20 (comment)
As I see it, the value of wicg/TT (to distinguish from StitchNG's proposal) is threefold:
I'll take these ideas one at a time:
This seems like a fine option for certain applications but any candidate solution to DOM XSS will have to be a good migration target for existing apps, and blanket policies are simply not good migration targets. If one part of an application relies on getting carefully crafted, unsanitized content to a browser API, then no part of the application can run under this regime, unless you have a way of marking certain content as exempt from this default policy. The larger the app, the more likely there is at least one case where content cannot be auto-sanitized, so you need exemptions; preferably auditable, type-safe exemptions. That's why wicg/TT treats TrustedXYZ as exempt from the default policy for strings. wicg/TT does enable exactly this posture via its default policy mechanism, but when an app grows to need to do something funky, you're not out of options.
This seems like a great idea and would make writing certain wicg/TT policies much easier.
First, these examples all show developers creating a policy and then immediately using it with a sink. Back to your question: Do they really? Let me table out what happens when a developer fails to think about document.getElementById('main-page').innerHTML = x;
† - ok, but without explicit auditing a security specialist can't double check. Yes, if you're going to use a sink correctly you have to have some basic knowledge about the kind of input that suggests. That's true either way. But it seems to me that with wicg/tt the developer has to think about strictly less. The developer who is using wicg/tt doesn't have to worry about whether it's trustworthy, just whether it's HTML. And via the reporting API, they can get feedback about when their assumptions are broken, which is unavailable to the developer who is not using wicg/tt. What am I missing?
This seems unlike other issues you've raised, in that it doesn't seem to come from a different design philosophy, different priorities, or different goals. As such, it seems like something that could be folded into either proposal if we succeed in hashing out our more fundamental differences. |
@mikesamuel i'll take it from here as i'm the author of this counter proposal. I'll now try to properly explain my proposal for TrustedTypes as i see there are certain parts of my proposal you do not completely recognize.
Firstly, I would like to say that my proposal has no /**!
* This is a fictitious (and contrived/feigned) API that is
* meant to expose the details of the policies registered
* via the "trusted-types" directive in the CSP header to the
* DOM sinks
*/
window.CSP = {}; // contrived API (partially part of my proposal. relevant only to make a point) /**!
* Define a trusted type policy as 'default'
*
*
*/
window.TrustedTypes.HTML.registerPolicySanitizer('default', function(){
return function(dirtyHTML){
return window.DOMPurify.sanitize(dirtyHTML)
}
}); <meta http-equiv="Content-Security-Policy" content="trusted-types default"> /**!
* This code monkey-patches of the `innerHTML` DOM API
* descriptor setter with the aim of becoming aware of trusted types
* policies and use them for sanitizing setter values (HTML strings)
*
*/
// get descriptor (may not work in chrome - luckily we have an alternative)
var originalDesc_innerHTML = Object.getOwnPropertyDescriptor(Element.prototype, 'innerHTML');
if(typeof originalDesc_innerHTML == "undefined"){
// use "__lookupSetter__" & "__defineSetter__" as Chrome/Safari doesn't let you access property descriptors
originalDesc_innerHTML = {set:Object.prototype.__lookupSetter__.call(HTMLElement.prototype, "innerHTML")}
Object.prototype.__defineSetter__.call(HTMLElement.prototype, "innerHTML", function value(value){
// { window.CSP.trustedTypesPolicies } is a conceptual API to get the trusted type policies listed in the CSP header
var trusted_types = window.CSP.trustedTypesPolicies || ['default'];
var registration = {trustedtype:{config:{},type:null},sanitizerFn:function(val){ return val; }}
if(typeof trusted_types === 'string'){
trusted_types = trusted_types.split(' ');
}
if(trusted_types.length == 1){
registration = window.TrustedTypes.htmlRegistrations[trusted_types[0]];
}
var old_value = String(value);
// the sanitizer registered to a trusted type policy is executed
var new_value = registration.sanitizerFn(old_value)
// if inclusions are configured to be blocked then do not execute the descriptor setter
if(registration.trustedType.config.blockIncludes){
return old_value;
}
return originalDesc_innerHTML.set.call(this, new_value);
});
} else {
Object.defineProperty(Element.prototype, 'innerHTML', {
configurable:originalDesc_innerHTML.configurable,
enumerable:originalDesc_innerHTML.enumerable,
get:originalDesc_innerHTML.get,
set: function innerHTML(value) {
var trusted_types = window.CSP.trustedTypesPolicies || ['default'];
var registration = {trustedtype:{config:{},type:null},sanitizerFn:function(val){ return val; }}
if(typeof trusted_types === 'string'){
trusted_types = trusted_types.split(' ');
}
if(trusted_types.length == 1){
// get the object that holds the "createHTML()` method
registration = window.TrustedTypes.htmlRegistrations[trusted_types[0]];
}
var old_value = String(value);
// the sanitizer registered to a trusted type policy is executed
var new_value = registration.sanitizerFn(old_value)
// if inclusions are configured to be blocked then do not execute the descriptor setter
if(registration.trustedType.config.blockIncludes){
return old_value;
}
// Call the original setter
return originalDesc_innerHTML.set.call(this, new_value);
}
});
} I do hope you can now understand the core of the idea in my proposal. The code above works on all major browsers (luckily in IE8 too).
As i said before, const TrustedTypePolicy = window.TrustedTypes.createPolicy('my-policy', {
createHTML(potentiallyUnsafeHtml){
return DOMPurify.sanitize(potentiallyUnsafeHtml)
}
});
document.getElementById("upper-card").classList.add(
// an attribute value could contain potentially malicious code so how do we sanitize ?
TrustedTypePolicy.createHTML(
document.body.getAttributeNode('class').nodeValue
)
); as opposed to this: document.getElementById("upper-card").classList.add(
document.body.getAttributeNode('class').nodeValue
); for the current spec direction for
I do know that code for policies have to be hidden away is some file. @stitchng was making a quick example that didn't require the elaborateness of how policies are defined in real-life web application projects.
This is what i believe will create developer apathy. I am a web developer and i think web developers already have a lot to think about while creating web apps. The current form of the API design for
I look forward to this and will work with you and other stakeholders to make this into either proposal. I do not wish that this counter proposal be seen as a competitor to the current proposal as is. It is not. rather it is meant to evoke more discussions around the durability of the current proposal and also a means of adding ideas into the current proposal such as this |
Thanks for your interest in this and the feedback! Could you briefly highlight the key difference between Trusted Types as specced and your proposal? I see a different API shape (it's not clear what exact shape is the proposed one, as we only get a few conflicting code examples), but I'm not sure I understand yet what the key difference is. From what I'm getting you essentially describe what our default policies do, and you have some additional behaviour that assumes existence of other native APIs (like URL sanitizers, URL types API, or CSP API) that simply don't exist, and some of them are very unlikely to exist in the future. You can check how TT API behaves - apart from the spec that describes it, the implementation is already in Chrome, there's also the polyfill available. So, for example:
Re: Developer cognitive load, I'm not sure that I understand yet what is the advantage of your proposal of the API. From what I can tell, your example is quite similar to https://gadgets.kotowicz.net/poc/tt/demo-dompurify/?tt=1, for which the crucial functionality is simply: TrustedTypes.createPolicy('default', {
createHTML: (s) => {
return 'Sanitizing anyway :) ' + DOMPurify.sanitize(s)
},
}); All the configuration knobs like |
There is a alternate proposal from @isocroft on TT which can be found here as a proof of concept and his thoughts around this is to help the ongoing discussions on how TT should be rolled out in browsers and also how simple the web developer experience should be. The idea is to "invert control" in a sense to make it possible to reduce the cognitive inertia that web developers today might have with the current spec direction of TT.
So, DOM sinks like
innerHTML
for example have knowledge of using a registered policy santizer to act on any (perhaps potentially unsafe) HTML string passed to it. Its behavior can also be modified accordingly too.The above actually proposes programmatic configurability over declarative as it is more cheaper and also doesn't require the web developer to keep all 70 DOM sinks in mind as he/she writes code based on TT. It also proposes that types should not be proliferated to deal with each kind on data passed around on the front-end. The use of a policy trusted ( types group ) or ( types form - a he (@isocroft) calls it) might be more efficient going forward.
For example: URIs for scripts / dynamic resources / stylesheets can come under a single types group or types form : URL
So, for stylesheets, there would also be:
We would love your take on this alternate proposal on TT. The POC implementation of the above is here. You can try it out yourselves to see how it works.
The text was updated successfully, but these errors were encountered: