-
Notifications
You must be signed in to change notification settings - Fork 210
Allow custom
to act as a Fallback Backend
#672
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
base: master
Are you sure you want to change the base?
Conversation
This looks like a rehash of the custom backend which we had in getrandom v0.2. We intentionally removed it in v0.3. Please read the previous discussion which led to it. |
I have read through that issue, and I see this as distinct in that the current Is there a specific concern with this approach beyond its similarity to previous versions of this crate? |
Firstly, this feature most certainly should not be enabled by default (it would be practically impossible to disable it). Next, there is a danger of users enabling this feature unconditionally in library crates (e.g. for testing on Web WASM), which could cause cryptic linking errors for downstream users. Finally, enabling an opt-in backend in I guess it could work as one potential option for experimentation. I am interested in hearing @josephlr's opinion on it. As a minor bikeshedding, I think the feature should be named |
Assuming by "this feature" you mean support for a fallback implementation, then agreed. This PR explicitly has the fallback disabled by default using a default feature
Unlike enabling a feature on I appreciate that Rust does have a problem with conflating Wasm with Web Browser, but I don't believe the current
This would be pretty easy to resolve for the
I appreciate you being open to discussion here. I know this is a very important crate you're protecting, and I do want to make it clear that I appreciate you engaging in the discussion.
Happy to call this a |
The crate feature should be additive, i.e. enabling the crate future should enable the custom fallback. As I wrote, it will be practically impossible for you to disable the This feature then should be enabled by crates which define the "custom" function. Note that it's not sufficient to just add the crate to your dependency tree because of how crates get linked, see the custom backend section docs for more information. In v0.2 we had the |
Also add `getrandom_no_fallback` escape hatch for security-concerned final binaries.
Ah ok I see what you were referring to now. I agree, I've inverted the feature and renamed it to
I believe it actually is sufficient to just include the relevant crate in your dependency graph by explicitly declaring the implementing function as |
Objective
As highlighted in #671, there is room for ergonomic improvements in how
getrandom
handles unsupported platforms. Sincegetrandom
already has a backend which supports external integration,custom
, it would be beneficial to end users to have a way to allow 3rd party dependencies to provide agetrandom
backend without modifyingRUSTFLAGS
.Solution
first-party-backends-only
, which will throw a compiler error if a first party backend is not available for the current target (either due to misconfiguration or lack of support). This is sem-ver compatible with a patch release.first-party-backends-only
by default to preserve the current behaviour, where an unsupported configuration will fail to compile. This is not required for sem-ver compatibility since it would at worst allow compilation where it was already failing. This is done as a conservative guard against a hypothetical supply chain attack.backends.rs
cfg_if
statement to fall back tocustom
if no other option is available. Note thatcustom
is still included as the first branch to preserve current behaviour.Intended Use
This fallback would allow HAL-style crates to include a dependency as a way to provide a fallback backend for
getrandom
. Since it relies onextern
linking, defining multiple fallback backends will cause a compile-time error, as will failing to provide one. Consider the contentiouswasm_js
backend. In order to work aroundgetrandom
's use ofRUSTFLAGS
to enable that backend,uuid
just vendored the backend. This is highly undesirable as an outcome, since there is now an increased risk ofuuid
containing a bug or vulnerability that is fixed ingetrandom
but not in their copy.With this fallback functionality, now
rust-random
or a 3rd party could publish thewasm_js
backend forgetrandom
as a dedicated crate, allowing users or libraries to activate it as they require. This may be a desirable path forward for more experimental backends anyway, as it would allow iteration without changes togetrandom
directly.Notes
getrandom
to allow this approach to work, but I don't see many difficulties here since it would be in service of improved ergonomics for their consumers.custom
backend to allow overriding theu32
andu64
methods, and potentially providing error messages back to the user, but that's a larger breaking change that isn't required for the ergonomic benefits discussed here.