Honor provided php configuration (in exec_background)

[dvzrv]

Hi! I'm currently maintaining cacti for Arch Linux and for the sake of improving the security of this web application (and many others), I'm about to introduce a more strict separation of the different applications.

To achieve this, I would like to be able to run each (php) application with its own (sub)set of php configuration (just exactly what it needs, but not more). This can be achieved by running cacti in an application container (e.g. uwsgi).

Unfortunately, cacti doesn't handle this very well, as it doesn't honor the configuration options passed to it, when it spawns backgrounded php processes.
This first manifests during the install process, where it becomes impossible (without modifying the system php.ini in /etc/php/php.ini) to pass the initial setup, as the (available) php modules are not recognized.

The same happens for the background processes spawned by poller.php, in which exec_background unconditionally reuses the system's php.ini, but is unaware of the environment from which it has been launched.

To give context, doing something like this to run poller.php in a systemd environment will not work, as the entry script doesn't pass down its environment:

[Unit]
Description=Cacti Poller

[Service]
Type=simple
User=http
Group=http
ExecStart=/usr/bin/php -c /etc/webapps/cacti/php.ini /usr/share/webapps/cacti/poller.php -d
PrivateDevices=yes
PrivateTmp=yes
ProtectSystem=full
ReadWriteDirectories=/etc/webapps/cacti /usr/share/webapps/cacti /var/lib/cacti /var/cache/cacti /run/mysqld/mysqld.sock
ProtectHome=yes
NoNewPrivileges=yes

In this example the paths are of course very Arch Linux specific, but the idea (php -c /path/to/a/config/file.ini cacti/poller.php) is straight forward and can be applied from any (unix) system.

The same happens, if cacti is run in a uwsgi application container, e.g.:

[uwsgi]
procname-master = %n
plugins = php
master = true
socket = /run/uwsgi/%n.sock
stats = /run/uwsgi/%n-stats.sock
uid = http
gid = http
processes = 10
cheaper = 2
cheaper-step = 1
idle = 600
die-on-idle = true
; reload whenever this config file changes
touch-reload = %p

php-allowed-ext = .php
php-docroot = /usr/share/webapps/%n
php-index = index.php
php-set = date.timezone=Europe/Berlin
php-set = open_basedir=/tmp/:/usr/share/webapps/%n:/etc/webapps/%n:/var/cache/%n:/var/lib/%n:/var/log/%n:/proc/meminfo:/usr/bin/rrdtool:/usr/bin/snmpget:/usr/bin/snmpwalk:/usr/bin/snmpbulkwalk:/usr/bin/snmpgetnext:/usr/bin/snmptrap:/usr/bin/sendmail:/usr/bin/php:/usr/bin/spine:/usr/share/fonts/TTF/
php-set = memory_limit = 512M
php-set = max_execution_time = 60
php-set = session.save_path=/tmp
php-set = session.gc_maxlifetime  21600
php-set = session.gc_divisor  500
php-set = session.gc_probability  1
php-set = post_max_size=64M
php-set = upload_max_filesize=64M
php-set = extension=gd.so
php-set = extension=gettext.so
php-set = extension=gmp.so
php-set = extension=pdo_mysql.so
php-set = extension=ldap.so
php-set = extension=snmp
php-set = extension=sockets.so

cron = -3 -1 -1 -1 -1 /usr/bin/php -c /etc/webapps/%n/php.ini /usr/share/webapps/cacti/poller.php

In this case the cron call takes care of the poller.php, but fails for the same reasons.

It would be very awesome, if cacti would pass on its environment, so all the php processes are actually using the same.
One easy solution would probably be to reuse the configuration in use and prepending its path explicitely to the arguments of the exec_background calls with the help of php_ini_loaded_file().

[cigamit]

@dvzrv, please review poller.php. In that file, we 'calculate' the php path, and then set an database variable called: 'path_php_binary'. If you review that code, you can then capture the path from an input parameter, somthing like 'php -q poller.php --php-path=/some/path' and then use that value instead. Does this go far enough?

[cigamit]

Alternatively, we might be able to pickup the path from the pollers environment. Let us know the way that'll work best.

[cigamit]

The more I think of this thing, the more I am thinking you want to pass the path to the ini file to anything that exec() or shell_exec() a php process. That needs more though. Everything is spawned as a sub-shell to the poller.php process. I wonder why PHP can not pick up the php.ini from the environment. It has to be there somewhere.

