From: Mateusz Majewski Date: Thu, 24 Dec 2020 10:47:44 +0000 (+0100) Subject: Fix mask calculation X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=f4be1fa5d9259c10996917c386e33ea2f40d97eb;p=platform%2Fcore%2Fconnectivity%2Fstc-iptables.git Fix mask calculation 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 --- diff --git a/src/helper/helper-ip6tables.c b/src/helper/helper-ip6tables.c index 28926f5..046428d 100755 --- a/src/helper/helper-ip6tables.c +++ b/src/helper/helper-ip6tables.c @@ -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; } diff --git a/src/helper/helper-iptables.c b/src/helper/helper-iptables.c index 999b40d..8678ba6 100755 --- a/src/helper/helper-iptables.c +++ b/src/helper/helper-iptables.c @@ -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; }