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

provide a mechanism to mark a custom getrandom implementation as being SecureRandom #1734

Closed
japaric opened this issue Oct 13, 2023 · 5 comments · Fixed by #1754
Closed

provide a mechanism to mark a custom getrandom implementation as being SecureRandom #1734

japaric opened this issue Oct 13, 2023 · 5 comments · Fixed by #1754

Comments

@japaric
Copy link
Contributor

japaric commented Oct 13, 2023

Context: I'm working on adding no-std support to rustls and as part of that I'm building a small no-std demo that uses libc (instead of libstd) to perform networking. I'm using the "OS agnostic" x86_64-unknown-none target, which lacks a std implementation, to ensure libstd doesn't get sneakily used by a dependency but the demo will run on x86_64 Linux (for simplicity of testing it on CI but the demo should run on any (RT)OS that has a POSIX layer but lacks a libstd port).

I'm trying to use ring as the cryptographic backend of rustls in no-std mode and I'm able to build rustls with the ring Cargo feature enabled and the "std" feature disabled (WIP PR: rustls/rustls#1502) for the x86_64-unknown-linux-gnu target but the build fails for the x86_64-unknown-none target. I have tracked down the issue to this SecureRandom implementation:

ring/src/rand.rs

Lines 127 to 149 in 2e8363b

#[cfg(any(
target_os = "android",
target_os = "dragonfly",
target_os = "freebsd",
target_os = "haiku",
target_os = "illumos",
target_os = "ios",
target_os = "tvos",
target_os = "linux",
target_os = "macos",
target_os = "netbsd",
target_os = "openbsd",
target_os = "redox",
target_os = "solaris",
target_os = "windows",
target_os = "vita",
all(
feature = "wasm32_unknown_unknown_js",
target_arch = "wasm32",
target_os = "unknown",
),
))]
impl sealed::SecureRandom for SystemRandom {

The x86_64-unknown-none target has its "target_os" set to "none" so it doesn't match any of those conditionals. That means the SystemRandom type cannot be used with API like RsaKeyPair::sign and EphemeralPrivateKey::generate , which rustls uses when configured to use ring as its default crypto provider.

Would it be possible for ring to provide a mechanism to mark a custom SystemRandom as being SecureRandom? As of ring v0.17.0, one can enable getrandom's "custom" feature to provide a custom getrandom implementation which is what SystemRandom uses. My demo will run on Linux so my custom getrandom implementation is the same as the x86_64-unknown-linux-gnu implementation so it's also a "secure" getrandom implementation.

Perhaps a Cargo feature can be used as the mechanism? Similar to rustls' "dangerous" Cargo feature. Let's say "secure-custom-getrandom":

# *ring*'s Cargo.toml
[features]
secure-custom-getrandom = ["getrandom/custom"]
// ring/src/rand.rs
#[cfg(any(
    feature = "secure-custom-getrandom",
    target_os = "android",
    // ..
))]
impl sealed::SecureRandom for SystemRandom { /* .. */ }

I think adding the feature = at the any level is fine because getrandom's documentation states that custom implementations only have an effect on unsupported targets so you cannot, for example, override the Linux getrandom implementation.

@briansmith
Copy link
Owner

As part of the FIPS project we'll likely implement a CSPRNG within ring that then only requires an entropy source. Then we'll likely provide a pluggable API for the entropy source. Not too different from getrandom. If you want to help with creating this, I would be happy to collaborate with you on it.

@briansmith
Copy link
Owner

Also, please see #1406 (review) where I describe a mechanism to use getrandom as-is without building the FIPS-compliant CSPRNG, by introducing a new less-safe-getrandom-rdrand feature. I believe this will get you what you need for targetting X86-64 if you are happy with RDRAND.

@briansmith
Copy link
Owner

Also, please see #1406 (review) where I describe a mechanism to use getrandom as-is without building the FIPS-compliant CSPRNG, by introducing a new less-safe-getrandom-rdrand feature. I believe this will get you what you need for targetting X86-64 if you are happy with RDRAND.

We can do something just like less-safe-getrandom-rdrand but for getrandom's custom feature: add a less-safe-getrandom-custom feature to ring which would then activate get getrandom's custom feature. This is how getrandom supports the kinds of environments you seem to be targeting; see register_custom_getrandom!.

@japaric
Copy link
Contributor Author

japaric commented Oct 16, 2023

We can do something just like less-safe-getrandom-rdrand but for getrandom's custom feature: add a less-safe-getrandom-custom feature to ring which would then activate get getrandom's custom feature.

this is more or less what I had in mind. I'll prepare a PR

@japaric
Copy link
Contributor Author

japaric commented Oct 16, 2023

this is more or less what I had in mind. I'll prepare a PR

submitted PR #1754

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

Successfully merging a pull request may close this issue.

2 participants