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

DietPi-Software | Pi-hole: Block access to blocking page from web by default #3054

Merged
merged 8 commits into from
Aug 19, 2019
Merged

DietPi-Software | Pi-hole: Block access to blocking page from web by default #3054

merged 8 commits into from
Aug 19, 2019

Conversation

alpha-tango-kilo
Copy link
Contributor

@alpha-tango-kilo alpha-tango-kilo commented Aug 12, 2019

Status: Ready

  • IPv6 Support
  • Make use of this config optional

Reference: #3024

Commit list/description:

  • Took security enhancements and appended them to the lighttpd.pihole.conf file
  • Redid security enchancements so they're actually correct
  • Added optional extra security

Edited 13/8

@alpha-tango-kilo
Copy link
Contributor Author

Where in the dietpi-software install is this config file used? I'm struggling to find it, and to make this optional I'd need to know where the config file is referenced so I can conditionally append to it before it's added in. Cheers

@alpha-tango-kilo
Copy link
Contributor Author

Also, how do I check for IPv6 addresses? Is that still a part of $HTTP["remoteip"] or is it called something different?

@MichaIng
Copy link
Owner

MichaIng commented Aug 12, 2019

@AtkLordOverAll
First of all many thanks for this. As mentioned in the comment, we do not want to block all non-Nextcloud access here. This was only the special need of one user. We only want to block /pihole and /html/pihole for now.

About IPv6. I will check, I think it can be added to the same regex, allowing ^fd..: as well. I think ~ reads to extended regex and not glob, so the prior fd??: I posted in the referenced issue is wrong. dot as random character, question mark makes the previous character optional.

But actually I am not sure if there is any user which an IPv6-only local network. Also this does not make any sense. So adding local IPv6 support IMO is not important. Also DietPi-Config/network setup currently only supports IPv4, IPv6 addresses are available through SLAAC but not used in local network.


The config is pulled here: https://github.com/MichaIng/DietPi/blob/dev/dietpi/dietpi-software#L8756
I would not make it optional, this is too complicated and as said, noone ever requires a blocking page accessible by the public. For the admin panel perhaps, indeed making this optional would be an idea, either by replacing the regex in case, or via an additional drop-in config/module?

@MichaIng
Copy link
Owner

MichaIng commented Aug 12, 2019

I tested with IPv6. I always get two IPv6 addresses assigned. One with the external IPv6 prefix, that matches the one from the router, with an internal suffix. That allows to access a specific local device remotely without a NAT.
And then I get a shorter local IPv6 address as well, always starting with fe80. I am a bid confused now, since I though these local IPv6 start with fd always...
Found some German link about this, that divides the local IPv6 address (for individual devices) into:

  • fe80::/10 -- fe80:: - febf for "Link Local Unicast", which matches the second IPv6 my devices get. This seems to be assigned by the devices themselves, especially if the router does not assign anything else, e.g. via DHCPv6.
  • fc00::/7 -- fc00:: - fdff:: for "Unique Local Unicast", which I can enable in my router, which I don't have.
  • https://www.ipv6-portal.de/informationen/einfuehrung/adressbereiche.html

The problem is now, that I can ping6 devices on all those up to 3 IPv6 addresses, the one with external prefix, the fe80 one and most likely the fc/fd one as well, if assigned/enabled in router settings.
And depending on which IPv6 I ping, the remote IP tracked on the other device matches this as well. So when I ping6 the fe80 address, the other device receives pings from remote fe80 as well, when I ping the address with external prefix, also the other device receives pings with the this prefix as remote address as well, etc.

All this is without any remote access to any of the two devices, so all this can happen within local network/access only.


So best we can do is allowing Link Local Unicast and Unique Local Unicast addresses, since those are for sure only assigned and reachable within the LAN.
But I see no chance to check for a local IPv6 access, when the external IPv6 prefix is used to access. Only probably when comparing the whole prefix with the devices own IPv6 addresses, much too complicated. However, really, who the hack is doing that? 😄

Regex would be: ^f(e[89ab].|[cd]..):

Michalng knows best #3054 (comment)
@alpha-tango-kilo
Copy link
Contributor Author

Okay I've updated my code as per the review, and shall look at the rest tomorrow.

If noone needs access to /admin remotely and anyone sees a security reason to block it

Could make a prompt asking this if you wanted? Something like "Are you going to need to remotely access the admin page? y/N" (no default for enhanced security)

+ DietPi-Software | Pi-hole: Add 127.* address range (reserved for localhost) to local IP regex
@MichaIng
Copy link
Owner

MichaIng commented Aug 13, 2019

@AtkLordOverAll
Added 127.* as of above otherwise localhost itself would not be able to access 😉.

