geoip: read source IP from EDNS0 subnet if provided (#5183)

* geoip: read source IP from EDNS0 subnet if provided This patch implements EDNS backend processing (similar in powerdns: https://doc.powerdns.com/authoritative/settings.html#setting-edns-subnet-processing). This feature comes very handy to test whether your geo config is working properly. Signed-off-by: Balazs Nagy <julsevern@gmail.com>
2022-05-02 19:25:02 +02:00 · 2022-05-02 19:25:02 +02:00 · 4ae29a449c
commit 4ae29a449c
parent 66f2ac7568
5 changed files with 122 additions and 50 deletions
--- a/plugin/geoip/README.md
+++ b/plugin/geoip/README.md
@ -1,9 +1,11 @@
 # geoip

 ## Name
+
 *geoip* - Lookup maxmind geoip2 databases using the client IP, then add associated geoip data to the context request.

 ## Description
+
 The *geoip* plugin add geo location data associated with the client IP, it allows you to configure a [geoIP2 maxmind database](https://dev.maxmind.com/geoip/docs/databases) to add the geo location data associated with the IP address.

 The data is added leveraging the *metadata* plugin, values can then be retrieved using it as well, for example:
@ -16,8 +18,8 @@ import (
 // ...
 if getLongitude := metadata.ValueFunc(ctx, "geoip/longitude"); getLongitude != nil {
    if longitude, err := strconv.ParseFloat(getLongitude(), 64); err == nil {
-		// Do something useful with longitude.
-	}
+        // Do something useful with longitude.
+    }
 } else {
    // The metadata label geoip/longitude for some reason, was not set.
 }
@ -25,26 +27,47 @@ if getLongitude := metadata.ValueFunc(ctx, "geoip/longitude"); getLongitude != n
 ```

 ## Databases
+
 The supported databases use city schema such as `City` and `Enterprise`. Other databases types with different schemas are not supported yet.

 You can download a [free and public City database](https://dev.maxmind.com/geoip/geolite2-free-geolocation-data).

 ## Syntax
-```txt
+
+```text
 geoip [DBFILE]
 ```
-* **DBFILE** the mmdb database file path.
+
+or
+
+```text
+geoip [DBFILE] {
+    [edns-subnet]
+}
+```
+
+* **DBFILE** the mmdb database file path. We recommend updating your mmdb database periodically for more accurate results.
+* `edns-subnet`: Optional. Use [EDNS0 subnet](https://en.wikipedia.org/wiki/EDNS_Client_Subnet) (if present) for Geo IP instead of the source IP of the DNS request. This helps identifying the closest source IP address through intermediary DNS resolvers, and it also makes GeoIP testing easy: `dig +subnet=1.2.3.4 @dns-server.example.com www.geo-aware.com`.
+
+  **NOTE:** due to security reasons, recursive DNS resolvers may mask a few bits off of the clients' IP address, which can cause inaccuracies in GeoIP resolution.
+
+  There is no defined mask size in the standards, but there are examples: [RFC 7871's example](https://datatracker.ietf.org/doc/html/rfc7871#section-13) conceals the last 72 bits of an IPv6 source address, and NS1 Help Center [mentions](https://help.ns1.com/hc/en-us/articles/360020256573-About-the-EDNS-Client-Subnet-ECS-DNS-extension) that ECS-enabled DNS resolvers send only the first three octets (eg. /24) of the source IPv4 address.

 ## Examples
-The following configuration configures the `City` database.
+
+The following configuration configures the `City` database, and looks up geolocation based on EDNS0 subnet if present.
+
 ```txt
 . {
-    geoip /opt/geoip2/db/GeoLite2-City.mmdb
+    geoip /opt/geoip2/db/GeoLite2-City.mmdb {
+      edns-subnet
+    }
    metadata # Note that metadata plugin must be enabled as well.
 }
 ```

 ## Metadata Labels
+
 A limited set of fields will be exported as labels, all values are stored using strings **regardless of their underlying value type**, and therefore you may have to convert it back to its original type, note that numeric values are always represented in base 10.

 | Label                                | Type      | Example          | Description
--- a/plugin/geoip/geoip.go
+++ b/plugin/geoip/geoip.go
@ -20,8 +20,9 @@ var log = clog.NewWithPlugin(pluginName)
 // GeoIP is a plugin that add geo location data to the request context by looking up a maxmind
 // geoIP2 database, and which data can be later consumed by other middlewares.
 type GeoIP struct {
-	Next plugin.Handler
-	db   db
+	Next  plugin.Handler
+	db    db
+	edns0 bool
 }

 type db struct {
@ -37,7 +38,7 @@ const (

 var probingIP = net.ParseIP("127.0.0.1")

-func newGeoIP(dbPath string) (*GeoIP, error) {
+func newGeoIP(dbPath string, edns0 bool) (*GeoIP, error) {
 	reader, err := geoip2.Open(dbPath)
 	if err != nil {
 		return nil, fmt.Errorf("failed to open database file: %v", err)
@ -66,7 +67,7 @@ func newGeoIP(dbPath string) (*GeoIP, error) {
 		return nil, fmt.Errorf("database does not provide city schema")
 	}

-	return &GeoIP{db: db}, nil
+	return &GeoIP{db: db, edns0: edns0}, nil
 }

 // ServeDNS implements the plugin.Handler interface.
@ -79,6 +80,17 @@ func (g GeoIP) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (
 func (g GeoIP) Metadata(ctx context.Context, state request.Request) context.Context {
 	srcIP := net.ParseIP(state.IP())

+	if g.edns0 {
+		if o := state.Req.IsEdns0(); o != nil {
+			for _, s := range o.Option {
+				if e, ok := s.(*dns.EDNS0_SUBNET); ok {
+					srcIP = e.Address
+					break
+				}
+			}
+		}
+	}
+
 	switch {
 	case g.db.provides&city == city:
 		data, err := g.db.City(srcIP)
--- a/plugin/geoip/geoip_test.go
+++ b/plugin/geoip/geoip_test.go
@ -3,59 +3,91 @@ package geoip
 import (
 	"context"
 	"fmt"
+	"net"
 	"testing"

 	"github.com/coredns/coredns/plugin/metadata"
 	"github.com/coredns/coredns/plugin/test"
 	"github.com/coredns/coredns/request"
+
+	"github.com/miekg/dns"
 )

 func TestMetadata(t *testing.T) {

 	tests := []struct {
-		dbPath        string
 		label         string
 		expectedValue string
 	}{
-		{cityDBPath, "geoip/city/name", "Cambridge"},
+		{"geoip/city/name", "Cambridge"},

-		{cityDBPath, "geoip/country/code", "GB"},
-		{cityDBPath, "geoip/country/name", "United Kingdom"},
+		{"geoip/country/code", "GB"},
+		{"geoip/country/name", "United Kingdom"},
 		// is_in_european_union is set to true only to work around bool zero value, and test is really being set.
-		{cityDBPath, "geoip/country/is_in_european_union", "true"},
+		{"geoip/country/is_in_european_union", "true"},

-		{cityDBPath, "geoip/continent/code", "EU"},
-		{cityDBPath, "geoip/continent/name", "Europe"},
+		{"geoip/continent/code", "EU"},
+		{"geoip/continent/name", "Europe"},

-		{cityDBPath, "geoip/latitude", "52.2242"},
-		{cityDBPath, "geoip/longitude", "0.1315"},
-		{cityDBPath, "geoip/timezone", "Europe/London"},
-		{cityDBPath, "geoip/postalcode", "CB4"},
+		{"geoip/latitude", "52.2242"},
+		{"geoip/longitude", "0.1315"},
+		{"geoip/timezone", "Europe/London"},
+		{"geoip/postalcode", "CB4"},
 	}

-	for i, _test := range tests {
-		geoIP, err := newGeoIP(_test.dbPath)
-		if err != nil {
-			t.Fatalf("Test %d: unable to create geoIP plugin: %v", i, err)
-		}
-		state := request.Request{
-			W: &test.ResponseWriter{RemoteIP: "81.2.69.142"}, // This IP should be be part of the CDIR address range used to create the database fixtures.
-		}
-		ctx := metadata.ContextWithMetadata(context.Background())
-		rCtx := geoIP.Metadata(ctx, state)
-		if fmt.Sprintf("%p", ctx) != fmt.Sprintf("%p", rCtx) {
-			t.Errorf("Test %d: returned context is expected to be the same one passed in the Metadata function", i)
-		}
+	knownIPAddr := "81.2.69.142" // This IP should be be part of the CDIR address range used to create the database fixtures.
+	for _, tc := range tests {

-		fn := metadata.ValueFunc(ctx, _test.label)
-		if fn == nil {
-			t.Errorf("Test %d: label %q not set in metadata plugin context", i, _test.label)
-			continue
-		}
-		value := fn()
-		if value != _test.expectedValue {
-			t.Errorf("Test %d: expected value for label %q should be %q, got %q instead",
-				i, _test.label, _test.expectedValue, value)
-		}
+		t.Run(fmt.Sprintf("%s/%s", tc.label, "direct"), func(t *testing.T) {
+			geoIP, err := newGeoIP(cityDBPath, false)
+			if err != nil {
+				t.Fatalf("unable to create geoIP plugin: %v", err)
+			}
+			state := request.Request{
+				Req: new(dns.Msg),
+				W:   &test.ResponseWriter{RemoteIP: knownIPAddr},
+			}
+			testMetadata(t, state, geoIP, tc.label, tc.expectedValue)
+		})
+
+		t.Run(fmt.Sprintf("%s/%s", tc.label, "subnet"), func(t *testing.T) {
+			geoIP, err := newGeoIP(cityDBPath, true)
+			if err != nil {
+				t.Fatalf("unable to create geoIP plugin: %v", err)
+			}
+			state := request.Request{
+				Req: new(dns.Msg),
+				W:   &test.ResponseWriter{RemoteIP: "127.0.0.1"},
+			}
+			state.Req.SetEdns0(4096, false)
+			if o := state.Req.IsEdns0(); o != nil {
+				addr := net.ParseIP(knownIPAddr)
+				o.Option = append(o.Option, (&dns.EDNS0_SUBNET{
+					SourceNetmask: 32,
+					Address:       addr,
+				}))
+			}
+			testMetadata(t, state, geoIP, tc.label, tc.expectedValue)
+		})
 	}
 }
+
+func testMetadata(t *testing.T, state request.Request, geoIP *GeoIP, label, expectedValue string) {
+	ctx := metadata.ContextWithMetadata(context.Background())
+	rCtx := geoIP.Metadata(ctx, state)
+	if fmt.Sprintf("%p", ctx) != fmt.Sprintf("%p", rCtx) {
+		t.Errorf("returned context is expected to be the same one passed in the Metadata function")
+	}
+
+	fn := metadata.ValueFunc(ctx, label)
+	if fn == nil {
+		t.Errorf("label %q not set in metadata plugin context", label)
+		return
+	}
+	value := fn()
+	if value != expectedValue {
+		t.Errorf("expected value for label %q should be %q, got %q instead",
+			label, expectedValue, value)
+	}
+
+}
--- a/plugin/geoip/setup.go
+++ b/plugin/geoip/setup.go
@ -26,6 +26,7 @@ func setup(c *caddy.Controller) error {

 func geoipParse(c *caddy.Controller) (*GeoIP, error) {
 	var dbPath string
+	var edns0 bool

 	for c.Next() {
 		if !c.NextArg() {
@ -39,13 +40,16 @@ func geoipParse(c *caddy.Controller) (*GeoIP, error) {
 		if len(c.RemainingArgs()) != 0 {
 			return nil, c.ArgErr()
 		}
-		// The plugin should not have any config block.
-		if c.NextBlock() {
-			return nil, c.Err("unexpected config block")
+
+		for c.NextBlock() {
+			if c.Val() != "edns-subnet" {
+				return nil, c.Errf("unknown property %q", c.Val())
+			}
+			edns0 = true
 		}
 	}

-	geoIP, err := newGeoIP(dbPath)
+	geoIP, err := newGeoIP(dbPath, edns0)
 	if err != nil {
 		return geoIP, c.Err(err.Error())
 	}
--- a/plugin/geoip/setup_test.go
+++ b/plugin/geoip/setup_test.go
@ -52,14 +52,15 @@ func TestGeoIPParse(t *testing.T) {
 	}{
 		// Valid
 		{false, fmt.Sprintf("%s %s\n", pluginName, cityDBPath), "", city},
+		{false, fmt.Sprintf("%s %s { edns-subnet }", pluginName, cityDBPath), "", city},

 		// Invalid
 		{true, pluginName, "Wrong argument count", 0},
-		{true, fmt.Sprintf("%s %s {\n\tlanguages en fr es zh-CN\n}\n", pluginName, cityDBPath), "unexpected config block", 0},
+		{true, fmt.Sprintf("%s %s {\n\tlanguages en fr es zh-CN\n}\n", pluginName, cityDBPath), "unknown property \"languages\"", 0},
 		{true, fmt.Sprintf("%s %s\n%s %s\n", pluginName, cityDBPath, pluginName, cityDBPath), "configuring multiple databases is not supported", 0},
 		{true, fmt.Sprintf("%s 1 2 3", pluginName), "Wrong argument count", 0},
 		{true, fmt.Sprintf("%s { }", pluginName), "Error during parsing", 0},
-		{true, fmt.Sprintf("%s /dbpath { city }", pluginName), "unexpected config block", 0},
+		{true, fmt.Sprintf("%s /dbpath { city }", pluginName), "unknown property \"city\"", 0},
 		{true, fmt.Sprintf("%s /invalidPath\n", pluginName), "failed to open database file: open /invalidPath: no such file or directory", 0},
 		{true, fmt.Sprintf("%s %s\n", pluginName, unknownDBPath), "reader does not support the \"UnknownDbType\" database type", 0},
 	}