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

Retrieve crypto from global instead of Window in WASM #1508

Closed
wants to merge 2 commits into from

Conversation

oliverdunk
Copy link

@oliverdunk oliverdunk commented Aug 9, 2022

Overview

At 1Password we use ring in our WebExtension, and extensions are currently moving to Manifest V3, where extension code will be run within a service worker. To continue using ring, I developed this patch which allows the WASM code to access crypto from the global without panicking if window is missing (like in a worker).

How to test

  1. Clone https://github.com/oliverdunk/wasm-worker (which uses the current version of ring)
  2. Run make
  3. Open http://localhost:8080/, and see that there is an error in the console
  4. Checkout the update-test branch
  5. Run make again
  6. Open http://localhost:8080/ and see that things ran successfully

Notes

There are a couple of related things that may be worth looking in to in place of this PR as is:

  • get_crypto could be cached in a thread_local if that's a performance win. I haven't done any benchmarking there yet but am happy to do so.
  • Add optional wasm32_unknown_unknown_getrandom feature #1486 is a super interesting PR and could potentially solve the same problem. We could just provide a custom implementation there.

@briansmith
Copy link
Owner

See the draft PR #1531 where I propose we delegate the randomness stuff to the getrandom crate. I'm going to close this in favor of that; I'll reopen this if we don't merge that for some reason.

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

Successfully merging this pull request may close these issues.

None yet

2 participants