[cigamit]

Okay, I see what you are talking about now. It's likely a bigger problem and impacts plugins as well. But we have to start somewhere.

[dvzrv]

I might have to patch the cacti package to actually work within this new paradigm anyways. As soon as I'm getting anywhere useful, I'll open a pull request!

[dvzrv]

Looking further at the codebase, this doesn't only affect the custom exec_background(), but also some direct use of exec() and shell_exec().

[netniV]

Have you read https://www.php.net/manual/en/configuration.file.php? It describes how the INI is located. There must be an env var that can be set to say use this location if required.

[dvzrv]

@netniV Yes, as mentioned in the initial ticket text, php_ini_loaded_file() can be used for that.
Will open a pull request for that, but I stumbled upon the other usages which means there's a little more to patch :)

[netniV]

Ah, missed that sorry

[dvzrv]

There is also the custom exec_into_array() in lib/data_query.php, which I don't fully understand and won't patch.
Maybe someone else can have a look at what that actually does.

[dvzrv]

Hmm, I think this was merged a little prematurely. Applying this on top of 1.2.3 or 1.2.2 I'm unable to even finish the installer (for different reasons), when using a uwsgi setup.

I'll look further into this, but I have the feeling that the config approach was not very wise and that this will do more harm than good (mostly because there are so many bizarre calls to backgrounded php processes) in the long run.

[dvzrv]

Hm, that being said: I've just tried HEAD and that works (more or less without problems).

