Menu
▾
▴
Re: [Sshguard-users] Patch to fix blacklist loading with ipfw
|
From: Greg P. <gr...@n0...> - 2015-05-27 02:34:53
|
After starting normally with an non-existent blacklist.db, poked at it
from another host and it attempted to block, but failed:
May 26 21:16:52 fbsd sshguard[18479]: Offender '192.168.1.53:4' scored 40 danger in 1 abuses (threshold 40) -> blacklisted.
May 26 21:16:52 fbsd sshguard[18479]: Blocking 192.168.1.53:4 for >0secs: 40 danger in 4 attacks over 1 seconds (all: 40d in 1 abuses over 1s).
May 26 21:16:52 fbsd sshguard[18479]: Command "/sbin/ipfw add 55003 drop ip from 192.168.1.53 to me" exited 64
May 26 21:16:52 fbsd sshguard[18479]: Blocking command failed. Exited: -1
While sshguard was still running, confirmed it did not add the entry. Then I
stopped sshguard. I copy & pasted the command from the log and it went in
without issue.
Copied a blacklist.db file with 213 entries. Upon start, it attempted to
insert, but failed. It made a long command, but was chopped off on the
67th IP address (that address had its last digit cut off). and there are
still 146 more after that one that never made the list.
The latter command clearly could not be added as it was truncated. However,
unsure why it didn't like the first one.
Greg
Kevin Zheng said:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Hi there,
>
> A patch that fixes blacklist loading when using the `ipfw` backend is
> available and attached here. It is mostly of interest to FreeBSD.
>
> This patch has not been committed because it relies on the
> non-portable functions `strlcpy` and `strlcat`. While I work on
> bringing these to SSHGuard, FreeBSD users can enjoy a working
> blacklist now.
>
> I've done rudimentary testing and this patch appears to work; before
> this hits the ports tree someone should really test it.
>
> Thanks,
> Kevin Zheng
>
> - --
> Kevin Zheng
> kev...@gm... | ke...@kd... | PGP: 0xC22E1090
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
>
> iQEcBAEBCAAGBQJVZRxRAAoJEOrPD3bCLhCQN2MIAJOMmgslZPV5aYsYEnX1quC+
> IXMc6t/rpFDybZPKz4LC4YI+WcsQ+fykKQ3mFZfJ2HITqqyBorNUe8JKzR8p59tX
> sX5ePTq4Jld+LOFklKOSS3NSZauMi6zS8tcCpz5gVdQ0iBizDssW/f70ZTD927lB
> 44VgAdv8FrHXsPpgEgcrZCsNm3uK8j48eh3aAo3elThM4BAIhoMYobLZl1Jgnq59
> hjWVk49Z1njypiP2SYASXVdy5x8AINQDY4R8Wqa0/mNGfzFKT2y5HPw/70YbAm3M
> E1o/V9apCH3p1Trq/NshZwvP9sFxfV0oJtATRXUvJxuI0BDHIM5F+/w72TJCVU4=
> =SKWp
> -----END PGP SIGNATURE-----
> diff --git a/src/fwalls/ipfw.c b/src/fwalls/ipfw.c
> index 29045b0..9bee0ad 100644
> --- a/src/fwalls/ipfw.c
> +++ b/src/fwalls/ipfw.c
> @@ -20,6 +20,7 @@
>
> #include <assert.h>
> #include <errno.h>
> +#include <limits.h>
> #include <time.h>
> #include <time.h>
> #include <string.h>
> @@ -37,8 +38,6 @@
>
> #define IPFWMOD_ADDRESS_BULK_REPRESENTATIVE "FF:FF:FF:FF:FF:FF:FF:FF"
>
> -#define MAXIPFWCMDLEN 90
> -
> #ifndef IPFW_RULERANGE_MIN
> #define IPFW_RULERANGE_MIN 55000
> #endif
> @@ -56,14 +55,14 @@ struct addr_ruleno_s {
> };
>
> static list_t addrrulenumbers;
> -static char command[MAXIPFWCMDLEN], args[MAXIPFWCMDLEN];
> +static char command[PATH_MAX], args[ARG_MAX];
>
> /* generate an IPFW rule ID for inserting a rule */
> static ipfw_rulenumber_t ipfwmod_getrulenumber(void);
> /* execute an IPFW command */
> -static int ipfwmod_runcommand(char *command, char *args);
> +static int ipfwmod_runcommand(const char *command, const char *args);
> /* build an IPFW rule for blocking a list of addresses, all of the given kind */
> -static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind, char *restrict command, char *restrict args);
> +static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind);
>
> static size_t ipfw_rule_meter(const void *el) { return sizeof(struct addr_ruleno_s); }
> static int ipfw_rule_comparator(const void *a, const void *b) {
> @@ -95,7 +94,7 @@ int fw_block(const char *restrict addr, int addrkind, int service) {
> ruleno = ipfwmod_getrulenumber();
> addresses[0] = addr;
> addresses[1] = NULL;
> - if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind, command, args) != FWALL_OK)
> + if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind) != FWALL_OK)
> return FWALL_ERR;
>
> /* run command */
> @@ -108,7 +107,7 @@ int fw_block(const char *restrict addr, int addrkind, int service) {
> sshguard_log(LOG_DEBUG, "Command exited %d.", ret);
>
> /* success, save rule number */
> - strcpy(addendum.addr, addr);
> + strlcpy(addendum.addr, addr, sizeof(addendum.addr));
> addendum.ruleno = ruleno;
> addendum.addrkind = addrkind;
>
> @@ -134,7 +133,7 @@ int fw_block_list(const char *restrict addresses[], int addrkind, const int serv
>
> ruleno = ipfwmod_getrulenumber();
> /* insert rules under this rule number (in chunks of max_addresses_per_rule) */
> - if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind, command, args) != FWALL_OK)
> + if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind) != FWALL_OK)
> return FWALL_ERR;
>
> /* run command */
> @@ -147,7 +146,7 @@ int fw_block_list(const char *restrict addresses[], int addrkind, const int serv
> sshguard_log(LOG_DEBUG, "Command exited %d.", ret);
>
> /* insert a placeholder for the bulk */
> - strcpy(addendum.addr, IPFWMOD_ADDRESS_BULK_REPRESENTATIVE);
> + strlcpy(addendum.addr, IPFWMOD_ADDRESS_BULK_REPRESENTATIVE, sizeof(addendum.addr));
> addendum.ruleno = ruleno;
> addendum.addrkind = addrkind;
> list_append(& addrrulenumbers, & addendum);
> @@ -161,7 +160,7 @@ int fw_release(const char *restrict addr, int addrkind, int service) {
> int pos, ret = 0;
>
> /* retrieve ID of rule blocking "addr" */
> - strcpy(data.addr, addr);
> + strlcpy(data.addr, addr, sizeof(data.addr));
> data.addrkind = addrkind;
> if ((pos = list_locate(& addrrulenumbers, &data)) < 0) {
> sshguard_log(LOG_ERR, "could not get back rule ID for address %s", addr);
> @@ -172,22 +171,22 @@ int fw_release(const char *restrict addr, int addrkind, int service) {
> switch (data.addrkind) {
> case ADDRKIND_IPv4:
> /* use ipfw */
> - sprintf(command, IPFW_PATH "/ipfw");
> + strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
> break;
> case ADDRKIND_IPv6:
> #ifdef FWALL_HAS_IP6FW
> /* use ip6fw if found */
> - sprintf(command, IPFW_PATH "/ip6fw");
> + strlcpy(command, IPFW_PATH "/ip6fw", sizeof(command));
> #else
> /* use ipfw, assume it supports IPv6 rules as well */
> - sprintf(command, IPFW_PATH "/ipfw");
> + strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
> #endif
> break;
> default:
> return FWALL_UNSUPP;
> }
> /* build command arguments */
> - snprintf(args, MAXIPFWCMDLEN, "delete %u", data.ruleno);
> + snprintf(args, sizeof(args), "delete %u", data.ruleno);
>
> sshguard_log(LOG_DEBUG, "running: '%s %s'", command, args);
>
> @@ -216,19 +215,19 @@ int fw_flush(void) {
> data = (struct addr_ruleno_s *)list_iterator_next(& addrrulenumbers);
> switch (data->addrkind) {
> case ADDRKIND_IPv4:
> - snprintf(command, MAXIPFWCMDLEN, IPFW_PATH "/ipfw");
> + strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
> break;
> case ADDRKIND_IPv6:
> #ifdef FWALL_HAS_IP6FW
> /* use ip6fw if found */
> - sprintf(command, IPFW_PATH "/ip6fw");
> + strlcpy(command, IPFW_PATH "/ip6fw", sizeof(command));
> #else
> /* use ipfw, assume it supports IPv6 rules as well */
> - sprintf(command, IPFW_PATH "/ipfw");
> + strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
> #endif
> break;
> }
> - sprintf(args, "delete %u", data->ruleno);
> + snprintf(args, sizeof(args), "delete %u", data->ruleno);
> sshguard_log(LOG_DEBUG, "running: '%s %s'", command, args);
> ret = ipfwmod_runcommand(command, args);
> if (ret != 0) {
> @@ -250,7 +249,7 @@ static ipfw_rulenumber_t ipfwmod_getrulenumber(void) {
> return (rand() % (IPFW_RULERANGE_MAX - IPFW_RULERANGE_MIN)) + IPFW_RULERANGE_MIN;
> }
>
> -static int ipfwmod_runcommand(char *command, char *args) {
> +static int ipfwmod_runcommand(const char *command, const char *args) {
> char *argsvec[20];
> pid_t pid;
> int i, j, ret;
> @@ -258,8 +257,8 @@ static int ipfwmod_runcommand(char *command, char *args) {
>
> sshguard_log(LOG_DEBUG, "Running command: '%s %s'.", command, args);
>
> - argsvec[0] = command;
> - strcpy(locargs, args);
> + argsvec[0] = strdup(command);
> + strlcpy(locargs, args, sizeof(locargs));
>
> /* tokenize command */
> argsvec[1] = locargs;
> @@ -280,6 +279,7 @@ static int ipfwmod_runcommand(char *command, char *args) {
> sshguard_log(LOG_ERR, "Unable to run command: %s", strerror(errno));
> _Exit(1);
> }
> + free(argsvec[0]);
> free(locargs);
> waitpid(pid, &ret, 0);
> ret = WEXITSTATUS(ret);
> @@ -287,7 +287,7 @@ static int ipfwmod_runcommand(char *command, char *args) {
> return ret;
> }
>
> -static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind, char *restrict command, char *restrict args) {
> +static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind) {
> int i;
>
> assert(addresses != NULL);
> @@ -307,19 +307,19 @@ static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restr
> switch (addrkind) {
> case ADDRKIND_IPv4:
> /* use ipfw */
> - sprintf(command, IPFW_PATH "/ipfw");
> - sprintf(args, "add %u drop ip", ruleno);
> + strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
> + snprintf(args, sizeof(args), "add %u drop ip", ruleno);
> break;
>
> case ADDRKIND_IPv6:
> #ifdef FWALL_HAS_IP6FW
> /* use ip6fw if found */
> - sprintf(command, IPFW_PATH "/ip6fw");
> + strlcpy(command, IPFW_PATH "/ip6fw", sizeof(command));
> #else
> /* use ipfw, assume it supports IPv6 rules as well */
> - sprintf(command, IPFW_PATH "/ipfw");
> + strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
> #endif
> - sprintf(args, "add %u drop ipv6", ruleno);
> + snprintf(args, sizeof(args), "add %u drop ipv6", ruleno);
> break;
>
> default:
> @@ -327,13 +327,17 @@ static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restr
> }
>
> /* add the rest of the rule */
> - sprintf(args + strlen(args), " from %s", addresses[0]);
> + strlcat(args, " from ", sizeof(args));
> + strlcat(args, addresses[0], sizeof(args));
> for (i = 1; addresses[i] != NULL; ++i) {
> - sprintf(args + strlen(args), ",%s", addresses[i]);
> + strlcat(args, ",", sizeof(args));
> + strlcat(args, addresses[i], sizeof(args));
> + }
> + if (strlcat(args, " to me", sizeof(args)) >= sizeof(args)) {
> + fprintf(stderr, "Fatal: Argument buffer too small\n");
> + exit(EXIT_FAILURE);
> }
> - strcat(args, " to me");
>
> return FWALL_OK;
> }
>
> -
> ------------------------------------------------------------------------------
> _______________________________________________
> Sshguard-users mailing list
> Ssh...@li...
> https://lists.sourceforge.net/lists/listinfo/sshguard-users
|