Could make a prompt asking this if you wanted? Something like "Are you going to need to remotely access the admin page? y/N" (no default for enhanced security)

Actually, as the Pi-hole installer asks a lot of questions anyway, it would not hurt. But it would need to be implemented a way that can be simply reverted. E.g. when Pi-hole is reinstalled, and users chooses different, it should be able to change the behaviour.

  • A sed of the regex should work:
    Block admin panel: sed -i 's#/pihole#/(pihole|admin)#' /etc/lighttpd/conf-available/99-dietpi-pihole.conf
    Allow admin panel: sed -i 's#/(pihole|admin)#/pihole#' /etc/lighttpd/conf-available/99-dietpi-pihole.conf
  • Or better create a separate config file, to be a bid failsafe, e.g. if someone manually edits our config? /etc/lighttpd/conf-available/99-dietpi-pihole-block_nonlocal_admin.conf (or any shorter name that roughly makes clear what it is for 😅)

Let's see if there are some other opinions about admin panel block and/or IPv6: https://dietpi.com/phpbb/viewtopic.php?p=19137#p19137

Uses G_WHIP_YESNO dialogue during install to ask, config is always made available to lighttpd even if it's not enabled so that the user may do so themselves later on
@alpha-tango-kilo
Copy link
Contributor Author

How's that @MichaIng? Optional admin panel blocking 😄

Added 127.* as of above otherwise localhost itself would not be able to access

We do not talk about this 😂

So apart from maybe IPv6 support I consider this PR mostly done, so I'm marking it as ready for review

@alpha-tango-kilo alpha-tango-kilo marked this pull request as ready for review August 13, 2019 13:45
+ DietPi-Software | Pi-hole: Fix regex, since admin penel URL is /html/admin or /admin (symlink). Simplify and failsafe regex, so that all access to either /admin/<anything> or /admin<end> is blockt, not only php scripts.
+ DietPi-Software | Pi-hole: Minor tab alignment and tiny comment for user info
+ DietPi-Software | Pi-hole: G_WHIP_* variables are reset after every call, thus they always need to be set before every G_WHIP call
+ DietPi-Software | Pi-hole: Add missing fi, respectively render it obsolete by simplifying code + minor tab alignment
@MichaIng MichaIng self-requested a review August 14, 2019 16:04
@MichaIng
Copy link
Owner

MichaIng commented Aug 14, 2019

@AtkLordOverAll
Okay lets go with this for now. IPv6 support can be added at a later date. IPv6-only networks are anyway not supported currently by dietpi-config, there there is always an IPv4 address assigned as well.

I fixed the admin panel block a bid, it is located beside /pihole, so /pihole is the blocking page and /admin the admin panel. However nice work, I think it is best like this to give user the choice and always install the config, so it can be easily enabled/disabled.

I will run some final test later and would merge then, if finished from your side.

EDIT: Changed the config name on server, to allow Nginx and Apache configs later as well.

+ DietPi-Software | Pi-hole: Rename admin block config on server, so we can add those for other webbrowsers later
+ DietPi-Software | Pi-hole: Admin block config has been renamed to allow configs for other webservers at a later time
@alpha-tango-kilo
Copy link
Contributor Author

Looks good, what you've changed makes sense, happy to have helped 😃

@MichaIng
Copy link
Owner

MichaIng commented Aug 15, 2019

@AtkLordOverAll
Damn, we are not done, actually we did not even achieve the initial aim.

The problem is that Pi-hole requires the blocking page index.php accessible in webroot. So it creates a symlink from webroot to html/pihole/index.php, more precisely from html/index.php, since it expects webroot there. We then move/create it at the actual webroot /var/www.

Nasty is that Lighttpd does not allow to block access based on local directory structure, at least I could not find any directive that would allow this, but only on URL. So the blocking page is always available by accessing the domain/IP without any appending path... This would additionally require to block access to "/$" and "/index.php". But then, if the blocking page is disabled, this location contains the default webserver test page and users might want to create any other entering page there. So to be sure, either we need to only block this, if blocking page is enabled, then user needs to remove this block manually when disabling the blocking page, or we need to check the symlink target or something, but that seems to be not possible via Lighttpd either...

One other idea I have is to not create/use any symlinks, but add rewrite rules. But I guess $HTTP["url"] does not change via rewrite, only via permanent redirect. And redirect means a second request from clients, being a huge access overhead and some clients to not follow redirects, e.g. curl without "-L" option etc.

No idea currently how to solve this reliable, as long as there is no option to block dir/file access via absolute path in Lighttpd, including symlink targets. There is no equivalent to Apache's directive, that is a petty...

However I will test the rewrite solution later, as this would be the only acceptable solution.