However, there are still quirks during install, which makes me think of expanding the fixes for the internally spawned background processes (e.g. during install), as they will still default back to the global default php config, when none is selected (but will not pick up the ones setup by an application server such as uwsgi - on top of the default configuration).
It might be more feasible to set whatever extensions need to be activated and whatever special settings (e.g. date.timezone, memory_limit) using the -d flag to php (after loading the php configuration with the -c flag, as introduced in #2652 ).

I think this would be the most flexible setup, but I ran into various bugs with 1.2.2 and 1.2.3 during testing.

[cigamit]

I did fix some issues. So, it's working fine right now. Not sure about issues on your end.

[dvzrv]

@cigamit yes, on their own 1.2.2/1.2.3 won't be able to be installed. Both get stuck in the install/step_json.php step, when finalizing the installation (endless loop).

So, for the sake of argument, I'll focus on the current develop branch (i.e. soon to be 1.2.4, I assume) and try to explain what my remaining issues are:

• uwsgi uses the global default php configuration (i.e. /etc/php/php.ini) and applies configuration on top (i.e. php-set = <identifier>=<value> as seen in the initial issue text), which follows the principle of least permissions per application, based on a global configuration (which can be completely unset!)
• on the develop branch it is now possible to launch an explicit cron job from uwsgi (i.e. cron = -3 -1 -1 -1 -1 /usr/bin/php -c /etc/webapps/%n/php.ini /usr/share/webapps/cacti/poller.php), which will require a separate php.ini
• on the develop branch the installation procedure still requires the user to set up the php extensions and basic configuration in the global php configuration (i.e. /etc/php/php.ini) when using uwsgi, because of the default behavior of php (use configuration from the default global location, if not explicitely disabled). My patch only fixes re-applying the ini file in use (e.g global php configuration has no settings -> any child process launched with exec/shell_exec has no settings)
• to achieve an implicit cron job with uwsgi (e.g. cron = -3 -1 -1 -1 -1 /usr/share/webapps/cacti/poller.php) and the complete "staying in your environment" during installation - for the sake of code de-duplication - the required configuration (if available) needs to be applied on top of the configuration parameter (i.e. php -c /my/config/php.ini -d extension=foo -d extension=bar -d date.timezone=some/where /path/to/script.php), as the global configuration needs to be allowed to be unset

I'll look into a patch set for that as well and have already something that seemingly worked (but then I ran into unrelated issues with 1.2.2/1.2.3, which ate a lot of time).

[cigamit]

Okay, well create a new pull request, and then let's place the required settings into lib/functions.php. However, please note, it will not make it into mainline until 1.2.5 if you are in urgent need or 1.3, which will take a while.

[netniV]

I have a big problem with this that hasn't been considered. By forcing which ini file we are using, it can lead to false positives for the installer and the website compared to the CLI. The website when it runs the PHP command line operates currently on the same CLI platform as the poller. This allows the installer to perform tests against what is installed/configured for the CLI.

I'm going to have to hold off on making the 1.2.4 release now because either I reverse this change and release, or we have to do some proper testing of what happens in all scenarios. I may actually decide to undo these changes because I want to get some fixes out for there other things such as the installers ability to get past the default location of files not being present.

[dvzrv]

@netniV I agree, that the pull request was a little suboptimal, as it only deals with the problem partially.

Would you care to elaborate on what you mean by

I have a big problem with this that hasn't been considered. By forcing which ini file we are using, it can lead to false positives for the installer and the website compared to the CLI. The website when it runs the PHP command line operates currently on the same CLI platform as the poller. This allows the installer to perform tests against what is installed/configured for the CLI.

though?
From my understanding the poller is triggered externally (and ideally with the same environment as the website, which was previously only guaranteed by the assumption of always using the global php configuration file) and not by the website, but similar to the poller the installation process also spawns backgrounded php processes (and all the backgrounded processes are now guaranteed to reuse the same configuration file).
For the installation process, the assumptions have therefore changed, but are also way more flexible.

However, I think that all the execs should additionally overload the settings properly (as it's not guaranteed, that a php process was actually launched with a populated configuration file, but with parameters instead).
This is still more of a sketch, so if you have a better idea for args composition, please do tell:

foreach (get_loaded_extensions() as $current_extension) {
  $args = $args . ' -d extension='.$current_extension.' ';
}
$args = $args . ' -d memory_limit='. ini_get('memory_limit') . ' ';
$args = $args . ' -d max_execution_time='. ini_get('max_execution_time') . ' ';
$args = $args . ' -d date.timezone='. ini_get('date.timezone') . ' ';

[netniV]

Systems such as Debian/Ubuntu have two separate ini file, one for the web and one for cli. When you run PHP at the command line, that would only use the CLI-based setup which is what the poller would utilise. When you run from the web, you are using the Apache-based setup which uses a completely separate ini.

By forcing the ini file, we are going to always be using the web based stuff for ANY exec() from the website. We introduced a lot of checks within 1.2 to make sure that all CLI stuff was properly enabled and that functionality will be broken. Or at least that is my suspicion and needs properly testing across all OS's.

Therefore, to give us more time for testing, I will be rolling back develop to remove this from the 1.2.4 release (but I will be cherry picking two fixes that @cigamit has done since we committed this).

Once that is done, and the release has been made, we can examine this a bit more. But if my suspicion is correct, we will probably have to move this to the 1.3 branch to properly test and work out the kinks of how we implement this properly because I see the reasoning behind trying to ensure the correct environment is present.

[dvzrv]

When looking at the many patches and changes Debian (and by proxy also Ubuntu) introduces, it becomes clear, that their (integrated) setup is meant to be apache centric.
However, I hope cacti is not meant to only be run on apache ;-)
Looking at the Debian apache integration specifically, I can't see a separate php.ini being defined.
That being said: Only because Debian chooses to do thing a certain way, doesn't mean that it is the preferred way or even best way to do something. There should of course be a headsup, in case the assumptions for their integration changes significantly.

Even after applying #2653 (ontop of develop) I have to provide a separate php configuration file or explicit configuration options (i.e. using the -d flag to php) for the poller (e.g. when running it via the uwsgi cron feature).
I assume, that this is because uwsgi doesn't hand down the configuration to the cron taks either (which is weird, as this works for many other web applications I tried this with, e.g. nextcloud) or because my patch is not good ;-)
btw: Debian relies on implicitely using the global php configuration) when calling the poller in cron.

Therefore, to give us more time for testing, I will be rolling back develop to remove this from the 1.2.4 release (but I will be cherry picking two fixes that @cigamit has done since we committed this).

I do understand. I hope we can find a solution that is benificial to all. Please have a look at my pull request regarding the composition of php arguments (#2653).

[cigamit]

Make sense. I had not even thought about the implications with Debian. So, @netniV and I are on the same page. We will push this, set an option or some other method. Make it more modular.

[dvzrv]

As I am no longer using this project and have no way of further testing, I'm closing this.

[TheWitness]

Thanks @dvzrv