TS-4284 Modernize the geoip_acl plugin

build/common.m4

@@ -556,30 +556,6 @@ AC_DEFUN([TS_ARG_ENABLE_VAR],[
   AC_SUBST(m4_join([_], $1, AS_TR_SH($2)))
 ])
 
-dnl
-dnl TS_SEARCH_LIBRARY(function, search-libs, [action-if-found], [action-if-not-found])
-dnl This macro works like AC_SEARCH_LIBS, except that $LIBS is not modified. If the library
-dnl is found, it is cached in the ts_cv_lib_${function} variable.
-dnl
-AC_DEFUN([TS_SEARCH_LIBRARY], [
-  __saved_LIBS="$LIBS"
-
-  AC_SEARCH_LIBS($1, $2, [
-    dnl action-if-found
-    case $ac_cv_search_$1 in
-    "none required"|"no") ts_cv_search_$1="" ;;
-    *) ts_cv_search_$1=$ac_cv_search_$1 ;;
-    esac
-    m4_default([$3], [true])
-  ], [
-    dnl action-if-not-found
-    m4_default([$4], [true])
-  ])
-
-  LIBS="$__saved_LIBS"
-  unset __saved_LIBS
-])
-
 dnl TS_CHECK_SOCKOPT(socket-option, [action-if-found], [action-if-not-found]
 AC_DEFUN([TS_CHECK_SOCKOPT], [
   AC_MSG_CHECKING([for $1 socket option])

configure.ac

@@ -1406,14 +1406,14 @@ AC_SUBST(use_hwloc)
 # GeoIP as a "helper" plugin, which other plugins can then use. Such a plugin could
 # then manage which libraries to use via explicit dlopen()'s.
 #
-enable_maxmind_geoip=no
-TS_SEARCH_LIBRARY([GeoIP_id_by_code], [GeoIP], [
-  GEOIP_LIBS=$ts_cv_search_GeoIP_id_by_code
-  AC_CHECK_HEADERS([GeoIP.h], [ enable_maxmind_geoip=yes ])
+AC_CHECK_HEADERS([GeoIP.h], [
+  AC_CHECK_LIB([GeoIP], [GeoIP_new], [
+    AC_SUBST([GEO_LIBS], ["-lGeoIP"])
+  ], [
+    AC_SUBST([GEO_LIBS], [""])
+  ])
 ])
 
-AC_SUBST(GEOIP_LIBS)
-AM_CONDITIONAL([BUILD_GEOIP_PLUGIN], [ test "x${enable_maxmind_geoip}" = x"yes" ])
 
 # Right now, the healthcheck plugins requires inotify_init (and friends)
 AM_CONDITIONAL([BUILD_HEALTHCHECK_PLUGIN], [ test "$ac_cv_func_inotify_init" = "yes" ])

plugins/experimental/geoip_acl/Makefile.am

@@ -16,11 +16,7 @@
 
 include $(top_srcdir)/build/plugins.mk
 
-if BUILD_GEOIP_PLUGIN
-
 pkglib_LTLIBRARIES = geoip_acl.la
 geoip_acl_la_SOURCES = acl.cc geoip_acl.cc
 geoip_acl_la_LDFLAGS = $(TS_PLUGIN_LDFLAGS)
-geoip_acl_la_LIBADD = $(GEOIP_LIBS)
-
-endif
+geoip_acl_la_LIBADD = $(GEO_LIBS)

plugins/experimental/geoip_acl/acl.cc

@@ -24,10 +24,88 @@
 #include "acl.h"
 #include "lulu.h"
 
-// Global GeoIP object.
-GeoIP *gGI;
 
-// Implementation of the ACL base class.
+// Implementation of the ACL base class. This wraps the underlying Geo library
+// that we've found and used.
+GeoDBHandle Acl::_geoip;
+GeoDBHandle Acl::_geoip6;
+
+// Maxmind v1 APIs
+#if HAVE_GEOIP_H
+bool
+Acl::init()
+{
+  TSDebug(PLUGIN_NAME, "initialized IPv4 GeoIP DB");
+  _geoip = GeoIP_new(GEOIP_MMAP_CACHE); // GEOIP_STANDARD seems to break threaded apps...
+
+  // Setup IPv6 if possible
+  if (GeoIP_db_avail(GEOIP_COUNTRY_EDITION_V6)) {
+    _geoip6 = GeoIP_open_type(GEOIP_COUNTRY_EDITION_V6, GEOIP_MMAP_CACHE | GEOIP_MEMORY_CACHE);
+    TSDebug(PLUGIN_NAME, "initialized IPv6 GeoIP DB");
+  }
+
+  return true;
+}
+
+int
+Acl::country_id_by_code(const std::string &str) const
+{
+  return GeoIP_id_by_code(str.c_str());
+}
+
+int
+Acl::country_id_by_addr(const sockaddr *addr) const
+{
+  int iso = -1;
+  int v = 4;
+
+  switch (addr->sa_family) {
+  case AF_INET: {
+    uint32_t ip = ntohl(reinterpret_cast<const struct sockaddr_in *>(addr)->sin_addr.s_addr);
+
+    iso = GeoIP_id_by_ipnum(_geoip, ip);
+  } break;
+  case AF_INET6: {
+    geoipv6_t ip;
+
+    memcpy(ip.s6_addr, reinterpret_cast<const struct sockaddr_in6 *>(addr)->sin6_addr.s6_addr, sizeof(ip));
+    iso = GeoIP_id_by_ipnum_v6(_geoip6, ip);
+    v = 6;
+  } break;
+  default:
+    break;
+  }
+
+  TSDebug(PLUGIN_NAME, "eval(): Client IPv%d seems to come from ISO=%d", v, iso);
+  return iso;
+}
+#else  /* !HAVE_GEOIP_H */
+
+// No library available, nothing will work :)
+bool
+Acl::init()
+{
+  TSDebug(PLUGIN_NAME, "No Geo library available!");
+  TSError("[%s] No Geo library available!", PLUGIN_NAME);
+
+  return false;
+}
+
+int
+Acl::country_id_by_code(const std::string &str) const
+{
+  return -1;
+}
+
+int
+Acl::country_id_by_addr(const sockaddr *addr) const
+{
+  return -1;
+}
+#endif /* HAVE_GEOIP_H */
+
+
+// This is the rest of the ACL baseclass, which is the same for all underlying Geo libraries.
 void
 Acl::read_html(const char *fn)
 {
@@ -39,7 +117,7 @@ Acl::read_html(const char *fn)
     f.close();
     TSDebug(PLUGIN_NAME, "Loaded HTML from %s", fn);
   } else {
-    TSError("[geoip_acl] Unable to open HTML file %s", fn);
+    TSError("[%s] Unable to open HTML file %s", PLUGIN_NAME, fn);
   }
 }
 
@@ -52,11 +130,13 @@ RegexAcl::parse_line(const char *filename, const std::string &line, int lineno)
   std::string regex, tmp;
   std::string::size_type pos1, pos2;
 
-  if (line.empty())
+  if (line.empty()) {
     return false;
+  }
   pos1 = line.find_first_not_of(_SEPARATOR);
-  if (line[pos1] == '#' || pos1 == std::string::npos)
+  if (line[pos1] == '#' || pos1 == std::string::npos) {
     return false;
+  }
 
   pos2 = line.find_first_of(_SEPARATOR, pos1);
   if (pos2 != std::string::npos) {
@@ -66,12 +146,12 @@ RegexAcl::parse_line(const char *filename, const std::string &line, int lineno)
       pos2 = line.find_first_of(_SEPARATOR, pos1);
       if (pos2 != std::string::npos) {
         tmp = line.substr(pos1, pos2 - pos1);
-        if (tmp == "allow")
+        if (tmp == "allow") {
           _acl->set_allow(true);
-        else if (tmp == "deny")
+        } else if (tmp == "deny") {
           _acl->set_allow(false);
-        else {
-          TSError("[geoip_acl] Bad action on in %s:line %d: %s", filename, lineno, tmp.c_str());
+        } else {
+          TSError("[%s] Bad action on in %s:line %d: %s", PLUGIN_NAME, filename, lineno, tmp.c_str());
           return false;
         }
         // The rest are "tokens"
@@ -102,11 +182,12 @@ RegexAcl::compile(const std::string &str, const char *filename, int lineno)
   if (NULL != _rex) {
     _extra = pcre_study(_rex, 0, &error);
     if ((NULL == _extra) && error && (*error != 0)) {
-      TSError("[geoip_acl] Failed to study regular expression in %s:line %d at offset %d: %s", filename, lineno, erroffset, error);
+      TSError("[%s] Failed to study regular expression in %s:line %d at offset %d: %s", PLUGIN_NAME, filename, lineno, erroffset,
+              error);
       return false;
     }
   } else {
-    TSError("[geoip_acl] Failed to compile regular expression in %s:line %d: %s", filename, lineno, error);
+    TSError("[%s] Failed to compile regular expression in %s:line %d: %s", PLUGIN_NAME, filename, lineno, error);
     return false;
   }
 
@@ -134,14 +215,12 @@ CountryAcl::add_token(const std::string &str)
 {
   int iso = -1;
 
-  Acl::add_token(str);
-  iso = GeoIP_id_by_code(str.c_str());
-
+  iso = country_id_by_code(str.c_str());
   if (iso > 0 && iso < NUM_ISO_CODES) {
     _iso_country_codes[iso] = true;
-    TSDebug(PLUGIN_NAME, "Added %s(%d) to remap rule, ACL=%d", str.c_str(), iso, _allow);
+    TSDebug(PLUGIN_NAME, "Added %s(%d) to remap rule, ACL=%s", str.c_str(), iso, _allow ? "allow" : "deny");
   } else {
-    TSError("[geoip_acl] Tried setting an ISO code (%d) outside the supported range", iso);
+    TSError("[%s] Tried setting an ISO code (%d) outside the supported range", PLUGIN_NAME, iso);
   }
 }
 
@@ -159,20 +238,22 @@ CountryAcl::read_regex(const char *fn)
     while (!f.eof()) {
       getline(f, line);
       ++lineno;
-      if (!acl)
+      if (!acl) {
         acl = new RegexAcl(new CountryAcl());
+      }
       if (acl->parse_line(fn, line, lineno)) {
-        if (NULL == _regexes)
+        if (NULL == _regexes) {
           _regexes = acl;
-        else
+        } else {
           _regexes->append(acl);
+        }
         acl = NULL;
       }
     }
     f.close();
     TSDebug(PLUGIN_NAME, "Loaded regex rules from %s", fn);
   } else {
-    TSError("[geoip_acl] Unable to open regex file %s", fn);
+    TSError("[%s] Unable to open regex file %s", PLUGIN_NAME, fn);
   }
 }
 
@@ -195,40 +276,22 @@ CountryAcl::eval(TSRemapRequestInfo *rri, TSHttpTxn txnp) const
     } while ((acl = acl->next()));
   }
 
-  // This is a special case for when there are no ISO codes. It got kinda wonky without it.
-  if (0 == _added_tokens)
-    return _allow;
-
   // None of the regexes (if any) matched, so fallback to the remap defaults if there are any.
-  int iso = -1;
-  const sockaddr *addr = TSHttpTxnClientAddrGet(txnp);
-
-  switch (addr->sa_family) {
-  case AF_INET: {
-    uint32_t ip = ntohl(reinterpret_cast<const struct sockaddr_in *>(addr)->sin_addr.s_addr);
+  int iso = country_id_by_addr(TSHttpTxnClientAddrGet(txnp));
 
-    iso = GeoIP_id_by_ipnum(gGI, ip);
-    if (TSIsDebugTagSet(PLUGIN_NAME)) {
-      const char *c = GeoIP_country_code_by_ipnum(gGI, ip);
-      TSDebug(PLUGIN_NAME, "eval(): IP=%u seems to come from ISO=%d / %s", ip, iso, c);
-    }
-  } break;
-  case AF_INET6:
-    return true;
-  default:
-    break;
-  }
-
-  if ((iso <= 0) || (!_iso_country_codes[iso]))
+  if ((iso <= 0) || (!_iso_country_codes[iso])) {
     return !_allow;
+  }
 
   return _allow;
 }
 
 
-void
+int
 CountryAcl::process_args(int argc, char *argv[])
 {
+  int tokens = 0;
+
   for (int i = 3; i < argc; ++i) {
     if (!strncmp(argv[i], "allow", 5)) {
       _allow = true;
@@ -240,6 +303,9 @@ CountryAcl::process_args(int argc, char *argv[])
       read_html(argv[i] + 6);
     } else { // ISO codes assumed for the rest
       add_token(argv[i]);
+      ++tokens;
     }
   }
+
+  return tokens;
 }

plugins/experimental/geoip_acl/acl.h

@@ -29,14 +29,14 @@
 #endif
 
 #include <string>
-
 #include "lulu.h"
 
 #if HAVE_GEOIP_H
 #include <GeoIP.h>
-#endif
-
-extern GeoIP *gGI;
+typedef GeoIP *GeoDBHandle;
+#else  /* !HAVE_GEOIP_H */
+typedef void *GeoDBHandle;
+#endif /* HAVE_GEOIP_H */
 
 // See http://www.iso.org/iso/english_country_names_and_code_elements
 // Maxmind allocates 253 country codes,even though there are only 248 according to the above
@@ -47,20 +47,16 @@ static const int NUM_ISO_CODES = 253;
 class Acl
 {
 public:
-  Acl() : _allow(true), _added_tokens(0) {}
+  Acl() : _allow(true) {}
 
   virtual ~Acl() {}
 
   // These have to be implemented for each ACL type
   virtual void read_regex(const char *fn) = 0;
-  virtual void process_args(int argc, char *argv[]) = 0;
+  virtual int process_args(int argc, char *argv[]) = 0;
   virtual bool eval(TSRemapRequestInfo *rri, TSHttpTxn txnp) const = 0;
+  virtual void add_token(const std::string &str) = 0;
 
-  virtual void
-  add_token(const std::string & /* str */)
-  {
-    ++_added_tokens;
-  }
   void
   set_allow(bool allow)
   {
@@ -79,10 +75,17 @@ class Acl
 
   void read_html(const char *fn);
 
+  int country_id_by_code(const std::string &str) const;
+  int country_id_by_addr(const sockaddr *addr) const;
+
+  static bool init();
+
 protected:
   std::string _html;
   bool _allow;
   int _added_tokens;
+  static GeoDBHandle _geoip;
+  static GeoDBHandle _geoip6;
 };
 
 
@@ -111,10 +114,10 @@ class RegexAcl
   bool
   match(const char *str, int len) const
   {
-    // TODO: Not 100% sure this is absolutely correct, and not sure why adding
-    // PCRE_NOTEMPTY to the options doesn't work ...
-    if (0 == len)
+    if (0 == len) {
       return false;
+    }
+
     return (pcre_exec(_rex, _extra, str, len, 0, PCRE_NOTEMPTY, NULL, 0) != -1);
   }
 
@@ -138,7 +141,7 @@ class CountryAcl : public Acl
   CountryAcl() : _regexes(NULL) { memset(_iso_country_codes, 0, sizeof(_iso_country_codes)); }
 
   void read_regex(const char *fn);
-  void process_args(int argc, char *argv[]);
+  int process_args(int argc, char *argv[]);
   bool eval(TSRemapRequestInfo *rri, TSHttpTxn txnp) const;
   void add_token(const std::string &str);