virtualmin / virtualmin-gpl

Virtualmin web hosting control panel for Webmin
https://www.virtualmin.com
GNU General Public License v3.0
376 stars 118 forks source link

Unexpected changes to user ACL file after installing new PHP version and re-running config check #988

Closed iliajie closed 4 months ago

iliajie commented 4 months ago

Hello Jamie,

Using a Debian 12 (Nginx system) snapshot with a clean Virtualmin 7.20.2 and a single PHP version installed, I added PHP version 8.0. Note that at that point in time, the domain had PHP-FPM 8.2 configured on the "PHP Options" page, and it was displayed correctly without any errors. After the installation, nothing changed. However, running the Virtualmin config check revealed the following change, which I'm not sure should happen at all, e.g.:

image

I refer to this change:

-php_inis=/etc/php/8.2/fpm/pool.d/172355607241220.conf=PHP configuration for debian12-pro.virtualmin.dev
+php_inis=/etc/php/8.0/fpm/pool.d/172355607241220.conf=PHP configuration for debian12-pro.virtualmin.dev

I don't think this is expected:

root@debian12-pro:/etc# ls -lsa /etc/php/8.0/fpm/pool.d/172355607241220.conf
ls: cannot access '/etc/php/8.0/fpm/pool.d/172355607241220.conf': No such file or directory
jcameron commented 4 months ago

When you say you added PHP 8.0 , did this replace version 8.2 or did the system now have both versions available?

Also in the domain's config file under /etc/webmin/virtual-server/domains, what is php_fpm_version set to?

iliajie commented 4 months ago

When you say you added PHP 8.0 , did this replace version 8.2 or did the system now have both versions available?

Of course it had both!

Also in the domain's config file under /etc/webmin/virtual-server/domains, what is php_fpm_version set to?

That part didn't change! I kept track of it intentionally!


I had Ubuntu 24.04 with Virtualmin 7.10 and was trying to reproduce the same issue using the exact steps but couldn't. I even tried upgrading to 7.20.2 first and then installing another PHP version, but the change originally described didn't occur.

Both systems had latest nightly Webmin version.

jcameron commented 4 months ago

That part didn't change! I kept track of it intentionally!

Just to confirm, it was set to 8.2, right?

Looking at the code, the FPM pool config file is set based on the FPM directory, which in turn comes from the domain's FPM version (see https://github.com/virtualmin/virtualmin-gpl/blob/master/feature-webmin.pl#L869 and https://github.com/virtualmin/virtualmin-gpl/blob/master/php-lib.pl#L2064)

So I can't see how the wrong version could possibly be set!

What does the command virtualmin list-php-versions --multiline output on this system?

iliajie commented 4 months ago

Just to confirm, it was set to 8.2, right?

Yes, that's right! And, still is. See this screenshot before I installed PHP 8.0:

image

So I can't see how the wrong version could possibly be set!

Where is the code that updates php_inis directive in phpini module for users ACL?

What does the command virtualmin list-php-versions --multiline output on this system?

~# virtualmin list-php-versions --multiline

8.0
    Command: /bin/php-cgi8.0
    CLI: /bin/php8.0
    PHP modes: fpm fcgid cgi
    FPM support: Yes
    Full version: 8.0.30
8.2
    Command: /bin/php-cgi8.2
    CLI: /bin/php8.2
    PHP modes: fpm fcgid cgi
    FPM support: Yes
    Full version: 8.2.26
jcameron commented 4 months ago

The code that updates the .acl file is here : https://github.com/virtualmin/virtualmin-gpl/blob/master/feature-webmin.pl#L869

I am mystified as to why this could happen!

iliajie commented 4 months ago

I am mystified as to why this could happen!

This issue only occurs on Nginx installations. I reproduced it on a different Ubuntu 22.04 system...

The code that updates the .acl file is here : feature-webmin.pl#L869

No, the actual update in this case happens here: feature-webmin.pl/#L921-L924.

...I have dug in further, and I could reproduce it with 100% accuracy—here is a small piece of the strace output on the PID for config check:

openat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl.lock", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
newfstatat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl.lock", 0xffffef7d1128, AT_SYMLINK_NOFOLLOW) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl.lock", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 3
ioctl(3, TCGETS, 0xffffef7d0e30)        = -1 ENOTTY (Inappropriate ioctl for device)
lseek(3, 0, SEEK_CUR)                   = 0
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=0, ...}, AT_EMPTY_PATH) = 0
flock(3, LOCK_EX|LOCK_NB)               = 0
getpid()                                = 33791
write(3, "33791\n", 6)                  = 6
flock(3, LOCK_UN)                       = 0
close(3)                                = 0
newfstatat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl", {st_mode=S_IFREG|0644, st_size=160, ...}, 0) = 0
readlinkat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl", 0xffffef7d01d8, 4095) = -1 EINVAL (Invalid argument)
openat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl", O_RDONLY|O_CLOEXEC) = 3
ioctl(3, TCGETS, 0xffffef7d0e30)        = -1 ENOTTY (Inappropriate ioctl for device)
lseek(3, 0, SEEK_CUR)                   = 0
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=160, ...}, AT_EMPTY_PATH) = 0
read(3, "noconfig=1\nanyfile=0\nglobal=0\nph"..., 8192) = 160
read(3, "", 8192)                       = 0
close(3)                                = 0
getuid()                                = 0
getpid()                                = 33791
newfstatat(AT_FDCWD, "/var/webmin/locks/33791", {st_mode=S_IFDIR|0700, st_size=448, ...}, 0) = 0
symlinkat("/etc/webmin/phpini/ubuntu22-gpl.acl.lock", AT_FDCWD, "/var/webmin/locks/33791/1734694814-18") = 0
newfstatat(AT_FDCWD, "/etc/webmin/miniserv.conf", {st_mode=S_IFREG|0600, st_size=1816, ...}, 0) = 0
newfstatat(AT_FDCWD, "/etc/webmin/webmin.groups", 0xaaaadf4374a8, 0) = -1 ENOENT (No such file or directory)
newfstatat(AT_FDCWD, "/etc/webmin/miniserv.conf", {st_mode=S_IFREG|0600, st_size=1816, ...}, 0) = 0
newfstatat(AT_FDCWD, "/etc/webmin/phpini", {st_mode=S_IFDIR|0711, st_size=86, ...}, 0) = 0
openat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl", O_RDONLY|O_CLOEXEC) = 3
ioctl(3, TCGETS, 0xffffef7d0e30)        = -1 ENOTTY (Inappropriate ioctl for device)
lseek(3, 0, SEEK_CUR)                   = 0
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=160, ...}, AT_EMPTY_PATH) = 0
read(3, "noconfig=1\nanyfile=0\nglobal=0\nph"..., 8192) = 160
read(3, "", 8192)                       = 0
close(3)                                = 0
newfstatat(AT_FDCWD, "/usr/share/webmin//prefs.info", 0xaaaadf4374a8, 0) = -1 ENOENT (No such file or directory)
newfstatat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl", {st_mode=S_IFREG|0644, st_size=160, ...}, AT_SYMLINK_NOFOLLOW) = 0
newfstatat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl", {st_mode=S_IFREG|0644, st_size=160, ...}, 0) = 0
newfstatat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl", {st_mode=S_IFREG|0644, st_size=160, ...}, 0) = 0
getpid()                                = 33791
openat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl.webmintmp.33791", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 3
ioctl(3, TCGETS, 0xffffef7d0e30)        = -1 ENOTTY (Inappropriate ioctl for device)
lseek(3, 0, SEEK_CUR)                   = 0
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=0, ...}, AT_EMPTY_PATH) = 0
fchmodat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl.webmintmp.33791", 0100644) = 0
write(3, "noconfig=1\nanyfile=0\nglobal=0\nph"..., 160) = 160
close(3)                                = 0
newfstatat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl", {st_mode=S_IFREG|0644, st_size=160, ...}, 0) = 0
renameat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl.webmintmp.33791", AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl") = 0
fchmodat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl", 0100644) = 0
fchownat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl", 0, 0, 0) = 0
newfstatat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl.lock", {st_mode=S_IFREG|0644, st_size=6, ...}, AT_SYMLINK_NOFOLLOW) = 0
unlinkat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl.lock", 0) = 0
newfstatat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl", {st_mode=S_IFREG|0644, st_size=160, ...}, 0) = 0
readlinkat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl", 0xffffef7d01d8, 4095) = -1 EINVAL (Invalid argument)
geteuid()                               = 0
umask(0700)                             = 022
newfstatat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl.webminorig", 0xffffef7d1128, AT_SYMLINK_NOFOLLOW) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl.webminorig", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 3
ioctl(3, TCGETS, 0xffffef7d0e30)        = -1 ENOTTY (Inappropriate ioctl for device)
lseek(3, 0, SEEK_CUR)                   = 0
newfstatat(3, "", {st_mode=S_IFREG|066, st_size=0, ...}, AT_EMPTY_PATH) = 0
write(3, "noconfig=1\nanyfile=0\nglobal=0\nph"..., 160) = 160
close(3)                                = 0
umask(022)                              = 0700
pipe2([3, 5], O_CLOEXEC)                = 0
pipe2([6, 7], O_CLOEXEC)                = 0
clone(child_stack=NULL, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0xffffb87b70f0) = 34183
close(7)                                = 0
close(5)                                = 0
read(6, "", 4)                          = 0
close(6)                                = 0
fcntl(3, F_SETFD, FD_CLOEXEC)           = 0
ioctl(3, TCGETS, 0xffffef7d0db0)        = -1 ENOTTY (Inappropriate ioctl for device)
lseek(3, 0, SEEK_CUR)                   = -1 ESPIPE (Illegal seek)
newfstatat(3, "", {st_mode=S_IFIFO|0600, st_size=0, ...}, AT_EMPTY_PATH) = 0
read(3, "4c4\n< php_inis=/etc/php/8.1/fpm/"..., 8192) = 218
read(3, "", 8192)                       = 0
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=34183, si_uid=0, si_status=1, si_utime=0, si_stime=0} ---
newfstatat(3, "", {st_mode=S_IFIFO|0600, st_size=0, ...}, AT_EMPTY_PATH) = 0
close(3)                                = 0
wait4(34183, [{WIFEXITED(s) && WEXITSTATUS(s) == 1}], 0, NULL) = 34183
newfstatat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl.webminorig", {st_mode=S_IFREG|066, st_size=160, ...}, AT_SYMLINK_NOFOLLOW) = 0
unlinkat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl.webminorig", 0) = 0
fchmodat(AT_FDCWD, "/etc/webmin/phpini/ubuntu22-gpl.acl", 0600) = 0
iliajie commented 4 months ago

Anyway, I have found the source of the bug. Here is the patch https://github.com/virtualmin/virtualmin-nginx/commit/a4de729b7526855de2c99de4303260199372b19b.

jcameron commented 4 months ago

Thanks for the patch!

iliajie commented 4 months ago

Thanks for the patch!

Sure! However, could you please also check other instances of virtual_server::get_php_fpm_config in virtualmin-nginx/virtual_feature.pl? It seems there are more cases where virtual_server::get_php_fpm_config is used without passing a domain object...

jcameron commented 4 months ago

Thanks, I'll fix those up

iliajie commented 4 months ago

Excellent! Thank you!