Skip to content
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 support in workbox-window's register() #2855

Closed
budarin opened this issue May 31, 2021 · 11 comments · Fixed by #2872
Closed

Trusted types support in workbox-window's register() #2855

budarin opened this issue May 31, 2021 · 11 comments · Fixed by #2872
Labels
Feature Request TypeScript Issues related to our TypeScript annotations. workbox-window

Comments

@budarin
Copy link

budarin commented May 31, 2021

Library Affected:
workbox--window

Browser & Platform:
Any browser with support of CSP 2.0

Issue or Feature Request Description:
When installing the CSP policy "require-trusted-types-for 'script'" - the security error occurs in the browser

image

Usefull link : Prevent DOM-based cross-site scripting vulnerabilities with Trusted Types

@jeffposnick
Copy link
Contributor

Hello @budarin—thanks for reaching out about this.

Is that code you shared from workbox-window, or is it your own code for registering a service worker? It's transpiled, so I can't tell for sure.

I'm not 100% clear on the implications of using trusted types and how Workbox could help with the TrustedScriptURL requirement, but I'll escalate the question to some of my teammates that work in that space and see what they have to say.

@jeffposnick jeffposnick added the Needs More Info Waiting on additional information from the community. label Jun 1, 2021
@jeffposnick
Copy link
Contributor

...actually, does doing something like what's outlined in Example 2 of https://w3c.github.io/webappsec-trusted-types/dist/spec/#policies-hdr help, where you pass in an object with a createScriptURL() function to createPolicy(), and the return value of passing your service worker's URL to that createScriptURL() is then passed to navigator.serviceWorker.register()? From reading the spec, that seems to be what's required...

@budarin
Copy link
Author

budarin commented Jun 1, 2021

Hi!

Is that code you shared from workbox-window, or is it your own code for registering a service worker? It's transpiled, so I can't tell for sure.

It's transpiled workbox code

...actually, does doing ...

I 've got what you're talking about.
I thought you'd implement this on the library side - user should pass in constructor a trusted type object

Well, okay - this option solves the problem!
Thanks for the tip

@budarin
Copy link
Author

budarin commented Jun 1, 2021

I don't know if I should close this discussion - will you still think about the implementation?

@budarin
Copy link
Author

budarin commented Jun 1, 2021

I implemented a simple policy for verifying that a URL belongs to the same domain as the document is, which can also be implemented in a package

if (window.trustedTypes && window.trustedTypes.createPolicy) {
    window.trustedTypes.createPolicy('default', {
        createScriptURL: (urlStr: string) => {
            if (typeof urlStr !== 'string') {
                // eslint-disable-next-line fp/no-throw
                throw new TypeError('invalid URL');
            }

           const url = new URL(urlStr, window.location.origin);
           if (url.origin !== window.location.origin) {
                // eslint-disable-next-line fp/no-throw
                throw new TypeError('invalid URL');
           }

            return urlStr;
        },
    });
}

but its not a protection - its a simple stub to mute security errors

@jeffposnick
Copy link
Contributor

Okay, let's leave this open to track the feature request of having workbox-window handle that for you.

But, for what it's worth, navigator.serviceWorker.register() already has a built-in same-origin check, and will throw and fail registration if you pass in a cross-origin URL. (This behavior is part of the service worker specification.) So I'm not sure how useful this security policy actually is in practice, but I understand that folks might need to implement something in order to meet the trusted types requirements.

@jeffposnick jeffposnick added Feature Request workbox-window and removed Needs More Info Waiting on additional information from the community. labels Jun 1, 2021
@jeffposnick jeffposnick changed the title [Security] Error: "This document requires 'TrustedScriptURL' assignment." Trusted types support in workbox-window's register() Jun 1, 2021
@budarin

This comment has been minimized.

@budarin
Copy link
Author

budarin commented Jun 5, 2021

Without changes in WorkBox - I can not apply trusted type security on my site.
You should provide a constructor with custom TrustedType policy object parameter with createScriptURL.

let wb;

