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

Question: What is the best way to use this library within a Web Worker? #1657

Open
rubensworks opened this issue Sep 1, 2021 · 12 comments
Open

Comments

@rubensworks
Copy link

I've been trying to use this library to fetch resources within the browser via a Web Worker, without much success.
I will describe below what I have tried up until now, and was wondering if someone could give me some pointers on what would be the best way to go forward.

My intended workflow:

  1. Authenticate the user within the browser itself.
  2. Serialize the session and send it to the Worker within a message.
  3. Deserialize the session within the Worker.
  4. Do fetch requests using the authenticated session.

Step 1 works without problems, but I'm unsure how to handle steps 2-3.
So far, I tried two approaches:

Serialization attempt 1

Main browser process:

// session is authenticated at this stage
const sessionInfoRaw = session.clientAuthentication.getSessionInfo(session.info.sessionId);
sendToWorker(sessionInfoRaw);

(I am aware that session.clientAuthentication is private, so this is probably not a good solution)

Worker:

const session = new Session({ sessionInfo: sessionInfoRaw });
await session.login({});

=> Throws a window is not defined error at BrowserStorage.js:5 (possibly caused by #308).

I can observe that the session already contains the correct webId value.

Serialization attempt 2

Main browser process:

// session is authenticated at this stage
const sessionInfoRaw = session.clientAuthentication.getSessionInfo(session.info.sessionId);
sendToWorker({
  refreshToken: sessionInfoRaw.refreshToken,
  clientId: sessionInfoRaw.clientAppId,
  clientSecret: sessionInfoRaw.clientAppSecret,
  oidcIssuer: sessionInfoRaw.issuer,
});

Worker:

const session = new Session();
await session.login(sessionInfoRaw);

=> Throws a window is not defined error at BrowserStorage.js:5 (possibly caused by #308).

In this case, the session contains an undefined webId value.

@NSeydoux
Copy link
Contributor

NSeydoux commented Sep 2, 2021

I'm afraid the main issue is that I don't believe the session is serializable: sensitive information is kept in-memory in the fetch closure, which isn't readable on purpose due to security concerns. This means that the Web Worker would have to do the full authentication process in order to get an authenticated session, which involves a redirection in most cases, so I don't think this could work.

One thing to consider would be to do the front channel exchange in the main window, since that's where the redirection happens, and to handle the incoming redirect in the Web Worker. Provided the worker is given a session with access to the local storage (to be able to retrieve the PKCE token), if the main window forwarded the redirect IRI (containing the authorization code), the Web Worker should be able to perform the back channel exchange. However, I'm not sure to what extent this works for you, because I expect you'd want to spawn workers for the lifetime of a given query, rather than keep one worker around for a given session ? And also, doing this will prevent the main window from being authenticated, since the authorization code is single-use.

And you're absolutely right, #308 would also need to be resolved, but I feel the issues I mentioned above are a bit more fundamental to the design. If we resolve these first, #308 shouldn't be too much.

@rubensworks
Copy link
Author

Thanks for your reply @NSeydoux!

Provided the worker is given a session with access to the local storage (to be able to retrieve the PKCE token), if the main window forwarded the redirect IRI (containing the authorization code), the Web Worker should be able to perform the back channel exchange. However, I'm not sure to what extent this works for you, because I expect you'd want to spawn workers for the lifetime of a given query, rather than keep one worker around for a given session ?

You're right, new workers can be spawned, and are expected to make use of the same session. Requiring the user to re-authenticate for every query execution may lead to poor UX.

I'm afraid the main issue is that I don't believe the session is serializable: sensitive information is kept in-memory in the fetch closure, which isn't readable on purpose due to security concerns.

I'm wondering if there really would be a security issue regarding the serialization of this session.
I would suspect that from the moment that an attacker is able to obtain the session object, that the attacker can already do everything he wants, independent of whether or not serialization is possible.

Enabling something like this may fix the Worker problem:

// In main window
const encodedSession = session.serialize();

// In worker
const session = Session.deserialize(encodedSession);

In any case, I'm happy to help further with finding/implementing a solution for this 🙂

@NoelDeMartin
Copy link
Contributor

I faced this problem in one of my apps (Media Kraken), so even though it's a workaround, I'll share my solution in case it's useful to others.

Basically, as mentioned in this issue it's not possible to serialize the session. It's also not possible to handle the redirection in the worker because Web Workers can't use localStorage.

So the workaround I came up with is to implement a fetch method in my worker that posts a message to the main thread, which in turn executes the authenticated request using the library and returns the response (posting a message back to the worker). The performance is probably not great, but at least the developer experience is quite good (I haven't found any issues so far).

In case you want to look at my implementation, these are the relevant parts:

@rubensworks
Copy link
Author

Thanks for your input @NoelDeMartin. This solution probably works well in most of the cases. Unfortunately, it won't work in my case, since it is very request-heavy and performance-dependent, and makes use of potentially very long streaming HTTP responses, which are not easily/efficiently handled via this approach.

@rubensworks
Copy link
Author

@NSeydoux I have implemented a prototype that makes authenticated requests work within a Web Worker,
and I would like to run this idea by you to see if it makes sense for me to polish this prototype and submit it as a PR.

In essence, the overall idea is that a communication channel is set up between the main window and the worker.
The following flow applies:

  • Worker wants to do an authenticated request.
  • Worker sends request information to the main Window.
  • Window constructs authenticated headers for the request information.
  • Window sends back authenticated headers to the Worker.
  • Worker performs request using the authenticated headers.

This approach ensures that all secrets never leave the session object within the main window,
but the actual fetch requests are still being done from within the worker (so there is no significant loss in performance).
Furthermore, this communication channel requires an explicit opt-in from both worker and window.

I've implemented some helpers that implement this flow, so that users don't have to be concerned about the implementation details of this flow.

\cc @pmcb55

@RubenVerborgh
Copy link
Contributor

RubenVerborgh commented Nov 19, 2021

@rubensworks This is the flow I've always wanted to implement, back in the days of solid-auth-client even (and we had a prototype with the same flow as well).

Given that the exchanged information from the main window to the worker is request-specific, and not usable for other requests or contexts nor leaking any other information, I think this is the way to go.

A conceptual summary of this modus operandi is that the main window has a service running that signs requests (buildAuthenticatedHeaders); and this same service can be accessed by the main window and by workers (but the main window probably has more direct access to it). And the services for actually performing requests are different in the main window and workers, because of the separate contexts.

The main challenge being that the exchanged request details (in particular to headers) should be flat JSON, not special objects.

Also given the thumbs-up by Pat, let's go ahead and turn this into a PR. Thanks a lot!

@NSeydoux
Copy link
Contributor

I'm interested to see the PR, because I have some questions. In particular, the current model aims at ensuring that having a malicious script loaded from the same origin as the app has limited consequences. If there exists a function to generate authentication headers, doesn't that mean that such a malicious script could grab the access token, and sign any header they want, enabling a third-party actor to issue authenticated requests for the lifetime of the token ?

@RubenVerborgh
Copy link
Contributor

RubenVerborgh commented Nov 22, 2021 via email

@rubensworks
Copy link
Author

@NSeydoux I'll submit a PR for review asap (hopefully later this week, but possibly only next week).

@elf-pavlik
Copy link

@NSeydoux could you post here a concise summary of how exactly the PR didn't meet the security requirements?
I think running the client in WebWorker in the browser would allow following the best practice of only running UI-related functionality on the main thread. There must be a way to accomplish it.

@elf-pavlik
Copy link

Do you happen to have any updates on this? I'm starting the implementation of updated WebPush Notifications.

Other issues may be related to restoring the session from the Service Worker. It would be excellent to know what is the latest status here.

@rubensworks
Copy link
Author

@elf-pavlik FYI, we're still using my fork for Comunica's web client, which is also available on npm: https://www.npmjs.com/package/@rubensworks/solid-client-authn-browser

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

5 participants