open-eid / chrome-token-signing

DEPRECATED Chrome and Firefox extension for signing with your eID on the web
https://github.com/open-eid/chrome-token-signing/wiki
GNU Lesser General Public License v2.1
206 stars 75 forks source link

improved page script csp solution for #158 #211

Closed mbakhoff closed 1 year ago

mbakhoff commented 2 years ago

Issue https://github.com/open-eid/chrome-token-signing/issues/158 was resolved by a workaround: have each page that uses token signing include the page script manually. This has some drawbacks:

This PR proposes a better solution: inject the script tag but link directly to the page.js inside the extension. The result is something like this:

<script type="text/javascript" data-name="TokenSigning" src="chrome-extension://ckjefchnfjhjfedoccjbhjpbncimppeg/page.js"></script>

This is better because it's no longer a inline script (does not need unsafe-inline permissions), so it's possible to use a more specific exception in the CSP. But even that doesn't seem to be necessary: the extension urls seem to bypass CSP automatically. The following CSP works without any changes: Content-Security-Policy: default-src 'self';

See also https://stackoverflow.com/a/9517879

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging bfb5329faeca2350fd634e487c8f46b73f91930a into b0b9f266395f1419bf3687007e4749dca31add35 - view on LGTM.com

fixed alerts:

taneltm commented 2 years ago

That seems like a great idea!

The only problem I see is that web_accessible_resources was added for Safari 14. So this would break compatibility with earlier Safari versions, for example for users using old MacBook Pro/Air or iMac from 2011 and earlier.
Those users could use Chrome or Firefox instead, but the problem is that they wouldn't get any notification that they would need to do so. For them it would seem like a website which worked before just doesn't work anymore.

mbakhoff commented 2 years ago

Based on https://stackoverflow.com/a/47435990 webextension support for Safari was first introduced in Safari 14, so earlier versions shouldn't be able to use this webextension even now. The repo also contains /SafariAppExtension/TokenSigningExtension/script.js - is that for the older safari versions?

Additionally https://developer.chrome.com/docs/extensions/mv3/manifest/web_accessible_resources/ has this note

Prior to Manifest V2 all resources within an extension could be accessed from any page on the web.

So it seems web_accessible_resources is not an issue for older browser versions. However manifest v2 support may still be an issue if you're interested in backwards compatibility with very old browsers. One option would be to feature detect it as follows

if (chrome.runtime.getURL) {
  s.src = chrome.runtime.getURL('/page.js');
} else {
  s.innerHTML = "(" + pageScript + ")();";
}

For that we'd need to keep the pageScript copy inside the content script.

kristelmerilain commented 2 years ago

Thank You for the contribution! Please note that the active development and management of the Token Signing component has ended due to the transition to the new web authentication and signing solution (Web eID). We are happy to accept your proposals in the new Web eID project repository: https://github.com/web-eid. We won't be accepting pull requests or responding to issues for this project anymore.