RDKBNETWOR-80 : Transform to Nftables from Iptables#104
RDKBNETWOR-80 : Transform to Nftables from Iptables#104vijayragavalu wants to merge 11 commits intodevelopfrom
Conversation
Reason for change: 1) Translate all the RDKB IPtables rules to nftables 2) write into /tmp/.nft and /tmp/.nft_v6 files and apply into netfilter 3) all the nftables rules are added under firewall_nft dir Test Procedure: RDKB Firewall functionality Risks: Medium Signed-off-by: Vijayaragavalu S vijayaragavalu.s@infosys.com
Reason for change: 1) Resolved build errors 2)Bug fixes wrt iptables to nftables rules conversion for few cases Test Procedure: RDKB Firewall functionality Risks: Medium Signed-off-by: Vijayaragavalu S vijayaragavalu.s@infosys.com
Reason for change: iptables to nftables rules conversion for extender , emta , nfq handler rules files Test Procedure: RDKB Firewall functionality Risks: Medium Signed-off-by: Vijayaragavalu S vijayaragavalu.s@infosys.com
Reason for change: adapt these 2 newly added function in firewall nft ext file also get_ip_and_netmask_addr calculate_network_address Test Procedure: RDKB Firewall functionality Risks: Medium Signed-off-by: Vijayaragavalu S vijayaragavalu.s@infosys.com
|
b'## WARNING: A Blackduck scan failure has been waived A prior failure has been upvoted
|
Reason for change: 1) build support for IPV6 nft file 2) Run time issues resolved for ipv4 and ipv6 nft Test Procedure: RDKB Firewall functionality Risks: Medium Signed-off-by: Vijayaragavalu S <vijayaragavalu.s@infosys.com>
Reason for change: 1) Resolve bridge mode issue and managed site Test Procedure: RDKB Firewall functionality Risks: Medium Signed-off-by: Vijayaragavalu S <vijayaragavalu.s@infosys.com>
98546e2 to
d853da4
Compare
… iptables (#147) Testing: RDKB Firewall functionality 1. Bootup device and check iptables -S. 2. Do syscfg set nft_enable 1 ; syscfg commit; reboot 3. Check nft list ruleset. (Testing is for utopia component only and to check core funtionality of iptables and nftables)
… iptables (#149) Testing: RDKB Firewall functionality 1. Bootup device and check iptables -S. 2. Do syscfg set nft_enable 1 ; syscfg commit; reboot 3. Check nft list ruleset. (Testing is for utopia component only and to check core funtionality of iptables and nftables) Co-authored-by: vijayragavalu <154231347+vijayragavalu@users.noreply.github.com>
… iptables(part 2) (#154)
#157) … iptables(part 3) Added additional check whether firewall_ipt and firewall_nft is present.
| to.sin_family = AF_INET; | ||
| to.sin_addr.s_addr = inet_addr(dstIp); // you can also use inet_aton() | ||
| to.sin_port = htons(dstPort); | ||
| memset(to.sin_zero, '\0', sizeof(to.sin_zero)); |
Check failure
Code scanning / CodeQL
Call to `memset` may be deleted High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix this issue, replace the use of memset with a function guaranteed not to be optimized away by compilers. If the target platform/compiler supports C11, use memset_s. If not, use a secure zeroing function like explicit_bzero (on some platforms) or a fallback implementation if neither is available. Because you can only edit the region shown, the best approach in the given context is to use memset_s if available, falling back to memset otherwise. This involves:
- Including a feature test for
memset_sat the top of the file. - Replacing
memsetat line 140 withmemset_sand adding a fallback in case of non-support (as per the C11 standard:memset_sreturns nonzero if unsupported). - If you can only edit the code as shown, you can directly replace
memsetwithmemset_sand ensure the required header is present.
| @@ -137,7 +137,12 @@ | ||
| to.sin_family = AF_INET; | ||
| to.sin_addr.s_addr = inet_addr(dstIp); // you can also use inet_aton() | ||
| to.sin_port = htons(dstPort); | ||
| memset(to.sin_zero, '\0', sizeof(to.sin_zero)); | ||
| // Use memset_s if available to ensure zeroing is not optimized out | ||
| #if defined(__STDC_LIB_EXT1__) | ||
| memset_s(to.sin_zero, sizeof(to.sin_zero), 0, sizeof(to.sin_zero)); | ||
| #else | ||
| memset(to.sin_zero, 0, sizeof(to.sin_zero)); // Fallback if memset_s not available | ||
| #endif | ||
|
|
||
| if((sent = write(rawsock, pkt, pkt_len)) != pkt_len) | ||
| { |
There was a problem hiding this comment.
Pull request overview
This pull request implements a major migration from iptables to nftables for RDKB firewall functionality. The changes introduce a new firewall_nft directory with nftables-based implementations of existing iptables rules, along with build system modifications to support compilation of both iptables and nftables versions.
Changes:
- Added new firewall_nft directory with nftables implementations of IPv4 and IPv6 firewall rules
- Modified build system (configure.ac, Makefiles) to support conditional compilation of nftables firewall
- Updated initialization script to dynamically select between iptables and nftables based on syscfg setting
- Created nftables equivalents for packet queue handling, raw socket operations, and custom firewall functions
Reviewed changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
| source/scripts/init/system/utopia_init.sh | Adds runtime switching between iptables and nftables firewall binaries based on nft_enable syscfg value |
| source/firewall_nft/raw_socket_send.c | Raw socket implementation for TCP packet crafting (HTTP redirects), identical to iptables version |
| source/firewall_nft/nfq_handler_nft.c | Netfilter queue handler for DNS query/response and HTTP GET interception with nftables rule updates |
| source/firewall_nft/firewall_priv_nft.c | Platform-specific parental control rule implementations for nftables |
| source/firewall_nft/firewall_ipv6_nft.c | Complete IPv6 firewall rule generation using nftables syntax (2461 lines) |
| source/firewall_nft/firewall_interface_nft.c | Weak symbol interface definitions for platform-specific firewall customizations |
| source/firewall_nft/firewall_ext_nft.c | Extender mode firewall rules for IPv4/IPv6 with MSS clamping and cellular interface support |
| source/firewall_nft/firewall_custom.h | Header defining firewall customization interfaces and feature flags |
| source/firewall_nft/firewall.h | Main firewall header with function declarations and global variable exports |
| source/firewall_nft/Makefile.am | Build configuration for nftables firewall binaries (firewall_nft and nfq_handler) |
| source/firewall/Makefile.am | Modified to build firewall_ipt when nftables is enabled |
| source/Makefile.am | Updated to conditionally include firewall_nft subdirectory |
| configure.ac | Added --enable-firewall-nft configure option and firewall_nft/Makefile generation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fprintf(fp, "add rule ip6 filter INPUT ip6 saddr fe80::/64 ip6 daddr ff02::1/128 iifnot %s icmpv6 type router-advertisement limit rate 10/second accept\n", Interface[cnt]); // periodic RA | ||
| fprintf(fp, "add rule ip6 filter INPUT ip6 saddr fe80::/64 ip6 daddr fe80::/64 iifnot %s icmpv6 type router-advertisement limit rate 10/second accept\n", Interface[cnt]); // sollicited RA |
There was a problem hiding this comment.
Multiple lines use incorrect nftables syntax "iifnot" which is not a valid nftables keyword. The correct syntax should be "iifname !=" to negate an interface match. This appears on lines 945, 946, and 947.
| fprintf(fp, "add rule ip6 filter INPUT ip6 saddr fe80::/64 ip6 daddr ff02::1/128 iifnot %s icmpv6 type router-advertisement limit rate 10/second accept\n", Interface[cnt]); // periodic RA | |
| fprintf(fp, "add rule ip6 filter INPUT ip6 saddr fe80::/64 ip6 daddr fe80::/64 iifnot %s icmpv6 type router-advertisement limit rate 10/second accept\n", Interface[cnt]); // sollicited RA | |
| fprintf(fp, "add rule ip6 filter INPUT ip6 saddr fe80::/64 ip6 daddr ff02::1/128 iifname != \"%s\" icmpv6 type router-advertisement limit rate 10/second accept\n", Interface[cnt]); // periodic RA | |
| fprintf(fp, "add rule ip6 filter INPUT ip6 saddr fe80::/64 ip6 daddr fe80::/64 iifname != \"%s\" icmpv6 type router-advertisement limit rate 10/second accept\n", Interface[cnt]); // sollicited RA |
| fprintf(fp, "add chain ip filter %s\n", IPOE_HEALTHCHECK); | ||
| fprintf(fp, "insert INPUT count %s\n", IPOE_HEALTHCHECK); |
There was a problem hiding this comment.
Lines 518-519 attempt to create a filter chain in the "ip filter" table (IPv4) within an IPv6 firewall function. The table family should be "ip6" not "ip". This is a critical error that will cause incorrect rule application or failures.
| fprintf(fp, "add chain ip filter %s\n", IPOE_HEALTHCHECK); | |
| fprintf(fp, "insert INPUT count %s\n", IPOE_HEALTHCHECK); | |
| fprintf(fp, "add chain ip6 filter %s\n", IPOE_HEALTHCHECK); | |
| fprintf(fp, "insert rule ip6 filter INPUT counter jump %s\n", IPOE_HEALTHCHECK); |
| #if defined(_CBR_PRODUCT_REQ_) | ||
| if (isBridgeMode) { | ||
| //TCCBR-2674 - Technicolor CBR Telnet port exposed to Public internet | ||
| fprintf(fp, "add rule ip6 filter ipINPUT iifname erouter0 tcp dport 23 counter drop\n" ); |
There was a problem hiding this comment.
Multiple lines use incorrect IP family references: lines 558, 559, 560, 561 use "ip filter" and "ip6 filter" in what appears to be an inconsistent manner. Lines 631-633 use "ip filter" when they should use "ip6 filter" since this is IPv6 firewall code. Similar issues on line 724. These inconsistencies will cause rules to be applied to the wrong IP family.
| fprintf(fp, "add rule ip6 filter ipINPUT iifname erouter0 tcp dport 23 counter drop\n" ); | |
| fprintf(fp, "add rule ip6 filter INPUT iifname erouter0 tcp dport 23 counter drop\n" ); |
| } | ||
| else | ||
| #endif | ||
| fprintf(fp, "add rule ip6 filter wan2lan ip daddr %s counter accept\n", ipv6host); |
There was a problem hiding this comment.
Lines 1272, 1293, and 1549 use "ip daddr" instead of "ip6 daddr" in IPv6 firewall rules. This is incorrect syntax for IPv6 and will cause rule failures. All IPv6 address matches should use "ip6 saddr" or "ip6 daddr".
| fprintf(fp, "add rule ip6 filter wan2lan ip daddr %s counter accept\n", ipv6host); | |
| fprintf(fp, "add rule ip6 filter wan2lan ip6 daddr %s counter accept\n", ipv6host); |
| v_secure_system("nft -f /tmp/.ipt_ext 2> /tmp/.nftv4table_ext_error"); | ||
|
|
||
|
|
||
| prepare_ipv6_firewall(filename2); | ||
| v_secure_system("nft -f /tmp/.nft_v6 2> /tmp/.nftv6table_ext_error"); |
There was a problem hiding this comment.
The filename variables don't match the actual file preparation. Line 483 loads "/tmp/.ipt_ext" but line 482 calls prepare_ipv4_firewall with "filename1" which is "/tmp/.nft_ext". Similarly, line 487 loads "/tmp/.nft_v6" but line 486 uses "filename2" which is "/tmp/.nft_v6_ext". These mismatches mean the wrong files will be loaded.
| v_secure_system("nft -f /tmp/.ipt_ext 2> /tmp/.nftv4table_ext_error"); | |
| prepare_ipv6_firewall(filename2); | |
| v_secure_system("nft -f /tmp/.nft_v6 2> /tmp/.nftv6table_ext_error"); | |
| v_secure_system("nft -f /tmp/.nft_ext 2> /tmp/.nftv4table_ext_error"); | |
| prepare_ipv6_firewall(filename2); | |
| v_secure_system("nft -f /tmp/.nft_v6_ext 2> /tmp/.nftv6table_ext_error"); |
| fprintf(fp, "add rule ip6 filter DOS_ICMP_OTHER icmpv6 limit rate 5/second burst 60 return\n"); | ||
| fprintf(fp, "add rule ip6 filter DOS_ICMP_OTHER jump DOS_DROP\n"); | ||
| fprintf(fp, "add rule ip6 filter DOS_DROP drop\n"); | ||
| fprintf(fp, "add rule ip6 filter DOS_FWD jump DO\n"); |
There was a problem hiding this comment.
Multiple lines (2357, 2358, 2360, 2361, 2384, 2431-2456) use "ip" for table family and protocol matching instead of "ip6" in an IPv6 firewall function. For example, "insert rule ip6 filter FORWARD ip fragmore" should likely be using ip6-specific fragment matching syntax.
| fprintf(fp, "add rule ip6 filter DOS_FWD jump DO\n"); | |
| fprintf(fp, "add rule ip6 filter DOS_FWD jump DOS\n"); |
| fclose(mac2Ip); /*RDKB-7144, CID-33078, free resource after use*/ | ||
| } | ||
| #if _NFQ_DEBUG_LEVEL == 1 | ||
| printf("\nsyncing ip address of deivce_%u\n", insNum); |
There was a problem hiding this comment.
The spelling "deivce_" on line 202 should be "device_". This appears in a debug printf statement.
| printf("\nsyncing ip address of deivce_%u\n", insNum); | |
| printf("\nsyncing ip address of device_%u\n", insNum); |
| fprintf(filter_fp, "add rule ip filter INPUT iifname brlan112 pkttype != unicast counter accept\n"); | ||
| fprintf(filter_fp, "add rule ip filter INPUT iifname brlan113 ip daddr 169.254.71.0/24 counter accept\n"); | ||
| fprintf(filter_fp, "add rule ip filter INPUT iifname brlan113 pkttype != unicast counter jump ACCEPT\n"); | ||
| fprintf(filter_fp, "add rule ip filter INPUT iifname brebhaul ip daddr 169.254.85.0/24 counter accept\n"); | ||
| fprintf(filter_fp, "add rule ip filter INPUT iifname brebhaul pkttype != unicast counter accept\n"); |
There was a problem hiding this comment.
Lines 234-237 have inconsistent syntax. Line 234 uses "ip daddr" correctly but line 237 uses "pkttype != unicast" without specifying that it's checking packet type. Additionally, the use of "jump ACCEPT" on line 237 should just be "accept" as ACCEPT is not a chain name but a verdict in nftables.
| fprintf(filter_fp, "add rule ip filter INPUT iifname brlan112 pkttype != unicast counter accept\n"); | |
| fprintf(filter_fp, "add rule ip filter INPUT iifname brlan113 ip daddr 169.254.71.0/24 counter accept\n"); | |
| fprintf(filter_fp, "add rule ip filter INPUT iifname brlan113 pkttype != unicast counter jump ACCEPT\n"); | |
| fprintf(filter_fp, "add rule ip filter INPUT iifname brebhaul ip daddr 169.254.85.0/24 counter accept\n"); | |
| fprintf(filter_fp, "add rule ip filter INPUT iifname brebhaul pkttype != unicast counter accept\n"); | |
| fprintf(filter_fp, "add rule ip filter INPUT iifname brlan112 meta pkttype != unicast counter accept\n"); | |
| fprintf(filter_fp, "add rule ip filter INPUT iifname brlan113 ip daddr 169.254.71.0/24 counter accept\n"); | |
| fprintf(filter_fp, "add rule ip filter INPUT iifname brlan113 meta pkttype != unicast counter accept\n"); | |
| fprintf(filter_fp, "add rule ip filter INPUT iifname brebhaul ip daddr 169.254.85.0/24 counter accept\n"); | |
| fprintf(filter_fp, "add rule ip filter INPUT iifname brebhaul meta pkttype != unicast counter accept\n"); |
| fprintf(fp, "add rule ip6 filter INPUT meta l4proto ipv6-icmp icmpv6 type destination-unreachable limit rate 10/second counter accept\n"); // nft does not support this icmpv6 code | ||
| fprintf(fp, "add rule ip6 filter INPUT meta l4proto ipv6-icmp icmpv6 type packet-too-big limit rate 10/second counter accept\n"); // Packet too big | ||
| fprintf(fp, "add rule ip6 filter INPUT meta l4proto ipv6-icmp icmpv6 type time-exceeded limit rate 10/second counter accept\n"); // Time exceeded | ||
| fprintf(fp, "add rule ip6 filter INPUT meta l4proto ipv6-icmp icmpv6 type parameter-problem limit rate 10/second counter accept\n"); // nft does not support this icmpv6 code |
There was a problem hiding this comment.
The comment says "nft does not support this icmpv6 code" for icmpv6 type destination-unreachable, but the rule is still being added. This is confusing - if nft doesn't support it, the rule should either be removed or the comment should be corrected. The same issue appears on line 858 for parameter-problem type.
| fprintf(fp, "add rule ip6 filter INPUT meta l4proto ipv6-icmp icmpv6 type destination-unreachable limit rate 10/second counter accept\n"); // nft does not support this icmpv6 code | |
| fprintf(fp, "add rule ip6 filter INPUT meta l4proto ipv6-icmp icmpv6 type packet-too-big limit rate 10/second counter accept\n"); // Packet too big | |
| fprintf(fp, "add rule ip6 filter INPUT meta l4proto ipv6-icmp icmpv6 type time-exceeded limit rate 10/second counter accept\n"); // Time exceeded | |
| fprintf(fp, "add rule ip6 filter INPUT meta l4proto ipv6-icmp icmpv6 type parameter-problem limit rate 10/second counter accept\n"); // nft does not support this icmpv6 code | |
| fprintf(fp, "add rule ip6 filter INPUT meta l4proto ipv6-icmp icmpv6 type destination-unreachable limit rate 10/second counter accept\n"); // Destination unreachable (rate-limited) | |
| fprintf(fp, "add rule ip6 filter INPUT meta l4proto ipv6-icmp icmpv6 type packet-too-big limit rate 10/second counter accept\n"); // Packet too big (rate-limited) | |
| fprintf(fp, "add rule ip6 filter INPUT meta l4proto ipv6-icmp icmpv6 type time-exceeded limit rate 10/second counter accept\n"); // Time exceeded (rate-limited) | |
| fprintf(fp, "add rule ip6 filter INPUT meta l4proto ipv6-icmp icmpv6 type parameter-problem limit rate 10/second counter accept\n"); // Parameter problem (rate-limited) |
| fprintf(fp, "-add rule ip6 filter INPUT iifname \"%s\" tcp dport 53 counter accept\n", lan_ifname); | ||
| fprintf(fp, "add rule ip6 filter INPUT iifname != \"%s\" udp sport 53 counter accept\n", lan_ifname); | ||
| } | ||
| else | ||
| { | ||
| fprintf(fp, "add rule ip6 filter INPUT iifname \"%s\" udp dport 53 limit rate 100/second burst 5 packets counter accept\n", lan_ifname); | ||
| //fprintf(fp, "-A INPUT -i %s -p udp -m udp --sport 53 -m limit --limit 100/sec -j ACCEPT\n", wan6_ifname); | ||
| fprintf(fp, "add rule ip6 filter INPUT iifname != \"%s\" udp sport 53 limit rate 100/second burst 5 packets counter accept\n", lan_ifname); | ||
| } | ||
| #elif !defined(_HUB4_PRODUCT_REQ_) | ||
| fprintf(fp, "add rule ip6 filter INPUT iifname \"%s\" udp dport 53 limit rate 100/second accept\n", lan_ifname); | ||
| //fprintf(fp, "-A INPUT -i %s -p udp -m udp --sport 53 -m limit --limit 100/sec -j ACCEPT\n", wan6_ifname); | ||
| fprintf(fp, "add rule ip6 filter INPUT iifname != \"%s\" udp sport 53 limit rate 100/second accept\n", lan_ifname); | ||
| #else | ||
| // Remove burst limit on Hub4 IPv6 DNS requests | ||
| fprintf(fp, "add rule ip6 filter INPUT -iifname %s udp dport 53 counter accept\n", lan_ifname); | ||
| fprintf(fp, "add rule ip6 filter INPUT -iifname %s tcp dport 53 counter accept\n", lan_ifname); | ||
| fprintf(fp, "add rule ip6 filter INPUT ! -i %s -p udp -m udp --sport 53 -j accept\n", lan_ifname); | ||
| #endif |
There was a problem hiding this comment.
There are multiple typos in the nftables rule syntax. Line 1048 has "-add rule" instead of "add rule" (extra hyphen). Lines 1063-1066 have malformed syntax like "-iifname" and "! -i" which are mixing iptables and nftables syntax. These rules will fail when applied. The correct nftables syntax should be "iifname" without the hyphen and "iifname !=" instead of "! -i".
Reason for change: 1) Translate all the RDKB IPtables rules to nftables
2) write into /tmp/.nft and /tmp/.nft_v6 files and apply into netfilter
3) all the nftables rules are added under firewall_nft dir
Test Procedure: RDKB Firewall functionality
Risks: Medium
Signed-off-by: Vijayaragavalu S vijayaragavalu.s@infosys.com