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

Allow passing custom PRNG to generate masks #355

Open
stackinspector opened this issue May 23, 2023 · 7 comments
Open

Allow passing custom PRNG to generate masks #355

stackinspector opened this issue May 23, 2023 · 7 comments

Comments

@stackinspector
Copy link

It is possible to include a place for RNG state in the appropriate place and accept any rand_core::RngCore + rand_core::CryptoRng when creating protocol state. But the amount of work to make this change seems a bit much.
In addition, if I use ws without handshake and both server and client agree, masks seem to become useless (since no http is involved). Given that handshake is already an optional feature, can we consider making mask an optional feature as well? (This request may be exceeded.)

@stackinspector
Copy link
Author

No need for handshake, no need for TLS, no need for masks... Maybe it's time for me to fork.

@daniel-abramov
Copy link
Member

Given that handshake is already an optional feature, can we consider making mask an optional feature as well?

Since the crate is generic over Read + Write stream, turning off masking would indeed be possible (it would solve the rationale for which masking was introduced in RFC6455). However, strictly speaking, if we're talking about RFC6455, we must make sure that it's activated (though it's true that for very specific cases we could allow the user to disable masking if they know what they're doing as it would not be strict RFC6455 then). I imagine one of the ways of doing that would be to have an option in the WebSocketConfig.

No need for handshake, no need for TLS, no need for masks...

Everything except masking is already a feature and could be disabled.

@stackinspector
Copy link
Author

But my main purpose is to optionally remove dependency rand...

@stackinspector
Copy link
Author

stackinspector commented May 23, 2023

Perhaps we can use the same way as ed25519-dalek...
However the only use rand::random() is already "hardcoded", so I could have done the replacement myself.

@daniel-abramov daniel-abramov changed the title enhance: use replaceable PRNG to generate masks Allow passing custom PRNG to generate masks May 27, 2023
@marceline-cramer
Copy link

marceline-cramer commented Dec 4, 2023

If a custom RNG (or an implementation of a new MaskSource trait, perhaps) were to be configured in WebSocketConfig, what would be a reasonable default if the rand dependency isn't enabled? Should it be optional, default to None if no rand feature, and have the client panic if a mask source isn't given?

@marceline-cramer
Copy link

I'm interested in this issue because I'd like to use tungstenite in a bare-bones Wasm environment with no browser APIs, so the getrandom dependency introduced by rand's std feature is problematic. I don't necessarily need the rand dependency completely gone.

@marceline-cramer
Copy link

marceline-cramer commented Dec 4, 2023

I'm also slightly concerned about the performance hit of using Box<dyn RngCore + CryptoRng> to avoid adding a generic to WebSocketConfig. A dynamic dispatch per mask generation seems expensive. One solution would be to buffer lots of masks in advance using one dynamic dispatch, but that'd require a buffering system to be implemented.

Benching is probably needed.

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

No branches or pull requests

3 participants