RDKBNETWOR-80 : Transform to Nftables from Iptables - #292
Open
vsai1990 wants to merge 11 commits into
Open
Conversation
Contributor
Contributor
|
b'## WARNING: A Blackduck scan failure has been waived A prior failure has been upvoted
|
snayak002c
requested changes
May 11, 2026
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
snayak002c
previously approved these changes
Jun 22, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an nftables-based firewall implementation alongside the existing iptables-based firewall, with build-time support (--enable-firewall-nft) and runtime selection via syscfg nft_enable.
Changes:
- Adds a new
source/firewall_nft/implementation (firewall + nfqueue handler + support headers) intended to generate/apply nftables rules. - Updates build system (autotools + Makefile conditionals) to optionally build the nftables firewall and to build the legacy firewall binary as
firewall_ipt. - Updates runtime launcher script and a utapi port-forwarding path to conditionally use nft vs iptables.
Reviewed changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| source/utapi/lib/utapi.c | Adds runtime switch to attempt nft rules for ephemeral port forwarding. |
| source/utapi/lib/Makefile.am | Adds -DNFT_ENABLE when FIREWALL_NFT is enabled. |
| source/scripts/init/service.d/service_firewall/firewall_log_handle.sh | Switches between legacy and nft firewall binaries based on syscfg nft_enable. |
| source/Makefile.am | Adds firewall_nft subdir when FIREWALL_NFT is enabled. |
| source/firewall/Makefile.am | Builds legacy firewall as firewall_ipt under FIREWALL_NFT. |
| source/firewall_nft/raw_socket_send.c | Adds raw packet send helper (copied from legacy). |
| source/firewall_nft/nfq_handler_nft.c | Adds nft-oriented nfqueue handler implementation. |
| source/firewall_nft/Makefile.am | Builds firewall_nft and an nfqueue handler binary. |
| source/firewall_nft/firewallnft.h | Adds nft firewall header/API surface. |
| source/firewall_nft/firewall_priv_nft.c | Adds nft versions of custom rule helpers. |
| source/firewall_nft/firewall_ipv6_nft.c | Adds nft IPv6 firewall rule generation. |
| source/firewall_nft/firewall_interface_nft.c | Adds weak stubs for platform hooks in nft firewall. |
| source/firewall_nft/firewall_ext_nft.c | Adds extender-mode nft firewall logic. |
| source/firewall_nft/firewall_custom.h | Adds nft firewall custom header and shared declarations/macros. |
| configure.ac | Adds --enable-firewall-nft configure option and generates source/firewall_nft/Makefile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+398
to
+401
| /* These rules are needed to accept IPv6 traffic with HBH extension header and No-Next-Header option */ | ||
| /* To enable ipv6header module support need to set CONFIG_IP6_NF_MATCH_IPV6HEADER=m kernel config */ | ||
| fprintf(filter_fp,"-I FORWARD 1 -o erouter0 -m ipv6header --soft --header hop-by-hop -j ACCEPT\n"); | ||
| fprintf(filter_fp,"-I FORWARD 1 -o erouter0 -m ipv6header --soft --header hop-by-hop -j LOG --log-prefix \"UTOPIA: FW.IPv6 FORWARD Hop-by-Hop\" --log-level 6\n"); |
snayak002c
previously approved these changes
Jul 2, 2026
|
All contributors have signed the CLA ✍️ ✅ |
Resolve nftable command errors and flags.
|
📋 PR Format Reminder
Expected: |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 16 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (8)
source/utapi/lib/utapi.c:7662
- In the TCP port-forwarding path, the
if (isNatReady)block opened above is never closed before the postrouting SNAT rule, which changes the original control flow (SNAT and subsequent rules become nested underisNatReady). Add the missing closing brace right after the#endiffor the DNAT rule.
v_secure_system("iptables -t nat -%c prerouting_fromlan -p tcp -m tcp -d %s --dport %s -s %s -j DNAT --to-destination %s%s",
ciptableOprationCode,natip4, external_dest_port, external_ip, toip, port_modifier);
#endif
#ifdef NFT_ENABLE
source/utapi/lib/utapi.c:7680
- The TCP branch never closes the
if (!isNatRedirectionBlocked)scope before adding the router-modewan2lan_forwarding_acceptrule, which makes the forward-accept rule conditional on NAT redirection state. Close the outerifblock before the router-mode section.
#endif
}
/* it will applicable during router mode */
source/utapi/lib/utapi.c:7765
- In the UDP port-forwarding path, the inner
if (isNatReady)block is not closed before the SNAT rule, changing the intended behavior. Add the missing closing brace after the DNAT#endif.
v_secure_system("iptables -t nat -%c prerouting_fromlan -p udp -m udp -d %s --dport %s -s %s -j DNAT --to-destination %s%s",
ciptableOprationCode,natip4, external_dest_port, external_ip, toip, port_modifier);
#endif
#ifdef NFT_ENABLE
source/utapi/lib/utapi.c:7781
- The UDP branch doesn’t close the
if (!isNatRedirectionBlocked)scope before the router-modewan2lan_forwarding_acceptrule, which makes the forward-accept rule conditional on NAT-redirection settings. Close the outerifblock before the router-mode section.
v_secure_system("iptables -t nat -%c postrouting_tolan -s %s.0/%s -p udp -m udp -d %s --dport %s -j SNAT --to-source %s",
ciptableOprationCode,lan_3_octets, lan_netmask, toip, dport, lan_ipaddr);
#endif
}
source/scripts/init/service.d/service_firewall/firewall_log_handle.sh:50
- When
nft_enable=1and/usr/bin/firewall_nftis missing/not executable, the script currently skips firewall setup entirely. Add a fallback to the legacy firewall binary so devices don’t boot without any firewall rules applied.
else
if [ -x /usr/bin/firewall_nft ];then
/usr/bin/firewall_nft "$@"
fi
fi
configure.ac:58
- The
--enable-firewall-nfthelp text says the value istrue/false, but the parser only acceptsyes/noand errors ontrue. Accept bothyes/noandtrue/false(and keep the error for other values) to match the documented interface.
AC_ARG_ENABLE([firewall-nft],
[ --enable-firewall-nft=val Turn on nft Feature, val=true or false],
[case "${enableval}" in
yes) firewall_nft=true ;;
no) firewall_nft=false ;;
source/firewall_nft/firewallnft.h:135
do_ipv6_filter_table()is declared twice in this header, which is redundant and can mask future signature mismatches. Keep only one declaration.
void do_ipv6_sn_filter(FILE *fp);
void do_ipv6_nat_table(FILE *fp);
void do_ipv6_filter_table(FILE *fp);
void do_ipv6_UIoverWAN_filter(FILE* fp);
void do_ipv6_filter_table(FILE *fp);
source/firewall/Makefile.am:32
- When
FIREWALL_NFTis enabled, bothsource/firewallandsource/firewall_nftbuild/install anfq_handlerbinary. This creates an install-time collision and can result in the iptables-oriented handler overwriting the nft-oriented one (or vice versa). In nft mode, only the nft variant should be installed.
if FIREWALL_NFT
bin_PROGRAMS = firewall_ipt nfq_handler
else
bin_PROGRAMS = firewall nfq_handler
endif
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 16 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (5)
source/scripts/init/service.d/service_firewall/firewall_log_handle.sh:50
- When nft_enable=="1" and /usr/bin/firewall_nft is missing/not executable, the script silently skips starting any firewall binary (it just continues). This can leave the system without firewall rules applied. Add a fallback to firewall_ipt and/or firewall for this branch too.
else
if [ -x /usr/bin/firewall_nft ];then
/usr/bin/firewall_nft "$@"
fi
fi
source/firewall/Makefile.am:32
- With FIREWALL_NFT enabled, both source/firewall and source/firewall_nft build/install a binary named "nfq_handler" (see source/firewall_nft/Makefile.am:23). This creates an install-time collision where one nfq_handler will overwrite the other, making which implementation runs nondeterministic.
if FIREWALL_NFT
bin_PROGRAMS = firewall_ipt nfq_handler
else
bin_PROGRAMS = firewall nfq_handler
endif
configure.ac:61
- The --enable-firewall-nft help text says the value is "true or false", but the parser only accepts "yes" or "no". This is confusing for users/configure scripts and can lead to misconfiguration.
#Enable NFT compile time flags if firewall-nft is set as yes
AC_ARG_ENABLE([firewall-nft],
[ --enable-firewall-nft=val Turn on nft Feature, val=true or false],
[case "${enableval}" in
yes) firewall_nft=true ;;
no) firewall_nft=false ;;
*) AC_MSG_ERROR([bad value ${enableval} for --enable-firewall-nft]) ;;
esac],[firewall_nft=false])
AM_CONDITIONAL(FIREWALL_NFT, test x"$firewall_nft" = x"true")
source/utapi/lib/utapi.c:7678
- The newly added NFT_ENABLE blocks appear to have disturbed the brace structure in the TCP nat-redirection path: the SNAT (postrouting_tolan) section is now nested under the inner "if ( isNatReady )" and the closing braces for the surrounding "if (0 == strcmp("none", fromip))" / "if (!isNatRedirectionBlocked)" blocks look inconsistent. This likely changes behavior vs the previous logic and may even result in unbalanced blocks depending on build flags.
if ( isNatReady )
{
#ifdef NFT_ENABLE
if (atoi(nft_enable) == 0)
{
v_secure_system("iptables -t nat -%c prerouting_fromlan -p tcp -m tcp -d %s --dport %s -s %s -j DNAT --to-destination %s%s",
ciptableOprationCode,natip4, external_dest_port, external_ip, toip, port_modifier);
}
else
{
v_secure_system("nft %s rule ip nat prerouting_fromlan ip saddr %s ip daddr %s tcp dport %s counter dnat to %s%s",
(ciptableOprationCode == 'A') ? "add" : "delete", external_ip, natip4, external_dest_port, toip, port_modifier);
}
#else
v_secure_system("iptables -t nat -%c prerouting_fromlan -p tcp -m tcp -d %s --dport %s -s %s -j DNAT --to-destination %s%s",
ciptableOprationCode,natip4, external_dest_port, external_ip, toip, port_modifier);
#endif
#ifdef NFT_ENABLE
if (atoi(nft_enable) == 0)
{
v_secure_system("iptables -t nat -%c postrouting_tolan -s %s.0/%s -p tcp -m tcp -d %s --dport %s -j SNAT --to-source %s",
ciptableOprationCode,lan_3_octets, lan_netmask, toip, dport, lan_ipaddr);
}
else
{
v_secure_system("nft %s rule ip nat postrouting_tolan ip saddr %s.0/%s ip daddr %s tcp dport %s counter snat to %s",
(ciptableOprationCode == 'A') ? "add" : "delete", lan_3_octets, lan_netmask, toip, dport, lan_ipaddr);
}
#else
v_secure_system("iptables -t nat -%c postrouting_tolan -s %s.0/%s -p tcp -m tcp -d %s --dport %s -j SNAT --to-source %s",
ciptableOprationCode,lan_3_octets, lan_netmask, toip, dport, lan_ipaddr);
#endif
}
source/utapi/lib/utapi.c:7782
- Same structural issue in the UDP nat-redirection path: the SNAT (postrouting_tolan) block is now nested under the inner "if ( isNatReady )" and the surrounding condition blocks' closing braces appear inconsistent after adding NFT_ENABLE branches. This can alter rule installation behavior compared to the original iptables-only flow.
if ( isNatReady )
{
#ifdef NFT_ENABLE
if (atoi(nft_enable) == 0)
{
v_secure_system("iptables -t nat -%c prerouting_fromlan -p udp -m udp -d %s --dport %s -s %s -j DNAT --to-destination %s%s",
ciptableOprationCode,natip4, external_dest_port, external_ip, toip, port_modifier);
}
else
{
v_secure_system("nft %s rule ip nat prerouting_fromlan ip saddr %s ip daddr %s udp dport %s counter dnat to %s%s",
(ciptableOprationCode == 'A') ? "add" : "delete", external_ip, natip4, external_dest_port, toip, port_modifier);
}
#else
v_secure_system("iptables -t nat -%c prerouting_fromlan -p udp -m udp -d %s --dport %s -s %s -j DNAT --to-destination %s%s",
ciptableOprationCode,natip4, external_dest_port, external_ip, toip, port_modifier);
#endif
#ifdef NFT_ENABLE
if (atoi(nft_enable) == 0)
{
v_secure_system("iptables -t nat -%c postrouting_tolan -s %s.0/%s -p udp -m udp -d %s --dport %s -j SNAT --to-source %s",
ciptableOprationCode,lan_3_octets, lan_netmask, toip, dport, lan_ipaddr);
}
else
{
v_secure_system("nft %s rule ip nat postrouting_tolan ip saddr %s.0/%s ip daddr %s udp dport %s counter snat to %s",
(ciptableOprationCode == 'A') ? "add" : "delete", lan_3_octets, lan_netmask, toip, dport, lan_ipaddr);
}
#else
v_secure_system("iptables -t nat -%c postrouting_tolan -s %s.0/%s -p udp -m udp -d %s --dport %s -j SNAT --to-source %s",
ciptableOprationCode,lan_3_octets, lan_netmask, toip, dport, lan_ipaddr);
#endif
}
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 16 changed files in this pull request and generated 3 comments.
Suppressed comments (3)
source/utapi/lib/utapi.c:7785
- Same brace imbalance exists in the UDP branch:
if (!isNatRedirectionBlocked)andif (0 == strcmp("none", fromip))are opened but not closed before the router-mode forwarding rule block, which can break compilation or unintentionally gate forwarding rules behind NAT-redirection conditions.
}
/* it will applicable during router mode */
if( 0 == isBridgeMode )
{
source/firewall_nft/Makefile.am:31
- With
FIREWALL_NFTenabled, the build includes bothsource/firewallandsource/firewall_nft, and both Makefiles declare anfq_handlerinbin_PROGRAMS. This creates an install/build name collision fornfq_handler(two different implementations produce the same binary name).
bin_PROGRAMS = firewall_nft nfq_handler
firewall_nft_SOURCES = firewall_nft.c firewall_ipv6_nft.c firewall_priv_nft.c firewall_interface_nft.c firewall_ext_nft.c
if CPC_FIREWALL_ENABLE
firewall_nft_SOURCES += firewall_lib.c firewall_dsl.c rabid.c
AM_LDFLAGS += -lrdkconfig
endif
nfq_handler_SOURCES = raw_socket_send.c nfq_handler_nft.c
firewall_nft_LDADD = $(top_builddir)/source/syscfg/lib/libsyscfg.la \
configure.ac:61
--enable-firewall-nfthelp text says the value istrue/false, but the parser only acceptsyes/no(and errors otherwise). This makes the documented invocation fail.
#Enable NFT compile time flags if firewall-nft is set as yes
AC_ARG_ENABLE([firewall-nft],
[ --enable-firewall-nft=val Turn on nft Feature, val=true or false],
[case "${enableval}" in
yes) firewall_nft=true ;;
no) firewall_nft=false ;;
*) AC_MSG_ERROR([bad value ${enableval} for --enable-firewall-nft]) ;;
esac],[firewall_nft=false])
AM_CONDITIONAL(FIREWALL_NFT, test x"$firewall_nft" = x"true")
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 16 changed files in this pull request and generated 1 comment.
Suppressed comments (7)
source/firewall/Makefile.am:32
- With
FIREWALL_NFTenabled, bothsource/firewallandsource/firewall_nftbuild and install a binary namednfq_handler, which will cause an install-time collision. Only onenfq_handlershould be built/installed for the nft build.
if FIREWALL_NFT
bin_PROGRAMS = firewall_ipt nfq_handler
else
bin_PROGRAMS = firewall nfq_handler
endif
source/utapi/lib/utapi.c:7729
if (isNatReady)is never closed after adding the UDP prerouting_fromwan rule, so the NAT redirection and router-mode forwarding rules become unintentionally scoped underisNatReady. This changes behavior vs the TCP path and can skip LAN-side DNAT/SNAT and filter rules when WAN is not ready.
#endif
if ( !isNatRedirectionBlocked )
{
configure.ac:60
- The
--enable-firewall-nfthelp text says the value istrue/false, but the implementation only acceptsyes/no. This is confusing for users of./configure. Either accepttrue|falseas aliases or update the help string to match.
#Enable NFT compile time flags if firewall-nft is set as yes
AC_ARG_ENABLE([firewall-nft],
[ --enable-firewall-nft=val Turn on nft Feature, val=true or false],
[case "${enableval}" in
yes) firewall_nft=true ;;
no) firewall_nft=false ;;
*) AC_MSG_ERROR([bad value ${enableval} for --enable-firewall-nft]) ;;
esac],[firewall_nft=false])
source/firewall_nft/nfq_handler_nft.c:490
main()readsargv[1]to decide IPv4 vs IPv6 without validatingargc, which can segfault if the program is started without arguments.
int main(int argc, char *argv[])
{
struct nfq_handle *nfqHandle;
struct nfq_q_handle *queueHandle;
int fd, rv;
source/firewall_nft/nfq_handler_nft.c:519
- The error message has a typo (“maxium”) and doesn’t explain what the maximum allowed length is, which makes debugging misconfiguration harder.
if (strlen(argv[2]) >= sizeof(srcMac))
{
fprintf(stderr, "nfq_handler: maxium length of srcMac %s\n", __FUNCTION__);
exit(1);
}
source/firewall_nft/raw_socket_send.c:248
- In the IPv4 branch of
CreateIPHeader(),malloc()return is not checked and the allocated header is not zero-initialized before fields are written. This can lead to NULL dereference on allocation failure and leaves unspecified fields uninitialized.
struct iphdr *ip_header;
ip_header = (struct iphdr *)malloc(sizeof(struct iphdr));
ip_header->version = 4;
source/firewall_nft/raw_socket_send.c:289
CreateTcpHeader()does not checkmalloc()and does not zero-initializestruct tcphdr, so TCP flag/bitfield members that aren’t explicitly set may contain garbage. This can produce malformed packets.
static struct tcphdr *CreateTcpHeader(int family, unsigned short sport, unsigned short dport, unsigned long seqNum, unsigned long ackNum, unsigned char fin)
{
struct tcphdr *tcp_header;
/* Check /usr/include/linux/tcp.h for header definiation */
tcp_header = (struct tcphdr *)malloc(sizeof(struct tcphdr));
tcp_header->source = htons(sport);
tcp_header->dest = htons(dport);
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reason for change:
Test Procedure: RDKB Firewall functionality
Risks: Medium