whatwg/loader

Proposal: CSP-compatible hooks

guybedford opened this issue · 6 comments

This is in response to #69 (comment).

The current loader hooks effectively provide inline eval support and do not provide the ability to ensure source text integrity.

In order to support CSP, SubResource Integrity and nonces, it is important to be able to provide these guarantees.

I've included a suggestion here, to illustrate how this might work with an adjusted fetch hook to aid discussion along these lines.

Illustrative Proposal

  1. Instead of having the fetch hook return a source string, we can have the fetch hook itself return either a URL to fetch or undefined if there is nothing to fetch:

      System.loader[Reflect.Loader.fetch] = function(entry, key) {
        return encodeURI(key);
      };

    It is important that we can return a URL that is different to the key, as while the key will likely be a URL, there may still be some modifications necessary before fetching such as encoding and stripping plugin metadata.

    The environment Fetch implementation is then delegated to for loading that URL entirely. Perhaps with some default headers or fetch options.

    The fetch hook payload is then populated as the fetched source (despite the hook itself returning a URL). This way the source is available to the subsequent translate and instantiate hooks as the fetch hook result.

  2. A flag can then conditionally disable the translate hook. Perhaps from a security perspective translate should be disabled by default, and turned on with a flag rather:

      System.loader.unsafeTranslate = true;
      System.loader[Reflect.loader.translate] = function() {
        // ...
      };

    Without unsafeTranslate set, the loader by default skips the translate stage.

With the above, we're able to maintain full support for the current hook use cases, while providing a guarantee in the loader that the source that was fetched is the source that was executed. These guarantees then give a path to inline <module> nonce support, SubResource Integrity support for <module integrity="..."> and CSP compatibility.

The unsafeTranslate or loaderTranslate flag would of course need to be an environment-level option.

Perhaps simply following the CSP policy unsafe-inline itself, or even via a custom HTML meta tag.

why a new issue? let's keep the discussion in one place.

How is this:

System.loader[Reflect.Loader.fetch] = function(entry, key) {
    return encodeURI(key);
};

different from

System.loader[Reflect.Loader.fetch] = function(entry, key) {
    return 'here is my content to be evaluated?';
};

I don't think this problem is in the realm of the pipeline, this security measures can be implemented at the lowest level by just marking the content that can be executed, whether that content was fetched via default browser, or just a fetch process on the user-land with the proper content type.

Sure, that's understandable - I just didn't want the proposal to be missed at the bottom of #69 which is marked as a question, while I believe this is an important issue, but I can copy it across if you would prefer?

In the two cases you show above they are different in that in case (1), with translate disabled via a flag, the source that is executed can't be modified, while in case (2) the content string is an execution injection vector, unless the fetch method on the loader can be somehow protected or frozen reliably.

Won't the environment fetch use window.fetch in the browser? If so you can use Service Workers to violate the fetch integrity anyways. I'm not a fan of getting rid of fetch (and turning it into locate).

@matthewp internal browser optimizations may find it better to skip the fetch spec, which is primarily a user-facing interface. That's a good point, but the scope of the service worker isn't the page itself so that there isn't a direct injection vector, just like for normal script loads.

@caridy closing to track in #69.