Re: [LEDE-DEV] [PATCHv2] firewall: fix logging of dropped & rejected packets

2018-04-09 Thread Philip Prindeville 
“guest” or “salon”?



> On Apr 3, 2018, at 8:51 AM, Alin Nastac  wrote:
> 
> From: Alin Nastac 
> 
> Reproduction scenario:
> - use 3 interfaces with 3 different zones - lan, wan and guest
> - configure firewall to allow forwarding from lan to wan
> - add DROP rule to prevent forwarding from lan to guest
> - although packets are forwarded from lan to wan, "DROP(dest guest)"
> traces are generated by zone_guest_dest_DROP chain
> 
> Signed-off-by: Alin Nastac 
> ---
> zones.c | 72 ++---
> 1 file changed, 60 insertions(+), 12 deletions(-)
> 
> diff --git a/zones.c b/zones.c
> index e00d527..9f00aca 100644
> --- a/zones.c
> +++ b/zones.c
> @@ -20,6 +20,8 @@
> #include "ubus.h"
> #include "helpers.h"
> 
> +#define filter_target(t) \
> + ((t == FW3_FLAG_REJECT) ? "reject" : fw3_flag_names[t])
> 
> #define C(f, tbl, tgt, fmt) \
>   { FW3_FAMILY_##f, FW3_TABLE_##tbl, FW3_FLAG_##tgt, fmt }
> @@ -401,6 +403,19 @@ print_zone_chain(struct fw3_ipt_handle *handle, struct 
> fw3_state *state,
>   set(zone->flags, handle->family, handle->table);
> }
> 
> +static const char*
> +jump_target(enum fw3_flag t, bool src, struct fw3_zone *zone, char *buf, 
> size_t size)
> +{
> + if ((zone->log & FW3_ZONE_LOG_FILTER) && t > FW3_FLAG_ACCEPT)
> + {
> + snprintf(buf, size, "%s_%s_%s", fw3_flag_names[t],
> + src ? "src" : "dest", zone->name);
> + return buf;
> + }
> +
> + return filter_target(t);
> +}
> +
> static void
> print_interface_rule(struct fw3_ipt_handle *handle, struct fw3_state *state,
>bool reload, struct fw3_zone *zone,
> @@ -420,9 +435,6 @@ print_interface_rule(struct fw3_ipt_handle *handle, 
> struct fw3_state *state,
>   "forward", "FORWARD",
>   };
> 
> -#define jump_target(t) \
> - ((t == FW3_FLAG_REJECT) ? "reject" : fw3_flag_names[t])
> -
>   if (handle->table == FW3_TABLE_FILTER)
>   {
>   for (t = FW3_FLAG_ACCEPT; t <= FW3_FLAG_DROP; t++)
> @@ -430,7 +442,7 @@ print_interface_rule(struct fw3_ipt_handle *handle, 
> struct fw3_state *state,
>   if (has(zone->flags, handle->family, 
> fw3_to_src_target(t)))
>   {
>   r = fw3_ipt_rule_create(handle, NULL, dev, 
> NULL, sub, NULL);
> - fw3_ipt_rule_target(r, jump_target(t));
> + fw3_ipt_rule_target(r, jump_target(t, true, 
> zone, buf, sizeof(buf)));
>   fw3_ipt_rule_extra(r, zone->extra_src);
> 
>   if (t == FW3_FLAG_ACCEPT && 
> !state->defaults.drop_invalid)
> @@ -455,7 +467,7 @@ print_interface_rule(struct fw3_ipt_handle *handle, 
> struct fw3_state *state,
>   }
> 
>   r = fw3_ipt_rule_create(handle, NULL, NULL, 
> dev, NULL, sub);
> - fw3_ipt_rule_target(r, jump_target(t));
> + fw3_ipt_rule_target(r, jump_target(t, false, 
> zone, buf, sizeof(buf)));
>   fw3_ipt_rule_extra(r, zone->extra_dest);
>   fw3_ipt_rule_replace(r, "zone_%s_dest_%s", 
> zone->name,
>fw3_flag_names[t]);
> @@ -503,7 +515,7 @@ print_interface_rule(struct fw3_ipt_handle *handle, 
> struct fw3_state *state,
>   {
>   if (zone->log & FW3_ZONE_LOG_MANGLE)
>   {
> - snprintf(buf, sizeof(buf) - 1, "MSSFIX(%s): ", 
> zone->name);
> + snprintf(buf, sizeof(buf), "MSSFIX(%s): ", 
> zone->name);
> 
>   r = fw3_ipt_rule_create(handle, , NULL, 
> dev, NULL, sub);
>   fw3_ipt_rule_addarg(r, false, "--tcp-flags", 
> "SYN,RST");
> @@ -640,30 +652,46 @@ print_zone_rule(struct fw3_ipt_handle *handle, struct 
> fw3_state *state,
>   {
>   if (has(zone->flags, handle->family, 
> fw3_to_src_target(t)))
>   {
> + fw3_ipt_create_chain(handle, 
> "%s_src_%s",
> +  fw3_flag_names[t], 
> zone->name);
> +
>   r = fw3_ipt_rule_new(handle);
> 
> - snprintf(buf, sizeof(buf) - 1, "%s(src 
> %s)",
> + snprintf(buf, sizeof(buf), "%s(src %s)",
>fw3_flag_names[t], zone->name);
> 
>   fw3_ipt_rule_limit(r, >log_limit);
>   fw3_ipt_rule_target(r, "LOG");
>

[LEDE-DEV] [PATCHv2] firewall: fix logging of dropped & rejected packets

2018-04-03 Thread Alin Nastac 
From: Alin Nastac 

Reproduction scenario:
 - use 3 interfaces with 3 different zones - lan, wan and guest
 - configure firewall to allow forwarding from lan to wan
 - add DROP rule to prevent forwarding from lan to guest
 - although packets are forwarded from lan to wan, "DROP(dest guest)"
 traces are generated by zone_guest_dest_DROP chain

Signed-off-by: Alin Nastac 
---
 zones.c | 72 ++---
 1 file changed, 60 insertions(+), 12 deletions(-)

diff --git a/zones.c b/zones.c
index e00d527..9f00aca 100644
--- a/zones.c
+++ b/zones.c
@@ -20,6 +20,8 @@
 #include "ubus.h"
 #include "helpers.h"
 
+#define filter_target(t) \
+   ((t == FW3_FLAG_REJECT) ? "reject" : fw3_flag_names[t])
 
 #define C(f, tbl, tgt, fmt) \
{ FW3_FAMILY_##f, FW3_TABLE_##tbl, FW3_FLAG_##tgt, fmt }
@@ -401,6 +403,19 @@ print_zone_chain(struct fw3_ipt_handle *handle, struct 
fw3_state *state,
set(zone->flags, handle->family, handle->table);
 }
 
+static const char*
+jump_target(enum fw3_flag t, bool src, struct fw3_zone *zone, char *buf, 
size_t size)
+{
+   if ((zone->log & FW3_ZONE_LOG_FILTER) && t > FW3_FLAG_ACCEPT)
+   {
+   snprintf(buf, size, "%s_%s_%s", fw3_flag_names[t],
+   src ? "src" : "dest", zone->name);
+   return buf;
+   }
+
+   return filter_target(t);
+}
+
 static void
 print_interface_rule(struct fw3_ipt_handle *handle, struct fw3_state *state,
 bool reload, struct fw3_zone *zone,
@@ -420,9 +435,6 @@ print_interface_rule(struct fw3_ipt_handle *handle, struct 
fw3_state *state,
"forward", "FORWARD",
};
 
-#define jump_target(t) \
-   ((t == FW3_FLAG_REJECT) ? "reject" : fw3_flag_names[t])
-
if (handle->table == FW3_TABLE_FILTER)
{
for (t = FW3_FLAG_ACCEPT; t <= FW3_FLAG_DROP; t++)
@@ -430,7 +442,7 @@ print_interface_rule(struct fw3_ipt_handle *handle, struct 
fw3_state *state,
if (has(zone->flags, handle->family, 
fw3_to_src_target(t)))
{
r = fw3_ipt_rule_create(handle, NULL, dev, 
NULL, sub, NULL);
-   fw3_ipt_rule_target(r, jump_target(t));
+   fw3_ipt_rule_target(r, jump_target(t, true, 
zone, buf, sizeof(buf)));
fw3_ipt_rule_extra(r, zone->extra_src);
 
if (t == FW3_FLAG_ACCEPT && 
!state->defaults.drop_invalid)
@@ -455,7 +467,7 @@ print_interface_rule(struct fw3_ipt_handle *handle, struct 
fw3_state *state,
}
 
r = fw3_ipt_rule_create(handle, NULL, NULL, 
dev, NULL, sub);
-   fw3_ipt_rule_target(r, jump_target(t));
+   fw3_ipt_rule_target(r, jump_target(t, false, 
zone, buf, sizeof(buf)));
fw3_ipt_rule_extra(r, zone->extra_dest);
fw3_ipt_rule_replace(r, "zone_%s_dest_%s", 
zone->name,
 fw3_flag_names[t]);
@@ -503,7 +515,7 @@ print_interface_rule(struct fw3_ipt_handle *handle, struct 
fw3_state *state,
{
if (zone->log & FW3_ZONE_LOG_MANGLE)
{
-   snprintf(buf, sizeof(buf) - 1, "MSSFIX(%s): ", 
zone->name);
+   snprintf(buf, sizeof(buf), "MSSFIX(%s): ", 
zone->name);
 
r = fw3_ipt_rule_create(handle, , NULL, 
dev, NULL, sub);
fw3_ipt_rule_addarg(r, false, "--tcp-flags", 
"SYN,RST");
@@ -640,30 +652,46 @@ print_zone_rule(struct fw3_ipt_handle *handle, struct 
fw3_state *state,
{
if (has(zone->flags, handle->family, 
fw3_to_src_target(t)))
{
+   fw3_ipt_create_chain(handle, 
"%s_src_%s",
+fw3_flag_names[t], 
zone->name);
+
r = fw3_ipt_rule_new(handle);
 
-   snprintf(buf, sizeof(buf) - 1, "%s(src 
%s)",
+   snprintf(buf, sizeof(buf), "%s(src %s)",
 fw3_flag_names[t], zone->name);
 
fw3_ipt_rule_limit(r, >log_limit);
fw3_ipt_rule_target(r, "LOG");
fw3_ipt_rule_addarg(r, false, 
"--log-prefix", buf);
-   fw3_ipt_rule_append(r, "zone_%s_src_%s",
-   zone->name, 
fw3_flag_names[t]);