if (window.trustedTypes && window.trustedTypes.createPolicy) {
    const customPolicy = window.trustedTypes.createPolicy('myPolicy', {
        createScriptURL: (urlStr: string) => {
            if (typeof urlStr !== 'string') {
                // eslint-disable-next-line fp/no-throw
                throw new TypeError('invalid URL');
            }

           const url = new URL(urlStr, window.location.origin);
           if (url.origin !== window.location.origin) {
                // eslint-disable-next-line fp/no-throw
                throw new TypeError('invalid URL');
           }

            return urlStr;
        },
    });
    
    // pass in the policy
    wb = new WorkBox(customPolicy, '/sw.js'); // ...navigator.serviceWorker.register(customPolicy.createScriptURL('/sw.js'))
} else {
    wb = new WorkBox('/sw.js');
}

https://webappsec.dev/assets/pub/Google_IO-Securing_Web_Apps_with_Modern_Platform_Features.pdf

@jeffposnick
Copy link
Contributor

I took a closer look at this, and I can see how we can improve things from a TypeScript perspective, but I'm not sure that it is necessary to overload the constructor to explicitly support trustedTypes.

What I can do is modify the constructor so that it's clear that the parameter can either be a string or a TrustedScriptURL. From what I can tell, the "official" TypeScript runtime libraries don't have trusted types support yet, so I was planning on pulling in the definition from https://github.com/DefinitelyTyped/DefinitelyTyped/blob/cd7943ca0fa97529efb644168073ab494f9bf1bf/types/trusted-types/lib/index.d.ts#L18-L21. Is that what you're using for your types?

Here's a quick standalone page that works for me (except for the TypeScript types), allowing you to just pass the createScriptURL return value in to the constructor:

<html>
  <head>
    <title>Trusted types</title>
    <meta http-equiv="Content-Security-Policy" content="require-trusted-types-for 'script';">
  </head>
  <body>
    <script type="module">
      import {Workbox} from 'https://storage.googleapis.com/workbox-cdn/releases/6.1.5/workbox-window.dev.mjs';

      let urlToRegister;
      const untrustedURL = './sw.js';

      if (window.trustedTypes?.createPolicy) {
        const customPolicy = window.trustedTypes.createPolicy('myPolicy', {
            createScriptURL: (urlStr) => {
              const url = new URL(urlStr, window.location.origin);
              if (url.origin !== window.location.origin) {
                  throw new TypeError('invalid URL');
              }
              return urlStr;
            },
        });
        
        urlToRegister = customPolicy.createScriptURL(untrustedURL);
      } else {
        urlToRegister = untrustedURL;
      }

      const wb = new Workbox(urlToRegister);
      wb.register();
    </script>
  </body>
</html>

@jeffposnick jeffposnick added the TypeScript Issues related to our TypeScript annotations. label Jun 17, 2021
@jeffposnick
Copy link
Contributor

Okay, I actually had to add in a few explicit .toString() calls, so there were some code changes in #2872

This seems to work, for anyone looking for an example:

import {Workbox} from 'path/to/workbox-window';
import {TrustedScriptURL, TrustedTypesWindow} from 'trusted-types/lib';

const ttWindow = window as unknown as TrustedTypesWindow;

let urlToRegister: string | TrustedScriptURL;
const untrustedURL = './sw.js';

if (ttWindow.trustedTypes?.createPolicy) {
  const customPolicy = ttWindow.trustedTypes.createPolicy('myPolicy', {
      createScriptURL: (urlStr) => {
        const url = new URL(urlStr, window.location.origin);
        if (url.origin !== window.location.origin) {
            throw new TypeError('invalid URL');
        }
        return urlStr;
      },
  });
  
  urlToRegister = customPolicy.createScriptURL(untrustedURL);
} else {
  urlToRegister = untrustedURL;
}

const wb = new Workbox(urlToRegister);
wb.register();

@jeffposnick
Copy link
Contributor

The fix is now deployed in a pre-release of Workbox v6.2.0: https://github.com/GoogleChrome/workbox/releases/tag/v6.2.0-alpha.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request TypeScript Issues related to our TypeScript annotations. workbox-window
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants