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

ServiceWorker verification #1720

Open
alvarolm opened this issue Jun 1, 2024 · 3 comments
Open

ServiceWorker verification #1720

alvarolm opened this issue Jun 1, 2024 · 3 comments

Comments

@alvarolm
Copy link

alvarolm commented Jun 1, 2024

Due to the sensitive capabilities (provided by the service worker) like request interception and others. The service worker script should be subjected at least to the same standards as common scripts in regards to security.

please keep in mind that currently:

  • there is absolutely no safeguard in place to verify the integrity or ownership of the service worker script.
  • it's not uncommon to see service workers hosted in shared domains where the domain administration is not in the control of the web author.
  • there is no certainty of when the service worker script could be updated.

Taking into consideration the unnecessary complexity and lack of adoption of previous initiatives like: "XML Signature Syntax and Processing" I believe a simpler approach is needed.

Acknowledging previous valid concerns like: #578 (comment) from @annevk here is my proposal:

Allow the web author to reject a service worker installation within a reasonable time frame, after which it is forcibly installed. In this manner we have time to properly address the situation and gracefully recover while being in control.

Implementation:

The "verify" event in ServiceWorkerRegistration: which callback is executed after downloading and before installing a service worker.
This event permits the author to access the downloaded service worker script using the "event.ServiceWorkerScript" property and define their own verification logic using the event methods: "event.accept()" or "event.reject()". By default if no event callback is defined It is understood that it is always accepted.

Add the option "rejectTimeout" to the ServiceWorkerContainer: register() method.
This option specifies the maximum duration (in seconds) where the rejected verification is respected, default value is 86400.

In the case of a rejected verification:

  • on first rejection: do not install the new version of service worker, keep the current one.
  • compute and store "rejected expiration": timestamp of rejection plus "rejectTimeout" option value if not stored already.
  • from now on every time a service worker installation request is triggered and verification rejected again check if the "rejected expiration" has been reached (current time is equal or greater than "rejected expiration"), if it has been reached proceed to install the service worker as if the verification had been accepted  

In the case of an accepted verification:

  • clear "rejected expiration" if it exists.
  • proceed with new service worker installation.

I hope there is at least a conversation about this subject of importance as involves a major security issue.

álvaro


faq:

How would the verification be done? Would it be based on the worker script contents?

it would be left to the user to implement the verification as it's just a callback placed before installing the service worker that accepts or rejects the new service worker script.
I personally would program the http server to respond the service worker script + an Ed25519 signature as a header value, and distribute the public key with a common script backed by a Subresource Integrity (SRI) checksum.
In this manner the integrity of the web application could be checked from the serving html, there are tools like https://github.com/tasn/webext-signed-pages that would facilitate this step.

Would it be done by the installing page, or by the worker itself when updating?

it would be defined in a common script in ServiceWorkerRegistration context, not in the worker itself.

Why would the rejection be temporary?

in order to avoid locking the service worker in a degraded state as result of some misconfiguration or any other reason.
for example: to reverse a bad deployment with a faulty verification method like a compromised secret signing key.

it (the porposal) also would allow the flexibility to specify the rejection duration, after which it would install the worker anyways.
This gives us time to react and properly regain control or fix the issue that generated the rejection.
As developers or authors of web applications commonly do not have complete control of the hosting environment, this provides us with great reassurance.

@alvarolm
Copy link
Author

alvarolm commented Jun 2, 2024

maybe some of the participants of related issues might be interested ?
@h2non @Sean4572435243 @jdalton @mkruisselbrink

@Sean4572435243
Copy link

I see what you're trying to accomplish, but here are my thoughts (hopefully without offending). Your idea creates the potential of a schism where some clients are operating on the latest service worker, while others are not, which creates a potentially very complex inconsistent environment that could corrupt the shared domain's intent behind updating their service worker, upending the basic premise behind who controls the service worker and forcing domain admins to accommodate potentially many historical versions of serviceworkers if they change frequently enough. It's doubtful the client will be smart enough to know what's the best decision, particularly if the serviceworker update addresses a critical security flaw/bug.

I believe the existing approach of scoping serviceworkers to domains where all clients update in sync represents the more encapsulated approach, and much easier to digest conceptually. In other words, if your use case is to control the serviceworker version, then take control by owning a specific domain for the relevant clients, rather than trying to make a shared domain fit, unintuitively inverting control of the updates to the client.

I've been an advocate of disabling serviceworker updates entirely in scenarios where the serviceworker cannot change (IPFS), but you're still looking for updateability, so it's a different use case.

@alvarolm
Copy link
Author

alvarolm commented Jun 3, 2024

I see what you're trying to accomplish, but here are my thoughts (hopefully without offending). Your idea creates the potential of a schism where some clients are operating on the latest service worker, while others are not, which creates a potentially very complex inconsistent environment that could corrupt the shared domain's intent behind updating their service worker, upending the basic premise behind who controls the service worker and forcing domain admins to accommodate potentially many historical versions of serviceworkers if they change frequently enough. It's doubtful the client will be smart enough to know what's the best decision, particularly if the serviceworker update addresses a critical security flaw/bug.

I believe the existing approach of scoping serviceworkers to domains where all clients update in sync represents the more encapsulated approach, and much easier to digest conceptually. In other words, if your use case is to control the serviceworker version, then take control by owning a specific domain for the relevant clients, rather than trying to make a shared domain fit, unintuitively inverting control of the updates to the client.

I've been an advocate of disabling serviceworker updates entirely in scenarios where the serviceworker cannot change (IPFS), but you're still looking for updateability, so it's a different use case.

I understand your worries, and think out of sync will always be an issue, these might be due to networking, caching or from any number of reasons.

the proposal aims to preserve the correct state of the web application as defined by the author (as it should be), while also having the flexibility to fallback into the default state, I believe this is a really well balanced solution that could appease fears from both sides.

offcourse being backwards compatible this proposal allows you to chose the option to verify, if not then there will be no side effect. Understanding the proposal as simple optional step that would bring peace of mind and assurance for my self, and others developers is key.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants