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

Disable HTTPS & shift functions to block page #1420

Merged
merged 5 commits into from Sep 14, 2017

Conversation

Projects
None yet
5 participants
@WaLLy3K
Copy link
Collaborator

WaLLy3K commented May 2, 2017

By submitting this pull request, I confirm the following:

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

5


The changes here primarily shift the work of header functions, JS redirects, etc, to the block page as per #1416.

Also, by commenting out include-conf-enabled.pl and replacing it with include_shell "find /etc/lighttpd/conf-enabled -name '*.conf' -a ! -name 'letsencrypt.conf' -printf 'include \"%p\"\n' 2>/dev/null", the following can be added to external.conf to enable HTTPS, without affecting web browsing load speeds:

# Only enable SSL for primary FQDN
$HTTP["host"] == "rpi.ddns.com" {
  include_shell "cat conf-enabled/letsencrypt.conf 2>/dev/null"
}

A blocked HTTPS query will be instantly dropped, but may provide an error message similar to: curl: (35) error:14077458:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 unrecognized name. Autofilling $HTTP["host"] == "" within external.conf with the proposed FQDN key from setupVars.conf is not a part of this PR and can be developed separately.

WaLLy3K added some commits May 2, 2017

Update lighttpd.conf.debian
* Disable `include-conf-enabled.pl`, as blindly enabling HTTPS (as Let's Encrypt does by having a file in that folder) creates Block Page inefficiencies
* Make Block page handle JS request rewrite, allowing users to better utilise their `lighttpd` service
* Make Block page handle debugging Pi-hole header
* Make Block page redirect users from `pi.hole` to `http://pi.hole/admin`
Update lighttpd.conf.fedora
Mirror changes found in `lighttpd.conf.debian`

@WaLLy3K WaLLy3K changed the title [WIP] Disable HTTPS & shift functions to block page Disable HTTPS & shift functions to block page May 2, 2017

This was referenced May 5, 2017

@PromoFaux

This comment has been minimized.

Copy link
Member

PromoFaux commented Jul 15, 2017

@WaLLy3K what steps do I need to take to test that this is working? So far it doesn't appear to be affecting any normal operations, but what should I be looking for specifically?

@WaLLy3K

This comment has been minimized.

Copy link
Collaborator Author

WaLLy3K commented Jul 16, 2017

If you have HTTPS enabled on your install (in my case, with Let's Encrypt), you'll see the following when you check the headers of a blocked HTTPS site (it's working as expected):

curl -I https://doubleclick.net
curl: (35) error:14077458:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 unrecognized name

This is an example of it not working, when the HTTPS enabled is attempting to serve the wrong certificate to a blocked domain:

curl -I https://doubleclick.net
curl: (51) SSL: no alternative certificate subject name matches target host name 'doubleclick.net'
@PromoFaux

This comment has been minimized.

Copy link
Member

PromoFaux commented Jul 16, 2017

I get this (and doubleclick is def blocked)

fap@fappotron:/etc/.pihole$ curl -I https://doubleclick.net
curl: (7) Failed to connect to doubleclick.net port 443: Connection refused
fap@fappotron:/etc/.pihole$ nslookup doubleclick.net
Server:         127.0.0.1
Address:        127.0.0.1#53

Name:   doubleclick.net
Address: 192.168.0.2

@WaLLy3K WaLLy3K added this to the v3.3 milestone Jul 20, 2017

@PromoFaux

This comment has been minimized.

Copy link
Member

PromoFaux commented Jul 29, 2017

I would approve this, but I'm not set up with HTTPS on my pi-hole installation (and quite frankly, I don't have a clue how to go about getting my setup into a state to be able to fully test this) Logic seems sound, however.

@spacedingo

This comment has been minimized.

Copy link
Contributor

spacedingo commented Aug 16, 2017

I have tested this type of "breaking" the https request and the results are great with respect to page-load times. Those curl HTTPS error codes are similar to the ones I got after I configured nginx and it was flying. I may have screenshots of browser dev-tools page load times before and after. If it can be done with lighttpd, then all the better.

@spacedingo

This comment has been minimized.

Copy link
Contributor

spacedingo commented Aug 16, 2017

The following curl fetch times were taken with a regular pi-hole and the a pi-hole with nginx configured to give the same https error (35) as above.

Fail to connect (for https):
v4 http 0.031000
v6 http 0.000000
v4 https 0.204000
v6 https 0.203000

Protocol error (for https):
v4 http 0.000000
v6 http 0.031000
v4 https 0.094000
v6 https 0.110000

The https response times are halved. It does cut a 5-6 second page load time to 2-3 seconds from what I have seen, considering that there are so few http content. Sorry I'm using nginx - i didn't know how to configure lighttpd for the same result.

Regular pi-hole (fail to connect)

G:\>curl -4 -I -w %{time_total} http://scorecardresearch.com/somethingsomething.jpg
HTTP/1.1 200 OK
Content-type: text/html; charset=UTF-8
Date: Wed, 16 Aug 2017 03:45:07
Server: lighttpd/1.4.45

0.031000

G:\>curl -4 -I -w %{time_total} https://scorecardresearch.com/somethingsomething.jpg
0.204000curl: (7) Failed to connect to scorecardresearch.com port 443: Connection refused

G:\>curl -6 -I -w %{time_total} http://scorecardresearch.com/somethingsomething.jpg
HTTP/1.1 200 OK
Content-type: text/html; charset=UTF-8
Date: Wed, 16 Aug 2017 03:45:48
Server: lighttpd/1.4.45

0.000000

G:\>curl -6 -I -w %{time_total} https://scorecardresearch.com/somethingsomething.jpg
0.203000curl: (7) Failed to connect to scorecardresearch.com port 443: Connection refused

G:\>curl -6 -I -w %{time_total} https://scorecardresearch.com/somethingsomething.jpg
0.218000curl: (7) Failed to connect to scorecardresearch.com port 443: Connection refused

Pihole/nginx (error 35)

G:\>curl -4 -I -w %{time_total} http://scorecardresearch.com/somethingsomething.jpg
HTTP/1.1 200 OK
Server: nginx/1.10.3
Date: Wed, 16 Aug 2017 03:39:12
Content-Type: image/gif
Content-Length: 43
Last-Modified: Mon, 28 Sep 1970 06:00:00
Connection: close
Expires: Wed, 16 Aug 2017 03:39:11
Cache-Control: no-cache
Strict-Transport-Security: includeSubdomains;
X-Frame-Options: DENY
X-Content-Type-Options: nosniff

0.000000
G:\>curl -4 -I -w %{time_total} https://scorecardresearch.com/somethingsomething.jpg
0.094000curl: (35) Unknown SSL protocol error in connection to scorecardresearch.com:443

G:\>curl -6 -I -w %{time_total} http://scorecardresearch.com/somethingsomething.jpg
HTTP/1.1 200 OK
Server: nginx/1.10.3
Date: Wed, 16 Aug 2017 03:39:38
Content-Type: image/gif
Content-Length: 43
Last-Modified: Mon, 28 Sep 1970 06:00:00
Connection: close
Expires: Wed, 16 Aug 2017 03:39:37
Cache-Control: no-cache
Strict-Transport-Security: includeSubdomains;
X-Frame-Options: DENY
X-Content-Type-Options: nosniff

0.031000
G:\>curl -6 -I -w %{time_total} https://scorecardresearch.com/somethingsomething.jpg
0.110000curl: (35) Unknown SSL protocol error in connection to scorecardresearch.com:443
# If the URL does not start with /admin, then it is a query for an ad domain
$HTTP["url"] =~ "^(?!/admin)/.*" {
# Create a response header for debugging using curl -I
setenv.add-response-header = ( "X-Pi-hole" => "A black hole for Internet advertisements." )

This comment has been minimized.

@Mcat12

Mcat12 Aug 16, 2017

Member

I think we want to keep the X-Pi-hole header for non-admin queries (it's being checked for in the dev debug script).

This comment has been minimized.

@WaLLy3K

WaLLy3K Aug 17, 2017

Author Collaborator

IIRC it's replaced by this codeblock.

@WaLLy3K WaLLy3K referenced this pull request Aug 28, 2017

Closed

Add SSL configuration for web interface #1672

5 of 5 tasks complete
@PromoFaux

This comment has been minimized.

Copy link
Member

PromoFaux commented Sep 13, 2017

Is this one good to go? I still don't really get it, so am unable to test... so we may have to go on trust on this one... AFAIK, this is the only one blocking BlockPage 2.0?

@WaLLy3K

This comment has been minimized.

Copy link
Collaborator Author

WaLLy3K commented Sep 14, 2017

There's not much to it really, but this requires features that the block page needs to add first.

@PromoFaux PromoFaux modified the milestones: v3.3, v3.2 Sep 14, 2017

@PromoFaux

This comment has been minimized.

Copy link
Member

PromoFaux commented Sep 14, 2017

Approved

Approved with PullApprove

@WaLLy3K WaLLy3K merged commit c458e4a into pi-hole:development Sep 14, 2017

4 checks passed

codacy/pr Good work! A positive pull request.
Details
code-review/pullapprove Approved by PromoFaux, WaLLy3K
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@WaLLy3K WaLLy3K deleted the WaLLy3K:new/https branch Sep 14, 2017

@WaLLy3K WaLLy3K referenced this pull request Oct 3, 2017

Merged

Add comment for include_shell feature #1721

5 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.