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

/dev/urandom warning seems unnecessary #5530

Closed
dchest opened this issue Jun 24, 2017 · 2 comments · Fixed by #10840
Closed

/dev/urandom warning seems unnecessary #5530

dchest opened this issue Jun 24, 2017 · 2 comments · Fixed by #10840
Labels
enhancement feature: install and update good first issue Small tasks with clear documentation about how and in which place you need to fix things in.

Comments

@dchest
Copy link

dchest commented Jun 24, 2017

Hello,

I've noticed there's a check for existence of /dev/urandom (pointed out by this article):

It outputs "/dev/urandom is not readable by PHP which is highly discouraged for security reasons. Further information can be found in our documentation".

However in PHP 7 on most recent platforms /dev/urandom is not used, and this check is confusing, especially in OpenBSD, as its web server is always chrooted, so there's no /dev/urandom. PHP 7 random_bytes and random_int use:

  • getrandom on recent Linux
  • CryptGenRandom on Windows
  • arc4random on OpenBSD and NetBSD

On platforms that read randomness from /dev/urandom (like older Linux versions), the inability to open it by random_bytes will throw an exception: https://github.com/php/php-src/blob/696bd37e6757d77dc7ed44f3ea6451944ebaba96/ext/standard/random.c#L150

Since Nextcloud's SecureRandom uses random_int, it will throw if it fails to generate random bytes.

Proposal: instead of checking for existence of /dev/urandom, check if SecureRandom can successfully generate random bytes. This should be a setup error, not just a warning.

@MorrisJobke MorrisJobke added enhancement good first issue Small tasks with clear documentation about how and in which place you need to fix things in. feature: install and update labels Aug 30, 2017
@binarious
Copy link

getrandom uses /dev/urandom by default:

By default, getrandom() draws entropy from the urandom source (i.e.,
the same source as the /dev/urandom device).

@dchest
Copy link
Author

dchest commented Jun 6, 2018

@binarious it uses the same source of entropy as /dev/urandom, it doesn't actually open the file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement feature: install and update good first issue Small tasks with clear documentation about how and in which place you need to fix things in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants