Closed niblo closed 4 years ago
@niblo: I really like the improvement this feature would provide. We should really check during runtime whether getrandom
is available, instead of during compile time. That would make the file more portable among all linux platforms.
I would prefer to always check first if getrandom
exists, then falling back to /dev/urandom
if it does not. That would mean that we have to bypass libc though, cause we will want the getrandom functionality to be implemented even if it is not implemented on the platform where the code is compiled. (This is the same as your implementation at https://github.com/dsprenkels/randombytes/compare/master...niblo:use-devrandom.)
I would be interested in implementing something to this effect.
Awesome. Go ahead. I will be available for mentoring, if you are interested. :)
I would prefer to always check first if
getrandom
exists, then falling back to/dev/urandom
if it does not.
Agreed.
Awesome. Go ahead. I will be available for mentoring, if you are interested. :)
Thanks for the offer.
Kind of unrelated, I should start looking at initializing this kind of state lazily and reusing it in later calls to randombytes
.
I was thinking about hardcoding the
SYS_getrandom
value in the code, but syscalls are different for every platform. So I think we will always need either libc or the kernel headers (but old machines will not have the recent version).
Even if they have an older version, it will surely have dlsym
I think. So you would not need to use any hardcoded symbols related to getrandom
at all. Or am I mistaken?
Kind of unrelated, I should start looking at initializing this kind of state lazily and reusing it in later calls to
randombytes
.
From what I understand, calling dlsym
with RTLD_DEFAULT
will search through all loaded libraries for the first occurrence, so it would be bad to do it on every call to randombytes
. Just doing it the first time randombytes
is called could set a flag, which on subsequent calls will determine whether getrandom
or /dev/{u}random
is used.
Even if they have an older version, it will surely have dlsym I think. So you would not need to use any hardcoded symbols related to getrandom at all. Or am I mistaken?
Yes, my mistake. I briefly forgot that the idea was to use dlsym
.
From what I understand, calling
dlsym
withRTLD_DEFAULT
will search through all loaded libraries for the first occurrence, so it would be bad to do it on every call torandombytes
. Just doing it the first timerandombytes
is called could set a flag, which on subsequent calls will determine whethergetrandom
or/dev/{u}random
is used.
Yes, that's the idea. However, it may be tricky to guarantee that this happens in a thread-safe manner. (I am personally not familiar with thread-safety in C.) To solve this, we could put this global flag in thread-local storage. Then, the initialization will be done for every thread in the process, but that may be a tradeoff worth to consider.
In the case of a global flag, the worst case is that a thread will see that it has not been determined yet which of getrandom
and /dev/{u}random
to use, and make another call to check whether getrandom
is available. But at some point, all threads should have "caught up".
In the case of per thread flag, I can imagine it causing confusion for the developer that is writing a program that starts lots of threads that makes at least one call to randombytes
. That first call to randombytes
in each thread will look odd performance-wise compared to all subsequent calls, especially if those first calls are made more or less at the same time.
As a third option, what do you think about adding another function to the API that is exclusive to Linux, that determines whether to use getrandom
or /dev/{u}random
? If you have a multi-threaded program, you would run this before starting the threads.
#if defined(__linux__)
randombytes_detect_getrandom();
#endif
Not calling it would mean that /dev/{u}random
is used. This feature of detecting getrandom
could be added with a compiler flag like RANDOMBYTES_DETECT_GETRANDOM
, so it may not have to break the current API. On the other hand it would probably be a lot more work than to break compatibility with the current API (for Linux). An example:
Compiling on Linux w/ getrandom:
- getrandom() assumed
- current behavior, so no break in the API
Compiling on Linux w/ getrandom and RANDOMBYTES_DETECT_GETRANDOM
- defaults to /dev/{u}random, detect getrandom() with call to randombytes_detect_getrandom()
Compiling on Linux w/o getrandom
- cannot compile
Compiling on Linux w/o getrandom and RANDOMBYTES_DETECT_GETRANDOM:
- same as 2
In the case of a global flag, the worst case is that a thread will see that it has not been determined yet which of
getrandom
and/dev/{u}random
to use, and make another call to check whethergetrandom
is available. But at some point, all threads should have "caught up".
Building the initialization such that it is idempotent, feels like it should not be very hard. This is, in my opinion, the preferable option. :)
In the case of per thread flag, I can imagine it causing confusion for the developer that is writing a program that starts lots of threads that makes at least one call to
randombytes
. That first call torandombytes
in each thread will look odd performance-wise compared to all subsequent calls, especially if those first calls are made more or less at the same time.
While it is better if the state is initialized only once, we mention in README.md
that the user should not assume any performance properties. So this performance degradation is really acceptable.
As a third option, what do you think about adding another function to the API that is exclusive to Linux, that determines whether to use
getrandom
or/dev/{u}random
? If you have a multi-threaded program, you would run this before starting the threads.
I don't really like this option. I think expanding the API with randombytes_detect_getrandom
and the RANDOMBYTES_DETECT_GETRANDOM
flag adds complexity for the user. If it's possible to keep the API the same as it is now, I think that we should do that.
I realized that there is the case where a Linux system may have kernel 3.17 (released Oct 5 2014), which introduces the SYS_getrandom
syscall, but not have glibc 2.25 (released Feb 5 2017) which introduces the wrapper function getrandom
. Using dlsym
to detect support for getrandom
does obviously not account for that. Needs some more thinking.
EDIT: Mistyped the release year of glibc 2.25 as 2014 instead of 2017.
You could forego SYS_getrandom
support for systems with kernel >= 3.17 but glibc < 2.25. But then again, the gap of 28 months between the two is so large that there may be many systems like that.
One can definitely compile on a newer system that has SYS_getrandom
, and have the binary be supported on systems without getrandom
but with SYS_getrandom
. syscall(SYS_getrandom, ...)
will simply indicate that the system call is unavailable, and then you fall back to /dev/{u}random
.
But I don't see how it is practical to make it possible to compile on a kernel without SYS_getrandom
and have the resulting binary be supported on systems without getrandom
but with SYS_getrandom
. You would have to include the corresponding syscall numbers for all platforms that you want to support compilation on. If there was a way to get the SYS_getrandom
symbol at runtime, you could, but as far as I know, that is not possible.
Do we actually need getrandom
in glibc? Can't we just use SYS_getrandom
, and fallback if the syscall returns an errno
telling us that the syscall is invalid?
Update: The current implementation does not use getrandom
, only SYS_getrandom
, imported through sys/sycall.h
.
Do we actually need
getrandom
in glibc? Can't we just useSYS_getrandom
, and fallback if the syscall returns anerrno
telling us that the syscall is invalid?
If you are compiling on an older system it is still possible to make the binary work with getrandom
on a newer system, but that does not work for SYS_getrandom
.
I guess the question is if it is interesting to support compilation on older systems and having the resulting binary use getrandom
on newer systems if it is available. If that is important, then we should try to use getrandom
via dlsym
. (And fallback to SYS_getrandom
first if the compiling machine has SYS_getrandom
, then /dev/{u}random
.)
If it is only important to be able to compile the binary on a newer system with SYS_getrandom
and have it work on older systems without SYS_getrandom
, then we can just as you suggest, check if syscall
fails to determine whether SYS_getrandom
is available or not.
I guess the question is if it is interesting to support compilation on older systems and having the resulting binary use
getrandom
on newer systems if it is available. If that is important, then we should try to usegetrandom
viadlsym
. (And fallback toSYS_getrandom
first if the compiling machine hasSYS_getrandom
, then/dev/{u}random
.) Yeah, I don't really think this is important, asgetrandom
will usesyscall
internally anyway.If it is only important to be able to compile the binary on a newer system with
SYS_getrandom
and have it work on older systems withoutSYS_getrandom
, then we can just as you suggest, check ifsyscall
fails to determine whetherSYS_getrandom
is available or not.
I would press "merge" on both solutions. :)
Here is a long overdue update.
The initial fork/patch that I made is sufficient for my needs, and I can't make my mind up about the getrandom()
mess. Some related reading (lwn.net):
The RANDOMBYTES_USE_DEVRANDOM
flag lets me compile on a newer system that has getrandom()
, but still end up using /dev/urandom
in the resulting binary so that it can be used on pre 3.17 kernel systems too. Good enough. If this becomes an issue for me, I'll be sure to contribute a patch.
Feel free to close this issue.
@niblo Thank you for all your input!
For future readers: I'm closing this issue, but I would not consider this as "wontfix". Having a single binary work on any platform would on any version of Linux would be a great improvement to the randombytes
file. Feel free to comment and (request to) reopen this issue.
Futhermore, if there are any users of this library that have an opinion on this issue, don't hesitate to speak up. :)
I think it would be useful to be able to build one binary and run it on Linux machines with and without
getrandom()
support.One option is to add a compile time flag, so that you can build for
/dev/{u}random
even though the compiling machine hasgetrandom()
. See https://github.com/niblo/randombytes/tree/use-devrandom for a quick attempt. The downside is that it would of course not usegetrandom()
even when it's supported.Another alternative is to add runtime support for detecting
getrandom()
, and fall back to/dev/{u}random
if it cannot detect it. Here's a sketch:The upside is that the
getrandom()
symbol does not have to be defined.It doesn't guarantee that
getrandom
is available, only that it's defined in libc, but I think the same assumption is made already inrandombytes_linux_randombytes_getrandom
.I would be interested in implementing something to this effect.