Fix mask calculation
authorMateusz Majewski <m.majewski2@samsung.com>
Thu, 24 Dec 2020 10:47:44 +0000 (11:47 +0100)
committerMateusz Majewski <m.majewski2@samsung.com>
Thu, 14 Jan 2021 12:12:48 +0000 (13:12 +0100)
The iptc library requires a mask in order to compare entries, which is
done in stc-iptables in order to detect duplicate entries and not insert
them. stc-iptables used to calculate the mask in an invalid way; it
marked the contiguous part of the entry that had the length of some
parts of the entry. In particular, in some cases, some parts which
weren't meant to be marked were marked, and some parts which were meant
to be marked weren't. For instance, the following two entries, which are
clearly different, were erronously determined to be the same, and the
second wasn't inserted:

    -A DOZE -m cgroup --cgroup 13 -j NFQUEUE --queue-num 1 --queue-bypass
    -A DOZE -m cgroup --cgroup 15 -j NFQUEUE --queue-num 1 --queue-bypass

This commit instead marks the whole entry. While iptables seems to be
using a more elaborate method of mask calculation, doing it the simple
way was correct in my (admittedly limited) tests.

Change-Id: I71d404907d6f6d94e2944ed6194fbf238265e933

src/helper/helper-ip6tables.c
src/helper/helper-iptables.c

index 28926f581b19a03a3755361dea65976b33921cbd..046428dc1928712a368441fe95a1bcfd7a51c333 100755 (executable)
@@ -130,8 +130,7 @@ static unsigned int __add_iprange_match(ip6tables_ip_type_e sip_type,
        return __add_match(IP6TC_IPRANGE, start, 1, sizeof(ip6t_iprange_info_t), &iprange);
 }
 
-static void __add_iprange(unsigned char *entry, unsigned int *size_mask,
-               unsigned int *size_match, ip6tables_rule_s *rule)
+static void __add_iprange(unsigned char *entry, unsigned int *size_match, ip6tables_rule_s *rule)
 {
        ip6t_entry_t *e = (ip6t_entry_t *)(entry);
 
@@ -139,7 +138,6 @@ static void __add_iprange(unsigned char *entry, unsigned int *size_mask,
                rule->s_ip1, rule->s_ip2, rule->d_ip_type, rule->d_ip1,
                rule->d_ip2, (ip6t_entry_match_t *)(e->elems + (*size_match)));
 
-       (*size_mask) += sizeof(ip6t_entry_match_t);
        e->target_offset += SIZE_IPRANGE_MATCH;
        e->next_offset += SIZE_IPRANGE_MATCH;
 }
@@ -233,8 +231,8 @@ static unsigned int __add_port_match(ip6tables_protocol_type_e prot_type,
        return 0;
 }
 
