sasl_blacklist.c: several small improvements

- This module only works with Atheme v7.3, so if building against
  a lesser version, compile a stub module that complains about
  this instead of creating a compilation failure

  This would have bit us if we ever bumped modules/contrib/ to
  HEAD in the Atheme release/7.2 branch

- Use libathemecore memory allocator for storing new config
  entries with an intrusive node so that we don't have to check
  if mowgli_node_add() fails (or segfault otherwise)

- Make private variables compilation-unit-static

- Put function names on their own line

- Sprinkle some const-correctness everywhere

- Perform a service_find("saslserv") only once, on startup, and
  error out if it fails

- Depend on saslserv/main so that we are reloaded if it is

  Otherwise, given the above, mod_deinit() would call
  del_conf_item() on a stale pointer and probably segfault

- Collapse 2 almost identical functions for processing config
  entries into 1 function

- Collapse multiple instances of clearing a mowgli_list_t into 1
  function which does the same

- Indicate which configuration block the parameters are in when
  reporting errors with the configuration file

- Avoid some potential NULL dereferences

- Conform to Atheme v7.3 coding style (line wrapping, etc)

diff --git a/sasl_blacklist.c b/sasl_blacklist.c
index 413ea4cfb4992acf6cec5231d708140aba2af158..6edaed6e778eeed3abe1e3b4b7a6ee4655e2dec7 100644 (file)
--- a/sasl_blacklist.c
+++ b/sasl_blacklist.c
@@ -1,150 +1,226 @@
+/*
+ * Copyright (C) 2018 Atheme Development Group (https://atheme.github.io/)
+ * Rights to this code are documented in doc/LICENSE.
+ *
+ * Restrict authentication methods by name or by host.
+ */
+
 #include "atheme-compat.h"
 
-mowgli_list_t restricted_hosts;
+#if (CURRENT_ABI_REVISION >= 730000)
 
-static int c_restricted_hosts(mowgli_config_file_entry_t *ce)
+struct blacklist_entry
 {
-       mowgli_config_file_entry_t *subce;
-       mowgli_node_t *n, *tn;
+       mowgli_node_t   node;
+       char *          data;
+};
 
-       if(!ce->entries)
-               return 0;
+static mowgli_list_t restricted_hosts;
+static mowgli_list_t permitted_mechanisms;
 
-       MOWGLI_ITER_FOREACH_SAFE(n, tn, restricted_hosts.head)
-       {
-               free(n->data);
-               mowgli_node_delete(n, &restricted_hosts);
-               mowgli_node_free(n);
-       }
+static struct service *saslsvs = NULL;
 
-       MOWGLI_ITER_FOREACH(subce, ce->entries)
+static void
+blacklist_clear_list(mowgli_list_t *const restrict list)
+{
+       mowgli_node_t *n, *tn;
+
+       MOWGLI_ITER_FOREACH_SAFE(n, tn, list->head)
        {
-               if (subce->entries != NULL)
-                       conf_report_warning(ce, "Invalid restricted_hosts entry");
-               else
-                       mowgli_node_add(sstrdup(subce->varname), mowgli_node_create(), &restricted_hosts);
-       }
+               struct blacklist_entry *const entry = n->data;
 
-       return 0;
+               (void) mowgli_node_delete(n, list);
+               (void) sfree(entry->data);
+               (void) sfree(entry);
+       }
 }
 
-mowgli_list_t permitted_mechanisms;
-
-static int c_permitted_mechanisms(mowgli_config_file_entry_t *ce)
+static void
+blacklist_process_configentry(mowgli_config_file_entry_t *const restrict ce, mowgli_list_t *const restrict list,
+                              const char *const restrict name)
 {
-       mowgli_config_file_entry_t *subce;
-       mowgli_node_t *n, *tn;
+       if (! ce->entries)
+               return;
 
-       if(!ce->entries)
-               return 0;
+       (void) blacklist_clear_list(list);
 
-       MOWGLI_ITER_FOREACH_SAFE(n, tn, permitted_mechanisms.head)
-       {
-               free(n->data);
-               mowgli_node_delete(n, &permitted_mechanisms);
-               mowgli_node_free(n);
-       }
+       mowgli_config_file_entry_t *subce;
 
        MOWGLI_ITER_FOREACH(subce, ce->entries)
        {
-               if (subce->entries != NULL)
-                       conf_report_warning(ce, "Invalid permitted_mechanisms entry");
+               if (! subce->entries)
+               {
+                       struct blacklist_entry *const entry = smalloc(sizeof *entry);
+
+                       entry->data = sstrdup(subce->varname);
+
+                       (void) mowgli_node_add(entry, &entry->node, list);
+               }
                else
-                       mowgli_node_add(sstrdup(subce->varname), mowgli_node_create(), &permitted_mechanisms);
+                       (void) conf_report_warning(ce, "Invalid saslserv::%s entry", name);
        }
+}
+
+static int
+c_restricted_hosts(mowgli_config_file_entry_t *const restrict ce)
+{
+       (void) blacklist_process_configentry(ce, &restricted_hosts, "restricted_hosts");
+
+       return 0;
+}
+
+static int
+c_permitted_mechanisms(mowgli_config_file_entry_t *const restrict ce)
+{
+       (void) blacklist_process_configentry(ce, &permitted_mechanisms, "permitted_mechanisms");
 
        return 0;
 }
 
