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

[FEATURE] Version without WebAssembly #49

Closed
horejsek opened this issue Nov 11, 2022 · 7 comments
Closed

[FEATURE] Version without WebAssembly #49

horejsek opened this issue Nov 11, 2022 · 7 comments

Comments

@horejsek
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Now client-zip is using crc32 implementation with WebAssembly which needs to allow unsafe evals in CSP headers.

Why is your feature relevant to client-zip?
client-zip is using WebAssembly. :-)

Describe the solution you'd like
There could be an option to avoid WebAssembly and use to vanilla JavaScript implementation.

Describe alternatives you've considered
It could automatically fall back to the non-WebAssembly implementation when an exception is thrown.

Additional context
I can create a pull request. Would you be interested in such a feature?

@Touffy
Copy link
Owner

Touffy commented Nov 13, 2022

fflate's pure JavaScript (well, more like asm.js) CRC32 is about as fast as mine, and could replace it (not as a fallback ; bundling the same functionality twice sounds wasteful). I guess it's roughly the same amount of code, after you factor in the WASM boilerplate. But I have a faster CRC32 implementation waiting for browsers to bring better SIMD support, and it'd be a shame to deprive end users of the greater speed just because lazy developers want to skip the HTTP header configuration. If you are afraid of the unsafe-eval directive, remember that it's not worse than before CSP. And if you really care that much, set up subresource integrity.

@horejsek
Copy link
Contributor Author

I'm not lazy. I don't see how it is not worse than without unsafe-eval. Can you explain?

From my perspective, unsafe-eval opens doors for more security problems, and integrity will not cut it fully. In my case, the speed of zip is actually not an issue, but security is, and that's why I'm suggesting this. I'm fine with both the fallback or the configuration option. I wanted to ask what option you would prefer before implementing it.

@Touffy
Copy link
Owner

Touffy commented Nov 14, 2022

I said it's not worse than before CSP (but yes, it's worse than default CSP). No CSP can create vulnerabilities that didn't exist before CSP. CSP are just one additional layer to prevent vulnerabilities (mostly those based on unfiltered content injection).

As far as I understand, SRI allows you to vet a particular script, allowing it to do whatever it wants as long as it wasn't altered, without generally allowing unsafe-eval. Doesn't that fully cover you ? The problem with SRI is that if you use it, you have to use it for all your direct dependencies, and that seems like too much trouble for use cases where you don't include user-generated content.

If you want client-zip to not use WASM, like I said, I would prefer to just change the default CRC32 implementation to something like fflate's, with no fallback necessary. There's currently no way to do that and retain the capability to hot-swap one WASM module for another working on the same Memory, so it would change how I'd set up my faster WASM version.

@horejsek
Copy link
Contributor Author

I see. I misunderstood you. It's the other way around: I have strict CSP already in place, and I don't want to add unsafe-eval. I didn't use SRI yet, I just read about it, and yeah, it seems like too much trouble and still opens doors; not if all is configured correctly and the code is easy to follow, but it is very easy to make a mistake in my opinion. That's why I don't want to go in this direction, instead keep the CSP header strict.

If you agree, I can take a look into it and replace the current version. But don't want to break your plans for a faster WASM version. So you are saying that hiding WASM version behind configuration, for example, so that devs can choose, is not possible? (I didn't write WASM module before, so not entirely sure what is possible. Sorry if I'm asking obvious questions.)

@Touffy
Copy link
Owner

Touffy commented Nov 14, 2022

There's a 'strict-dynamic' option with SRI where you only configure the hashes of direct dependencies (also using 'unsafe-hashes' for things like client-zip) that allows anything imported (directly or indirectly) by them, from any origin, but disallows anything else. Seems like the best of both worlds (with hashes, you can prevent injections even from your own origin or other origins that are allowed on your page, but you don't have to follow the dependency tree of static code that you already trust). It seems to only be vulnerable to chain-of-supply attacks, and those are nearly impossible to prevent from your side anyway. But 'strict-dynamic' is mutually exclusive with all the other ways to allow dependencies, which is what I meant by "too much trouble" : you have to completely switch over to SRI hashes.

I don't want the WASM module to be configurable, I want to use feature detection to pick the best module for the user without the developer having to do anything (I want client-zip to work "out of the box" as much as possible, though I'd agree CSP is a problem there).

You should look at my PR for faster CRC32. The idea, currently, is to have a default module that can be loaded synchronously, and then to feature detect SIMD support (though in your case, it would fail because of CSP) and if it passes, hot-replace the CRC32 module (even while client-zip is processing a file) with the faster version if it is supported.

I think it could be done with a JavaScript implementation as the default, too. As I said, the way I did it in the PR wouldn't work, but reusing the Memory was just a nice detail (that reduced the necessary code, keeping the Memory with the precomputed table that was already generated). It's still doable without that trick.

@Touffy
Copy link
Owner

Touffy commented May 7, 2023

Merged and published in 2.4.0 and 1.6.0

@Touffy Touffy closed this as completed May 7, 2023
@xvilo
Copy link

xvilo commented Jun 3, 2024

I'd like to express my thanks of the new versions and this change. We had some issues with people not being to load the page because WASM is disabled by default in some cases. Updating the dependency did fix this 👍

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

3 participants