Add Access Control Lists (#31)

We settled on a powerful, but a bit complicated acl design.
This commit makes a couple of cosmetic improvements. It also removes
http.Transport, which was previously used to dial and write http
requests for insecure GET requests. Now we have to dial manually,
so we can check the access control list.

README.md

@@ -1,6 +1,6 @@
 # Secure forward proxy plugin for the Caddy web server
 
-[![Build Status](https://travis-ci.org/refraction-networking/utls.svg?branch=master)](https://travis-ci.org/refraction-networking/utls)
+[![Build Status](https://travis-ci.org/caddyserver/forwardproxy.svg?branch=master)](https://travis-ci.org/caddyserver/forwardproxy)
 
 This plugin enables [Caddy](https://caddyserver.com) to act as a forward proxy (as opposed to reverse proxy, Caddy's standard `proxy` directive) for HTTP/2.0 and HTTP/1.1 requests (HTTP/1.0 might work, but is untested).
 
@@ -22,18 +22,41 @@ forwardproxy {
     response_timeout 30
     dial_timeout     30
     upstream         https://user:password@extra-upstream-hop.com
+    acl {
+      allow     *.caddyserver.com
+      deny      192.168.1.1/32 192.168.0.0/16 *.prohibitedsite.com *.localhost
+      allow     ::1/128 8.8.8.8 github.com *.github.io
+      allowfile /path/to/whitelist.txt
+      denyfile  /path/to/blacklist.txt
+      allow     all
+      deny      all // unreachable rule, remaining requests are matched by `allow all`
+    }
 }
 ```
 
 (The square brackets `[ ]` indicate values you should replace; do not actually include the brackets.)
 
+##### Security
+
 - **basicauth [user] [password]**  
 Sets basic HTTP auth credentials. This property may be repeated multiple times. Note that this is different from Caddy's built-in `basicauth` directive. BE SURE TO CHECK THE NAME OF THE SITE THAT IS REQUESTING CREDENTIALS BEFORE YOU ENTER THEM.  
 _Default: no authentication required._
 
-- **ports [integer] [integer]...**  
-Whitelists ports forwardproxy will HTTP CONNECT to.  
-_Default: no restrictions._
+- **probe_resistance [secretlink.tld]**  
+Attempts to hide the fact that the site is a forward proxy.
+Proxy will no longer respond with "407 Proxy Authentication Required" if credentials are incorrect or absent,
+and will attempt to mimic a generic Caddy web server as if the forward proxy is not enabled.  
+Probing resistance works (and makes sense) only if basicauth is set up.
+To use your proxy with probe resistance, supply your basicauth credentials to your client configuration.
+If your proxy client(browser, operating system, [browser extension](https://chrome.google.com/webstore/detail/proxy-switchyomega/padekgcemlokbadohgkifijomclgjgif), etc.)
+allows you to preconfigure credentials, and sends credentials preemptively, you do not need secret link.  
+If your proxy client does not preemptively send credentials, you will have to visit your secret link in your browser to trigger the authentication.
+Make sure that specified domain name is visitable, does not contain uppercase characters, does not start with dot, etc.
+Only this address will trigger a 407 response, prompting browsers to request credentials from user and cache them for the rest of the session.
+It is possible to use any top level domain, but for secrecy reasons it is highly recommended to use `.localhost`.  
+_Default: no probing resistance._
+
+##### Privacy
 
 - **hide_ip**  
 If set, forwardproxy will not add user's IP to "Forwarded:" header.  
@@ -45,25 +68,66 @@ If set, forwardproxy will not add Via header, and prevents simple way to detect
 WARNING: there are other side-channels to determine this.  
 _Default: no hiding; Header in form of `Via: 2.0 caddy` will be sent out._
 
-- **probe_resistance [secretlink.tld]**  
-EXPERIMENTAL. (Here be dragons!) Attempts to hide the fact that the site is a forward proxy. Proxy will no longer respond with "407 Proxy Authentication Required" if credentials are incorrect or absent, and will attempt to mimic a generic Caddy web server as if the forward proxy is not configured. Since not all clients (browsers, operating systems, etc.) are able to be configured to send credentials right away (some only authenticate after receiving a 407), we will use a secret link. Make sure that specified domain name is visitable, does not contain uppercase characters, does not start with dot, etc. Only this address will trigger a 407 response, prompting browsers to request credentials from users and cache them for the rest of the session. It is possible to use any top level domain (tld), but for secrecy reasons it is highly recommended to use `.localhost`. Probing resistance works (and makes sense) only if basicauth is set up. To use your proxy with probe resistance, supply your basicauth credentials to your client configuration if possible. If your proxy client does not authenticate right away, you may then have to visit your secret link in your browser to trigger the authentication.  
-_Default: no probing resistance._
+##### Access Control
 
-- **serve_pac [/path.pac]**  
-Generate (in-memory) and serve a [Proxy Auto-Config](https://en.wikipedia.org/wiki/Proxy_auto-config) file on given path. If no path is provided, the PAC file will be served at `/proxy.pac`. NOTE: If you enable probe_resistance, your PAC file should also be served at a secret location; serving it at a predictable path can easily defeat probe resistance.  
-_Default: no PAC file will be generated or served by Caddy (you still can manually create and serve proxy.pac like a regular file)._
+- **ports [integer] [integer]...**  
+Specifies ports forwardproxy will whitelist for all requests. Other ports will be forbidden.  
+_Default: no restrictions._
+
+- **acl {  
+    acl_directive  
+    ...  
+    acl_directive  
+}**  
+Specifies **order** and rules for allowed destination IP networks, IP addresses and hostnames.
+The hostname in each forwardproxy request will be resolved to an IP address,
+and caddy will check the IP address and hostname against the directives in order until a directive matches the request.
+acl_directive may be:
+  - **allow [ip or subnet or hostname] [ip or subnet or hostname]...**
+  - **allowfile /path/to/whitelist.txt**
+  - **deny [ip or subnet or hostname] [ip or subnet or hostname]...**
+  - **denyfile /path/to/blacklist.txt**
+
+  If you don't want unmatched requests to be subject to the default policy, you could finish
+  your acl rules with one of the following to specify action on unmatched requests:
+  - **allow all**
+  - **deny all**
+  
+  For hostname, you can specify `*.` as a prefix to match domain and subdomains. For example,
+  `*.caddyserver.com` will match caddyserver.com, subdomain.caddyserver.com, but not fakecaddyserver.com.
+  Note that hostname rule, matched early in the chain, will override later IP rules,
+  so it is advised to put IP rules first, unless domains are highly trusted and should override the
+  IP rules. Also note that domain-based blacklists are easily circumventable by directly specifying the IP.  
+  For `allowfile`/`denyfile` directives, syntax is the same, and each entry must be separated by newline.  
+  This policy applies to all requests except requests to the proxy's own domain and port.
+  Whitelisting/blacklisting of ports on per-host/IP basis is not supported.  
+_Default policy:_  
+acl {  
+    deny 10.0.0.0/8 127.0.0.0/8 172.16.0.0/12 192.168.0.0/16 ::1/128 fe80::/10  
+    allow all  
+}  
+_Default deny rules intend to prohibit access to localhost and local networks and may be expanded in future._
+
+##### Timeouts
 
 - **response_timeout [integer]**  
-Sets timeout (in seconds) for HTTP requests made by proxy on behalf of users (does not affect `CONNECT`-method requests).  
-_Default: no timeout (other timeouts will eventually close the connection)._
+Sets timeout (in seconds) to get full response for HTTP requests made by proxy on behalf of users (does not affect `CONNECT`-method requests).  
+_Default: no timeout._
 
 - **dial_timeout [integer]**  
 Sets timeout (in seconds) for establishing TCP connection to target website. Affects all requests.  
 _Default: 20 seconds._
 
+##### Other
+
+- **serve_pac [/path.pac]**  
+Generate (in-memory) and serve a [Proxy Auto-Config](https://en.wikipedia.org/wiki/Proxy_auto-config) file on given path. If no path is provided, the PAC file will be served at `/proxy.pac`. NOTE: If you enable probe_resistance, your PAC file should also be served at a secret location; serving it at a predictable path can easily defeat probe resistance.  
+_Default: no PAC file will be generated or served by Caddy (you still can manually create and serve proxy.pac like a regular file)._
+
 - **upstream [https://username:password@upstreamproxy.site:443]**  
 Sets upstream proxy to route all forwardproxy requests through it.
-This setting does not affect non-forwardproxy requests nor requests with wrong credentials.  
+This setting does not affect non-forwardproxy requests nor requests with wrong credentials.
+Upstream is incompatible with `acl` and `ports` subdirectives.  
 Supported schemes to remote host: https.  
 Supported schemes to localhost: socks5, http, https(certificate check is ignored).  
 _Default: no upstream proxy._

acl.go

@@ -0,0 +1,96 @@
+package forwardproxy
+
+import (
+	"errors"
+	"net"
+	"strings"
+)
+
+type aclDecision uint8
+
+const (
+	aclDecisionAllow = iota
+	aclDecisionDeny
+	aclDecisionNoMatch
+)
+
+type aclRule interface {
+	tryMatch(ip net.IP, domain string) aclDecision
+}
+
+type aclIPRule struct {
+	net   net.IPNet
+	allow bool
+}
+
+func (a *aclIPRule) tryMatch(ip net.IP, domain string) aclDecision {
+	if !a.net.Contains(ip) {
+		return aclDecisionNoMatch
+	}
+	if a.allow {
+		return aclDecisionAllow
+	}
+	return aclDecisionDeny
+
+}
+
+type aclDomainRule struct {
+	domain            string
+	subdomainsAllowed bool
+	allow             bool
+}
+
+func (a *aclDomainRule) tryMatch(ip net.IP, domain string) aclDecision {
+	if strings.HasSuffix(domain, ".") {
+		domain = domain[:len(domain)-1]
+	}
+	if domain == a.domain ||
+		a.subdomainsAllowed && strings.HasSuffix(domain, "."+a.domain) {
+		if a.allow {
+			return aclDecisionAllow
+		}
+		return aclDecisionDeny
+	}
+	return aclDecisionNoMatch
+}
+
+type aclAllRule struct {
+	allow bool
+}
+
+func (a *aclAllRule) tryMatch(ip net.IP, domain string) aclDecision {
+	if a.allow {
+		return aclDecisionAllow
+	}
+	return aclDecisionDeny
+}
+
+func newAclRule(ruleSubject string, allow bool) (aclRule, error) {
+	if ruleSubject == "all" {
+		return &aclAllRule{allow: allow}, nil
+	}
+	_, ipNet, err := net.ParseCIDR(ruleSubject)
+	if err != nil {
+		ip := net.ParseIP(ruleSubject)
+		// support specifying just an IP
+		if ip.To4() != nil {
+			_, ipNet, err = net.ParseCIDR(ruleSubject + "/32")
+		} else if ip.To16() != nil {
+			_, ipNet, err = net.ParseCIDR(ruleSubject + "/128")
+		}
+	}
+	if err == nil {
+		return &aclIPRule{net: *ipNet, allow: allow}, nil
+	}
+
+	subdomainsAllowed := false
+	if strings.HasPrefix(ruleSubject, `*.`) {
+		subdomainsAllowed = true
+		ruleSubject = ruleSubject[2:]
+	}
+	err = isValidDomainLite(ruleSubject)
+	if err != nil {
+		return nil, errors.New(ruleSubject + " could not be parsed as either IP, IP network, or domain: " + err.Error())
+	}
+	return &aclDomainRule{domain: ruleSubject, subdomainsAllowed: subdomainsAllowed, allow: allow}, nil
+}

acl_test.go

@@ -0,0 +1,199 @@
+package forwardproxy
+
+import (
+	"net/http"
+	"testing"
+)
+
+/*
+test port blocking working
+test blacklist allowed
+test blacklist refused with correct status
+
+*/
+
+func TestWhitelistAllowing(t *testing.T) {
+	useTls := true
+	for _, httpTargetVer := range testHttpVersions {
+		for _, resource := range testResources {
+			response, err := getViaProxy(caddyTestTarget.addr, resource, caddyForwardProxyWhiteListing.addr, httpTargetVer,
+				"", useTls)
+			if err != nil {
+				t.Fatal(err)
+			} else if err = responseExpected(response, caddyTestTarget.contents[resource]); err != nil {
+				t.Fatal(err)
+			}
+		}
+	}
+}
+
+func TestWhitelistBlocking(t *testing.T) {
+	useTls := true
+	for _, httpTargetVer := range testHttpVersions {
+		for _, resource := range testResources {
+			response, err := getViaProxy(caddyHTTPTestTarget.addr, resource, caddyForwardProxyWhiteListing.addr, httpTargetVer,
+				"", useTls)
+			if err != nil {
+				t.Fatal(err)
+			} else if response.StatusCode != http.StatusForbidden {
+				t.Fatal("Expected response \"403 Forbidden\", got:", response.StatusCode)
+			}
+		}
+	}
+
+	for _, httpTargetVer := range testHttpVersions {
+		for _, resource := range testResources {
+			response, err := getViaProxy("google.com:6451", resource, caddyForwardProxyWhiteListing.addr, httpTargetVer,
+				"", useTls)
+			if err != nil {
+				t.Fatal(err)
+			} else if response.StatusCode != http.StatusForbidden {
+				t.Fatal("Expected response \"403 Forbidden\", got:", response.StatusCode)
+			}
+		}
+	}
+}
+
+func TestLocalhostDefaultForbidden(t *testing.T) {
+	useTls := true
+	for _, httpTargetVer := range testHttpVersions {
+		for _, resource := range testResources {
+			response, err := getViaProxy("localhost:6451", resource, caddyForwardProxyNoBlacklistOverride.addr, httpTargetVer,
+				"", useTls)
+			if err != nil {
+				t.Fatal(err)
+			} else if response.StatusCode != http.StatusForbidden {
+				t.Fatal("Expected response \"403 Forbidden\", got:", response.StatusCode)
+			}
+		}
+	}
+
+	for _, httpTargetVer := range testHttpVersions {
+		for _, resource := range testResources {
+			response, err := getViaProxy("127.0.0.1:808", resource, caddyForwardProxyNoBlacklistOverride.addr, httpTargetVer,
+				"", useTls)
+			if err != nil {
+				t.Fatal(err)
+			} else if response.StatusCode != http.StatusForbidden {
+				t.Fatal("Expected response \"403 Forbidden\", got:", response.StatusCode)
+			}
+		}
+	}
+
+	for _, httpTargetVer := range testHttpVersions {
+		for _, resource := range testResources {
+			response, err := getViaProxy("[::1]:8080", resource, caddyForwardProxyNoBlacklistOverride.addr, httpTargetVer,
+				"", useTls)
+			if err != nil {
+				t.Fatal(err)
+			} else if response.StatusCode != http.StatusForbidden {
+				t.Fatal("Expected response \"403 Forbidden\", got:", response.StatusCode)
+			}
+		}
+	}
+}
+
+func TestLocalNetworksDefaultForbidden(t *testing.T) {
+	useTls := true
+	for _, httpTargetVer := range testHttpVersions {
+		for _, resource := range testResources {
+			response, err := getViaProxy("10.0.0.0:80", resource, caddyForwardProxyNoBlacklistOverride.addr, httpTargetVer,
+				"", useTls)
+			if err != nil {
+				t.Fatal(err)
+			} else if response.StatusCode != http.StatusForbidden {
+				t.Fatal("Expected response \"403 Forbidden\", got:", response.StatusCode)
+			}
+		}
+	}
+
+	for _, httpTargetVer := range testHttpVersions {
+		for _, resource := range testResources {
+			response, err := getViaProxy("127.222.34.1:443", resource, caddyForwardProxyNoBlacklistOverride.addr, httpTargetVer,
+				"", useTls)
+			if err != nil {
+				t.Fatal(err)
+			} else if response.StatusCode != http.StatusForbidden {
+				t.Fatal("Expected response \"403 Forbidden\", got:", response.StatusCode)
+			}
+		}
+	}
+
+	for _, httpTargetVer := range testHttpVersions {
+		for _, resource := range testResources {
+			response, err := getViaProxy("172.16.0.1:8080", resource, caddyForwardProxyNoBlacklistOverride.addr, httpTargetVer,
+				"", useTls)
+			if err != nil {
+				t.Fatal(err)
+			} else if response.StatusCode != http.StatusForbidden {
+				t.Fatal("Expected response \"403 Forbidden\", got:", response.StatusCode)
+			}
+		}
+	}
+
+	for _, httpTargetVer := range testHttpVersions {
+		for _, resource := range testResources {
+			response, err := getViaProxy("192.168.192.168:888", resource, caddyForwardProxyNoBlacklistOverride.addr, httpTargetVer,
+				"", useTls)
+			if err != nil {
+				t.Fatal(err)
+			} else if response.StatusCode != http.StatusForbidden {
+				t.Fatal("Expected response \"403 Forbidden\", got:", response.StatusCode)
+			}
+		}
+	}
+}
+
+func TestBlacklistBlocking(t *testing.T) {
+	useTls := true
+	for _, httpTargetVer := range testHttpVersions {
+		for _, resource := range testResources {
+			response, err := getViaProxy(blacklistedDomain, resource, caddyForwardProxyBlackListing.addr, httpTargetVer,
+				"", useTls)
+			if err != nil {
+				t.Fatal(err)
+			} else if response.StatusCode != http.StatusForbidden {
+				t.Fatal("Expected response \"403 Forbidden\", got:", response.StatusCode)
+			}
+		}
+	}
+
+	for _, httpTargetVer := range testHttpVersions {
+		for _, resource := range testResources {
+			response, err := getViaProxy(blacklistedIPv4, resource, caddyForwardProxyBlackListing.addr, httpTargetVer,
+				"", useTls)
+			if err != nil {
+				t.Fatal(err)
+			} else if response.StatusCode != http.StatusForbidden {
+				t.Fatal("Expected response \"403 Forbidden\", got:", response.StatusCode)
+			}
+		}
+	}
+
+	for _, httpTargetVer := range testHttpVersions {
+		for _, resource := range testResources {
+			response, err := getViaProxy(blacklistedIPv6, resource, caddyForwardProxyBlackListing.addr, httpTargetVer,
+				"", useTls)
+			if err != nil {
+				t.Fatal(err)
+			} else if response.StatusCode != http.StatusForbidden {
+				t.Fatal("Expected response \"403 Forbidden\", got:", response.StatusCode)
+			}
+		}
+	}
+}
+
+func TestBlacklistAllowing(t *testing.T) {
+	useTls := true
+	for _, httpTargetVer := range testHttpVersions {
+		for _, resource := range testResources {
+			response, err := getViaProxy(caddyTestTarget.addr, resource, caddyForwardProxyBlackListing.addr, httpTargetVer,
+				"", useTls)
+			if err != nil {
+				t.Fatal(err)
+			} else if err = responseExpected(response, caddyTestTarget.contents[resource]); err != nil {
+				t.Fatal(err)
+			}
+		}
+	}
+}

common_test.go

@@ -38,6 +38,10 @@ Empty/Correct/Wrong -- tries different credentials
 var testResources = []string{"", "/pic.png"}
 var testHttpVersions = []string{"HTTP/2.0", "HTTP/1.1"}
 
+var blacklistedDomain = "google-public-dns-a.google.com" // supposed to ever resolve to one of 2 IP addresses below
+var blacklistedIPv4 = "8.8.8.8"
+var blacklistedIPv6 = "2001:4860:4860::8888"
+
 type caddyTestServer struct {
 	*caddy.Instance
 	addr string // could be http or https
@@ -56,11 +60,15 @@ var (
 	caddyForwardProxyProbeResist caddyTestServer // requires auth, and has probing resistance on
 	caddyDummyProbeResist        caddyTestServer // same as caddyForwardProxyProbeResist, but w/o forwardproxy
 
+	caddyForwardProxyWhiteListing        caddyTestServer
+	caddyForwardProxyBlackListing        caddyTestServer
+	caddyForwardProxyNoBlacklistOverride caddyTestServer // to test default blacklist
+
 	// authenticated server upstreams to authenticated https proxy with different credentials
 	caddyAuthedUpstreamEnter caddyTestServer
 
-	caddyTestTarget     caddyTestServer
-	caddyHTTPTestTarget caddyTestServer
+	caddyTestTarget     caddyTestServer // whitelisted by caddyForwardProxyWhiteListing
+	caddyHTTPTestTarget caddyTestServer // serves plain http on 6480
 )
 
 func (c *caddyTestServer) marshal() []byte {
@@ -88,7 +96,6 @@ func (c *caddyTestServer) marshal() []byte {
 			"}"}
 		mainBlock = append(mainBlock, redirectBlock...)
 	}
-	// fmt.Println(strings.Join(mainBlock, "\n"))
 	return []byte(strings.Join(mainBlock, "\n"))
 }
 
@@ -116,24 +123,27 @@ func (c *caddyTestServer) StartTestServer() {
 }
 
 func TestMain(m *testing.M) {
-	caddyForwardProxy = caddyTestServer{addr: "127.0.0.1:1984", root: "./test/forwardproxy",
+	caddyForwardProxy = caddyTestServer{addr: "127.0.0.2:1984", root: "./test/forwardproxy",
 		directives:   []string{"tls self_signed"},
-		proxyEnabled: true, proxyDirectives: []string{"serve_pac"}}
+		proxyEnabled: true, proxyDirectives: []string{"serve_pac",
+			"acl {\nallow all\n}"}}
 	caddyForwardProxy.StartTestServer()
 
-	caddyForwardProxyAuth = caddyTestServer{addr: "127.0.0.1:4891", root: "./test/forwardproxy",
+	caddyForwardProxyAuth = caddyTestServer{addr: "127.0.0.2:4891", root: "./test/forwardproxy",
 		directives:   []string{"tls self_signed"},
-		proxyEnabled: true, proxyDirectives: []string{"basicauth test pass"}}
+		proxyEnabled: true, proxyDirectives: []string{"basicauth test pass",
+			"acl {\nallow all\n}"}}
 	caddyForwardProxyAuth.StartTestServer()
 
-	caddyForwardProxyProbeResist = caddyTestServer{addr: "127.0.0.1:8888", root: "./test/forwardproxy",
+	caddyForwardProxyProbeResist = caddyTestServer{addr: "127.0.0.2:8888", root: "./test/forwardproxy",
 		directives: []string{"tls self_signed"}, HTTPRedirectPort: "8880",
 		proxyEnabled: true, proxyDirectives: []string{"basicauth test pass",
 			"probe_resistance test.localhost",
-			"serve_pac superhiddenfile.pac"}}
+			"serve_pac superhiddenfile.pac",
+			"acl {\nallow all\n}"}}
 	caddyForwardProxyProbeResist.StartTestServer()
 
-	caddyDummyProbeResist = caddyTestServer{addr: "127.0.0.1:9999", root: "./test/forwardproxy",
+	caddyDummyProbeResist = caddyTestServer{addr: "127.0.0.2:9999", root: "./test/forwardproxy",
 		directives: []string{"tls self_signed"}, HTTPRedirectPort: "9980",
 		proxyEnabled: false}
 	caddyDummyProbeResist.StartTestServer()
@@ -149,12 +159,30 @@ func TestMain(m *testing.M) {
 		proxyEnabled: false}
 	caddyHTTPTestTarget.StartTestServer()
 
-	caddyAuthedUpstreamEnter = caddyTestServer{addr: "127.0.0.1:6585", root: "./test/upstreamingproxy",
+	caddyAuthedUpstreamEnter = caddyTestServer{addr: "127.0.0.2:6585", root: "./test/upstreamingproxy",
 		directives:   []string{"tls self_signed"},
 		proxyEnabled: true, proxyDirectives: []string{"upstream https://test:pass@127.0.0.1:4891",
 			"basicauth upstreamtest upstreampass"}}
 	caddyAuthedUpstreamEnter.StartTestServer()
 
+	caddyForwardProxyWhiteListing = caddyTestServer{addr: "127.0.0.2:8776", root: "./test/forwardproxy",
+		directives:   []string{"tls self_signed"},
+		proxyEnabled: true, proxyDirectives: []string{"acl {\nallow localhost\n deny all\n}",
+			"ports 6451"}}
+	caddyForwardProxyWhiteListing.StartTestServer()
+
+	caddyForwardProxyBlackListing = caddyTestServer{addr: "127.0.0.2:6676", root: "./test/forwardproxy",
+		directives:   []string{"tls self_signed"},
+		proxyEnabled: true, proxyDirectives: []string{"acl {\ndeny " + blacklistedIPv4 + "/30\n" +
+			"deny " + blacklistedIPv6 + "\nallow all\n}"},
+	}
+	caddyForwardProxyBlackListing.StartTestServer()
+
+	caddyForwardProxyNoBlacklistOverride = caddyTestServer{addr: "127.0.0.2:6679", root: "./test/forwardproxy",
+		directives:   []string{"tls self_signed"},
+		proxyEnabled: true, proxyDirectives: []string{}}
+	caddyForwardProxyNoBlacklistOverride.StartTestServer()
+
 	retCode := m.Run()
 
 	caddyForwardProxy.Stop()
@@ -164,6 +192,9 @@ func TestMain(m *testing.M) {
 	caddyTestTarget.Stop()
 	caddyHTTPTestTarget.Stop()
 	caddyAuthedUpstreamEnter.Stop()
+	caddyForwardProxyWhiteListing.Stop()
+	caddyForwardProxyBlackListing.Stop()
+	caddyForwardProxyNoBlacklistOverride.Stop()
 
 	os.Exit(retCode)
 }
@@ -218,24 +249,6 @@ func TestTheTest(t *testing.T) {
 	}
 }
 
-func TestIsSubdomain(t *testing.T) {
-	testSubDomain := func(s, domain string, expectedResult bool) {
-		result := isSubdomain(s, domain)
-		if result != expectedResult {
-			t.Fatalf("Expected: isSubdomain(%s, %s) is %v, Got: %v", s, domain, expectedResult, result)
-		}
-	}
-	testSubDomain("hoooli.abc", "hooya.ya", false)
-	testSubDomain("", "hooya.ya", false)
-	testSubDomain("hoooli.abc", "", false)
-	testSubDomain("hoooli.abc", "hiddenlink.localhost", false)
-	testSubDomain("www.hoooli.abc", "hoooli.abc", true)
-	testSubDomain("hoooli.abc", "hoooli.abc", true)
-	testSubDomain(".hoooli.abc", "hoooli.abc", true)
-	testSubDomain("sup.hoooli.abc", "hoooli.abc", true)
-	testSubDomain("qwe.qwe.qwe.hoooli.abc", "hoooli.abc", true)
-}
-
 func debugIoCopy(dst io.Writer, src io.Reader, prefix string) (written int64, err error) {
 	buf := make([]byte, 32*1024)
 	flusher, ok := dst.(http.Flusher)
@@ -277,7 +290,7 @@ func httpdump(r interface{}) string {
 		if v == nil {
 			return "httpdump: nil"
 		}
-		b, err := httputil.DumpRequest(v, false)
+		b, err := httputil.DumpRequest(v, true)
 		if err != nil {
 			return err.Error()
 		} else {
@@ -287,7 +300,7 @@ func httpdump(r interface{}) string {
 		if v == nil {
 			return "httpdump: nil"
 		}
-		b, err := httputil.DumpResponse(v, false)
+		b, err := httputil.DumpResponse(v, true)
 		if err != nil {
 			return err.Error()
 		} else {

forwardproxy.go

@@ -17,6 +17,7 @@
 package forwardproxy
 
 import (
+	"bufio"
 	"crypto/subtle"
 	"errors"
 	"fmt"
@@ -32,30 +33,49 @@ import (
 )
 
 type ForwardProxy struct {
-	httpTransport      http.Transport
-	Next               httpserver.Handler
-	authRequired       bool
-	authCredentials    [][]byte // slice with base64-encoded credentials
-	hideIP             bool
-	hideVia            bool
-	whitelistedPorts   []int
+	Next httpserver.Handler
+
+	authRequired    bool
+	authCredentials [][]byte // slice with base64-encoded credentials
+
+	hideIP  bool
+	hideVia bool
+
+	pacFilePath string
+
+	hostname string // do not intercept requests to the hostname (except for hidden link)
+	port     string // port on which chain with forwardproxy is listening on
+
 	probeResistDomain  string
-	pacFilePath        string
 	probeResistEnabled bool
-	dialTimeout        time.Duration // for initial tcp connection
-	hostname           string        // do not intercept requests to the hostname (except for hidden link)
-	port               string        // port on which chain with forwardproxy is listening on
+
+	dialTimeout     time.Duration  // for initial tcp connection
+	responseTimeout *time.Duration // for getting response (affects GET requests only)
 
 	// overridden dial allows to redirect requests to upstream proxy
 	dial     func(network, address string) (net.Conn, error)
 	upstream string // address of upstream proxy
+
+	aclRules         []aclRule
+	whitelistedPorts []int
 }
 
 var bufferPool sync.Pool
 
-// TODO?: getStatusCode(err) that casts to http.Error, net Error, etc. and returns correct http status code
+func (fp *ForwardProxy) hostIsAllowed(hostname string, ip net.IP) bool {
+	for _, rule := range fp.aclRules {
+		switch rule.tryMatch(ip, hostname) {
+		case aclDecisionDeny:
+			return false
+		case aclDecisionAllow:
+			return true
+		}
+	}
+	fmt.Println("ERROR: no acl match for ", hostname, ip) // shouldn't happen
+	return false
+}
 
-func (fp *ForwardProxy) connectPortIsAllowed(port string) bool {
+func (fp *ForwardProxy) portIsAllowed(port string) bool {
 	portInt, err := strconv.Atoi(port)
 	if err != nil {
 		return false
@@ -158,17 +178,6 @@ func (fp *ForwardProxy) checkCredentials(r *http.Request) error {
 	return errors.New("Invalid credentials")
 }
 
-// returns true if `s` is `domain` or subdomain of `domain`. Inputs are expected to be sanitized.
-func isSubdomain(s, domain string) bool {
-	if s == domain {
-		return true
-	}
-	if strings.HasSuffix(s, "."+domain) {
-		return true
-	}
-	return false
-}
-
 // borrowed from `proxy` plugin
 func stripPort(address string) string {
 	// Keep in mind that the address might be a IPv6 address
@@ -225,6 +234,53 @@ func (fp *ForwardProxy) servePacFile(w http.ResponseWriter) (int, error) {
 	return 0, nil
 }
 
+// bool indicates whether it was rejected as "Forbidden"
+// TODO: after custom errors are implemented package-wide, remove the bool
+func (fp *ForwardProxy) dialRequestedAddress(r *http.Request) (net.Conn, error, bool) {
+	var err error
+	var conn net.Conn
+	if fp.upstream != "" {
+		// if upstreaming -- do not resolve locally nor check acl
+		conn, err = fp.dial("tcp", r.URL.Host)
+		return conn, err, false
+	}
+	port := r.URL.Port()
+	if port == "" {
+		switch r.Method {
+		case http.MethodGet:
+			port = "80" // implicit port for GET requests
+		case http.MethodConnect:
+			return nil, errors.New("port is required for CONNECT " + r.URL.String()), true
+		default:
+			return nil, errors.New("Method " + r.Method + " is not allowed"), true
+		}
+	}
+	if !fp.portIsAllowed(port) {
+		return nil, errors.New("port " + r.URL.Hostname() + " is not allowed"), true
+	}
+
+	// in case IP was provided, net.LookupIP will simply return it
+	IPs, err := net.LookupIP(r.URL.Hostname())
+	if err != nil {
+		return nil, errors.New(fmt.Sprintf("Lookup of %s failed: %v",
+			r.URL.Hostname(), err)), false
+	}
+
+	// This is net.Dial's default behavior: if the host resolves to multiple IP addresses,
+	// Dial will try each IP address in order until one succeeds
+	for _, ip := range IPs {
+		if !fp.hostIsAllowed(r.URL.Hostname(), ip) {
+			continue
+		}
+
+		conn, err = fp.dial("tcp", net.JoinHostPort(ip.String(), port))
+		if err == nil {
+			return conn, err, false
+		}
+	}
+	return nil, errors.New("No allowed IP addresses for " + r.URL.Hostname()), true
+}
+
 func (fp *ForwardProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
 	var authErr error
 	if fp.authRequired {
@@ -233,7 +289,7 @@ func (fp *ForwardProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) (int,
 	if fp.probeResistEnabled && len(fp.probeResistDomain) > 0 && stripPort(r.Host) == fp.probeResistDomain {
 		return serveHiddenPage(w, authErr)
 	}
-	if isSubdomain(stripPort(r.Host), fp.hostname) && (r.Method != http.MethodConnect || authErr != nil) {
+	if stripPort(r.Host) == fp.hostname && (r.Method != http.MethodConnect || authErr != nil) {
 		// Always pass non-CONNECT requests to hostname
 		// Pass CONNECT requests only if probe resistance is enabled and not authenticated
 		if fp.shouldServePacFile(r) {
@@ -256,6 +312,21 @@ func (fp *ForwardProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) (int,
 		return http.StatusHTTPVersionNotSupported, errors.New("Unsupported HTTP major version: " + strconv.Itoa(r.ProtoMajor))
 	}
 
+	targetConn, err, forbidden := fp.dialRequestedAddress(r)
+	if forbidden {
+		return http.StatusForbidden, err
+	}
+	if err != nil {
+		// failed, but not because it's forbidden
+		return http.StatusBadGateway, errors.New(fmt.Sprintf("dial %s failed: %v", r.URL.Host, err))
+	}
+	if targetConn == nil {
+		// safest to check both error and targetConn afterwards, in case fp.dial (potentially unstable
+		// from x/net/proxy) misbehaves and returns both nil or both non-nil
+		return http.StatusForbidden, errors.New("hostname " + r.URL.Hostname() + " is not allowed")
+	}
+	defer targetConn.Close()
+
 	if r.Method == http.MethodConnect {
 		if r.ProtoMajor == 2 {
 			if len(r.URL.Scheme) > 0 || len(r.URL.Path) > 0 {
@@ -263,16 +334,6 @@ func (fp *ForwardProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) (int,
 			}
 		}
 
-		if !fp.connectPortIsAllowed(r.URL.Port()) {
-			return http.StatusForbidden, errors.New("CONNECT port not allowed for " + r.URL.String())
-		}
-
-		targetConn, err := fp.dial("tcp", r.URL.Hostname()+":"+r.URL.Port())
-		if err != nil {
-			return http.StatusBadGateway, errors.New(fmt.Sprintf("Dial %s failed: %v", r.URL.String(), err))
-		}
-		defer targetConn.Close()
-
 		switch r.ProtoMajor {
 		case 1: // http1: hijack the whole flow
 			return serveHijack(w, targetConn)
@@ -310,15 +371,22 @@ func (fp *ForwardProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) (int,
 		if !fp.hideVia {
 			r.Header.Add("Via", strconv.Itoa(r.ProtoMajor)+"."+strconv.Itoa(r.ProtoMinor)+" caddy")
 		}
-		response, err := fp.httpTransport.RoundTrip(r)
-		if err != nil {
-			if response != nil {
-				if response.StatusCode != 0 {
-					return response.StatusCode, errors.New("failed to do RoundTrip(): " + err.Error())
-				}
-			}
-			return http.StatusBadGateway, errors.New("failed to do RoundTrip(): " + err.Error())
+
+		if fp.responseTimeout != nil {
+			targetConn.SetDeadline(time.Now().Add(*fp.responseTimeout))
 		}
+
+		var response *http.Response
+		err = r.Write(targetConn)
+		if err != nil {
+			return http.StatusBadGateway, errors.New("failed to write http request: " + err.Error())
+		}
+		response, err = http.ReadResponse(bufio.NewReader(targetConn), r)
+		if err != nil {
+			return http.StatusBadGateway, errors.New("failed to read http response: " + err.Error())
+		}
+
+		// TODO?: check 301 and 302 redirects against ACL and follow them
 		return 0, forwardResponse(w, response)
 	}
 }

forwardproxy_test.go

@@ -61,7 +61,7 @@ func connectAndGetViaProxy(targetHost, resource, proxyAddr, httpTargetVer, proxy
 		connectRequest.Header.Set("Proxy-Authorization", proxyCredentials)
 	}
 	connectRequest.Host = targetHost
-	connectRequest.URL, err = url.Parse("http://" + connectRequest.Host)
+	connectRequest.URL, err = url.Parse("https://" + connectRequest.Host)
 	if err != nil {
 		return nil, err
 	}

setup.go

@@ -33,21 +33,14 @@ import (
 	"github.com/mholt/caddy"
 	"github.com/mholt/caddy/caddyhttp/httpserver"
 	"golang.org/x/net/proxy"
+	"os"
 )
 
 func setup(c *caddy.Controller) error {
 	httpserver.GetConfig(c).FallbackSite = true
-	fp := &ForwardProxy{dialTimeout: time.Second * 20,
-		hostname: httpserver.GetConfig(c).Host(), port: httpserver.GetConfig(c).Port(),
-		httpTransport: http.Transport{
-			Proxy:                 http.ProxyFromEnvironment,
-			MaxIdleConns:          100,
-			IdleConnTimeout:       90 * time.Second,
-			TLSHandshakeTimeout:   10 * time.Second,
-			ExpectContinueTimeout: 1 * time.Second,
-		}}
-	fp.httpTransport.DialTLS = func(network, addr string) (net.Conn, error) {
-		return nil, &http.ProtocolError{ErrorString: "Proxy does not fetch TLS resources, use CONNECT instead"}
+	fp := &ForwardProxy{
+		dialTimeout: time.Second * 20,
+		hostname:    httpserver.GetConfig(c).Host(), port: httpserver.GetConfig(c).Port(),
 	}
 
 	c.Next() // skip the directive name
@@ -66,11 +59,11 @@ func setup(c *caddy.Controller) error {
 				return c.ArgErr()
 			}
 			if len(args[0]) == 0 {
-				return errors.New("Parse error: empty usernames are not allowed")
+				return c.Err("empty usernames are not allowed")
 			}
 			// TODO: Evaluate policy of allowing empty passwords.
 			if strings.Contains(args[0], ":") {
-				return errors.New("Parse error: character ':' in usernames is not allowed")
+				return c.Err("character ':' in usernames is not allowed")
 			}
 			if fp.authCredentials == nil {
 				fp.authCredentials = [][]byte{}
@@ -85,13 +78,13 @@ func setup(c *caddy.Controller) error {
 				return c.ArgErr()
 			}
 			if len(fp.whitelistedPorts) != 0 {
-				return errors.New("Parse error: ports subdirective specified twice")
+				return c.Err("ports subdirective specified twice")
 			}
 			fp.whitelistedPorts = make([]int, len(args))
 			for i, p := range args {
 				intPort, err := strconv.Atoi(p)
 				if intPort <= 0 || intPort > 65535 || err != nil {
-					return errors.New("Parse error: ports are expected to be space-separated" +
+					return c.Err("ports are expected to be space-separated" +
 						" and in 0-65535 range. Got: " + p)
 				}
 				fp.whitelistedPorts[i] = intPort
@@ -123,7 +116,7 @@ func setup(c *caddy.Controller) error {
 				return c.ArgErr()
 			}
 			if len(fp.pacFilePath) != 0 {
-				return errors.New("Parse error: serve_pac subdirective specified twice")
+				return c.Err("serve_pac subdirective specified twice")
 			}
 			if len(args) == 1 {
 				fp.pacFilePath = args[0]
@@ -143,9 +136,10 @@ func setup(c *caddy.Controller) error {
 				return c.ArgErr()
 			}
 			if timeout < 0 {
-				return errors.New("Parse error: response_timeout cannot be negative.")
+				return c.Err("response_timeout cannot be negative.")
 			}
-			fp.httpTransport.ResponseHeaderTimeout = time.Second * time.Duration(timeout)
+			responseTimeout := time.Duration(timeout) * time.Second
+			fp.responseTimeout = &responseTimeout
 		case "dial_timeout":
 			if len(args) != 1 {
 				return c.ArgErr()
@@ -155,7 +149,7 @@ func setup(c *caddy.Controller) error {
 				return c.ArgErr()
 			}
 			if timeout < 0 {
-				return errors.New("Parse error: dial_timeout cannot be negative.")
+				return c.Err("dial_timeout cannot be negative.")
 			}
 			fp.dialTimeout = time.Second * time.Duration(timeout)
 		case "upstream":
@@ -163,14 +157,97 @@ func setup(c *caddy.Controller) error {
 				return c.ArgErr()
 			}
 			fp.upstream = args[0]
+		case "acl":
+			if len(args) != 0 {
+				return c.Err("acl should be only subdirective on the line")
+			}
+			args := c.RemainingArgs()
+			if len(args) > 0 {
+				return c.ArgErr()
+			}
+			c.Next()
+			if c.Val() != "{" {
+				return c.Err("acl directive must be followed by opening curly braces \"{\"")
+			}
+			for {
+				if !c.Next() {
+					return c.Err("acl blockmust be ended by closing curly braces \"}\"")
+				}
+				aclDirective := c.Val()
+				args := c.RemainingArgs()
+				if aclDirective == "}" {
+					break
+				}
+				if len(args) == 0 {
+					return c.ArgErr()
+				}
+				var ruleSubjects []string
+				var err error
+				aclAllow := false
+				switch aclDirective {
+				case "allow":
+					ruleSubjects = args[:]
+					aclAllow = true
+				case "allowfile":
+					if len(args) != 1 {
+						return c.Err("allowfile accepts a single filename argument")
+					}
+					ruleSubjects, err = readLinesFromFile(args[0])
+					if err != nil {
+						return err
+					}
+					aclAllow = true
+				case "deny":
+					ruleSubjects = args[:]
+				case "denyfile":
+					if len(args) != 1 {
+						return c.Err("denyfile accepts a single filename argument")
+					}
+					ruleSubjects, err = readLinesFromFile(args[0])
+					if err != nil {
+						return err
+					}
+				default:
+					return c.Err("expected acl directive: allow/allowfile/deny/denyfile." +
+						"got: " + aclDirective)
+				}
+				for _, rs := range ruleSubjects {
+					ar, err := newAclRule(rs, aclAllow)
+					if err != nil {
+						return err
+					}
+					fp.aclRules = append(fp.aclRules, ar)
+				}
+			}
 		default:
 			return c.ArgErr()
 		}
 	}
 
+	if fp.upstream != "" && (fp.aclRules != nil || len(fp.whitelistedPorts) != 0) {
+		return c.Err("upstream subdirective is incompatible with acl/ports subdirectives")
+	}
+
+	for _, ipDeny := range []string{
+		"10.0.0.0/8",
+		"127.0.0.0/8",
+		"172.16.0.0/12",
+		"192.168.0.0/16",
+		"::1/128",
+		"fe80::/10",
+	} {
+		ar, err := newAclRule(ipDeny, false)
+		if err != nil {
+			panic(err)
+		}
+		fp.aclRules = append(fp.aclRules, ar)
+	}
+	fp.aclRules = append(fp.aclRules, &aclAllRule{allow: true})
+
 	if fp.probeResistEnabled {
 		if !fp.authRequired {
-			return errors.New("Parse error: probing resistance requires authentication")
+			return c.Err("probing resistance requires authentication: " +
+				"add `basicauth username password` to forwardproxy")
 		}
 		if len(fp.probeResistDomain) > 0 {
 			log.Printf("Secret domain used to connect to proxy: %s\n", fp.probeResistDomain)
@@ -183,13 +260,15 @@ func setup(c *caddy.Controller) error {
 		DualStack: true,
 	}
 
+	fp.dial = dialer.Dial
+
 	if fp.upstream != "" {
 		upstreamURL, err := url.Parse(fp.upstream)
 		if err != nil {
 			return errors.New("failed to parse upstream address: " + err.Error())
 		}
 
-		if !isLocalhost(upstreamURL) && upstreamURL.Scheme != "https" {
+		if !isLocalhost(upstreamURL.Hostname()) && upstreamURL.Scheme != "https" {
 			return errors.New("insecure schemes are only allowed to localhost upstreams")
 		}
 
@@ -208,10 +287,6 @@ func setup(c *caddy.Controller) error {
 			return errors.New("failed to create proxy to upstream: " + err.Error())
 		}
 		fp.dial = newDialer.Dial
-		fp.httpTransport.Dial = newDialer.Dial
-	} else {
-		fp.dial = dialer.Dial
-		fp.httpTransport.DialContext = dialer.DialContext
 	}
 
 	httpserver.GetConfig(c).AddMiddleware(func(next httpserver.Handler) httpserver.Handler {
@@ -246,8 +321,8 @@ func NewHTTPDialer(dialer *net.Dialer, useHTTPS bool, upstream *url.URL) *HTTPDi
 	}
 	if useHTTPS {
 		d.tlsConf = &tls.Config{ServerName: upstream.Hostname()}
-		if isLocalhost(upstream) {
-			log.Println("Localhost upstream detected, disabling verification of TLS ceritifcate")
+		if isLocalhost(upstream.Hostname()) {
+			log.Println("Localhost upstream detected, disabling verification of TLS certificate")
 			d.tlsConf.InsecureSkipVerify = true
 		}
 	}
@@ -285,10 +360,47 @@ func (d *HTTPDialer) Dial(network, addr string) (net.Conn, error) {
 	return c, nil
 }
 
-func isLocalhost(u *url.URL) bool {
-	if u.Hostname() == "localhost" || u.Hostname() == "127.0.0.1" ||
-		u.Hostname() == "::1" {
+func isLocalhost(hostname string) bool {
+	if hostname == "localhost" || hostname == "127.0.0.1" || hostname == "::1" {
 		return true
 	}
 	return false
 }
+
+func readLinesFromFile(filename string) ([]string, error) {
+	file, err := os.Open(filename)
+	if err != nil {
+		return nil, err
+	}
+	defer file.Close()
+
+	var hostnames []string
+	scanner := bufio.NewScanner(file)
+	for scanner.Scan() {
+		hostnames = append(hostnames, scanner.Text())
+	}
+
+	return hostnames, scanner.Err()
+}
+
+// isValidDomainLite shamelessly rejects non-LDH names. returns nil if domains seems valid
+func isValidDomainLite(domain string) error {
+	for i := 0; i < len(domain); i++ {
+		c := domain[i]
+		if 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || c == '_' || '0' <= c && c <= '9' ||
+			c == '-' || c == '.' {
+			continue
+		}
+		return errors.New("character " + string(c) + " is not allowed")
+	}
+	sections := strings.Split(domain, ".")
+	for _, s := range sections {
+		if len(s) == 0 {
+			return errors.New("empty section between dots in domain name or trailing dot")
+		}
+		if len(s) > 63 {
+			return errors.New("domain name section is too long")
+		}
+	}
+	return nil
+}

setup_test.go

@@ -120,4 +120,18 @@ func TestSetup(t *testing.T) {
 	testParsing([]string{"upstream http://proxy.site"}, false)
 	testParsing([]string{"upstream https://proxy.site https://proxy.site"}, false)
 	testParsing([]string{"upstream https://proxy.site"}, true)
+	testParsing([]string{"upstream https://caddyserver.com", "acl {\nallow all\n}"}, false)
+	testParsing([]string{"upstream https://caddyserver.com", "ports 123"}, false)
+
+	testParsing([]string{"acl {\nallow all\n}"}, true)
+	testParsing([]string{"acl {\nallow localhost 128.32.22.1/32 1.1.1.1 caddyserver.com\n deny all\n}"}, true)
+	testParsing([]string{"acl {\nallowfile test/parseable_acl.txt\n}"}, true)
+	testParsing([]string{"acl {\ndenyfile test/parseable_acl.txt\n}"}, true)
+	testParsing([]string{"acl {\nallowfile test/unparseable_acl.txt\n}"}, false)
+	testParsing([]string{"acl {\ndenyfile test/unparseable_acl.txt\n}"}, false)
+	//testParsing([]string{"acl {\nallow all\n"}, false) // doesn't fail, but should: caddy itself doesn't demand curly brace to be closed
+	testParsing([]string{"acl {\nallow all\n", "serve_pac"}, false) // this does fail
+	testParsing([]string{"acl \nallow all\n}"}, false)
+	testParsing([]string{"acl {allow all\n}"}, false)
+	//testParsing([]string{"acl {\nallow all}"}, false) // '}' is not on the next line, "all}" parses as regexp
 }

test/parseable_acl.txt

@@ -0,0 +1,6 @@
+128.12.2.3
+123.32.1.1/32
+qwe.com
+localhost
+lalalala
+usetor.usesignal

test/unparseable_acl.txt

@@ -0,0 +1,4 @@
+128.3.3.1/23
+previous.line.is.parseable.com
+but.next.line.is.not.parseable.com
+(za0zaz