* fix valgrind checks in f2b_cmd_*() & improve testing

configs/conf-available/20-jail-global.conf

@@ -0,0 +1,9 @@
+[jail:global]
+; this jail used by remote controller
+; no source/filter needed here
+state    = yes
+enabled  = yes
+bantime  = 86400 ; will be redefined at runtime
+expiretime = 14400
+bantime_extend = 0
+findtime_extend = 0

configs/f2b.conf.in

@@ -20,7 +20,6 @@ expiretime = 14400
 bantime_extend = 0.2
 findtime_extend = 0.07
 banscore = 50
-source   = files:/var/log/messages
 backend  = exec-ipset:banned
 
 [csocket]

debian/f2b.install

@@ -2,6 +2,7 @@ etc/f2b/conf-available/*-source-files.conf
 etc/f2b/conf-available/*-source-portknock.conf
 etc/f2b/conf-available/*-filter-preg.conf
 etc/f2b/conf-available/*-backend-exec-*.conf
+etc/f2b/conf-available/*-jail-global.conf
 etc/f2b/conf-enabled
 etc/f2b/f2b.conf
 usr/bin/f2b-*-test

src/commands.c

@@ -137,22 +137,22 @@ f2b_cmd_create(const char *line) {
 
   assert(line != NULL);
 
+  while (isspace(*line)) line++;
+  if (strlen(line) <= 0)
+    return NULL; /* empty string */
+
   if ((cmd = calloc(1, sizeof(f2b_cmd_t))) == NULL)
     return NULL;
 
-  if (f2b_buf_alloc(&cmd->data, strlen(line))) {
-    if (f2b_cmd_parse(cmd, line))
-      return cmd;
-    free(cmd);
-    cmd = NULL;
-  }
-
-
-  return cmd;
+  if (f2b_cmd_parse(cmd, line))
+    return cmd;
+  free(cmd);
+  return NULL;
 }
 
 void
 f2b_cmd_destroy(f2b_cmd_t *cmd) {
+  if (!cmd) return;
   f2b_buf_free(&cmd->data);
   free(cmd);
 }
@@ -168,6 +168,9 @@ f2b_cmd_parse(f2b_cmd_t *cmd, const char *src) {
   while (isblank(*src))
     src++;
 
+  if (strlen(src) == 0)
+    return false; /* empty string */
+
   f2b_buf_alloc(&cmd->data, strlen(src) + 1);
   f2b_buf_append(&cmd->data, src, 0);
 
@@ -234,6 +237,7 @@ f2b_cmd_parse(f2b_cmd_t *cmd, const char *src) {
     }
   }
   cmd->type = CMD_UNKNOWN;
+  memset(cmd->args, 0x0, sizeof(cmd->args));
   f2b_buf_free(&cmd->data);
 
   return false;

src/csocket-test.c

@@ -15,6 +15,8 @@ cmd_handler(const f2b_cmd_t *cmd, f2b_buf_t *res) {
   }
   if (cmd->type == CMD_HELP) {
     f2b_buf_append(res, f2b_cmd_help(), 0);
+  } else {
+    f2b_buf_append(res, "+ok\n", 0);
   }
   return;
 }

src/csocket.c