-static void __add_port(unsigned char *entry, unsigned int *size_mask,
-               unsigned int *size_match, ip6tables_rule_s *rule, unsigned int match_size)
+static void __add_port(unsigned char *entry, unsigned int *size_match,
+               ip6tables_rule_s *rule, unsigned int match_size)
 {
        if ((rule->s_port_type > IP6TABLES_PORT_NONE &&
                rule->s_port_type <= IP6TABLES_PORT_RANGE) ||
@@ -248,7 +246,6 @@ static void __add_port(unsigned char *entry, unsigned int *size_mask,
                        rule->d_port_type, rule->d_port1, rule->d_port2,
                        (ip6t_entry_match_t *) (e->elems + (*size_match)));
 
-               (*size_mask) += sizeof(ip6t_entry_match_t);
                e->target_offset += match_size;
                e->next_offset += match_size;
        }
@@ -305,7 +302,6 @@ static int __create_entry_data(unsigned char *entry, unsigned char *mask,
 {
        ip6t_entry_t *e = NULL;
        ip6t_entry_target_t *target = NULL;
-       unsigned int size_mask = 0;
        unsigned int size_match = 0;
 
        if (!rule->chain) {
@@ -318,7 +314,6 @@ static int __create_entry_data(unsigned char *entry, unsigned char *mask,
        /* entry size */
        e->target_offset = SIZE_ENTRY;
        e->next_offset = SIZE_ENTRY;
-       size_mask = sizeof(ip6t_entry_t);
 
        if (rule->ifname && rule->ifname[0] != '\0') {
                switch (rule->direction) {
@@ -388,7 +383,7 @@ static int __create_entry_data(unsigned char *entry, unsigned char *mask,
 
        if (rule->s_ip_type == IP6TABLES_IP_RANGE ||
                rule->d_ip_type == IP6TABLES_IP_RANGE)
-               __add_iprange(entry, &size_mask, &size_match, rule);
+               __add_iprange(entry, &size_match, rule);
 
        /* -p tcp */
 
@@ -396,11 +391,11 @@ static int __create_entry_data(unsigned char *entry, unsigned char *mask,
        switch (rule->protocol) {
        case IP6TABLES_PROTOCOL_TCP:
                e->ipv6.proto = IPPROTO_TCP;
-               __add_port(entry, &size_mask, &size_match, rule, SIZE_TCP_MATCH);
+               __add_port(entry, &size_match, rule, SIZE_TCP_MATCH);
                break;
        case IP6TABLES_PROTOCOL_UDP:
                e->ipv6.proto = IPPROTO_UDP;
-               __add_port(entry, &size_mask, &size_match, rule, SIZE_UDP_MATCH);
+               __add_port(entry, &size_match, rule, SIZE_UDP_MATCH);
                break;
        case IP6TABLES_PROTOCOL_ICMP:
                e->ipv6.proto = IPPROTO_ICMP;
@@ -427,7 +422,6 @@ static int __create_entry_data(unsigned char *entry, unsigned char *mask,
        /* -m cgroup --cgroup 33 */
        if (rule->classid > 0) {
                size_match += __add_cgroup_match(rule->classid, (ip6t_entry_match_t *) e->elems);
-               size_mask += sizeof(ip6t_entry_match_t);
                e->target_offset += SIZE_CGROUP_MATCH;
                e->next_offset += SIZE_CGROUP_MATCH;
        }
@@ -435,7 +429,6 @@ static int __create_entry_data(unsigned char *entry, unsigned char *mask,
        /* -m nfacct --nfacct-name  c2_1_33_seth_w0 */
        if (rule->nfacct_name && rule->nfacct_name[0] != '\0') {
                size_match += __add_nfacct_match(rule->nfacct_name, (ip6t_entry_match_t *) (e->elems + size_match));
-               size_mask += sizeof(ip6t_entry_match_t);
                e->target_offset += SIZE_NFACCT_MATCH;
                e->next_offset += SIZE_NFACCT_MATCH;
        }
@@ -458,7 +451,7 @@ static int __create_entry_data(unsigned char *entry, unsigned char *mask,
                break;
        }
 
-       memset(mask, 0xFF, size_mask);
+       memset(mask, 0xFF, e->next_offset);
 
        return STC_ERROR_NONE;
 }
index 999b40deb41bcd1de7d479888bf08d043afcad6a..8678ba6c7ac85bd3fc71ea3e79d4a8ef91d7fb86 100755 (executable)
@@ -129,8 +129,7 @@ static unsigned int __add_iprange_match(iptables_ip_type_e sip_type,
        return __add_match(IPTC_IPRANGE, start, 1, sizeof(ipt_iprange_info_t), &iprange);
 }
 
-static void __add_iprange(unsigned char *entry, unsigned int *size_mask,
-               unsigned int *size_match, iptables_rule_s *rule)
+static void __add_iprange(unsigned char *entry, unsigned int *size_match, iptables_rule_s *rule)
 {
        ipt_entry_t *e = (ipt_entry_t *)(entry);
 
@@ -138,7 +137,6 @@ static void __add_iprange(unsigned char *entry, unsigned int *size_mask,
                rule->s_ip1, rule->s_ip2, rule->d_ip_type, rule->d_ip1,
                rule->d_ip2, (ipt_entry_match_t *)(e->elems + (*size_match)));
 
-       (*size_mask) += sizeof(ipt_entry_match_t);
        e->target_offset += SIZE_IPRANGE_MATCH;
        e->next_offset += SIZE_IPRANGE_MATCH;
 }
@@ -232,8 +230,8 @@ static unsigned int __add_port_match(iptables_protocol_type_e prot_type,
        return 0;
 }
 
-static void __add_port(unsigned char *entry, unsigned int *size_mask,
-               unsigned int *size_match, iptables_rule_s *rule, unsigned int match_size)
+static void __add_port(unsigned char *entry, unsigned int *size_match,
+               iptables_rule_s *rule, unsigned int match_size)
 {
        if ((rule->s_port_type > IPTABLES_PORT_NONE &&
                rule->s_port_type <= IPTABLES_PORT_RANGE) ||
@@ -247,7 +245,6 @@ static void __add_port(unsigned char *entry, unsigned int *size_mask,
                        rule->d_port_type, rule->d_port1, rule->d_port2,
                        (ipt_entry_match_t *) (e->elems + (*size_match)));
 
-               (*size_mask) += sizeof(ipt_entry_match_t);
                e->target_offset += match_size;
                e->next_offset += match_size;
        }
@@ -304,7 +301,6 @@ static int __create_entry_data(unsigned char *entry, unsigned char *mask,
 {
        ipt_entry_t *e = NULL;
        ipt_entry_target_t *target = NULL;
-       unsigned int size_mask = 0;
        unsigned int size_match = 0;
 
        if (!rule->chain) {
@@ -317,7 +313,6 @@ static int __create_entry_data(unsigned char *entry, unsigned char *mask,
        /* entry size */
        e->target_offset = SIZE_ENTRY;
        e->next_offset = SIZE_ENTRY;
-       size_mask = sizeof(ipt_entry_t);
 
        if (rule->ifname && rule->ifname[0] != '\0') {
                switch (rule->direction) {
@@ -369,17 +364,17 @@ static int __create_entry_data(unsigned char *entry, unsigned char *mask,
 
        if (rule->s_ip_type == IPTABLES_IP_RANGE ||
                rule->d_ip_type == IPTABLES_IP_RANGE)
-               __add_iprange(entry, &size_mask, &size_match, rule);
+               __add_iprange(entry, &size_match, rule);
 
        /* -p tcp */
        switch (rule->protocol) {
        case IPTABLES_PROTOCOL_TCP:
                e->ip.proto = IPPROTO_TCP;
-               __add_port(entry, &size_mask, &size_match, rule, SIZE_TCP_MATCH);
+               __add_port(entry, &size_match, rule, SIZE_TCP_MATCH);
                break;
        case IPTABLES_PROTOCOL_UDP:
                e->ip.proto = IPPROTO_UDP;
-               __add_port(entry, &size_mask, &size_match, rule, SIZE_UDP_MATCH);
+               __add_port(entry, &size_match, rule, SIZE_UDP_MATCH);
                break;
        case IPTABLES_PROTOCOL_ICMP:
                e->ip.proto = IPPROTO_ICMP;
@@ -408,7 +403,6 @@ static int __create_entry_data(unsigned char *entry, unsigned char *mask,
        if (rule->classid > 0) {
                size_match += __add_cgroup_match(rule->classid,
                        (ipt_entry_match_t *) (e->elems + size_match));
-               size_mask += sizeof(ipt_entry_match_t);
                e->target_offset += SIZE_CGROUP_MATCH;
                e->next_offset += SIZE_CGROUP_MATCH;
        }
@@ -417,7 +411,6 @@ static int __create_entry_data(unsigned char *entry, unsigned char *mask,
        if (rule->nfacct_name && rule->nfacct_name[0] != '\0') {
                size_match += __add_nfacct_match(rule->nfacct_name,
                        (ipt_entry_match_t *) (e->elems + size_match));
-               size_mask += sizeof(ipt_entry_match_t);
                e->target_offset += SIZE_NFACCT_MATCH;
                e->next_offset += SIZE_NFACCT_MATCH;
        }
@@ -440,7 +433,7 @@ static int __create_entry_data(unsigned char *entry, unsigned char *mask,
                break;
        }
 
-       memset(mask, 0xFF, size_mask);
+       memset(mask, 0xFF, e->next_offset); // TODO: is this the correct way to write the mask?
 
        return STC_ERROR_NONE;
 }