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

Display false warning when run as non-root user with setuid bit set #7758

Closed
dbjpanda opened this issue Oct 30, 2018 · 9 comments
Closed

Display false warning when run as non-root user with setuid bit set #7758

dbjpanda opened this issue Oct 30, 2018 · 9 comments

Comments

@dbjpanda
Copy link

dbjpanda commented Oct 30, 2018

I configured my php executable i.e /usr/local/bin/php with a setuid for user deploy. So that if any user calls that php binary it should be executed as deploy only. As you can see below php binary is chowned by a non-root user deploy.

> ls -al /usr/local/bin/php
 -rwxr-xr-x    1 root     root     /usr/local/bin/php

> chown deploy:deploy /usr/local/bin/php 

> chmod u+s /usr/local/bin/php

> ls -al /usr/local/bin/php
 -rwsr-xr-x    1 deploy   deploy   /usr/local/bin/php

> ls -al composer.phar 
 -rwxr-xr-x    1 deploy   1000     composer.phar

> whoami 
 root

> php composer.phar -V
Do not run Composer as root/super user! See https://getcomposer.org/root for details
Composer version 1.7.2 2018-08-16 16:57:12

Output getting
Do not run Composer as root/super user! .

Expected result:
I shouldn't get that warning as php binary is owned by deploy and setuid bit is set.

However even if executing php composer.phar install as root the vendor dir created is owned by deploy. Infact the composer process ignores the setuid bit but when it creates any file/dir it obey setuid.

I inserted below inside the composer.phar and get the right user i.e deploy but with a warning Don't call as root .

$processUser = posix_getpwuid(posix_geteuid());
print $processUser['name'];

Output of composer diagnose:

# php composer diag
Do not run Composer as root/super user! See https://getcomposer.org/root for details
Checking composer.json: OK
Checking platform settings: OK
Checking git settings: OK
Checking http connectivity to packagist: OK
Checking https connectivity to packagist: OK
Checking github.com rate limit: OK
Checking disk free space: OK
Checking pubkeys: 
Tags Public Key Fingerprint: 57815BA2 7E54DC31 7ECC7CC5 573090D0  87719BA6 8F3BB723 4E5D42D0 84A14642
Dev Public Key Fingerprint: 4AC45767 E5EC2265 2F0C1167 CBBB8A2B  0C708369 153E328C AD90147D AFE50952
OK
Checking composer version: OK
Composer version: 1.7.2
PHP version: 7.2.11
PHP binary path: /usr/local/bin/php

@alcohol
Copy link
Member

alcohol commented Oct 30, 2018

The current conditional can be found here, and probably needs some double checking / adjustment:

if (!Platform::isWindows() && function_exists('exec') && !getenv('COMPOSER_ALLOW_SUPERUSER')) {
if (function_exists('posix_getuid') && posix_getuid() === 0) {
if ($commandName !== 'self-update' && $commandName !== 'selfupdate') {
$io->writeError('<warning>Do not run Composer as root/super user! See https://getcomposer.org/root for details</warning>');
}
if ($uid = (int) getenv('SUDO_UID')) {
// Silently clobber any sudo credentials on the invoking user to avoid privilege escalations later on
// ref. https://github.com/composer/composer/issues/5119
Silencer::call('exec', "sudo -u \\#{$uid} sudo -K > /dev/null 2>&1");
}
}
// Silently clobber any remaining sudo leases on the current user as well to avoid privilege escalations
Silencer::call('exec', 'sudo -K > /dev/null 2>&1');
}

@Seldaek
Copy link
Member

Seldaek commented Oct 31, 2018

Maybe setuid doesn't change what posix_getuid() returns.. I am not super familiar with it. Anyway not a huge deal IMO but if you manage to fix it feel free to send a PR.

@Seldaek Seldaek added this to the Bugs milestone Oct 31, 2018
@yassine-ah
Copy link
Contributor

I think it is about the difference between the effective and the real user ID:
posix_geteuid() vs posix_getuid()

dbjpanda added a commit to dbjpanda/composer that referenced this issue Nov 1, 2018
@dbjpanda dbjpanda changed the title composer gives false warning even if run as a non-root user Display false warning when run as non-root user with setuid bit set Nov 1, 2018
@dbjpanda
Copy link
Author

dbjpanda commented Nov 1, 2018