@@ -229,7 +229,14 @@ f2b_conn_process(f2b_conn_t *conn, bool in, void (*cb)(const f2b_cmd_t *cmd, f2b
 int
 f2b_sock_create_unix(const char *path) {
   struct sockaddr_un addr;
+  struct stat st;
   int sock = -1;
+  /* is socket already exists? */
+  if (stat(path, &st) == 0) {
+    if ((st.st_mode & S_IFMT) == S_IFSOCK) {
+      unlink(path);
+    }
+  }
   /* create socket  */
   if ((sock = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0)) < 0) {
     f2b_log_msg(log_error, "can't create control socket at %s", strerror(errno));
@@ -242,7 +249,7 @@ f2b_sock_create_unix(const char *path) {
   strlcpy(addr.sun_path, path, sizeof(addr.sun_path) - 1);
   /* try bind */
   if (bind(sock, (struct sockaddr *) &addr, sizeof(struct sockaddr_un)) != 0) {
-    f2b_log_msg(log_error, "bind() on socket failed: %s", strerror(errno));
+    f2b_log_msg(log_error, "bind() on socket failed: %s -- %s", path, strerror(errno));
     unlink(path);
     return -1;
   }
@@ -292,8 +299,11 @@ f2b_sock_create_inet(const char *addr) {
   res[0].ai_socktype |= SOCK_NONBLOCK;
   sock = socket(res[0].ai_family, res[0].ai_socktype, res[0].ai_protocol);
   if (sock >= 0) {
+    int enable = 1;
+    if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &enable, sizeof(enable)) < 0)
+      f2b_log_msg(log_warn, "setsockopt(SO_REUSEADDR) failed: %s", strerror(errno));
     if (bind(sock, res[0].ai_addr, res[0].ai_addrlen) != 0) {
-      f2b_log_msg(log_error, "bind() on socket failed: %s", strerror(errno));
+      f2b_log_msg(log_error, "bind() on socket failed: %s -- %s", addr, strerror(errno));
       sock = -1;
     }
   } else {

src/daemon.c

@@ -173,7 +173,7 @@ f2b_csocket_cmd_process(const f2b_cmd_t *cmd, f2b_buf_t *res) {
   }
 
   if (res->used == 0)
-    f2b_buf_append(res, "+ok\n", 3); /* default reply if not set above */
+    f2b_buf_append(res, "+ok\n", 0); /* default reply if not set above */
 
   return;
 }

src/jail.c

@@ -146,6 +146,7 @@ f2b_jail_apply_config(f2b_jail_t *jail, f2b_config_section_t *section) {
     if (strcmp(param->name, "source") == 0) {
       f2b_jail_parse_compound_value(param->value, name, init);
       jail->source = f2b_source_create(name, init);
+      jail->flags |= JAIL_HAS_SOURCE;
       continue;
     }
     if (strcmp(param->name, "filter") == 0) {
@@ -306,7 +307,9 @@ f2b_jail_process(f2b_jail_t *jail) {
 
   f2b_backend_ping(jail->backend);
   
-  while ((stag = f2b_source_next(jail->source, line, sizeof(line), reset)) > 0) {
+  while (jail->flags & JAIL_HAS_SOURCE) {
+    if ((stag = f2b_source_next(jail->source, line, sizeof(line), reset)) <= 0)
+      break; /* no more data */
     reset = false;
     if (jail->flags & JAIL_HAS_FILTER) {
       if ((ftag = f2b_filter_match(jail->filter, line, matchbuf, sizeof(matchbuf), &score)) == 0)
@@ -339,7 +342,7 @@ f2b_jail_process(f2b_jail_t *jail) {
     f2b_jail_evt_match(jail->name, addr, match);
     /* host is banned? */
     if (addr->banned) {
-      if (addr->banned_at != now)
+      if (addr->banned_at > (now - 5))
         f2b_log_msg(log_warn, "jail '%s': ip %s was already banned", jail->name, matchbuf);
       continue;
     }
@@ -348,12 +351,11 @@ f2b_jail_process(f2b_jail_t *jail) {
     f2b_matches_expire(&addr->matches, now - findtime - 600);
     /* ...so, check host score */
     score = f2b_matches_score(&addr->matches, findtime);
-    if (score < jail->banscore) {
-      f2b_log_msg(log_info, "jail '%s': new match for ip %s (%u/%u)",
-        jail->name, matchbuf, score, jail->banscore);
+    f2b_log_msg(log_info, "jail '%s': new match for ip %s (%u/%u)",
+      jail->name, matchbuf, score, jail->banscore);
+    if (score < jail->banscore)
       continue; /* lucky bastard */
-    }
-    /* score limit reached, ban ip */
+    /* else: score limit reached, ban ip */
     f2b_jail_ban(jail, addr);
     if (jail->flags & JAIL_HAS_STATE)
       jail->sfile->need_save = true;
@@ -410,17 +412,15 @@ f2b_jail_init(f2b_jail_t *jail, f2b_config_t *config) {
   assert(jail   != NULL);
   assert(config != NULL);
 
-  if (!jail->source) {
-    f2b_log_msg(log_error, "jail '%s': missing 'source' option", jail->name);
-    goto cleanup1;
-  }
-  if ((section = f2b_config_section_find(config->sources, jail->source->name)) == NULL) {
-    f2b_log_msg(log_error, "jail '%s': no source with name '%s'", jail->name, jail->source->name);
-    goto cleanup1;
-  }
-  if (!f2b_source_init(jail->source, section)) {
-    f2b_log_msg(log_error, "jail '%s': can't init source '%s' with %s", jail->name, jail->source->name, jail->source->init);
-    goto cleanup1;
+  if (jail->flags & JAIL_HAS_SOURCE) {
+    if ((section = f2b_config_section_find(config->sources, jail->source->name)) == NULL) {
+      f2b_log_msg(log_error, "jail '%s': no source with name '%s'", jail->name, jail->source->name);
+      goto cleanup1;
+    }
+    if (!f2b_source_init(jail->source, section)) {
+      f2b_log_msg(log_error, "jail '%s': can't init source '%s' with %s", jail->name, jail->source->name, jail->source->init);
+      goto cleanup1;
+    }
   }
 
   if (jail->flags & JAIL_HAS_FILTER) {
@@ -432,7 +432,7 @@ f2b_jail_init(f2b_jail_t *jail, f2b_config_t *config) {
       f2b_log_msg(log_error, "jail '%s': no regexps loaded from '%s'", jail->name, jail->filter->init);
       goto cleanup2;
     }
-  } else if (jail->source->flags & MOD_NEED_FILTER) {
+  } else if (jail->source && jail->source->flags & MOD_NEED_FILTER) {
     f2b_log_msg(log_error, "jail '%s': source '%s' needs filter, but jail has no one", jail->name, jail->source->name);
     goto cleanup1;
   }
@@ -452,7 +452,7 @@ f2b_jail_init(f2b_jail_t *jail, f2b_config_t *config) {
   }
 
   /* start all */
-  if (!f2b_source_start(jail->source)) {
+  if (jail->source && !f2b_source_start(jail->source)) {
     f2b_log_msg(log_warn, "jail '%s': source action 'start' failed", jail->name);
     goto cleanup3;
   }
@@ -568,6 +568,7 @@ f2b_jail_cmd_status(char *res, size_t ressize, f2b_jail_t *jail) {
     "flags:\n"
     "  enabled: %s\n"
     "  state: %s\n"
+    "  source: %s\n"
     "  filter: %s\n";
   const char *fmt2 =
     "times:\n"
@@ -587,6 +588,7 @@ f2b_jail_cmd_status(char *res, size_t ressize, f2b_jail_t *jail) {
     jail->name, jail->banscore,
     jail->flags & JAIL_ENABLED     ? "yes" : "no",
     jail->flags & JAIL_HAS_STATE   ? "yes" : "no",
+    jail->flags & JAIL_HAS_SOURCE  ? "yes" : "no",
     jail->flags & JAIL_HAS_FILTER  ? "yes" : "no"
   );
   snprintf(buf, sizeof(buf), fmt2,

src/jail.h

@@ -16,6 +16,7 @@
 #define JAIL_ENABLED       0x01
 #define JAIL_HAS_STATE     0x02
 #define JAIL_HAS_FILTER    0x04
+#define JAIL_HAS_SOURCE    0x08
 
 /** jail metadata struct */
 typedef struct f2b_jail_t {

t/t_cmd.c

@@ -4,6 +4,10 @@
 
 int main() {
   f2b_cmd_t cmd;
+  f2b_cmd_t *c;
+
+  assert(f2b_cmd_parse(&cmd, "") == false);
+  assert(f2b_cmd_parse(&cmd, " ") == false);
 
   assert(f2b_cmd_parse(&cmd, "status") == true);
   assert(cmd.type = CMD_STATUS);
@@ -37,5 +41,19 @@ int main() {
   assert(strcmp(cmd.args[4], "7200") == 0);
   f2b_buf_free(&cmd.data);
 
+  c = f2b_cmd_create("");
+  assert(c == NULL);
+  c = f2b_cmd_create(" ");
+  assert(c == NULL);
+  c = f2b_cmd_create("\n\r\n");
+  assert(c == NULL);
+  c = f2b_cmd_create("test");
+  assert(c == NULL); /* no such command */
+  c = f2b_cmd_create("help");
+  assert(c != NULL);
+  f2b_cmd_destroy(c);
+
+  f2b_buf_free(&cmd.data);
+
   return EXIT_SUCCESS;
 }