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

Use configuration flags to switch between backends #504

Merged
merged 12 commits into from
Oct 7, 2024
Merged

Conversation

newpavlov
Copy link
Member

@newpavlov newpavlov commented Oct 4, 2024

This PR removes linux_disable_fallback, rdrand, js, test-in-browser, and custom crate features. As their replacement two new configuration flags are introduced: getrandom_browser_test and getrandom_backend. The latter can have the following values: custom, rdrand, linux_getrandom, wasm_js, esp_idf.

getrandom_backend allows to change default backends which resolves issues like #346 and provides more flexibility to users. For example, it allows to use RDRAND or RDRND (see #494) directly instead of syscall-based interfaces. We previously did not allow such overwrites because of security concerns, but they do not apply in this case since configuration flags used by a project can not be controlled by its upstream dependencies.

The register_custom_getrandom! macro is removed in favor of explicitly defining the __getrandom_custom function. It does not look like the macro was widely used in practice and it's probably easier to explain the extern fn approach (especially to embedded developers) than the "magical" registration macro.

The new configuration flags also allow a great simplification of our testing code.

Finally, ESP-IDF support is no longer enabled by default because of the concerns raised in #397. Users can enable it by enabling the esp_idf opt-in backend.

Closes #230
Closes #346
Closes #397
Closes #498

#[cfg(not(any(target_arch = "x86_64", target_arch = "x86")))]
compile_error!("`rdrand` backend can be enabled only for x86 and x86-64 targets!");
#[path = "rdrand.rs"] mod imp;
} else if #[cfg(getrandom_backend = "wasm_js")] {
Copy link
Member Author

@newpavlov newpavlov Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could split wasm_js feature further into wasm_node and wasm_web parts. But I don't know whether people need WASM files which can run both on Node and Web.

Alternatively, we could make the JS backend enabled by default and require users targeting non-JS WASM to enable the custom backend. But personally I dislike such approach since it's technically wrong and we play into the poor handling of the wasm32-unknown-unknown target by the Rust ecosystem.

// `len`).
let dest = uninit_slice_fill_zero(dest);
let ret = unsafe { __getrandom_custom(dest.as_mut_ptr(), dest.len()) };
let ret = unsafe { __getrandom_custom(dest.as_mut_ptr().cast(), dest.len()) };
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that in v0.3 we no longer need to zeroize the destination buffer since we can specify that uninitialized buffers may be passed to __getrandom_custom.

We also probably could use NonZeroUsize for len to improve code generation for __getrandom_custom provider.

@newpavlov newpavlov marked this pull request as ready for review October 4, 2024 18:33
#[path = "js.rs"] mod imp;
} else if #[cfg(getrandom_backend = "esp_idf")] {
#[cfg(not(target_os = "espidf"))]
compile_error!("`esp_idf` backend can be enabled only for ESP-IDF targets!");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ESP-IDF support is now optional because of the concerns raised in #397.

@newpavlov
Copy link
Member Author

cc @briansmith

@newpavlov newpavlov merged commit f0d2c3f into master Oct 7, 2024
52 checks passed
@newpavlov newpavlov deleted the backend_cfgs branch October 7, 2024 14:11
newpavlov added a commit that referenced this pull request Oct 11, 2024
Tweak the breaking changes section and add entries for #415, #440, #442,
#448, #504, and #512.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant