From 89567787d16d9bf1ea7741d7d23e1c55010984dc Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Wed, 17 Feb 2016 18:49:13 +1100 Subject: [PATCH 01/20] Don't allow includes of the / path. --- scanner.l | 5 +++++ t/Makefile.am | 4 +++- t/t_72_noincluderoot | 25 +++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100755 t/t_72_noincluderoot diff --git a/scanner.l b/scanner.l index 3f81242c..c939f249 100644 --- a/scanner.l +++ b/scanner.l @@ -194,6 +194,11 @@ void include_file(const char *name) { char *fn; int n; + if (strlen(name) == 1 && name[0] == '/' && name[1] == '\0') { + scan_err("warning: cannot include / path; skipping"); + return; + } + if (stat(name, &st)) { if (errno == ENOENT && (index(name, '*') != NULL || index(name, '?') != NULL || diff --git a/t/Makefile.am b/t/Makefile.am index debf28a3..00846833 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -68,7 +68,9 @@ TESTS = t_01_ccomment \ t_68_noop \ t_69_includeemptynoop \ t_70_includesemicolon \ - t_71_includeglob + t_71_includeglob \ + t_72_noincluderoot + check_PROGRAMS = scan parse emit convert factorise diff --git a/t/t_72_noincluderoot b/t/t_72_noincluderoot new file mode 100755 index 00000000..1d194f20 --- /dev/null +++ b/t/t_72_noincluderoot @@ -0,0 +1,25 @@ +#!/bin/sh -x + +TEST="that include paths don't start at root" + +testdir=`dirname $0` +. $testdir/testlib + +cat >$work/in < $work/out 2>&1 +if test $? -ne 0 ; then fail ; fi + +test "x$srcdir" = "x" && cat $work/out + +cat >$work/good < Date: Thu, 18 Feb 2016 09:33:52 +1100 Subject: [PATCH 02/20] Check return of strdup and report an error. --- scanner.l | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/scanner.l b/scanner.l index c939f249..15aacdd5 100644 --- a/scanner.l +++ b/scanner.l @@ -127,8 +127,12 @@ text return TOK_TEXT; [ \t]* /* eat whitespace after include */ [^ \t\n;]+ { /* include file name */ char * name = strdup(yytext); - include_file(name); - free(name); + if (name == NULL) { + scan_err("warning: internal error trying to alloc for include of %s: %s", yytext, strerror(errno)); + } else { + include_file(name); + free(name); + } BEGIN(INITIAL); } @@ -161,8 +165,9 @@ static void scan_err(const char *fmt, ...) { va_list args; va_start(args, fmt); - if (inc_stackptr >= 0) + if (inc_stackptr >= 0) { fprintf(stderr, "%s:%ld: ", filename(), lineno()); + } vfprintf(stderr, fmt, args); fprintf(stderr, "\n"); } @@ -238,7 +243,7 @@ void include_file(const char *name) { while (n--) { /* FIXME: assumes d_name */ if (namelist[n]->d_name[0] == '.') { - free(namelist[n]); + free(namelist[n]); continue; } if (asprintf(&fn, "%s/%s", name, namelist[n]->d_name) < 0) { @@ -250,7 +255,7 @@ void include_file(const char *name) { include_file(fn); free(fn); } - free(namelist[n]); + free(namelist[n]); } free(namelist); } From a7cf0cd50e907cc978818bbaf3c1c070531bd82b Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Thu, 18 Feb 2016 10:39:26 +1100 Subject: [PATCH 03/20] Normalize root paths. --- scanner.l | 16 +++++++++++----- t/t_72_noincluderoot | 2 ++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/scanner.l b/scanner.l index 15aacdd5..3034b319 100644 --- a/scanner.l +++ b/scanner.l @@ -21,6 +21,7 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ +#include #include #include #include @@ -199,11 +200,6 @@ void include_file(const char *name) { char *fn; int n; - if (strlen(name) == 1 && name[0] == '/' && name[1] == '\0') { - scan_err("warning: cannot include / path; skipping"); - return; - } - if (stat(name, &st)) { if (errno == ENOENT && (index(name, '*') != NULL || index(name, '?') != NULL || @@ -237,6 +233,16 @@ void include_file(const char *name) { } } else { if (S_ISDIR(st.st_mode)) { + char *b = strdup(name); + char *base = basename(b); + + if (strcmp("/", base) == 0) { + scan_err("warning: cannot include / path; skipping"); + free(b); + return; + } + free(b); + if ((n = scandir(name, &namelist, NULL, alphasort)) < 0) { scan_err("warning: scandir failed on %s: %s", name, strerror(errno)); } else { diff --git a/t/t_72_noincluderoot b/t/t_72_noincluderoot index 1d194f20..17536cd7 100755 --- a/t/t_72_noincluderoot +++ b/t/t_72_noincluderoot @@ -7,6 +7,7 @@ testdir=`dirname $0` cat >$work/in <$work/good < Date: Thu, 18 Feb 2016 10:51:00 +1100 Subject: [PATCH 04/20] Don't apply any flags if there are no flags to apply. --- filter.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/filter.c b/filter.c index 65338c5f..7a644383 100644 --- a/filter.c +++ b/filter.c @@ -415,8 +415,10 @@ void filter_noneg(struct filter **f) { if (f) { }; */ void filter_apply_flags(struct filter *f, long flags) { struct filter *s; - if (!f) + + if (!f || !flags) { return; + } switch (f->type) { /* Structural things */ case F_SIBLIST: From eacade60c1e66b4df1cefcb628c6381e950784a6 Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Thu, 18 Feb 2016 11:54:32 +1100 Subject: [PATCH 05/20] Don't create new filters if the subrule conversion returns NULL. --- glue.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/glue.c b/glue.c index abe120a9..f7ebb669 100644 --- a/glue.c +++ b/glue.c @@ -80,12 +80,15 @@ struct filter *convert_subrule_list(struct subrule_list_s *n, struct filter *convert_compound_specifier(struct compound_specifier_s *r, struct filtergen_opts *o) { - struct filter *res = NULL; + struct filter *res = NULL, *sub = NULL; eprint("converting compound_specifier\n"); if (r->list) { - res = new_filter_sibs(convert_subrule_list(r->list, o)); + sub = convert_subrule_list(r->list, o); + if (sub) { + res = new_filter_sibs(sub); + } } return res; } @@ -466,8 +469,9 @@ struct filter *convert_chaingroup_specifier(struct chaingroup_specifier_s *n, if (n->list) { sub = convert_subrule_list(n->list, o); - - res = new_filter_subgroup(name, sub); + if (sub) { + res = new_filter_subgroup(name, sub); + } } else { printf("error: no list in chaingroup\n"); } From df5c044d144e089e2a8d8e3206ea8cc7b5fefda2 Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Sat, 20 Feb 2016 10:30:21 +1100 Subject: [PATCH 06/20] Remove filter_apply_flags method as it only performs name resolution, which has already been done by the resolver module. This extra duplication costs seconds of CPU time in the worst case. --- fg-cisco.c | 1 - fg-ipchains.c | 1 - fg-ipfilter.c | 1 - fg-iptables.c | 1 - fg-iptrestore.c | 1 - filter.c | 71 ------------------------------------------------- filter.h | 5 +--- 7 files changed, 1 insertion(+), 80 deletions(-) diff --git a/fg-cisco.c b/fg-cisco.c index b1877772..2c197ef5 100644 --- a/fg-cisco.c +++ b/fg-cisco.c @@ -147,6 +147,5 @@ int fg_cisco(struct filter *filter, int flags) { "can generate broken rulesets."); filter_nogroup(filter); filter_unroll(&filter); - filter_apply_flags(filter, flags); return filtergen_cprod(filter, &cb_cisco, &misc); } diff --git a/fg-ipchains.c b/fg-ipchains.c index 277fb6c2..8681d927 100644 --- a/fg-ipchains.c +++ b/fg-ipchains.c @@ -266,7 +266,6 @@ int fg_ipchains(struct filter *filter, int flags) { }; filter_unroll(&filter); - filter_apply_flags(filter, flags); if (!(flags & FF_NOSKEL)) { oputs("for f in INPUT OUTPUT FORWARD; do " IPCHAINS " -P $f DENY; done"); oputs(IPCHAINS " -F; " IPCHAINS " -X"); diff --git a/fg-ipfilter.c b/fg-ipfilter.c index f4f13cc8..db39a984 100644 --- a/fg-ipfilter.c +++ b/fg-ipfilter.c @@ -136,6 +136,5 @@ int fg_ipfilter(struct filter *filter, int flags) { filter_nogroup(filter); filter_unroll(&filter); - filter_apply_flags(filter, flags); return filtergen_cprod(filter, &cb_ipfilter, &misc); } diff --git a/fg-iptables.c b/fg-iptables.c index d1dd8e24..c246eab0 100644 --- a/fg-iptables.c +++ b/fg-iptables.c @@ -397,7 +397,6 @@ static int fg_iptables_common(struct filter *filter, int flags, const int nchains = 3; filter_unroll(&filter); - filter_apply_flags(filter, flags); if (!(flags & FF_NOSKEL)) { oputs("CHAINS=\"INPUT OUTPUT FORWARD\""); diff --git a/fg-iptrestore.c b/fg-iptrestore.c index 8da07e32..0e512c65 100644 --- a/fg-iptrestore.c +++ b/fg-iptrestore.c @@ -403,7 +403,6 @@ static int fg_iptrestore_common(struct filter *filter, int flags, const int nchains = 3; filter_unroll(&filter); - filter_apply_flags(filter, flags); if (!(flags & FF_NOSKEL)) { oprintf("%s <type) { - /* Structural things */ - case F_SIBLIST: - for (s = f->u.sib; s; s = s->next) - filter_apply_flags(s, flags); - break; - case F_SUBGROUP: - filter_apply_flags(f->u.sub.list, flags); - break; - case F_NEG: - filter_apply_flags(f->u.neg, flags); - break; - /* Real things */ - case F_SPORT: - case F_DPORT: - if (flags & FF_LOOKUP) { - struct port_spec *p = &f->u.ports; - if (p->min == -1) { - fprintf(stderr, "warning: couldn't lookup service \"%s\"\n", p->minstr); - break; - } - free(p->minstr); - p->minstr = int_to_str_dup(p->min); - if (p->maxstr) { - if (p->max == -1) { - fprintf(stderr, "warning: couldn't lookup service \"%s\"\n", - p->minstr); - break; - } - free(p->maxstr); - p->maxstr = int_to_str_dup(p->max); - } - } - break; - case F_SOURCE: - case F_DEST: - if (flags & FF_LOOKUP) { - struct addr_spec *a = &f->u.addrs; - struct addrinfo hints; - struct addrinfo *info = NULL; - memset(&hints, 0, sizeof(struct addrinfo)); - hints.ai_family = a->family; - if (getaddrinfo(a->addrstr, NULL, &hints, &info) == 0) { - free(a->addrstr); - a->addrstr = malloc(NI_MAXHOST + 1); - if (getnameinfo(info->ai_addr, info->ai_addrlen, a->addrstr, NI_MAXHOST, - NULL, 0, NI_NUMERICHOST)) { - fprintf(stderr, "warning: can't stringify IP: %s\n", strerror(errno)); - } - freeaddrinfo(info); - } else { - fprintf(stderr, "warning: can't lookup name \"%s\"\n", a->addrstr); - } - } - break; - default: - break; - } - filter_apply_flags(f->child, flags); - filter_apply_flags(f->next, flags); -} diff --git a/filter.h b/filter.h index 96965b92..104694a7 100644 --- a/filter.h +++ b/filter.h @@ -145,7 +145,6 @@ struct filter *new_filter_oneway(void); void filter_unroll(struct filter **f); void filter_nogroup(struct filter *f); void filter_noneg(struct filter **f); -void filter_apply_flags(struct filter *f, long flags); /* from generated lexer and parer in filterlex.l */ int filter_fopen(const char *filename); @@ -215,10 +214,8 @@ enum flags { FF_LSTATE = (1 << 1), /* lightweight state matching */ FF_LOCAL = (1 << 2), /* assume packets are local only */ FF_ROUTE = (1 << 3), /* assume packets are forwarded */ - FF_LOOKUP = (1 << 4), /* translate host and service names into IP addresses - and port numbers */ + FF_NORESOLVE = (1 << 4), /* don't resolve hostnames, ports, or services */ FF_FLUSH = (1 << 5), /* just flush the ruleset instead */ - FF_NORESOLVE = (1 << 6), /* don't resolve hostnames, ports, or services */ }; /* filtergen.c */ From 019e43b4736f0aed0cbb18ad86f33331ba3eff88 Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Sat, 20 Feb 2016 10:46:30 +1100 Subject: [PATCH 07/20] Check to see if the google-perftools libprofiler is available. --- Makefile.am | 2 +- configure.ac | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index e51d50cf..ca762eef 100644 --- a/Makefile.am +++ b/Makefile.am @@ -19,7 +19,7 @@ filtergen_SOURCES = \ resolver.c \ icmpent.c -filtergen_LDADD = @GETOPT_LIBS@ +filtergen_LDADD = @GETOPT_LIBS@ @LIBPROFILER@ headers = filter.h util.h ast.h resolver.h icmpent.h diff --git a/configure.ac b/configure.ac index 6c67b599..51e5bf70 100644 --- a/configure.ac +++ b/configure.ac @@ -43,6 +43,22 @@ if test "x$HAVE_GETOPT" = xyes ; then AC_SUBST(GETOPT_LIBS) fi +# Google profiler +AC_MSG_CHECKING([whether to enable profiler]) +AC_ARG_WITH([profiler], + [AS_HELP_STRING([--with-profiler],[enable support for profiler [default=no]])], + [with_profiler=$withval], + [with_profiler=no] +) +AC_MSG_RESULT([$with_profiler]) + +if test "x${with_profiler}" = "xyes"; then + AC_CHECK_LIB([profiler], [ProfilerStart], + [AC_SUBST([LIBPROFILER], ["-lprofiler"])], + [AC_MSG_FAILURE([check for profiler failed. Have you installed google-perftools-devel?])], + ) +fi + dnl ----------------- dnl set warning level dnl ----------------- From 8483da10cf5a6d4e9d27ec352a98d44a4fa841d6 Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Sat, 20 Feb 2016 21:32:17 +1100 Subject: [PATCH 08/20] Remove filter_unroll, unused by the compilation phase. --- fg-cisco.c | 1 - fg-ipchains.c | 1 - fg-ipfilter.c | 1 - fg-iptables.c | 2 -- fg-iptrestore.c | 2 -- filter.c | 45 --------------------------------------------- filter.h | 1 - 7 files changed, 53 deletions(-) diff --git a/fg-cisco.c b/fg-cisco.c index 2c197ef5..9d47e151 100644 --- a/fg-cisco.c +++ b/fg-cisco.c @@ -146,6 +146,5 @@ int fg_cisco(struct filter *filter, int flags) { oputs("# Warning: This backend is not complete and " "can generate broken rulesets."); filter_nogroup(filter); - filter_unroll(&filter); return filtergen_cprod(filter, &cb_cisco, &misc); } diff --git a/fg-ipchains.c b/fg-ipchains.c index 8681d927..e934671e 100644 --- a/fg-ipchains.c +++ b/fg-ipchains.c @@ -265,7 +265,6 @@ int fg_ipchains(struct filter *filter, int flags) { .rule = cb_ipchains_rule, .group = cb_ipchains_group, }; - filter_unroll(&filter); if (!(flags & FF_NOSKEL)) { oputs("for f in INPUT OUTPUT FORWARD; do " IPCHAINS " -P $f DENY; done"); oputs(IPCHAINS " -F; " IPCHAINS " -X"); diff --git a/fg-ipfilter.c b/fg-ipfilter.c index db39a984..1645a72c 100644 --- a/fg-ipfilter.c +++ b/fg-ipfilter.c @@ -135,6 +135,5 @@ int fg_ipfilter(struct filter *filter, int flags) { fg_callback cb_ipfilter = {.rule = cb_ipfilter_rule, NULL}; filter_nogroup(filter); - filter_unroll(&filter); return filtergen_cprod(filter, &cb_ipfilter, &misc); } diff --git a/fg-iptables.c b/fg-iptables.c index c246eab0..481a2dd6 100644 --- a/fg-iptables.c +++ b/fg-iptables.c @@ -396,8 +396,6 @@ static int fg_iptables_common(struct filter *filter, int flags, }; const int nchains = 3; - filter_unroll(&filter); - if (!(flags & FF_NOSKEL)) { oputs("CHAINS=\"INPUT OUTPUT FORWARD\""); oputs(""); diff --git a/fg-iptrestore.c b/fg-iptrestore.c index 0e512c65..28e1c38f 100644 --- a/fg-iptrestore.c +++ b/fg-iptrestore.c @@ -402,8 +402,6 @@ static int fg_iptrestore_common(struct filter *filter, int flags, }; const int nchains = 3; - filter_unroll(&filter); - if (!(flags & FF_NOSKEL)) { oprintf("%s <child = x; } -/* - * The easy bit of a cross-product. Basically we ensure that no - * filter node has more than one path out. - * 1. We push sibling->child down to the end of the component - * sub-lists, and - * 2. Ensure that negated entries have only a single component - * hanging off them. - */ -void __filter_unroll(struct filter *f) { - struct filter *c, *s; - - if (!f) - return; - - /* depth first */ - __filter_unroll(c = f->child); - - /* check this node */ - switch (f->type) { - case F_SIBLIST: - for (s = f->u.sib; s; s = s->next) { - __filter_unroll(s); - filter_append(s, c); - } - f->child = NULL; - break; - case F_SUBGROUP: - __filter_unroll(f->u.sub.list); - break; - case F_NEG: - abort(); - default: - break; - } - - /* lastly, go sideways */ - __filter_unroll(f->next); -} - void __filter_neg_expand(struct filter **f, int neg) { if (!*f) return; @@ -377,12 +338,6 @@ void __filter_targets_to_end(struct filter **f) { } } -void filter_unroll(struct filter **f) { - __filter_neg_expand(f, 0); - __filter_targets_to_end(f); - __filter_unroll(*f); -} - void filter_nogroup(struct filter *f) { if (!f) return; diff --git a/filter.h b/filter.h index 104694a7..caad71eb 100644 --- a/filter.h +++ b/filter.h @@ -142,7 +142,6 @@ struct filter *new_filter_host(enum filtertype, const char *, sa_family_t); struct filter *new_filter_oneway(void); /* filter manipulations */ -void filter_unroll(struct filter **f); void filter_nogroup(struct filter *f); void filter_noneg(struct filter **f); From def2b7151b5393da1876cf6f78f4e981195c3e9f Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Sun, 21 Feb 2016 02:26:53 +1100 Subject: [PATCH 09/20] Replace abort() with a message and return and error state. --- fg-cisco.c | 7 ++++--- fg-ipchains.c | 17 ++++++++++------- fg-ipfilter.c | 7 ++++--- fg-iptables.c | 18 +++++++++++------- fg-iptrestore.c | 18 +++++++++++------- filter.c | 5 +++-- gen.c | 3 ++- 7 files changed, 45 insertions(+), 30 deletions(-) diff --git a/fg-cisco.c b/fg-cisco.c index 9d47e151..50cc7e95 100644 --- a/fg-cisco.c +++ b/fg-cisco.c @@ -71,8 +71,8 @@ static int cb_cisco_rule(const struct filterent *ent, APP(rule_r, "IN"); break; default: - fprintf(stderr, "unknown direction\n"); - abort(); + fprintf(stderr, "invalid direction: %d\n", ent->direction); + return -1; } /* target */ @@ -91,7 +91,8 @@ static int cb_cisco_rule(const struct filterent *ent, APPS(rule_r, "deny"); break; default: - abort(); + fprintf(stderr, "invalid target: %d\n", ent->target); + return -1; } /* protocol */ diff --git a/fg-ipchains.c b/fg-ipchains.c index e934671e..b14b58bd 100644 --- a/fg-ipchains.c +++ b/fg-ipchains.c @@ -103,8 +103,8 @@ static int cb_ipchains_rule(const struct filterent *ent, forrevchain = strdup("forward"); break; default: - fprintf(stderr, "unknown direction\n"); - abort(); + fprintf(stderr, "invalid direction: %d\n", ent->direction); + return -1; } if (ent->iface && strcmp(ent->iface, "*")) { @@ -187,7 +187,8 @@ static int cb_ipchains_rule(const struct filterent *ent, forrevtarget = strdup("forw_out"); break; default: - abort(); + fprintf(stderr, "invalid direction: %d\n", ent->direction); + return -1; } break; case DROP: @@ -221,11 +222,13 @@ static int cb_ipchains_rule(const struct filterent *ent, forrevtarget = strdup("forward"); break; default: - abort(); + fprintf(stderr, "invalid direction: %d\n", ent->direction); + return -1; } break; default: - abort(); + fprintf(stderr, "invalid target: %d\n", ent->target); + return -1; } if (ent->oneway) @@ -298,8 +301,8 @@ int flush_ipchains(enum filtertype policy) { ostr = strdup("REJECT"); break; default: - fprintf(stderr, "invalid filtertype %d\n", policy); - abort(); + fprintf(stderr, "invalid filtertype: %d\n", policy); + return -1; } oprintf("for f in $CHAINS; do " IPCHAINS " -P $f %s; done\n", ostr); oputs(IPCHAINS " -F; " IPCHAINS " -X"); diff --git a/fg-ipfilter.c b/fg-ipfilter.c index 1645a72c..30ffdfe0 100644 --- a/fg-ipfilter.c +++ b/fg-ipfilter.c @@ -80,7 +80,8 @@ static int cb_ipfilter_rule(const struct filterent *ent, APP(rule, "block return-icmp-as-dest(port-unr)"); break; default: - abort(); + fprintf(stderr, "invalid target: %d\n", ent->target); + return -1; } /* in or out? */ @@ -92,8 +93,8 @@ static int cb_ipfilter_rule(const struct filterent *ent, APPS(rule, "out"); break; default: - fprintf(stderr, "unknown direction\n"); - abort(); + fprintf(stderr, "invalid direction: %d\n", ent->direction); + return -1; } if (ESET(ent, LOG)) diff --git a/fg-iptables.c b/fg-iptables.c index 481a2dd6..eaf9cd5b 100644 --- a/fg-iptables.c +++ b/fg-iptables.c @@ -157,8 +157,8 @@ static int cb_iptables_rule_common(const struct filterent *ent, } break; default: - fprintf(stderr, "unknown direction\n"); - abort(); + fprintf(stderr, "invalid direction: %d\n", ent->direction); + return -1; } /* state and reverse rules here */ @@ -280,7 +280,8 @@ static int cb_iptables_rule_common(const struct filterent *ent, nattarget = strdup("REDIRECT"); break; default: - abort(); + fprintf(stderr, "invalid target: %d\n", target); + return -1; } } @@ -297,7 +298,8 @@ static int cb_iptables_rule_common(const struct filterent *ent, forrevtarget = strdup("FORW_OUT"); break; default: - abort(); + fprintf(stderr, "invalid direction: %d\n", ent->direction); + return -1; } break; case DROP: @@ -324,11 +326,13 @@ static int cb_iptables_rule_common(const struct filterent *ent, forrevtarget = strdup("FORWARD"); break; default: - abort(); + fprintf(stderr, "invalid direction: %d\n", ent->direction); + return -1; } break; default: - abort(); + fprintf(stderr, "invalid target: %d\n", target); + return -1; } if ((misc->flags & FF_LSTATE) && (target != T_REJECT)) @@ -478,7 +482,7 @@ static int flush_iptables_common(enum filtertype policy, sa_family_t family, break; default: fprintf(stderr, "invalid filtertype %d\n", policy); - abort(); + return -1; } oprintf("for f in $CHAINS; do %s -P $f %s; done\n", iptables, ostr); oprintf("%s -F; %s -X\n", iptables, iptables); diff --git a/fg-iptrestore.c b/fg-iptrestore.c index 28e1c38f..2c205006 100644 --- a/fg-iptrestore.c +++ b/fg-iptrestore.c @@ -154,8 +154,8 @@ static int cb_iptrestore_rule_common(const struct filterent *ent, } break; default: - fprintf(stderr, "unknown direction\n"); - abort(); + fprintf(stderr, "invalid direction: %d\n", ent->direction); + return -1; } /* state and reverse rules here */ @@ -276,7 +276,8 @@ static int cb_iptrestore_rule_common(const struct filterent *ent, nattarget = strdup("REDIRECT"); break; default: - abort(); + fprintf(stderr, "invalid target: %d\n", target); + return -1; } } @@ -293,7 +294,8 @@ static int cb_iptrestore_rule_common(const struct filterent *ent, forrevtarget = strdup("FORW_OUT"); break; default: - abort(); + fprintf(stderr, "invalid direction: %d\n", ent->direction); + return -1; } break; case DROP: @@ -320,11 +322,13 @@ static int cb_iptrestore_rule_common(const struct filterent *ent, forrevtarget = strdup("FORWARD"); break; default: - abort(); + fprintf(stderr, "invalid direction: %d\n", ent->direction); + return -1; } break; default: - abort(); + fprintf(stderr, "invalid target: %d\n", target); + return -1; } if ((misc->flags & FF_LSTATE) && (target != T_REJECT)) @@ -474,7 +478,7 @@ static int flush_iptrestore_common(enum filtertype policy) { break; default: fprintf(stderr, "invalid filtertype %d\n", policy); - abort(); + return -1; } oprintf(":INPUT %s [0:0]\n", ostr); oprintf(":OUTPUT %s [0:0]\n", ostr); diff --git a/filter.c b/filter.c index df4c9e77..ff8ffca2 100644 --- a/filter.c +++ b/filter.c @@ -190,8 +190,9 @@ struct filter *new_filter_host(enum filtertype type, const char *matchstr, } break; default: - fprintf(stderr, "can't parse netmask \"%s\" for unknown address family\n", - mask); + fprintf(stderr, + "can't parse netmask \"%s\" for invalid address family %d\n", + mask, family); return NULL; } } diff --git a/gen.c b/gen.c index 65c80691..21564e39 100644 --- a/gen.c +++ b/gen.c @@ -117,7 +117,7 @@ int __fg_applyone(struct filterent *e, const struct filter *f, fg_callback *cb, fprintf( stderr, "backend doesn't support grouping, but hasn't removed groups\n"); - abort(); + return -1; } e->target = f->type; e->subgroup = f->u.sub.name; @@ -167,6 +167,7 @@ int __fg_applyone(struct filterent *e, const struct filter *f, fg_callback *cb, return __fg_applylist(e, f->u.sib, cb, misc); default: + abort(); } From 130f958e60c2f48c9c032e662774f8e6fa1cf13f Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Sun, 21 Feb 2016 02:27:20 +1100 Subject: [PATCH 10/20] Only branch on n->arg if it exists. --- glue.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/glue.c b/glue.c index f7ebb669..cca3f6d7 100644 --- a/glue.c +++ b/glue.c @@ -126,7 +126,8 @@ convert_direction_argument_list(struct direction_argument_list_s *n, int type) { printf("warning: convert_direction_argument_list returned NULL\n"); } } else { - res = convert_direction_argument(n->arg, type); + if (n->arg) + res = convert_direction_argument(n->arg, type); } return res; @@ -204,7 +205,8 @@ struct filter *convert_host_argument_list(struct host_argument_list_s *n, printf("warning: convert_host_argument_list returned NULL\n"); } } else { - res = convert_host_argument(n->arg, type, o); + if (n->arg) + res = convert_host_argument(n->arg, type, o); } return res; @@ -272,7 +274,8 @@ convert_protocol_argument_list(struct protocol_argument_list_s *n) { printf("warning: convert_protocol_argument_list returned NULL\n"); } } else { - res = convert_protocol_argument(n->arg); + if (n->arg) + res = convert_protocol_argument(n->arg); } return res; @@ -336,7 +339,8 @@ struct filter *convert_port_argument_list(struct port_argument_list_s *n, printf("warning: convert_port_argument_list returned NULL\n"); } } else { - res = convert_port_argument(n->arg, type); + if (n->arg) + res = convert_port_argument(n->arg, type); } return res; @@ -403,7 +407,8 @@ convert_icmptype_argument_list(struct icmptype_argument_list_s *n) { printf("warning: convert_icmptype_argument_list returned NULL\n"); } } else { - res = convert_icmptype_argument(n->arg); + if (n->arg) + res = convert_icmptype_argument(n->arg); } return res; @@ -576,7 +581,8 @@ struct filter *convert_specifier_list(struct specifier_list_s *n, printf("warning: convert_specifier_list returned NULL\n"); } } else { - res = convert_negated_specifier(n->spec, o); + if (n->spec) + res = convert_negated_specifier(n->spec, o); } return res; From 7b672cfd29990aff9f27ad795c986cccf5e53784 Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Sun, 21 Feb 2016 02:33:12 +1100 Subject: [PATCH 11/20] Don't rash when invalid filter types are processed. --- gen.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gen.c b/gen.c index 21564e39..3897b99d 100644 --- a/gen.c +++ b/gen.c @@ -167,8 +167,8 @@ int __fg_applyone(struct filterent *e, const struct filter *f, fg_callback *cb, return __fg_applylist(e, f->u.sib, cb, misc); default: - - abort(); + fprintf(stderr, "invalid filter type %d\n", f->type); + return -1; } if (f->negate) From fbd9ed029071cedd8ee8a0c5bf6218a81db2e169 Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Sun, 21 Feb 2016 16:56:47 +1100 Subject: [PATCH 12/20] Update HISTORY --- HISTORY | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/HISTORY b/HISTORY index 1e38f231..4b5bc415 100644 --- a/HISTORY +++ b/HISTORY @@ -1,3 +1,8 @@ +0.12.8: + Fix some easy to cause crashes. + Remove some obsolete code that caused long compilation times. + Prevents inclusion of the root filesystem path directly. + 0.12.7: Supports iptables-restore output format. Supports IPv6 names. From ee309d6a8cf2bbf2644180d1b25494bef14c4c6f Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Sun, 21 Feb 2016 17:13:49 +1100 Subject: [PATCH 13/20] Free strdup'd memory. --- filtergen.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/filtergen.c b/filtergen.c index 3a253363..8018972f 100644 --- a/filtergen.c +++ b/filtergen.c @@ -234,6 +234,7 @@ int main(int argc, char **argv) { for (ft = filter_types; ft->name; ft++) if (!strcmp(ftn, ft->name)) break; + free(ftn); if (!ft->name) { fprintf(stderr, "%s: target filter unrecognised: %s\n", progname, ftn); usage(progname); @@ -293,6 +294,9 @@ int main(int argc, char **argv) { unlink(ofn); return 1; } + if (ofn) { + free(ofn); + } fprintf(stderr, "generated %d rules\n", l); return 0; } From 43d6d13acc25e579865cd3cc355ab3167f8da9db Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Sun, 21 Feb 2016 17:15:29 +1100 Subject: [PATCH 14/20] Convert filename() into a const char return and allocate the standard input name as a static. --- scanner.l | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scanner.l b/scanner.l index 3034b319..a11bfac0 100644 --- a/scanner.l +++ b/scanner.l @@ -40,10 +40,11 @@ struct inc_stack_s { }; struct inc_stack_s inc_stack[MAXINCLUDES] = { { .state = 0, .filename = NULL, .lineno = 1 } }; +static const char kStandardInput[] = "(standard input)"; int inc_stackptr = 0; long int lineno(); -char * filename(); +const char * filename(); static void scan_err(const char * fmt, ...) __attribute__((format(printf, 1, 2))); void include_file(const char *); %} @@ -156,10 +157,9 @@ long int lineno(void) { return inc_stack[inc_stackptr].lineno; } -/* FIXME: make this return an immutable string */ -char *filename(void) { +const char *filename(void) { return inc_stack[inc_stackptr].filename ? inc_stack[inc_stackptr].filename - : strdup("(standard input)"); + : kStandardInput; } static void scan_err(const char *fmt, ...) { From 2c9cf85e840ccd64b6db5e020b8a98f7777b4194 Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Sun, 21 Feb 2016 17:22:33 +1100 Subject: [PATCH 15/20] Free the AST after conversion to struct filter*. --- glue.c | 49 ++++++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/glue.c b/glue.c index cca3f6d7..3c589960 100644 --- a/glue.c +++ b/glue.c @@ -17,6 +17,7 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ +#include #include #include "filter.h" #include "ast.h" @@ -75,6 +76,7 @@ struct filter *convert_subrule_list(struct subrule_list_s *n, printf("error: no content in subrule_list\n"); } + free(n); return res; } @@ -90,6 +92,7 @@ struct filter *convert_compound_specifier(struct compound_specifier_s *r, res = new_filter_sibs(sub); } } + free(r); return res; } @@ -102,7 +105,7 @@ struct filter *convert_direction_argument(struct direction_argument_s *n, } else { printf("error: no direction argument contents\n"); } - + free(n); return res; } @@ -129,7 +132,7 @@ convert_direction_argument_list(struct direction_argument_list_s *n, int type) { if (n->arg) res = convert_direction_argument(n->arg, type); } - + free(n); return res; } @@ -156,7 +159,7 @@ struct filter *convert_direction(struct direction_specifier_s *n) { } else { printf("error: no direction argument list\n"); } - + free(n); return res; } @@ -181,7 +184,7 @@ struct filter *convert_host_argument(struct host_argument_s *n, int type, } else { printf("error: no host part\n"); } - + free(n); return res; } @@ -208,7 +211,7 @@ struct filter *convert_host_argument_list(struct host_argument_list_s *n, if (n->arg) res = convert_host_argument(n->arg, type, o); } - + free(n); return res; } @@ -236,7 +239,7 @@ struct filter *convert_host_specifier(struct host_specifier_s *n, } else { printf("error: no host argument list\n"); } - + free(n); return res; } @@ -250,7 +253,7 @@ struct filter *convert_protocol_argument(struct protocol_argument_s *n) { } else { printf("error: no protocol argument contents\n"); } - + free(n); return res; } @@ -277,7 +280,7 @@ convert_protocol_argument_list(struct protocol_argument_list_s *n) { if (n->arg) res = convert_protocol_argument(n->arg); } - + free(n); return res; } @@ -291,7 +294,7 @@ struct filter *convert_protocol_specifier(struct protocol_specifier_s *n) { } else { printf("error: no protocol argument list\n"); } - + free(n); return res; } @@ -315,7 +318,7 @@ struct filter *convert_port_argument(struct port_argument_s *n, int type) { } else { printf("error: no port argument contents\n"); } - + free(n); return res; } @@ -342,7 +345,7 @@ struct filter *convert_port_argument_list(struct port_argument_list_s *n, if (n->arg) res = convert_port_argument(n->arg, type); } - + free(n); return res; } @@ -369,7 +372,7 @@ struct filter *convert_port_specifier(struct port_specifier_s *n) { } else { printf("error: no port argument list\n"); } - + free(n); return res; } @@ -383,7 +386,7 @@ struct filter *convert_icmptype_argument(struct icmptype_argument_s *n) { } else { printf("error: no icmptype argument contents\n"); } - + free(n); return res; } @@ -410,7 +413,7 @@ convert_icmptype_argument_list(struct icmptype_argument_list_s *n) { if (n->arg) res = convert_icmptype_argument(n->arg); } - + free(n); return res; } @@ -424,7 +427,7 @@ struct filter *convert_icmptype_specifier(struct icmptype_specifier_s *n) { } else { printf("error: no icmptype argument list\n"); } - + free(n); return res; } @@ -450,7 +453,7 @@ struct filter *convert_option_specifier(struct option_specifier_s *n) { printf("error: incorrect option type encountered\n"); break; } - + free(n); return res; } @@ -480,7 +483,7 @@ struct filter *convert_chaingroup_specifier(struct chaingroup_specifier_s *n, } else { printf("error: no list in chaingroup\n"); } - + free(n); return res; } @@ -536,9 +539,10 @@ struct filter *convert_specifier(struct specifier_s *r, res = convert_option_specifier(r->option); } else if (r->chaingroup) { res = convert_chaingroup_specifier(r->chaingroup, o); - } else + } else { printf("error: no specifiers\n"); - + } + free(r); return res; } @@ -558,6 +562,7 @@ struct filter *convert_negated_specifier(struct negated_specifier_s *r, res = spec; } } + free(r); return res; } @@ -584,7 +589,7 @@ struct filter *convert_specifier_list(struct specifier_list_s *n, if (n->spec) res = convert_negated_specifier(n->spec, o); } - + free(n); return res; } @@ -595,6 +600,7 @@ struct filter *convert_rule(struct rule_s *r, struct filtergen_opts *o) { if (r->list) res = convert_specifier_list(r->list, o); + free(r); return res; } @@ -620,7 +626,7 @@ struct filter *convert_rule_list(struct rule_list_s *n, res = convert_rule(n->rule, o); } } - + free(n); return res; } @@ -631,6 +637,7 @@ struct filter *convert(struct ast_s *ast, struct filtergen_opts *o) { if (ast->list) res = convert_rule_list(ast->list, o); + return res; } From 313e6b6a8cb0ad498d04eccc6926b36bdb6fa8d4 Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Sun, 21 Feb 2016 17:33:17 +1100 Subject: [PATCH 16/20] Revert "Remove filter_unroll, unused by the compilation phase." This reverts commit 8483da10cf5a6d4e9d27ec352a98d44a4fa841d6. --- fg-cisco.c | 1 + fg-ipchains.c | 1 + fg-ipfilter.c | 1 + fg-iptables.c | 2 ++ fg-iptrestore.c | 2 ++ filter.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ filter.h | 1 + 7 files changed, 53 insertions(+) diff --git a/fg-cisco.c b/fg-cisco.c index 50cc7e95..258b071d 100644 --- a/fg-cisco.c +++ b/fg-cisco.c @@ -147,5 +147,6 @@ int fg_cisco(struct filter *filter, int flags) { oputs("# Warning: This backend is not complete and " "can generate broken rulesets."); filter_nogroup(filter); + filter_unroll(&filter); return filtergen_cprod(filter, &cb_cisco, &misc); } diff --git a/fg-ipchains.c b/fg-ipchains.c index b14b58bd..3ef26709 100644 --- a/fg-ipchains.c +++ b/fg-ipchains.c @@ -268,6 +268,7 @@ int fg_ipchains(struct filter *filter, int flags) { .rule = cb_ipchains_rule, .group = cb_ipchains_group, }; + filter_unroll(&filter); if (!(flags & FF_NOSKEL)) { oputs("for f in INPUT OUTPUT FORWARD; do " IPCHAINS " -P $f DENY; done"); oputs(IPCHAINS " -F; " IPCHAINS " -X"); diff --git a/fg-ipfilter.c b/fg-ipfilter.c index 30ffdfe0..0911b328 100644 --- a/fg-ipfilter.c +++ b/fg-ipfilter.c @@ -136,5 +136,6 @@ int fg_ipfilter(struct filter *filter, int flags) { fg_callback cb_ipfilter = {.rule = cb_ipfilter_rule, NULL}; filter_nogroup(filter); + filter_unroll(&filter); return filtergen_cprod(filter, &cb_ipfilter, &misc); } diff --git a/fg-iptables.c b/fg-iptables.c index eaf9cd5b..00b5d7c0 100644 --- a/fg-iptables.c +++ b/fg-iptables.c @@ -400,6 +400,8 @@ static int fg_iptables_common(struct filter *filter, int flags, }; const int nchains = 3; + filter_unroll(&filter); + if (!(flags & FF_NOSKEL)) { oputs("CHAINS=\"INPUT OUTPUT FORWARD\""); oputs(""); diff --git a/fg-iptrestore.c b/fg-iptrestore.c index 2c205006..6e094e9a 100644 --- a/fg-iptrestore.c +++ b/fg-iptrestore.c @@ -406,6 +406,8 @@ static int fg_iptrestore_common(struct filter *filter, int flags, }; const int nchains = 3; + filter_unroll(&filter); + if (!(flags & FF_NOSKEL)) { oprintf("%s <child = x; } +/* + * The easy bit of a cross-product. Basically we ensure that no + * filter node has more than one path out. + * 1. We push sibling->child down to the end of the component + * sub-lists, and + * 2. Ensure that negated entries have only a single component + * hanging off them. + */ +void __filter_unroll(struct filter *f) { + struct filter *c, *s; + + if (!f) + return; + + /* depth first */ + __filter_unroll(c = f->child); + + /* check this node */ + switch (f->type) { + case F_SIBLIST: + for (s = f->u.sib; s; s = s->next) { + __filter_unroll(s); + filter_append(s, c); + } + f->child = NULL; + break; + case F_SUBGROUP: + __filter_unroll(f->u.sub.list); + break; + case F_NEG: + abort(); + default: + break; + } + + /* lastly, go sideways */ + __filter_unroll(f->next); +} + void __filter_neg_expand(struct filter **f, int neg) { if (!*f) return; @@ -339,6 +378,12 @@ void __filter_targets_to_end(struct filter **f) { } } +void filter_unroll(struct filter **f) { + __filter_neg_expand(f, 0); + __filter_targets_to_end(f); + __filter_unroll(*f); +} + void filter_nogroup(struct filter *f) { if (!f) return; diff --git a/filter.h b/filter.h index caad71eb..104694a7 100644 --- a/filter.h +++ b/filter.h @@ -142,6 +142,7 @@ struct filter *new_filter_host(enum filtertype, const char *, sa_family_t); struct filter *new_filter_oneway(void); /* filter manipulations */ +void filter_unroll(struct filter **f); void filter_nogroup(struct filter *f); void filter_noneg(struct filter **f); From 0bfedcd96a3647d680f42bd5adc624566852b8aa Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Sun, 21 Feb 2016 19:06:08 +1100 Subject: [PATCH 17/20] Add a test to validate the example programs. --- t/Makefile.am | 3 ++- t/check_examples | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100755 t/check_examples diff --git a/t/Makefile.am b/t/Makefile.am index 00846833..324413f7 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -69,7 +69,8 @@ TESTS = t_01_ccomment \ t_69_includeemptynoop \ t_70_includesemicolon \ t_71_includeglob \ - t_72_noincluderoot + t_72_noincluderoot \ + check_examples check_PROGRAMS = scan parse emit convert factorise diff --git a/t/check_examples b/t/check_examples new file mode 100755 index 00000000..3f3d6adc --- /dev/null +++ b/t/check_examples @@ -0,0 +1,11 @@ +#!/bin/sh -ex + +TEST="that the example programs all compile" + +testdir=`dirname $0` +. $testdir/testlib + +for t in $here/../examples/*.filter; do + echo Checking $t + $here/../filtergen -R -c -t iptables $t +done From ad5fd1f04279d92b44cf38ea468dacb840e9ef7b Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Mon, 22 Feb 2016 22:56:43 +1100 Subject: [PATCH 18/20] Explicit braces around if. --- glue.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/glue.c b/glue.c index 3c589960..1b0c85ac 100644 --- a/glue.c +++ b/glue.c @@ -635,8 +635,9 @@ struct filter *convert(struct ast_s *ast, struct filtergen_opts *o) { eprint("converting ast\n"); - if (ast->list) + if (ast->list) { res = convert_rule_list(ast->list, o); +} return res; } From 629ccaf8b3e562332991c4753a89b544136afe51 Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Mon, 22 Feb 2016 22:57:42 +1100 Subject: [PATCH 19/20] Rename icmp.filter to icmp to avoid being included in the example test. --- examples/{icmp.filter => icmp} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename examples/{icmp.filter => icmp} (100%) diff --git a/examples/icmp.filter b/examples/icmp similarity index 100% rename from examples/icmp.filter rename to examples/icmp From 9eff5b4cad78ce594dbb22e994961fd0a685d947 Mon Sep 17 00:00:00 2001 From: Jamie Wilkinson Date: Mon, 22 Feb 2016 23:18:00 +1100 Subject: [PATCH 20/20] Bump to version 0.12.8 and update HISTORY. --- HISTORY | 1 + configure.ac | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/HISTORY b/HISTORY index 4b5bc415..536397f4 100644 --- a/HISTORY +++ b/HISTORY @@ -2,6 +2,7 @@ Fix some easy to cause crashes. Remove some obsolete code that caused long compilation times. Prevents inclusion of the root filesystem path directly. + Test that the example programs compile. 0.12.7: Supports iptables-restore output format. diff --git a/configure.ac b/configure.ac index 51e5bf70..8ab7d558 100644 --- a/configure.ac +++ b/configure.ac @@ -1,6 +1,6 @@ AC_PREREQ(2.50) -AC_INIT(filtergen, 0.12.7, jaq@spacepants.org) +AC_INIT(filtergen, 0.12.8, jaq@spacepants.org) AC_CONFIG_AUX_DIR(.) AC_CONFIG_SRCDIR(filtergen.c)