I already somewhat introduced this subject in a previous threads. I found several problems about current cronjobs starting server/cron_daily(.sh) and server/server(.sh): they are located in root's user crontab they needlessly call a shell script while the cronjob could call the PHP script directly This is the exact content of this crontab: Code: * * * * * /usr/local/ispconfig/server/server.sh > /dev/null 2>> /var/log/ispconfig/cron.log 30 00 * * * /usr/local/ispconfig/server/cron_daily.sh > /dev/null 2>> /var/log/ispconfig/cron.log About 1., I think I saw in the code that it was editing directly /var/spool/cron/crontabs/root. That should really be avoided as the crontab utility does some validation and notifies the cron daemon about a modification. Anyway, I'm not just suggesting to use a pipe to crontab to add it, but rather to create a new /etc/cron.d/ispconfig file with this same content. About 2., other than starting the PHP script, the current shell scripts are just loading a specific content for the PATH environment variable and (for only one) loading /etc/profile. Even writing to the log file is done from the crontab. I'm pretty sure the PATH is really needed anywhere from within those PHP scripts (as every utility called should be with its full access path). On Debian Lenny at least, this /etc/profile is almost empty, only setting things like the PATH (again) and some shell CLI usability (obviously not needed). I can't imagine what else could be read from /etc/profile which should be used from within those scripts. I guess that just replacing the calls to something like this would work without problem. Code: * * * * * /usr/bin/php /usr/local/ispconfig/server/server.php > /dev/null 2>> /var/log/ispconfig/cron.log 30 00 * * * /usr/bin/php /usr/local/ispconfig/server/cron_daily.php > /dev/null 2>> /var/log/ispconfig/cron.log We are going to test that in the next days.
The changes you suggested to run the server.php file directly will not work as the wrapper shell script is needed to set the path variable. Without the path varable, several script will fail. This is espaeciall a problem on redhat based distributions. Regarding the root crontab. The code works well and it tested, so please do not change it. I do not ant to risk to break a few thousand installs onyl be doing such a cosmetic change of code that has pooven to work fine.
No, and that's exactly the reason behind my thread: the PHP script could be modified to set environment (including PATH) exactly like it is currently done from the shell script calling it. And I volunteer to change it and thoroughly test it before submitting any change. This is not just cosmetic, as for a script running every minute, any CPU runtime second and MB of RAM counts. Especially on a server which is supposed to host many websites and e-mail domains. On Debian Lenny 32-bits, any Bash running is taking around 6-7MB RAM. Dash is taking only about 2MB RAM.
Ok. Then feel free to change it. I was referring to the code that writes the root crontab. The current code uses the cron utility and the installer does not modify any crontab file manually, so cron gets notified of the changes. Sorry for the misunderstanding as I did not quote the part of your post. If we change from root crontab to /etc/cron.d/ispconfig we would have to ensure that it does not cause a problem during updates.
Will do. Alright, I understand your reaction now. I assumed that because of the following line in install/dist/conf/debian40.conf.php: $conf['cron_tab'] = '/var/spool/cron/crontabs/root'; Which is also present in every other file of distribution supported. After searching, I can't find anywhere in the code something using this $conf['cron_tab']. So my bad also: I did not thoroughly check my assumptions, but I found a configuration variable never used (which is a subtle bug anyway, IMHO).
And I was not aware that we have this variable in the config files It can be removed of course if it is not used anywhere else.
Furthermore, the calls to external UNIX tools should be avoided whenever possible (using PHP code), but when you cannot avoid it, you should use the full path to the tool. I understand this might differ among distributions, so this is my plan: at installation (or upgrade) time, you should discover full paths of all tools used in ISPConfig and store them in the DB, then use those full paths whenever they are needed (an interface is probably needed to edit those in case user really knows what he is doing). This would at the same time leverage use of "which" commands in the code (except the install, of course).
The combination of setting a path together with relative baths for the binarys works quite well, especially when you have to support a wide variety of linux distributions like ISPConfig does. There is no need to replace this so I think we shall stay with the current setup. Changing this will break too many setups. It is ok if we avoid the .sh script and set the path in the php script (if that really works, I had some problems with setting the path variable from php in the past). But chnaging the paths of all binaries to a fixed path is not a good idea in my opinion.
We (well you, actually) are already doing this for iptables and ipchains (i.e. checking during install and storing in the DB for later use). The problem security-wise is for example if some evil succeeds to upload some fake tool used by ISPC to /usr/local/bin, it will get executed, as root. Having the actual path stored in the DB during installation/upgrade would completely avoid this.
I understand your concerns, but normal users like the web users do not have the permission to store any files in /usr/local/bin, only the root user and the staff group (which is empty by default and no web users get added to that group) has this permission. try e.g.: ISPConfig uses quite a lot external applications and you have to consider also that a user e.g. switches from mydns to bind without a reinstall, so binaries needed to configure a service get installed after the ispconfig install. We can switch to fixed paths but then we need to add mechanisms which detect these paths on the fly if they are not set in the configuration. e.g.: 1) Detect paths at install time and write them to the database. 2) if we have to run a application and there is no path set in the database yet, detect the path at runtime and write it to the database. 3) If we try to run a application where the path is wrong (the app does not exist anymore under the path that is stored in the db, then try to find the new correct path and additionally log a warning message for the administrator). We can make step 3 optional in a way that either the path gets corrected automatically plus a warning gets logged or that the operation logs a error and stops the script processing.
