Skip to content

prng: support getrandom & getentropy #1367

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.

Sign up for GitHub

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

Open
vapier wants to merge 1 commit into
base: master
Choose a base branch
from
+43 −1
Open

prng: support getrandom & getentropy #1367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ AC_CHECK_HEADERS(m4_normalize([
stdlib.h
string.h
sys/ioctl.h
sys/random.h
sys/resource.h
sys/socket.h
sys/stat.h
Expand Down Expand Up @@ -304,6 +305,8 @@ AC_TYPE_UINTPTR_T

# Checks for library functions.
AC_CHECK_FUNCS(m4_normalize([
getentropy
getrandom
gettimeofday
posix_memalign
cfmakeraw
Expand Down
41 changes: 40 additions & 1 deletion src/crypto/prng.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,20 @@
#ifndef PRNG_HPP
#define PRNG_HPP

#include "config.h"

#if !defined(HAVE_GETENTROPY) && !defined(HAVE_GETRANDOM)
# define HAVE_URANDOM 1
#else
# undef HAVE_URANDOM
#endif

#include <unistd.h>
#ifdef HAVE_SYS_RANDOM_H
# include <sys/random.h>
#endif

#include <algorithm>
#include <cstdint>
#include <fstream>
#include <string>
Expand All @@ -43,32 +57,57 @@

We rely on stdio buffering for efficiency. */

#ifdef HAVE_URANDOM
static const char rdev[] = "/dev/urandom";
#endif

using namespace Crypto;

class PRNG
{
private:
#ifdef HAVE_URANDOM
std::ifstream randfile;
#endif

/* unimplemented to satisfy -Weffc++ */
PRNG( const PRNG& );
PRNG& operator=( const PRNG& );

public:
PRNG() : randfile( rdev, std::ifstream::in | std::ifstream::binary ) {}
PRNG()
#ifdef HAVE_URANDOM
: randfile( rdev, std::ifstream::in | std::ifstream::binary )
#endif
{}

void fill( void* dest, size_t size )
{
if ( 0 == size ) {
return;
}

#if defined(HAVE_GETRANDOM)
if ( getrandom( dest, size, 0 ) != static_cast<ssize_t>( size ) ) {
Copy link
Collaborator

@achernya achernya Jan 20, 2026

Choose a reason for hiding this comment

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

Reading https://manpages.debian.org/trixie/manpages-dev/getrandom.2.en.html#Interruption_by_a_signal_handler I think we should use the same loop for getrandom as we do for getentropy. It looks like otherwise we might get EINTR, which I believe would have been handled for /dev/urandom and getentropy otherwise.

Copy link
Author

@vapier vapier Jan 20, 2026

Choose a reason for hiding this comment

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

i'm not sure std::basic_istream read() handles signals for you either tbh

not that this is an argument against being defensive against EINTR

Copy link
Collaborator

@achernya achernya Jan 20, 2026

Choose a reason for hiding this comment

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

Fair point, but given you already wrote the code, may as well reuse it?

Copy link
Author

@vapier vapier Jan 20, 2026

Choose a reason for hiding this comment

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

hmm, if we want to be defensive against EINTR, should we do it comprehensively ? from a quick scan of the codebase, i think the only time we register signal handlers is in the select wrapper. so if we leverage SA_RESTART, then the kernel should take care of this transparently for us normally.

--- a/src/util/select.h
+++ b/src/util/select.h
@@ -108,7 +108,7 @@ public:
     /* Register a handler, which will only be called when pselect()
        is interrupted by a (possibly queued) signal. */
     struct sigaction sa;
-    sa.sa_flags = 0;
+    sa.sa_flags = SA_RESTART;
     sa.sa_handler = &handle_signal;
     fatal_assert( 0 == sigfillset( &sa.sa_mask ) );
     fatal_assert( 0 == sigaction( signum, &sa, NULL ) );

Copy link
Collaborator

@achernya achernya Jan 20, 2026

Choose a reason for hiding this comment

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

SA_RESTART is probably worthwhile. I will note that https://man7.org/linux/man-pages/man7/signal.7.html says it's only "some" syscalls, but getrandom and most file IO are on there. But on the BSD side, FreeBSD https://man.freebsd.org/cgi/man.cgi?query=signal at leaste claims that getentropy is not on the list. But it also doesn't seem to list EINTR as a valid return, so maybe that's fine?

throw CryptoException( "getrandom fell short" );
}
#elif defined(HAVE_GETENTROPY)
// getentropy() can only read up to 256 bytes at a time :(.
const size_t max_read = 256;
while ( size ) {
size_t this_size = std::min( max_read, size );
if ( getentropy( dest, this_size ) ) {
throw CryptoException( "getentropy fell short" );
}
size -= this_size;
dest = static_cast<char*>( dest ) + this_size;
}
#else
randfile.read( static_cast<char*>( dest ), size );
if ( !randfile ) {
throw CryptoException( "Could not read from " + std::string( rdev ) );
}
#endif
}

uint8_t uint8()
Expand Down