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

getrandom custom target fails for target_os=none #499

Open
addd45 opened this issue Oct 30, 2024 · 3 comments
Open

getrandom custom target fails for target_os=none #499

addd45 opened this issue Oct 30, 2024 · 3 comments

Comments

@addd45
Copy link

addd45 commented Oct 30, 2024

I am unable to compile my library project which depends on mc-sgx-core-types when using a build target where os = none.

in mc-sgx-core-types lib.rs file, there is a call to register a custom getrandom method -

// For targets that don't have a random number source we force it to always
// fail.
// Per https://docs.rs/getrandom/latest/getrandom/macro.register_custom_getrandom.html
// this function will *only* be used if getrandom doesn't know of a native
// secure spng
#[cfg(target_os = "none")]
use getrandom::register_custom_getrandom;

#[cfg(target_os = "none")]
register_custom_getrandom!(always_fail);

#[cfg(target_os = "none")]
fn always_fail(_buf: &mut [u8]) -> Result<(), getrandom::Error> {
    Err(getrandom::Error::UNSUPPORTED)
}

This is not the correct way to register a custom getrandom per the documentation for register_custom_getrandom, per the doc (and bolding relevant part):

Registering a custom getrandom implementation

Functions can only be registered in the root binary crate. Attempting to register a function in a non-root crate will result in a linker error. This is similar to #[panic_handler] or #[global_allocator], where helper crates define handlers/allocators but only the binary crate actually uses the functionality.

Thus, when adding mc-sgx-core-sys-types as a dependency and compiling with a target where os=none indeed a linker error occurs.

error[E0432]: unresolved import `getrandom::register_custom_getrandom`
  --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/mc-sgx-core-types-0.11.0/src/lib.rs:44:5
   |
44 | use getrandom::register_custom_getrandom;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `register_custom_getrandom` in the root

By reading the comments I understand the goal is to default to an error scenario if no suitable get random implementation is available, but this should be something that is handled "down the line" where it is up to the users of the library to decide how they want to use getrandom.
For example, my use case is to use rdrand which is something that can just be enabled with the "rdrand" feature for getrandom, but since mc-sgx-core-types enforces the "custom" feature (and the issues mentioned already), I am not able to use this library with my current build target

@nick-mobilecoin
Copy link
Collaborator

It does seem like this was implemented incorrectly :(.

To set expectations, we probably won't be prioritizing any fixes or improvements in this repo until we have a need to update the SGX bindings.

@addd45
Copy link
Author

addd45 commented Nov 5, 2024

Fair enough :).

If you prefer, and if I get some time, I could open up a PR to take out the registration part of code.
I presume it would not break anything but want to be safe.

@nick-mobilecoin
Copy link
Collaborator

I'll need to spin back up on the what and why that I did there.

It seems like it should fail to link if one doesn't specify a get random function so doesn't seem like the force failure should be needed

So yes if you happen to get to creating a PR prior to I would be keen to reviewing it

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

2 participants