I tried changing from posix_getuid() to posix_geteuid() but It doesn't work. It doesn't show any warning message even if it is called as root

/var/www/github/composer # ls -al /usr/local/bin/php 
-rwxr-xr-x    1 root     root      14664264 Oct 15 18:56 /usr/local/bin/php
/var/www/github/composer # whoami
root
/var/www/github/composer # /usr/local/bin/php composer.phar -V
Composer version 1.8-dev (70557f3ab7b84a896179396509611fae66b19773) 2018-11-01 06:53:09
/var/www/github/composer # 

@Seldaek
Copy link
Member

Seldaek commented Nov 1, 2018 via email

@yassine-ah
Copy link
Contributor

I tested EUID behavior after reading those good explications :
https://unix.stackexchange.com/questions/191940/difference-between-owner-root-and-ruid-euid
https://unix.stackexchange.com/questions/399746/what-is-required-by-a-process-to-set-its-uid-to-0-root
http://timetobleed.com/5-things-you-dont-know-about-user-ids-that-will-destroy-you/

Here my test:

<?php

echo "RUID: " . posix_getuid() . "\n";
echo "EUID: " . posix_geteuid() . "\n";
echo file_put_contents('/root/euid_root.txt', 'test');
echo "\n\n";

// 48 is my apache user id
posix_seteuid(48);

echo "RUID: " . posix_getuid() . "\n";
echo "EUID: " . posix_geteuid() . "\n";
echo file_put_contents('/root/euid_apache.txt', 'test');
echo "\n";

// return EUID to root
// a process can change it EUID back to RUID/SUID
posix_seteuid(0);

echo "RUID: " . posix_getuid() . "\n";
echo "EUID: " . posix_geteuid() . "\n";
echo file_put_contents('/root/uid_apache.txt', 'test');
echo "\n";

And here the results:

RUID: 0
EUID: 0
4

RUID: 0
EUID: 48
PHP Warning:  file_put_contents(/root/euid_apache.txt): failed to open stream: Permission denied in /root/test.php on line 17
PHP Stack trace:
PHP   1. {main}() /root/test.php:0
PHP   2. file_put_contents() /root/test.php:17

Warning: file_put_contents(/root/euid_apache.txt): failed to open stream: Permission denied in /root/test.php on line 17

Call Stack:
    0.2006     229968   1. {main}() /root/test.php:0
    0.2009     230440   2. file_put_contents() /root/test.php:17


RUID: 0
EUID: 0
4

So if we execute a script with RUID set to root, we can switch the EUID as we wish during process, and from I know:

Plugins and scripts have full access to the user account which runs Composer

@Seldaek
Copy link
Member

Seldaek commented Nov 1, 2018

Right, so I think keeping the root warning makes sense even if seteuid was used. Closing this.

@Seldaek Seldaek closed this as completed Nov 1, 2018
@dbjpanda
Copy link
Author

dbjpanda commented Nov 1, 2018

What do you get if you run php -r 'var_dump(posix_geteuid());' ? I get 0 as root for both getuid and geteuid. Are you sure PHP is running as root in your test?

/var/www # ls -al /usr/local/bin/php
-rwsr-xr-x    1 deploy   root      14664264 Oct 15 18:56 /usr/local/bin/php
/var/www # id deploy
uid=1000(deploy) gid=2000(deploy) groups=2000(deploy)
/var/www # php -r 'var_dump(posix_geteuid());'
int(1000)
/var/www # php -r 'var_dump(posix_getuid());'
int(0) 

I am getting different ids. Have you both configured your php executable i.e /usr/local/bin/php with proper SUID ??
e.g

chown 1000 /usr/local/bin/php
chmod u+s  /usr/local/bin/php

Also I don't know how it is relevant to apache process ? I think composer uses CLI-SAPI. So any script that will be executed by CLI-SAPI with setuid bit set on it, should be take the ownership of that CLI-SAPI.

Please correct me if I am wrong.

@Seldaek
Copy link
Member

Seldaek commented Nov 1, 2018

@dbjpanda I think you're right but if php -r 'posix_seteuid(0); var_dump(posix_geteuid());' outputs 0, that means the process technically has access to the root user, even though it's pinned to the deploy user when it starts. So that only sandboxes things as far as the code running is playing nice.

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

4 participants