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

pihole -up/-r break lighttpd by incorrectly adding include "/etc/lighttpd/conf.d/pihole-admin.conf" to /etc/lighttpd/lighttpd.conf #5649

Open
neuffer opened this issue May 5, 2024 · 11 comments

Comments

@neuffer
Copy link

neuffer commented May 5, 2024

Versions

pihole -v
Pi-hole version is v5.18.2 (Latest: v5.18.2)
web version is v5.21 (Latest: v5.21)
FTL version is v5.25.1 (Latest: v5.25.1)

Platform

Linux pihole 6.1.0-20-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.85-1 (2024-04-11) x86_64 GNU/Linux
KVM VM
Debian Bullseye

Expected behavior

Normal working updates/repairs with
pihole -up
or
pihole -r

Actual behavior / bug

Updates and repairs FUBAR /etc/lighttpd/lighttpd.conf
by adding the following line:
include "/etc/lighttpd/conf.d/pihole-admin.conf"
to
/etc/lighttpd/lighttpd.conf

preventing lighttpd from starting.

In an Debian/Ubuntu environment this line should NOT be added.

Steps to reproduce

Run
pihole -up
or
pihole -r

will fail, since lighttpd will fail to (re-)start after the corruption of the config file
When the line is commented out, lighttpd will start again.

This error has been around for quite a while.

Debug Token

https://tricorder.pi-hole.net/MNknz3QR/

@rdwebdesign
Copy link
Member

You have an unusual and unexpected configuration.

Usually the directory /etc/lighttpd/conf.d is not present in Debian installs. It only exists in Fedora/CentOS/RedHat distributions.

Did you manually create this directory?

@neuffer
Copy link
Author

neuffer commented May 5, 2024 via email

@rdwebdesign
Copy link
Member

Typically Debian/Ubuntu families do not have this directory (it only exists in Fedora/CentOS/RedHat distributions), so the code was written taking this into account.

In your case, the code correctly identifies your distribution and installs the correct files, BUT in this particular install step the directory is (unexpectedly) found. Then the file is written to the wrong directory and the include line is created.

With both files (the one in conf.d/ and the other in conf-enabled/) included at the same time, the result is an invalid config.

Maybe @gstrauss can have a more detailed explanation, but I think the conf.d directory is present on your system by mistake and can be removed (your debug log shows Pi-hole file is the only file in this directory).

@gstrauss
Copy link
Contributor

gstrauss commented May 6, 2024

Maybe @gstrauss can have a more detailed explanation,

... I'll leave out the long history of pi-hole overwriting lighttpd.conf being unfriendly to use by anyone other than pi-hole 😉

but I think the conf.d directory is present on your system by mistake and can be removed (your debug log shows Pi-hole file is the only file in this directory).

The current configs are transitional to be more friendly and integrate better with other vhosts served by the same lighttpd instance. Once pi-hole drops support for Debian Buster (which runs an ancient version of lighttpd), the pi-hole lighttpd configs can be further simplified. Still, the shared pihole-admin.conf is most of the way there.

In the meantime, here's an incremental fix: adding include "/etc/lighttpd/conf.d/pihole-admin.conf" to /etc/lighttpd/lighttpd.conf should be done only for legacy installs, not for new installs. That's something that should be fixed in the basic-install.sh script. Untested patch:

--- a/automated install/basic-install.sh        
+++ b/automated install/basic-install.sh        
@@ -1423,7 +1423,7 @@ installConfigs() {
             install -D -m 644 -T ${PI_HOLE_LOCAL_REPO}/advanced/pihole-admin.conf /etc/lighttpd/conf.d/pihole-admin.conf
             if grep -q -F 'include "/etc/lighttpd/conf.d/pihole-admin.conf"' "${lighttpdConfig}"; then
                 :
-            else
+            else if grep -q -F "OVERWRITTEN BY PI-HOLE" "${lighttpdConfig}"; then
                 echo 'include "/etc/lighttpd/conf.d/pihole-admin.conf"' >> "${lighttpdConfig}"
             fi
             # Avoid some warnings trace from lighttpd, which might break tests

To be more robust about checking the platform, instead of checking for /etc/lighttpd/conf.d/ or /etc/lighttpd/conf-enabled/, the script could read /etc/os-release; or /etc/fedora-release or /etc/redhat-release or /etc/debian_version could be checked for existence. Is the issue reported here common? Has it be reported by others?

[Edit: the patch above should not be used on Fedora since the include line added to lighttpd.conf is needed even on newer pi-hole installations]

@neuffer
Copy link
Author

neuffer commented May 6, 2024

For me this is an old problem (+2 years).
When it appeared first, the problem was discussed on several websites outside Github.

@rdwebdesign
Copy link
Member

... adding include "/etc/lighttpd/conf.d/pihole-admin.conf" to /etc/lighttpd/lighttpd.conf should be done only for legacy installs, not for new installs.

I don't think this is a new/recent install.
Apparently this was an older Pi-hole installation (prior to v5.15) and neuffer kept updating that older version, like many other users.

Initially the installer identified the older file and used the correct files, in /etc/lighttpd/conf-enabled (without the include).

(This part is a guess, but I think this is what happened)
After that, the conf.d directory was created by some other install process. Then, in the next Pi-hole update the installer installed the file in conf.d and added the include, resulting in 2 files (one in conf.d and the previous one in conf-enabled), causing the issue.

Is the issue reported here common? Has it be reported by others?

No.
I never saw an issue with both paths (as far as I know, no one reported a similar issue).

@neuffer
Copy link
Author

neuffer commented May 7, 2024

@rdwebdesign
Yes, this installation has been running for several years.
I've fine tuned the local back and whitelists for my purposes over the years.
Why should I wipe that out?

If you don't support updating and require reinstall, you should make that clear and remove the "-up" update option.

Yes, the issue has been reported/discussed, but it seems not here. Just google for it. You'll find it.

@rdwebdesign
Copy link
Member

Why should I wipe that out?

Who said something about wipe out?

I just answered the comment where gstrauss said "adding include should be done only for legacy installs, not for new installs".

If you kept updating your Pi-hole installation, you actually has a normal and functional installation. The only (and very specific) issue is your lighttpd with 2 different config directories.

I never saw a similar report, but looking at your debug log, I think there is a simple solution to your case:

  1. uncomment the include line loading files from /etc/lighttpd/conf.d;
  2. remove the file /etc/lighttpd/conf-enabled/15-pihole-admin.conf. This file probably wasn't updated since the directory /etc/lighttpd/conf.d was created.

I think this will avoid the duplication and you will be able to update Pi-hole normally.

@PromoFaux
Copy link
Member

Worth pointing out that with V6 (currently in beta) does away with lighttpd as the web server as it is all handled by the FTL binary, so issues like this should be a thing of the past

@gstrauss
Copy link
Contributor

gstrauss commented May 7, 2024

Respectfully, the historical problems with pi-hole overwritting the system lighttpd.conf is not a problem with lighttpd, but rather with pi-hole installation scripts. This was mostly fixed in pi-hole over a year ago with a PR that I submitted and @PromoFaux reviewed and committed.

As the "OVERWRITTEN BY PI-HOLE" configs are still supported by pi-hole, and it seems like that is what @neuffer is running, one option for @neuffer is to uninstall pi-hole to remove the old configs, and then to reinstall pi-hole. I posted above that there appears to be some improvements to be made. Would you accept a PR for those items mentioned further above?

@PromoFaux
Copy link
Member

Thanks, but there is probably very little to be gained at this stage, as the aforementioned V6 will be released sooner rather than later. I'd like to be hopeful and say this year, even!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants