Merge pull request #415 from neilcook/geoip

Fix GeoIP2 lookup issues
PowerDNS · Jan 3, 2024 · c044195 · c044195
2 parents 23eae89 + 06a7d1e
commit c044195
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 31 deletions.
diff --git a/common/common-lua.cc b/common/common-lua.cc
@@ -474,5 +474,15 @@ void setupCommonLua(bool client,
   c_lua.registerFunction("lookupUIntValue", &WFGeoIP2DB::lookupUIntValue);
   c_lua.registerFunction("lookupBoolValue", &WFGeoIP2DB::lookupBoolValue);
   c_lua.registerFunction("lookupDoubleValue", &WFGeoIP2DB::lookupDoubleValue);
+
+  c_lua.registerMember("city", &WFGeoIPRecord::city);
+  c_lua.registerMember("country_code", &WFGeoIPRecord::country_code);
+  c_lua.registerMember("country_name", &WFGeoIPRecord::country_name);
+  c_lua.registerMember("region", &WFGeoIPRecord::region);
+  c_lua.registerMember("continent_code", &WFGeoIPRecord::continent_code);
+  c_lua.registerMember("postal_code", &WFGeoIPRecord::postal_code);
+  c_lua.registerMember("latitude", &WFGeoIPRecord::latitude);
+  c_lua.registerMember("longitude", &WFGeoIPRecord::longitude);
+
 #endif // HAVE_MMDB
 }
diff --git a/common/wforce-geoip2.cc b/common/wforce-geoip2.cc
@@ -76,40 +76,40 @@ std::string WFGeoIP2DB::lookupISP(const ComboAddress& address)
   return std::string(data.utf8_string, data.data_size);
 }
 
-WFGeoIPRecord WFGeoIP2DB::lookupCity(const ComboAddress& address)
+std::unique_ptr<WFGeoIPRecord> WFGeoIP2DB::lookupCity(const ComboAddress& address)
 {
-  WFGeoIPRecord ret_wfgir = {};
+  auto ret_wfgir = std::make_unique<WFGeoIPRecord>();
   MMDB_entry_data_s data;
   MMDB_lookup_result_s res;
 
   if (mmdbLookup(address.toString(), res)) {
     if ((MMDB_get_value(&res.entry, &data, "country", "iso_code", NULL)
          == MMDB_SUCCESS) && (data.has_data))
-      ret_wfgir.country_code = std::string(data.utf8_string, data.data_size);
+      ret_wfgir->country_code = std::string(data.utf8_string, data.data_size);
     if ((MMDB_get_value(&res.entry, &data, "country", "names", "en", NULL)
          == MMDB_SUCCESS) && (data.has_data))
-      ret_wfgir.country_name = std::string(data.utf8_string, data.data_size);
+      ret_wfgir->country_name = std::string(data.utf8_string, data.data_size);
     if ((MMDB_get_value(&res.entry, &data, "subdivisions", "0", "iso_code", NULL)
          == MMDB_SUCCESS) && (data.has_data))
-      ret_wfgir.region = std::string(data.utf8_string, data.data_size);
+      ret_wfgir->region = std::string(data.utf8_string, data.data_size);
     if ((MMDB_get_value(&res.entry, &data, "cities", "0", NULL)
          == MMDB_SUCCESS) && (data.has_data))
-      ret_wfgir.city = std::string(data.utf8_string, data.data_size);
+      ret_wfgir->city = std::string(data.utf8_string, data.data_size);
     else if ((MMDB_get_value(&res.entry, &data, "city", "names", "en", NULL)
               == MMDB_SUCCESS) && (data.has_data))
-      ret_wfgir.city = std::string(data.utf8_string, data.data_size);
+      ret_wfgir->city = std::string(data.utf8_string, data.data_size);
     if ((MMDB_get_value(&res.entry, &data, "continent", "code", NULL)
          == MMDB_SUCCESS) && (data.has_data))
-      ret_wfgir.continent_code = std::string(data.utf8_string, data.data_size);
+      ret_wfgir->continent_code = std::string(data.utf8_string, data.data_size);
     if ((MMDB_get_value(&res.entry, &data, "postal", "code", NULL)
          == MMDB_SUCCESS) && (data.has_data))
-      ret_wfgir.postal_code = std::string(data.utf8_string, data.data_size);
+      ret_wfgir->postal_code = std::string(data.utf8_string, data.data_size);
     if ((MMDB_get_value(&res.entry, &data, "location", "latitude", NULL)
          == MMDB_SUCCESS) && (data.has_data))
-      ret_wfgir.latitude = data.double_value;
+      ret_wfgir->latitude = data.double_value;
     if ((MMDB_get_value(&res.entry, &data, "location", "longitude", NULL)
          == MMDB_SUCCESS) && (data.has_data))
-      ret_wfgir.longitude = data.double_value;    
+      ret_wfgir->longitude = data.double_value;
   }
   return ret_wfgir;
 }

diff --git a/common/wforce-geoip2.hh b/common/wforce-geoip2.hh
@@ -45,7 +45,7 @@ public:
 
   std::string lookupCountry(const ComboAddress& address);
   std::string lookupISP(const ComboAddress& address);
-  WFGeoIPRecord lookupCity(const ComboAddress& address);
+  std::unique_ptr<WFGeoIPRecord> lookupCity(const ComboAddress& address);
   std::string lookupStringValue(const ComboAddress& address, const std::vector<std::pair<unsigned int, std::string>>& attrs);
   uint64_t lookupUIntValue(const ComboAddress& address, const std::vector<std::pair<unsigned int, std::string>>& attrs);
   bool lookupBoolValue(const ComboAddress& address, const std::vector<std::pair<unsigned int, std::string>>& attrs);

diff --git a/docker/Makefile.am b/docker/Makefile.am
@@ -5,7 +5,7 @@ REGRESSION_SERVICE = regression
 
 SUBDIRS= wforce_image
 
-$(COMPOSE_TARGET): $(COMPOSE_SOURCE) $(shell find ../common -type f) $(shell find ../wforce -type f) $(shell find ../report_api -type f)  $(shell find ../trackalert -type f) $(shell find ../ext -type f) $(shell find ../regression-tests -type f) $(shell find regression -type f) $(shell find logstash -type f) 
+$(COMPOSE_TARGET): $(COMPOSE_SOURCE) $(shell find ../common -type f) $(shell find ../wforce -type f)  $(shell find ../trackalert -type f) $(shell find ../ext -type f) $(shell find ../regression-tests -type f) $(shell find regression -type f) $(shell find logstash -type f)
 	$(DCMP) down -v
 	$(DCMP) build
 	touch $(COMPOSE_TARGET)

diff --git a/docker/regression/Dockerfile b/docker/regression/Dockerfile
@@ -44,7 +44,7 @@ RUN apt-get update && \
 # Disable Ipv6 for redis
 ARG MAXMIND_LICENSE_KEY
 RUN wget -O GeoLite2-City.mmdb.tar.gz "https://download.maxmind.com/app/geoip_download?edition_id=GeoLite2-City&license_key=${MAXMIND_LICENSE_KEY}&suffix=tar.gz" || true
- RUN wget -O GeoLite2-Country.mmdb.tar.gz "https://download.maxmind.com/app/geoip_download?edition_id=GeoLite2-Country&license_key=${MAXMIND_LICENSE_KEY}&suffix=tar.gz" || true
+RUN wget -O GeoLite2-Country.mmdb.tar.gz "https://download.maxmind.com/app/geoip_download?edition_id=GeoLite2-Country&license_key=${MAXMIND_LICENSE_KEY}&suffix=tar.gz" || true
 RUN wget -O GeoLite2-ASN.mmdb.tar.gz "https://download.maxmind.com/app/geoip_download?edition_id=GeoLite2-ASN&license_key=${MAXMIND_LICENSE_KEY}&suffix=tar.gz" || true
 RUN gunzip GeoLite2-*.mmdb.tar.gz || true
 RUN tar xvf GeoLite2-City.mmdb.tar || true

diff --git a/docker/wforce_image/Makefile.am b/docker/wforce_image/Makefile.am
@@ -11,7 +11,7 @@ export WFORCE_PASSWORD = super
 export WFORCE_PORT = 18084
 export TRACKALERT_PORT = 18085
 
-$(WFORCE_IMAGE_COMPOSE_TARGET): $(shell find weakforced/common -type f) $(shell find weakforced/wforce -type f) $(shell find weakforced/trackalert -type f) $(shell find weakforced/ext -type f) $(shell find weakforced/report_api -type f) $(COMPOSE_SOURCE)
+$(WFORCE_IMAGE_COMPOSE_TARGET): $(shell find weakforced/common -type f) $(shell find weakforced/wforce -type f) $(shell find weakforced/trackalert -type f) $(shell find weakforced/ext -type f)  $(COMPOSE_SOURCE)
 	$(DCMP) down -v
 	$(DCMP) build
 	touch $(WFORCE_IMAGE_COMPOSE_TARGET)

diff --git a/regression-tests/test_GeoIP.py b/regression-tests/test_GeoIP.py
@@ -5,40 +5,32 @@
 import os
 from test_helper import ApiTestCase
 
-maxmind_license = None
-
 class TestGeoIP(ApiTestCase):
     def setUp(self):
-        maxmind_license = os.getenv('MAXMIND_LICENSE_KEY', default=None)
         super(TestGeoIP, self).setUp()
 
     def test_geoIP(self):
-        # Don't do the geoip tests if there's no maxmind license key
-        if maxmind_license is None:
-            return
         # don't allow IPs from Japan (arbitrary)
         r = self.allowFunc('baddie', '112.78.112.20', "1234")
         j = r.json()
         self.assertEqual(j['status'], -1)
         self.assertRegex(json.dumps(j), "Japan")
         r.close()
 
-#    def test_geoIP2City(self):
-#        attrs = dict()
-#        attrs['ip'] = '128.243.1.1'
-#        r = self.customFuncWithName("geoip2", attrs)
-#        j = r.json()
-#        self.assertRegex(json.dumps(j), "Nottingham")
-#        r.close()
+    def test_geoIP2City(self):
+        attrs = dict()
+        attrs['ip'] = '128.243.1.1'
+        r = self.customFuncWithName("geoip2", attrs)
+        j = r.json()
+        self.assertRegex(json.dumps(j), "Nottingham")
+        r.close()
 
     def test_geoIP2LookupVals(self):
-        if maxmind_license is None:
-            return
         attrs = dict()
         attrs['ip'] = '128.243.21.1'
         r = self.customFuncWithName("geoip2_lookupValue", attrs)
         j = r.json()
         print(json.dumps(j))
         self.assertRegex(json.dumps(j['r_attrs']['city']), "Nottingham")
-        self.assertRegex(json.dumps(j['r_attrs']['latitude']), "52.9538")
+        self.assertRegex(json.dumps(j['r_attrs']['latitude']), "52.9044")
         r.close()