-static bool is_restricted_host(char const *host)
+static bool
+is_restricted_host(char const *const restrict host)
 {
+       if (! host || ! *host)
+               return false;
+
        mowgli_node_t *n;
-       char *entry;
 
        MOWGLI_ITER_FOREACH(n, restricted_hosts.head)
        {
-               entry = n->data;
-               if(!match(entry, host) || !match_ips(entry, host))
-                       return 1;
+               struct blacklist_entry *const entry = n->data;
+
+               if (match(entry->data, host) == 0 || match_ips(entry->data, host) == 0)
+                       return true;
        }
-       return 0;
+
+       return false;
 }
 
-static bool is_permitted_mechanism(char const *mech)
+static bool
+is_permitted_mechanism(char const *const restrict mech)
 {
+       if (! mech || ! *mech)
+               return false;
+
        mowgli_node_t *n;
+
        MOWGLI_ITER_FOREACH(n, permitted_mechanisms.head)
-               if(!strcmp(n->data, mech))
-                       return 1;
-       return 0;
+       {
+               struct blacklist_entry *const entry = n->data;
+
+               if (strcmp(entry->data, mech) == 0)
+                       return true;
+       }
+
+       return false;
 }
 
-static void can_login(hook_user_login_check_t *c)
+static void
+blacklist_can_login(hook_user_login_check_t *const restrict c)
 {
-       struct sasl_sourceinfo *ssi;
-       char const *log_target = service_get_log_target(c->si->service);
+       if (! c->si)
+               return;
 
-       if(c->si->service == service_find("saslserv"))
+       if (c->si->service == saslsvs)
        {
-               ssi = (struct sasl_sourceinfo *)c->si;
-               if((ssi->sess->host && is_restricted_host(ssi->sess->host)) || (ssi->sess->ip && is_restricted_host(ssi->sess->ip)))
-                       if(!is_permitted_mechanism(ssi->sess->mechptr->name))
+               const struct sasl_sourceinfo *const ssi = (struct sasl_sourceinfo *) c->si;
+
+               if (! ssi->sess || ! ssi->sess->mechptr || ! (ssi->sess->host || ssi->sess->ip))
+                       return;
+
+               if (is_restricted_host(ssi->sess->host) || is_restricted_host(ssi->sess->ip))
+               {
+                       char const *const log_target = service_get_log_target(saslsvs);
+
+                       if (! is_permitted_mechanism(ssi->sess->mechptr->name))
                        {
-                               slog(CMDLOG_LOGIN, "%s %s:%s failed LOGIN to \2%s\2 (%s not allowed)", log_target, entity(c->mu)->name, ssi->sess->uid, entity(c->mu)->name, ssi->sess->mechptr->name);
+                               (void) slog(CMDLOG_LOGIN, "%s %s:%s failed LOGIN to \2%s\2 ('%s' not allowed)",
+                                           log_target, entity(c->mu)->name, ssi->sess->uid, entity(c->mu)->name,
+                                           ssi->sess->mechptr->name);
+
                                c->allowed = false;
-                               return;
                        }
+               }
        }
-       else if(c->si->su)
+       else if (c->si->su)
        {
-               if(is_restricted_host(c->si->su->host) || is_restricted_host(c->si->su->chost) || is_restricted_host(c->si->su->vhost) || is_restricted_host(c->si->su->ip))
+               if (is_restricted_host(c->si->su->host) || is_restricted_host(c->si->su->chost) ||
+                   is_restricted_host(c->si->su->vhost) || is_restricted_host(c->si->su->ip))
                {
-                       logcommand(c->si, CMDLOG_LOGIN, "failed IDENTIFY to \2%s\2 (restricted address)", entity(c->mu)->name);
+                       (void) logcommand(c->si, CMDLOG_LOGIN, "failed IDENTIFY to \2%s\2 (restricted address)",
+                                         entity(c->mu)->name);
+
                        c->allowed = false;
-                       return;
                }
        }
 }
 
-static void can_register(hook_user_register_check_t *c)
+static void
+blacklist_can_register(hook_user_register_check_t *const restrict c)
+{
+       if (is_restricted_host(c->si->su->host) || is_restricted_host(c->si->su->chost) ||
+           is_restricted_host(c->si->su->vhost) || is_restricted_host(c->si->su->ip))
+       {
+               (void) logcommand(c->si, CMDLOG_LOGIN, "denied REGISTER of \2%s\2 (restricted address)", c->account);
+
+               c->approved++;
+       }
+}
+
+static void
+mod_init(module_t *const restrict m)
 {
-       if(is_restricted_host(c->si->su->host) || is_restricted_host(c->si->su->chost) || is_restricted_host(c->si->su->vhost) || is_restricted_host(c->si->su->ip))
+       // We do this to depend on saslserv/main, so that if it is reloaded, we are too
+       const struct sasl_core_functions *sasl_core_functions;
+       MODULE_TRY_REQUEST_SYMBOL(m, sasl_core_functions, "saslserv/main", "sasl_core_functions");
+
+       if (! (saslsvs = service_find("saslserv")))
        {
-               logcommand(c->si, CMDLOG_LOGIN, "denied REGISTER of \2%s\2 (restricted address)", c->account);
-               c->approved = 1;
+               (void) slog(LG_ERROR, "%s: could not find SASLServ (BUG!)", m->name);
+
+               m->mflags |= MODFLAG_FAIL;
+               return;
        }
+
+       (void) hook_add_event("user_can_login");
+       (void) hook_add_user_can_login(&blacklist_can_login);
+
+       (void) hook_add_event("user_can_register");
+       (void) hook_add_user_can_register(&blacklist_can_register);
+
+       (void) add_conf_item("RESTRICTED_HOSTS", &saslsvs->conf_table, &c_restricted_hosts);
+       (void) add_conf_item("PERMITTED_MECHANISMS", &saslsvs->conf_table, &c_permitted_mechanisms);
 }
 
-static void mod_init(module_t *m)
+static void
+mod_deinit(const module_unload_intent_t ATHEME_VATTR_UNUSED intent)
 {
-       hook_add_user_can_login(can_login);
-       hook_add_user_can_register(can_register);
+       (void) del_conf_item("RESTRICTED_HOSTS", &saslsvs->conf_table);
+       (void) del_conf_item("PERMITTED_MECHANISMS", &saslsvs->conf_table);
 
-       add_conf_item("RESTRICTED_HOSTS", &service_find("saslserv")->conf_table, c_restricted_hosts);
-       add_conf_item("PERMITTED_MECHANISMS", &service_find("saslserv")->conf_table, c_permitted_mechanisms);
+       (void) hook_del_user_can_login(&blacklist_can_login);
+       (void) hook_del_user_can_register(&blacklist_can_register);
+
+       (void) blacklist_clear_list(&restricted_hosts);
+       (void) blacklist_clear_list(&permitted_mechanisms);
 }
 
-static void mod_deinit(module_unload_intent_t intent)
+#else /* (CURRENT_ABI_REVISION >= 730000) */
+
+static void
+mod_init(module_t *const restrict m)
 {
-       mowgli_node_t *n, *tn;
+       (void) slog(LG_ERROR, "%s: this module only works with Atheme v7.3.0 or above", m->name);
 
-       hook_del_user_can_login(can_login);
-       hook_del_user_can_register(can_register);
+       m->mflags |= MODFLAG_FAIL;
+}
+
+static void
+mod_deinit(const module_unload_intent_t ATHEME_VATTR_UNUSED intent)
+{
 
-       del_conf_item("RESTRICTED_HOSTS", &service_find("saslserv")->conf_table);
-       MOWGLI_ITER_FOREACH_SAFE(n, tn, restricted_hosts.head)
-       {
-               free(n->data);
-               mowgli_node_delete(n, &restricted_hosts);
-               mowgli_node_free(n);
-       }
-       del_conf_item("PERMITTED_MECHANISMS", &service_find("saslserv")->conf_table);
-       MOWGLI_ITER_FOREACH_SAFE(n, tn, permitted_mechanisms.head)
-       {
-               free(n->data);
-               mowgli_node_delete(n, &permitted_mechanisms);
-               mowgli_node_free(n);
-       }
 }
 
+#endif /* (CURRENT_ABI_REVISION < 730000) */
+
 VENDOR_DECLARE_MODULE_V1("contrib/sasl_blacklist", MODULE_UNLOAD_CAPABILITY_OK, CONTRIB_VENDOR_FREENODE)