search

snapview / tungstenite-rs

Lightweight stream-based WebSocket implementation for Rust.
Apache License 2.0
1.94k stars 222 forks source link

Allow passing custom PRNG to generate masks #355

Open stackinspector opened 1 year ago

stackinspector commented 1 year ago

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 commented 1 year ago

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

daniel-abramov commented 1 year ago

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 commented 1 year ago

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

stackinspector commented 1 year ago

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.

marceline-cramer commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.