OC 9.2 / master random_bytes doesn't work with open_basedir

[PVince81]

Steps

1. Set open_basedir to /var/www
2. Check out master (0a6e09a) and run make
3. Open "index.php"

Expected

Works

Actual

White page. To see the actual error, needs this PR: #26075

I think the reason is because with open_basedir, ownCloud cannot access /dev/urandom any more.
Not sure why it worked before, maybe an update of the libraries made it more strict.

@DeepDiver1975 @butonic

[PVince81]

also needs to set php.ini's "sys_temp_dir" to something inside that, for example "/var/www/tmp".

[PVince81]

Exception occurred while logging exception: There is no suitable CSPRNG installed on your system
#0 /srv/www/htdocs/owncloud/lib/private/Security/CSRF/CsrfToken.php(50): random_bytes(32)
#1 /srv/www/htdocs/owncloud/lib/public/Util.php(505): OC\Security\CSRF\CsrfToken->getEncryptedValue()
#2 /srv/www/htdocs/owncloud/lib/private/legacy/template.php(77): OCP\Util::callRegister()
#3 /srv/www/htdocs/owncloud/lib/base.php(392): OC_Template->__construct('', 'update.admin', 'guest')
#4 /srv/www/htdocs/owncloud/lib/base.php(334): OC::printUpgradePage()
#5 /srv/www/htdocs/owncloud/lib/base.php(844): OC::checkUpgrade(true)
#6 /srv/www/htdocs/owncloud/index.php(46): OC::handleRequest()
#7 {main}

[PVince81]

This issue only exists on 9.2/master where we upgraded some libraries.

Ref: paragonie/random_compat#99.

If I understand correctly, it seems it's because the lib stopped trusting openssl so it's now throwing an exception instead of falling back.

The workaround is to add the fallback ourselves as stated in the release notes: https://github.com/paragonie/random_compat/releases/tag/v1.3.0

@Peter-Prochaska any thoughts on that ?

[PVince81]

One thing that we might want to do at least is to replace usages of random_bytes with calls to \OC::$server->getSecureRandom()->generate(32).

But that one seems to default to base64 chars, not sure if that's enough.
Also it internally uses \random_int which might fail as well with open_basedir. However if we decide to use the workaround we could put it in that one place instead of everywhere.

[PVince81]

@Peter-Prochaska let me know if this is an acceptable approach

[peterprochaska]

it should work with https://github.com/paragonie/random_compat/releases/tag/v1.4.0

[DeepDiver1975]

We have random_compat in 9.1 - see https://github.com/owncloud/3rdparty/tree/master/paragonie/random_compat

I guess it got lost as we remove some polyfills ....

[PVince81]

@DeepDiver1975 can we add it back then ?

[PVince81]

From what I see we still have "symfony/polyfill-php70" on master and this package indirectly requires "random_compat" in version v2.0.3: https://github.com/owncloud/core/blob/master/composer.lock#L1398

On stable9.1 we had version "v1.4.1".

I suppose that if we did a composer update on 3rdparty on stable9.1 it might update that library too...

Should we add an entry to our composer.json/lock to require random_compat < 2 ?

[PVince81]

@DeepDiver1975 @Peter-Prochaska please tell me if this is the correct approach and I'll adjust composer then

[PVince81]

Please make a decision regarding the library situation, thanks.

[PVince81]

Any update ? If we don't do anything this will break setups that use open_basedir with 10.0

[PVince81]

@Peter-Prochaska @DeepDiver1975 any update ? Time is running out and I don't like having to update or switch libraries that late...

If we don't fix this then any install using open_basedir will fail to work with 10.0.
It seems many people have been using open_basedir as a security measure to isolate the PHP process.

However I don't see any mention of it in the admin documentation, so not sure about official support...
But dropping support for this also doesn't feel like the right decision.

@felixboehm

[peterprochaska]

@PVince81 That's too much for 10.0. You have to set a temp dir for composer, a temp dir for php and install an random_compat version < 2.0, if you set open_basedir to /var/www.
Sure, we should set open_basedir in future, but we have to discuss this

[PVince81]

Thanks for the feedback. In the light of this, I'm moving this to 10.0.1 to discuss.

[PVince81]

@Peter-Prochaska will you have time to work on this for 10.0.1 ?

[peterprochaska]

@PVince81 When is the release of 10.0.1 planned?

[PVince81]

@Peter-Prochaska https://github.com/owncloud/core/wiki/Maintenance-and-Release-Schedule

May 23rd but we need to be done one week earlier before because of RC

[peterprochaska]

@PVince81 Then, this should be no problem. Has this worked with 9.x versions?

[PVince81]

#26076 (comment)

[ownclouders]

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 7 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.

[PVince81]

Interestingly no one complained about this yet since 10.0 is out... We still need to decide whether using open_basedir is officially supported.

[PVince81]

@settermjd thoughts on this ?

[DeepDiver1975]

Can we address this with additional documentation? Just tell people that open_basedir needs to include /dev/urandom.

[ownclouders]

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 7 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.)

[PVince81]

sounds good

[PVince81]

@mmattel are you familiar with this ?

[mmattel]

No I am not...

[stale]

This issue has been automatically closed.