Implements --trusted-ip option (#552)

* Implements --ip-whitelist option * Included IPWhitelist option to allow one-or-more selected CIDR ranges to bypass OAuth2 authentication. * Adds IPWhitelist, a fast lookup table for multiple CIDR ranges. * Renamed IPWhitelist ipCIDRSet * Fixed unessesary pointer usage in ipCIDRSet * Update CHANGELOG.md * Update CHANGELOG.md * Updated to not use err.Error() in printf statements * Imrpoved language for --ip-whitelist descriptions. * Improve IP whitelist options error messages * Clarify options single-host normalization * Wrote a book about ipCIDRSet * Added comment to IsWhitelistedIP in oauthproxy.go * Rewrite oauthproxy test case as table driven * oops * Support whitelisting by low-level remote address * Added more test-cases, improved descriptions * Move ip_cidr_set.go to pkg/ip/net_set.go * Add more whitelist test use cases. * Oops * Use subtests for TestIPWhitelist * Add minimal tests for ip.NetSet * Use switch statment * Renamed ip-whitelist to whitelist-ip * Update documentation with a warning. * Update pkg/apis/options/options.go * Update CHANGELOG.md Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * Update pkg/ip/net_set_test.go Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * Update pkg/ip/net_set_test.go Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * Update pkg/ip/net_set_test.go Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * Apply suggestions from code review Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * fix fmt * Move ParseIPNet into abstraction * Add warning in case of --reverse-proxy * Update pkg/validation/options_test.go * Rename --whitelist-ip to --trusted-ip * Update oauthproxy.go Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * fix Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>
2026-10-17 11:55:38 +02:00 · 2020-07-11 11:10:58 +01:00
parent e6903d8c1f
commit 64ae31b5a0
12 changed files with 541 additions and 17 deletions
@@ -21,21 +21,22 @@ type SignatureData struct {
 // Options holds Configuration Options that can be set by Command Line Flag,
 // or Config File
 type Options struct {
-	ProxyPrefix        string `flag:"proxy-prefix" cfg:"proxy_prefix"`
-	PingPath           string `flag:"ping-path" cfg:"ping_path"`
-	PingUserAgent      string `flag:"ping-user-agent" cfg:"ping_user_agent"`
-	ProxyWebSockets    bool   `flag:"proxy-websockets" cfg:"proxy_websockets"`
-	HTTPAddress        string `flag:"http-address" cfg:"http_address"`
-	HTTPSAddress       string `flag:"https-address" cfg:"https_address"`
-	ReverseProxy       bool   `flag:"reverse-proxy" cfg:"reverse_proxy"`
-	RealClientIPHeader string `flag:"real-client-ip-header" cfg:"real_client_ip_header"`
-	ForceHTTPS         bool   `flag:"force-https" cfg:"force_https"`
-	RawRedirectURL     string `flag:"redirect-url" cfg:"redirect_url"`
-	ClientID           string `flag:"client-id" cfg:"client_id"`
-	ClientSecret       string `flag:"client-secret" cfg:"client_secret"`
-	ClientSecretFile   string `flag:"client-secret-file" cfg:"client_secret_file"`
-	TLSCertFile        string `flag:"tls-cert-file" cfg:"tls_cert_file"`
-	TLSKeyFile         string `flag:"tls-key-file" cfg:"tls_key_file"`
+	ProxyPrefix        string   `flag:"proxy-prefix" cfg:"proxy_prefix"`
+	PingPath           string   `flag:"ping-path" cfg:"ping_path"`
+	PingUserAgent      string   `flag:"ping-user-agent" cfg:"ping_user_agent"`
+	ProxyWebSockets    bool     `flag:"proxy-websockets" cfg:"proxy_websockets"`
+	HTTPAddress        string   `flag:"http-address" cfg:"http_address"`
+	HTTPSAddress       string   `flag:"https-address" cfg:"https_address"`
+	ReverseProxy       bool     `flag:"reverse-proxy" cfg:"reverse_proxy"`
+	RealClientIPHeader string   `flag:"real-client-ip-header" cfg:"real_client_ip_header"`
+	TrustedIPs         []string `flag:"trusted-ip" cfg:"trusted_ips"`
+	ForceHTTPS         bool     `flag:"force-https" cfg:"force_https"`
+	RawRedirectURL     string   `flag:"redirect-url" cfg:"redirect_url"`
+	ClientID           string   `flag:"client-id" cfg:"client_id"`
+	ClientSecret       string   `flag:"client-secret" cfg:"client_secret"`
+	ClientSecretFile   string   `flag:"client-secret-file" cfg:"client_secret_file"`
+	TLSCertFile        string   `flag:"tls-cert-file" cfg:"tls_cert_file"`
+	TLSKeyFile         string   `flag:"tls-key-file" cfg:"tls_key_file"`

 	AuthenticatedEmailsFile  string   `flag:"authenticated-emails-file" cfg:"authenticated_emails_file"`
 	KeycloakGroup            string   `flag:"keycloak-group" cfg:"keycloak_group"`
@@ -186,6 +187,7 @@ func NewFlagSet() *pflag.FlagSet {
 	flagSet.String("https-address", ":443", "<addr>:<port> to listen on for HTTPS clients")
 	flagSet.Bool("reverse-proxy", false, "are we running behind a reverse proxy, controls whether headers like X-Real-Ip are accepted")
 	flagSet.String("real-client-ip-header", "X-Real-IP", "Header used to determine the real IP of the client (one of: X-Forwarded-For, X-Real-IP, or X-ProxyUser-IP)")
+	flagSet.StringSlice("trusted-ip", []string{}, "list of IPs or CIDR ranges to allow to bypass authentication. WARNING: trusting by IP has inherent security flaws, read the configuration documentation for more information.")
 	flagSet.Bool("force-https", false, "force HTTPS redirect for HTTP requests")
 	flagSet.String("tls-cert-file", "", "path to certificate file")
 	flagSet.String("tls-key-file", "", "path to private key file")
@@ -0,0 +1,112 @@
+package ip
+
+import (
+	"fmt"
+	"net"
+)
+
+// Fast lookup table for intersection of a single IP address within a collection of CIDR networks.
+//
+// Supports 4-byte (IPv4) and 16-byte (IPv6) networks.
+//
+// Provides O(1) best-case, O(log(n)) worst-case performance.
+// In practice netmasks included will generally only be of standard lengths:
+// - /8, /16, /24, and /32 for IPv4
+// - /64 and /128 for IPv6.
+// As a result, typical lookup times will lean closer to best-case rather than worst-case even when most of the internet
+// is included.
+type NetSet struct {
+	ip4NetMaps []ipNetMap
+	ip6NetMaps []ipNetMap
+}
+
+// Create a new NetSet with all of the provided networks.
+func NewNetSet() *NetSet {
+	return &NetSet{
+		ip4NetMaps: make([]ipNetMap, 0),
+		ip6NetMaps: make([]ipNetMap, 0),
+	}
+}
+
+// Check if `ip` is in the set, true if within the set otherwise false.
+func (w *NetSet) Has(ip net.IP) bool {
+	netMaps := w.getNetMaps(ip)
+
+	// Check all ipNetMaps for intersection with `ip`.
+	for _, netMap := range *netMaps {
+		if netMap.has(ip) {
+			return true
+		}
+	}
+	return false
+}
+
+// Add an CIDR network to the set.
+func (w *NetSet) AddIPNet(ipNet net.IPNet) {
+	netMaps := w.getNetMaps(ipNet.IP)
+
+	// Determine the size / number of ones in the CIDR network mask.
+	ones, _ := ipNet.Mask.Size()
+
+	var netMap *ipNetMap
+
+	// Search for the ipNetMap containing networks with the same number of ones.
+	for i := 0; len(*netMaps) > i; i++ {
+		if netMapOnes, _ := (*netMaps)[i].mask.Size(); netMapOnes == ones {
+			netMap = &(*netMaps)[i]
+			break
+		}
+	}
+
+	// Create a new ipNetMap if none with this number of ones have been created yet.
+	if netMap == nil {
+		netMap = &ipNetMap{
+			mask: ipNet.Mask,
+			ips:  make(map[string]bool),
+		}
+		*netMaps = append(*netMaps, *netMap)
+		// Recurse once now that there exists an netMap.
+		w.AddIPNet(ipNet)
+		return
+	}
+
+	// Add the IP to the ipNetMap.
+	netMap.ips[ipNet.IP.String()] = true
+}
+
+// Get the appropriate array of networks for the given IP version.
+func (w *NetSet) getNetMaps(ip net.IP) (netMaps *[]ipNetMap) {
+	switch {
+	case ip.To4() != nil:
+		netMaps = &w.ip4NetMaps
+	case ip.To16() != nil:
+		netMaps = &w.ip6NetMaps
+	default:
+		panic(fmt.Sprintf("IP (%s) is neither 4-byte nor 16-byte?", ip.String()))
+	}
+
+	return netMaps
+}
+
+// Hash-set of CIDR networks with the same mask size.
+type ipNetMap struct {
+	mask net.IPMask
+	ips  map[string]bool
+}
+
+// Check if the IP is in any of the CIDR networks contained in this map.
+func (m ipNetMap) has(ip net.IP) bool {
+	// Apply the mask to the IP to remove any irrelevant bits in the IP.
+	ipMasked := ip.Mask(m.mask)
+	if ipMasked == nil {
+		panic(fmt.Sprintf(
+			"Mismatch in net.IPMask and net.IP protocol version, cannot apply mask %s to %s",
+			m.mask.String(), ip.String()))
+	}
+
+	// Check if the masked IP is the same as any of the networks.
+	if _, ok := m.ips[ipMasked.String()]; ok {
+		return true
+	}
+	return false
+}
@@ -0,0 +1,117 @@
+package ip
+
+import (
+	"net"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestEmptyNetSet(t *testing.T) {
+	set := NewNetSet()
+
+	ips := []string{
+		"127.0.0.1",
+		"241.163.75.163",
+		"189.248.81.89",
+		"167.75.10.139",
+		"22.169.8.110",
+		"::1",
+		"114d:28ae:60a:7b12:b851:fd49:5ff9:9b9d",
+		"4e73:55a5:6cd:29c7:5db6:673a:4608:1a3f",
+		"f6b0:d7f3:8206:b529:ee3b:49ff:1b3b:9aac",
+		"09d8:a74c:652f:3763:b87d:f068:b593:b588",
+	}
+	for _, ip := range ips {
+		assert.Falsef(t, set.Has(net.ParseIP(ip)), "Empty NetSet must not have %q", ip)
+	}
+}
+
+func TestV4ContainsEverything(t *testing.T) {
+	set := NewNetSet()
+	set.AddIPNet(net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: net.CIDRMask(0, 32)})
+
+	ip4s := []string{
+		"127.0.0.1",
+		"250.169.183.192",
+		"175.125.175.160",
+		"178.244.137.158",
+		"190.13.12.194",
+	}
+	for _, ip := range ip4s {
+		assert.Truef(t, set.Has(net.ParseIP(ip)), "NetSet{\"0.0.0.0/0\"} must have %q", ip)
+	}
+	ip6s := []string{
+		"::1",
+		"7261:ca92:c9a5:4b79:2e:8889:b769:1d62",
+		"f54f:10cb:e6a4:89f5:3366:1e3e:2d22:f68e",
+		"d1f3:be6:60d:361:1717:4fe6:9812:1a6c",
+		"7eaa:2a8:365c:b55f:67cd:96dd:602a:4385",
+	}
+	for _, ip := range ip6s {
+		assert.Falsef(t, set.Has(net.ParseIP(ip)), "NetSet{\"0.0.0.0/0\"} must not have %q", ip)
+	}
+}
+
+func TestV6ContainsEverything(t *testing.T) {
+	set := NewNetSet()
+	set.AddIPNet(net.IPNet{IP: net.ParseIP("::"), Mask: net.CIDRMask(0, 128)})
+
+	ip4s := []string{
+		"127.0.0.1",
+		"216.163.140.97",
+		"175.55.31.226",
+		"115.112.186.19",
+		"250.83.241.122",
+	}
+	for _, ip := range ip4s {
+		assert.Falsef(t, set.Has(net.ParseIP(ip)), "NetSet{\"::/0\"} must not have %q", ip)
+	}
+	ip6s := []string{
+		"::1",
+		"fa3b:7f17:7521:d9f4:d855:51f9:4b63:de7f",
+		"4d47:c29:2479:41ce:69f9:3d33:306a:91c8",
+		"fe99:f364:f8cd:7a11:838:a36f:b9a1:965",
+		"fb9e:d809:881d:9cee:533d:1ba5:592e:ea9b",
+	}
+	for _, ip := range ip6s {
+		assert.Truef(t, set.Has(net.ParseIP(ip)), "NetSet{\"::/0\"} must have %q", ip)
+	}
+}
+
+func TestLocalhostOnly(t *testing.T) {
+	set := NewNetSet()
+	set.AddIPNet(net.IPNet{IP: net.ParseIP("127.0.0.0"), Mask: net.CIDRMask(8, 32)})
+	set.AddIPNet(net.IPNet{IP: net.ParseIP("::1"), Mask: net.CIDRMask(128, 128)})
+
+	included := []string{
+		"127.0.0.1",
+		"127.196.199.162",
+		"127.78.85.40",
+		"127.205.171.240",
+		"127.121.187.93",
+		"127.5.217.10",
+		"127.22.172.251",
+		"127.12.60.198",
+		"127.110.240.252",
+		"::1",
+	}
+	for _, ip := range included {
+		assert.Truef(t, set.Has(net.ParseIP(ip)), "NetSet{\"127.0.0.0/8\", \"::1\"} must have %q", ip)
+	}
+	excluded := []string{
+		"15.65.169.230",
+		"196.72.207.65",
+		"43.46.246.121",
+		"69.147.193.224",
+		"43.130.74.115",
+		"8375:b8f9:70bf:a247:f55e:b0c0:c24f:eebe",
+		"d325:aabf:ef49:2d46:ba47:1aa5:9e57:b0bf",
+		"d2f6:831:170:e04e:bd6d:7ae7:e14a:71c3",
+		"ca80:483f:bb7f:df2a:bceb:de5e:6129:9625",
+		"f3fe:f771:9138:5d63:8e83:7033:2e6f:4762",
+	}
+	for _, ip := range excluded {
+		assert.Falsef(t, set.Has(net.ParseIP(ip)), "NetSet{\"127.0.0.0/8\", \"::1\"} must not have %q", ip)
+	}
+}
@@ -0,0 +1,39 @@
+package ip
+
+import (
+	"net"
+	"strings"
+)
+
+func ParseIPNet(s string) *net.IPNet {
+	if !strings.ContainsRune(s, '/') {
+		ip := net.ParseIP(s)
+		if ip == nil {
+			return nil
+		}
+
+		var mask net.IPMask
+		switch {
+		case ip.To4() != nil:
+			mask = net.CIDRMask(32, 32)
+		case ip.To16() != nil:
+			mask = net.CIDRMask(128, 128)
+		default:
+			return nil
+		}
+
+		return &net.IPNet{
+			IP:   ip,
+			Mask: mask,
+		}
+	}
+
+	switch ip, ipNet, err := net.ParseCIDR(s); {
+	case err != nil:
+		return nil
+	case !ipNet.IP.Equal(ip):
+		return nil
+	default:
+		return ipNet
+	}
+}
@@ -58,6 +58,14 @@ func (p xForwardedForClientIPParser) GetRealClientIP(h http.Header) (net.IP, err
 	return ip, nil
 }

+// GetClientIP obtains the perceived end-user IP address from headers if p != nil else from req.RemoteAddr.
+func GetClientIP(p ipapi.RealClientIPParser, req *http.Request) (net.IP, error) {
+	if p != nil {
+		return p.GetRealClientIP(req.Header)
+	}
+	return getRemoteIP(req)
+}
+
 // getRemoteIP obtains the IP of the low-level connected network host
 func getRemoteIP(req *http.Request) (net.IP, error) {
 	if ipStr, _, err := net.SplitHostPort(req.RemoteAddr); err != nil {
@@ -225,6 +225,16 @@ func Validate(o *options.Options) error {
 		})
 	}

+	if len(o.TrustedIPs) > 0 && o.ReverseProxy {
+		fmt.Fprintln(os.Stderr, "WARNING: trusting of IPs with --reverse-proxy poses risks if a header spoofing attack is possible.")
+	}
+
+	for i, ipStr := range o.TrustedIPs {
+		if nil == ip.ParseIPNet(ipStr) {
+			msgs = append(msgs, fmt.Sprintf("trusted_ips[%d] (%s) could not be recognized", i, ipStr))
+		}
+	}
+
 	if len(msgs) != 0 {
 		return fmt.Errorf("invalid configuration:\n  %s",
 			strings.Join(msgs, "\n  "))
@@ -2,6 +2,7 @@ package validation

 import (
 	"crypto"
+	"errors"
 	"io/ioutil"
 	"net/url"
 	"os"
@@ -352,6 +353,45 @@ func TestRealClientIPHeader(t *testing.T) {
 	assert.Nil(t, o.GetRealClientIPParser())
 }

+func TestIPCIDRSetOption(t *testing.T) {
+	tests := []struct {
+		name       string
+		trustedIPs []string
+		err        error
+	}{
+		{
+			"TestSomeIPs",
+			[]string{"127.0.0.1", "10.32.0.1/32", "43.36.201.0/24", "::1", "2a12:105:ee7:9234:0:0:0:0/64"},
+			nil,
+		}, {
+			"TestOverlappingIPs",
+			[]string{"135.180.78.199", "135.180.78.199/32", "d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4", "d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4/128"},
+			nil,
+		}, {
+			"TestInvalidIPs",
+			[]string{"[::1]", "alkwlkbn/32"},
+			errors.New(
+				"invalid configuration:\n" +
+					"  trusted_ips[0] ([::1]) could not be recognized\n" +
+					"  trusted_ips[1] (alkwlkbn/32) could not be recognized",
+			),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			o := testOptions()
+			o.TrustedIPs = tt.trustedIPs
+			err := Validate(o)
+			if tt.err == nil {
+				assert.Nil(t, err)
+			} else {
+				assert.Equal(t, tt.err.Error(), err.Error())
+			}
+		})
+	}
+}
+
 func TestProviderCAFilesError(t *testing.T) {
 	file, err := ioutil.TempFile("", "absent.*.crt")
 	assert.NoError(t, err)