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

Feature-gating /dev/random fallback for Linux, Android and Solaris? #5

Closed
newpavlov opened this issue Jan 19, 2019 · 9 comments
Closed

Comments

@newpavlov
Copy link
Member

newpavlov commented Jan 19, 2019

Currently the listed targets fall back to reading /dev/random if getrandom syscall is not present. Maybe it's worth to feature-gate it? (i.e. by default only syscall will be used)

@dhardy
Copy link
Member

dhardy commented Jan 20, 2019

Isn't this just going to cause a support headache for any software deployed on old systems without getrandom? This was added to Linux in 3.17 released in 2014, so there might still be a few systems around without it.

I also don't see much motive for this; it removes only a tiny amount of code and since we do check that /dev/random is ready before using /dev/urandom, there shouldn't be any security risk.

@newpavlov
Copy link
Member Author

How about adding disabled-by-default force_syscall feature? To provide fallback we use 3 atomics and TLS, which feels quite redundant on modern Linux systems.

@dhardy
Copy link
Member

dhardy commented Feb 6, 2019

That's an interesting proposal. One drawback is that any crate which enables that feature forces it on in the build, which means that libraries shouldn't do this, and only the final binary should enable the feature — though this will often not depend directly on the getrandom crate.

This is just another case where Cargo features aren't a good fit. I'd rather this was not included for now, though am happy to discuss further.

@newpavlov
Copy link
Member Author

I think it's the same story as with WASM features: they should be enabled only by end applications and not by library crates. Yes, arguably it's not the best tool for the job, but it's the best we currently have. It's not an urgent matter, so I guess we can postpone this issue for the time being.

@dhardy
Copy link
Member

dhardy commented Feb 7, 2019

An environment variable or some type of global config would be a better fit. For the no_std stuff too.

Apparently the Cargo team are revising how features work, so it makes sense to hold off on this for now.

@josephlr
Copy link
Member

@newpavlov what was the motivation for this? As long as we are read a byte from /dev/random first, it doesn't seem like doing the file fallback is a bad thing.

@newpavlov
Copy link
Member Author

Mostly minimalism of the implementation, and as a consequence smaller resource usage, incl. binary size. (yeah, it's not really much, but stuff like that tends to add up)

@dhardy
Copy link
Member

dhardy commented Jun 17, 2019

Are there a significant class of users who would actually make use of it? Possibly for in-distro use? If there are is a large base of users who would actually benefit (slightly), then fine, but mostly this appears to be over-optimisation (the cost exceeding the benefit).

@newpavlov
Copy link
Member Author

Looks like pushing the minimum supported version of Linux kernel to 3.17 will not happen anytime soon, so we can close this issue.

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

3 participants