@alpha-tango-kilo
Copy link
Contributor Author

This is sad 😞

So we need to regex based on the browser URL as opposed to the file path? Or am I missing a layer of complexity here due to redirects?

@MichaIng
Copy link
Owner

@AtkLordOverAll
The other way round. It would be great to block based on absolute dir/file path, but $HTTP["url"] only contains the browser URL path the check against. Symlinks and I guess even internal URL rewrites (that are not represented in browsers address fields) are thus not blocked.

In contrast, Apache has a <Directory /path/to/dir> directive that allows to add settings/blocks based on the absolute path. It as well has a <Location /URL/path> directive that matches $HTTP["url"] from Lighttpd, so contains the URL path shown in browser instead of the local file system path.
Nginx has the same issue as Lighttpd, a location directive, that allows to check against the URL only, not against the full local fs path.

Best would be actually, if blocked requests would not be redirected to webroot, but to /pihole/index.php directly, so we could skip creating the symlink without breaking the blocking page. This would be better compatible with other websites/applications on the same webserver as well. See my post on the referenced Pi-hole issue above.

@MichaIng
Copy link
Owner

MichaIng commented Aug 16, 2019

Okay got some clarifications in the referenced issue and as well a solution for us:

  • Blocked requests (by Pi-hole DNS) are not redirected to webroot, but only the domain is replaced by the Pi-hole server IP.
  • This totally makes sense since a DNS has no influence on request path, query etc. I was not thinking this through very well 😅.
  • The blocking page is implemented as 404 handler. So e.g. you access bad.spam.domain.org/path/to/spam.png and bad.spam.domain.org is inside the block lists, it will be resolved with the local LAN IP of the Pi-hole server, so e.g. the request turns into 192.168.178.30/path/to/spam.png (my test VM IP). Of course /var/www/path/to/spam.png does not exist on my server, so it would answer with 404 (file not found). The default 404 handler on the Lighttpd config shipped by default Pi-hole install sets the 404 handler to the Pi-hole blocking page, so effectively for nearly all blocked requests the blocking page is shown, as desired.
    • This btw means that if some ad URL is bad.spam.domain.org/admin/index.php or contains any other path that actually exists on your server, you will not see the blocking page but this file/page/script instead 😉. However very unlikely case and indeed this is the best one can do via DNS based blocking.

So this means for us:

  • The symlink in webroot is not required. It shows the blocking page only, if one accesses a blocked domain without any appending path. Most likely I simply missed to test this thoroughly when re-implementing the blocking page some versions ago.
  • To correctly implement the blocking page, we need to set the 404 handler accordingly.
  • To block it for remote access, besides what we already did (block direct access), we need to set the 404 handler for local LAN access only, respectively reset it for WAN access.

Okay will implement this over the weekend for all webservers.
Actually I will implement a general non-LAN access config/mod for all webservers, which will include configs from an additional directory within the above remote IP address check.
So we can then add simpler "if location is ... then block" config snippets there, not only for Pi-hole but for other sensible websites as well. I anyway planned to add some software config menu by times, and it would be a great feature to toggle block/non-block for non-LAN access to every website we offer + custom path individually via GUI.

Hmm what fits best for this kind of access: non-local, non-LAN, remote, external, WAN, public? 🤔 😄

@alpha-tango-kilo
Copy link
Contributor Author

Sounds like you've got a handle on it, I'm on holiday currently so that's probably just as well. Best of luck 👍

I'd say non-local or public

@MichaIng
Copy link
Owner

@AtkLordOverAll
Okay I think we'll go with public. Its short and the term "local" is often used for localhost (127.* IP range) only in this context, not whole LAN.

I will merge this PR as a starting point. It even works when we remove the index.php symlink from webroot and for admin panel anyway.

@MichaIng MichaIng merged commit 9193d78 into MichaIng:dev Aug 19, 2019
@alpha-tango-kilo alpha-tango-kilo deleted the issue-3024 branch August 20, 2019 11:02
@MichaIng
Copy link
Owner

MichaIng commented Aug 22, 2019

<Directory not allowed in context

Okay the idea I had cannot be realised easily. The problem is that directory and location sections are "statically" parsed on server start. So those cannot existing within any conditional statement against a specific request, like remote IP check we need. And there are no usual ways to block access to certain paths without the related statement.

So it can only be the other way round, like

if location is, then
  if IP is, then
    block
  fi
fi

And since the IP check must be done anyway for each software title/dir/location individually, having a general public access control module is obsolete. No drama however and performance-wise it might be even better, since the IP check is not done on every request, but only those to defined paths.


PR is up to fix blocking page implementation and add related configs and public access control for all webservers: #3072

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

Successfully merging this pull request may close these issues.

2 participants