How does this work when you have a dependency included twice? #222
Comments
|
Thanks for pointing that out. We're actively working on addressing that use
case. One proposed solution is in
#220, another one is to
have a CSP keyword that allows duplicate names.
The API as specced now, will allow duplicate policy names when * is used in
a trusted-types directive (the Chrome impl. lags behind a bit), but we're
working on providing a better solution for this.
…On Thu, Sep 26, 2019, 10:32 Kevin Gibbons ***@***.***> wrote:
It is very common for multiple versions or copies of a library to be
present on a page, even within a single bundle. But if I understand
correctly only one of those will be able to call createPolicy with that
library's policy name (assuming libraries don't choose to include their
version number in their policy name, which, since updating a policy name
will require reconfiguring one's server, I imagine they would not). The
dependency which doesn't win that race will then presumably be broken.
Is there a better story for this than "avoid having multiple copies of a
library on your page"?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#222>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAA7JK4L5AIFGFSNRRPPUV3QLRXQ5ANCNFSM4I2WZVNA>
.
|
|
It looks like policies with a duplicate name will be allowed by using a CSP keyword (e.g. |
|
The current specced behavior conflates two things when a wildcard
I'd like to separate those tho thing, for the purpose of allowing colliding on policy names (e.g. when a dependency is included twice), without forcing authors into losing control over policy names used in their applications totally. To remind of the threat model - TT are not there to protect agains malicious developers, or malicious dependencies. Some setups of TT may help catch those cases, but we assume that if you already have JS execution in the application, a capablity of calling DOM XSS sink functions with arbitrary values is not really useful (you can already do much more). So, my proposal is to:
Does that sound good? |
|
Does this mean that when using |
|
Yes, in CSP the directive values are usually source lists (
https://w3c.github.io/webappsec-csp/#framework-directive-source-list), and
so far there's no directive that would give meaning to the positioning of a
given element (e.g. a keyword).
…On Sun, Nov 3, 2019 at 11:38 AM Emanuel Tesař ***@***.***> wrote:
Does this mean that when using allow-duplicate in trusted-types directive
all of the policy names can be duplicated? If all of them, can this "flag"
be used anywhere inside directive?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#222>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA7JKYBAZVYJF44T4Y3YA3QR2S25ANCNFSM4I2WZVNA>
.
--
Best regards,
Krzysztof Kotowicz
|
|
A small comment: Has there been any thought given to allowing policy names to occur multiple times, with the behavior that you could create only that many instances of a given policy? So for example |
Correct - it does relax the controls over which policies can be created in the code. However the threat model of TT is not to protect against malicious authors (of dependencies or 1st party ones). To protect the application from threats like that in the client alone, an integrity check on the whole codebase - including the dynamically introduced code - would be required. In other words, TT are not to stop XSS attacks via backdorred code - they cannot achieve that. For example, developers able to pass code to
We are thinking on how to enable more precise control, even when some policy names are used multiple times - the repeated name is one example (others were e.g. It seems like for now we were just looking into how to define the header (more precisely: directive) syntax that doesn't block us from introducing more precise controls later. The narrow problem we know we need to fix now is to allow name collisions while still controlling the names used, as this in an improvement over how |
|
I think I understand the problem, but I'm not super convinced of the solutions. I wonder: The named policy store is essentially a security-critical, global variable. Who is supposed to have the last word over its content? Whoever writes the CSP? Or each dependency for their own domain? Brainstorming a few alternatives for discussion purposes: Alternative: Alternative: Alternative: Alternative: Alternative: |
Hm. I had understood from discussions re: tc39/proposal-dynamic-code-brand-checks#1 that part of the purpose of this proposal was to allow code bases to be more effectively audited: that is, to make it so that a security team with control over the CSP of a page could review all current uses of Is that not the case? |
It is the case. Debugging which policy names are still available to use, and intentionally creating one that doesn't violate the explicit restrictions set for the application is hardly accidental. |
|
Maybe I am still missing something but for me this makes most sense to restrict this as much as possible. I personally, like the following syntax the most (but the syntax is not really important):
This of course contradicts #222 (comment). I think, that if we can implement the API with stronger policy restrictions, why not implement it? I think the same as in #222 (comment) and I guess the debugging and auditing might be easier with tighter rules. Because if you are able to specify the exact amount of policies, you know exactly how many to look for in the codebase (And I think this is very useful). |
I'd argue it should be whoever controls the headers (CSP in this case).
Thanks for those, some are quite interesting! I'll put some comments inline.
That's feels like a reintroduction of the
That is more precise solution, the only problem I see is the syntax. Perhaps As such:
Ouch. Sounds like quite surprising behavior (e.g. the code from a different module would run unexpectedly).
I didn't fully get it, sorry, but one problem I see is tying this to script URLs. Unfortunately these change often in the application, and the authors don't expect behavior changes just because they repackaged the application, or the algorithm for the bundler changed.
The problem here is that dependencies we're talking about can be packaged in various ways, e.g. together with the main script, added as a second |
Sure, but neither is any other use of
This is my favorite of the suggestions so far, fwiw. |
|
Perhaps we should think of approaches that remove specific mechanisms for controlling policy creation from the standard, and delegate this function to application-deployer or application-framework provided code. We are evidently finding it difficult to design an approach based on header-declared named policies that is both sufficiently rigorous (in that it achieves the goal of reasonably strongly confining security-relevant code into areas that are straightforwardly reviewed) and also sufficiently flexible. This suggests that it's possibly a mistake to bake a specific way of controlling policy creation into the standard. ProposalRemove the procedure by which policy creation is guarded based on CSP header (section 4.5.3) from the spec. Instead, provide a mechanism by which an application can install exactly one "meta policy" (some thoughts on details thereof below). This meta policy takes the form of a callback that is invoked by the TrustedTypePolicyFactory when createPolicy is called. The callback decides whether a createPolicy invocation should succeed. This permits application deployers / security reviewers to control policy creations using a mechanism that is suitable for their environment. In environments where all application source (including third-party dependencies) is processed by a compiler/bundler, more rigorous control can be achieved; in environments where this is not the case, less so. This "meta policy" callback can control policy creation along a variety of approaches, such as: monitoring only: Allow all createPolicy calls, but record policy names and stack traces of each createPolicy call. This record can be sent back as telemetry to servers, or stashed in a global where a custom web application scanner can analyse it and look for improper usage. name-based: Essentially what the current standard proposal does. The "meta policy" callback would refer to a list of allowed policy names, for example emitted by the server into a call-stack-based: Instead of just looking at policy names, the "meta policy" callback could inspect the stack trace of the policy-content-based: Policies could be allowed not based on their name, but rather a hash of the stringified policy code. name-based, with compiler/bundler-created labels: A compiler (compiler plugin) would collect all In the latter approach, the (compiler-generated) policy names/labels function effectively as nonces. This would afford an application deployer / security auditor fairly rigorous control (and opportunity to review) over all code that creates TT policies:
Security review happens at the source code level. This approach provides a strong binding between code that has been reviewed at the source level (and could be statically identified as security-sensitive) with code that is permitted to create TT policies at runtime. Meta Policy Callback RegistrationThis of course begs the question where the code for the "meta policy" callback comes from.
Note: Libraries should never call Conventions for library authors remain the same as before: They should call |
|
I'm worried that this may only work for some authors, namely those for which it's easy to control the JS code that runs in their application and add a code block will be included only once and will run first. We've already had this issue with a default policy - the fact that it's a singleton, and has to be registered early in JS, makes it problematic to use. The problem indeed is that config in JS is easier to do than in declarative CSP. That said, if CSP syntax would be enough to configure a practical enforcement that's "good enough" for a lot of authors, I think that has a value as well for simplicity purposes. The warning sign is indeed if we try to stretch the CSP syntax too much. OTOH I see the value of 'your security rules are part of your JS program' approach, I just worry it might be an overkill to ask authors to set up this to gain the benefits of TT. Sometimes the argument comes up that controlling JS is easier than controlling response headers. At some expense of complexity, we can have both solutions, with authors choosing the best method for their application. We need to make sure that this doesn't affect libraries, that should still simply call We're actually quite close to already supporting the config-in-js. Right now I want us to come up with an extendable MVP solution that doesn't unneccessarily raise the bar of adopting TT. We know already we have to address duplicate names, and that policy creator should specify a name. How about if we went with a list of names (or a *) + So we should probably at least remove the Does that sound reasonable to all on a thread? We are trying to come up with some middle ground that works for majority of cases, without locking ourselves out from supporting the other cases as well. I don't feel there's a perfect solution here, there can only be an acceptable one ;) |
|
If policy name or rules would only be inspected, but not modified by the JS meta policy callback, and (apart from arbitrary side effects) the decision relevant to TT from the meta callback could be simply to stop the policy creation, then using regular events would work. In an imaginary implementation: trustedTypes.browserInternalCreatePolicyImpl_ = function(name, rules) {
const details = {"detail": {
name: name,
rules: /* shallow clone to prevent mutation */ Object.assign({}, rules}}};
Object.freeze(details);
var event = new CustomEvent("beforecreatepolicy", details);
if (!trustedTypes.dispatchEvent(event)) {
// policy creation prevented. console.log something.
return;
}
actuallyCreatePolicy_(name, rules);
}the following code would work: trustedTypes.addEventListener('beforecreatepolicy', (e) => {
// inspect e.details.name or e.details.rules
// throw an Error to inspect the stack
e.preventDefault(); // don't let this policy be created.
}); In this model, multiple event handlers could be setup. Any of them could cancel policy creation, but none could undo that decision. You get inspectability into the body of the policies (so secrets in policy bodies could be revealed), and you could even call the inner policy functions, but that seems fine (inner policy functions return strings, not trusted types) Any code can already do that by playing with prototypes, so we're not giving capabilities that don't already exist. It also seems to reuse a model familiar to authors, and gives enough control to implement practical limitations without allowing too much surprising and hard to debug behavior (e.g. the policy rules being suddenly replaced for a dependency from the event handler). |
|
One downside of the event-based approach is that it essentially "fails open" and it's harder to centrally ensure that the event handler is actually in effect. A middleware or other centralized mechanism in a web framework can't easily ensure or verify that the event handler is installed (other than by modifying or inspecting HTML responses, which is pretty awkward). That's where I was going with With that, a central server/framework middleware can ensure the custom policy is in place by adding a header. Other than that, I like the idea of combining the simpler header-based approach with a mechanism that allows custom code to inspect and approve/reject policies. I agree that the meta-policy callback (or event handler, however it's implemented) should not be able to modify policy, only approve/deny. Re:
It's seems this might be a slightly different situation: The default policy modifies how individual DOM XSS sinks behave, and library authors would likely be inclined to set a default policy because it might remove the need to modify call sites to pass proper types. With regards to the meta policy, there would be really no incentive for individual libraries to set one. The default is already maximally permissive (or is whatever is given by the headers in your hybrid proposal). There shouldn't be a reason for a library or framework author to further restrict that. IOW, library/framework authors will want to just |
|
Pulling up a side-note as a separate comment: With trustedTypes.createPolicy(trustedTypes.getPolicyNames()[0], {...}}.becomes a common idiom to create policy without getting a security review. It might be worth getting rid of |
|
"meta policy": I like that it moves more logic into the script, but this does sound like a lot of complexity. Generally, it strikes me that the combination of script-chosen name plus name-based enforcement has more problems than I would have anticipated. Maybe an alternative would be to use the CSP to list all the resources that may create policies? (This would be similar to other CSP items, which I think are mostly resource lists.) E.g.: |
|
What if, instead of or in addition to policy names, we included policy hashes in the CSP header instead?
|
I don't think CSP, given even its syntax limitations is the best medium for introducing that precise control over the policies. For example, hashing is sensitive even to whitespace changes, and syncing a JS bundle with what headers are emitted is not a trivial task for every author. I'd rather the CSP controls the baseline checks (e.g. policy names) that may give enough control for most authors, and move more precise rules to JS. Paradoxically, only being able to whitelisting by hash vs policy names might offer less security. I imagine there might be policies that are less strict in their rules, and end up being secure because the data that flows to them is trusted (e.g. is sanitized server side, or comes directly from the API). It would still be advisable if such lax policy rules are not used throughout the application, but only looking at the hashes one cannot prevent that. |
That forces the authors to restructure how their application is bundled i.e. to get the benefits they have to move the TT logic not just to separate functions, but to separate bundles. In general whitelisting URLs via static lists didn't work so well for |
The flexibility of defining rules in the JS as opposed to declaratively indeed makes asserting that the rules were installed more tricky. That said, there are easy ways of making certain a given function was called in a runtime - just make it send a beacon and warn on its absence. In general, the closer the security rules are to your program, the easier they are to test, and the less likely they are to go out of sync and break your application. Any other method of defining the rules can fail as well (a CSP header is not sent, there's a typo in a header name, the script 404s etc.). I actually like the decentralization here (i.e. that there may be multiple event handlers), as that allows for modularity, iow creating experiments for a subset of audience, implementing custom report-only behavior for a subset of policies etc. On a more philosophical note: Perhaps asserting security posture by looking at the headers only gives us a false impression that we're actually controlling anything and are delivering value, as it's hard to see the bugs from such a vantage point. Maybe we should just switch to more dev-friendly approaches even if it requires rewiring some of the security controls we're used to. |
|
#245 allows for duplicate names by two means:
Recently, we added character set restrictions for policy names, allowing us to introduce a new syntax later - it does feel like a JS-based mechanism for restricting policy creation would be a better fit for authors that want more precise control. |
It is very common for multiple versions or copies of a library to be present on a page, even within a single bundle. But if I understand correctly only one of those will be able to call
createPolicywith that library's policy name (assuming libraries don't choose to include their version number in their policy name, which, since updating a policy name will require reconfiguring one's server, I imagine they would not). The dependency which doesn't win that race will then presumably be broken.Is there a better story for this than "avoid having multiple copies of a library on your page"?
The text was updated successfully, but these errors were encountered: