Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add iptlite packet filter modules #7541

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

P1F
Copy link

@P1F P1F commented Nov 7, 2022

Summary

This merge request aims to add a lightweight packet filter to NuttX, called iptlite (iptables lite), which was based on Linux firewall, iptables and netfilter. This first implementation was focused on the essential commands, such as adding a drop rule based on the 4-tuple (source IPv4 address, destination IPv4 address, source port and destination port), flush all rules and list all rules, for all ingress TCP packets.

The implementation was divided in two parts: the iptlite app, the CLI to the user, and the nflite modules (netfilter lite), which will provide the APIs to the iptlite app, that can be seen in another MR on the incubator-nuttx repository.

This project was considered the third-best security tool in the XXII Brazilian Symposium on Information Security and Computer Systems, and the related paper was accepted by this conference as well.

Impact

This lightweight packet filter could be an additional security feature, especially in the IoT environment, allowing the users to adopt, for instance, a zero trust policy, consequently, denying all ingress packet filter, except by the preset ones.

Testing

In order to give more context about the implementation that it was made, this following link will show a quick video demo of the project.

@P1F P1F changed the title Add iptlite packet filter module Add iptlite packet filter modules Nov 7, 2022
Comment on lines +39 to +40
chain *chain_head;
chain *last_rule;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Include "g_" before the variables names to indicate they are global variables.

in_addr_t destipaddr;
in_port_t srcport;
in_port_t destport;
chain *next;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Include "FAR chain *net" to be compatible with 8-bit MCU that requires it

in_port_t srcport, in_port_t destport)
{
chain *new_chainrule = (chain *)malloc(sizeof(chain));
if (new_chainrule == NULL) return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include:
{
}

Comment on lines +139 to +146
if ((current_rule->destipaddr == 0 \
|| current_rule->destipaddr == destipaddr) \
&& (current_rule->srcipaddr == 0 \
|| current_rule->srcipaddr == srcipaddr) \
&& (current_rule->destport == 0 \
|| current_rule->destport == destport) \
&& (current_rule->srcport == 0 \
|| current_rule->srcport == srcport)) return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto! ( { } )

chain *head = chain_head->next;
char **table = (char **)malloc(rules_counter * sizeof(char *));

for (int i = 0; i < rules_counter; i++)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move "int i" to top of the function to be C89 compatible

#ifdef CONFIG_NETUTILS_IPTLITE
/* Check if packet needs to be dropped */

bool is_valid_packet = nflite_verify_ipv4(dev);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Declare this variable at the top

/* Check if packet needs to be dropped */

bool is_valid_packet = nflite_verify_ipv4(dev);
if (!is_valid_packet) goto drop;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto


void nflite_initialize(void)
{
chain_head = (chain *)malloc(sizeof(chain));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Malloc could fail, check the return and move the function from void to int to return an error

@yamt
Copy link
Contributor

yamt commented Nov 9, 2022

Summary

This merge request aims to add a lightweight packet filter to NuttX, called iptlite (iptables lite), which was based on Linux firewall, iptables and netfilter. This first implementation was focused on the essential commands, such as adding a drop rule based on the 4-tuple (source IPv4 address, destination IPv4 address, source port and destination port), flush all rules and list all rules, for all ingress TCP packets.

The implementation was divided in two parts: the iptlite app, the CLI to the user, and the nflite modules (netfilter lite), which will provide the APIs to the iptlite app, that can be seen in another MR on the incubator-nuttx repository.

what's the exact relationship between these code and https://www.netfilter.org/ ?
is this code under GPL?

@xiaoxiang781216
Copy link
Contributor

@P1F @wengzhe has developed an infrastructure for iptable, you may port filter functionality less effort and more standardized now. Please reference the follow PR to learn the usage: apache/nuttx-apps#1479 and #7989.

@acassis
Copy link
Contributor

acassis commented Jun 15, 2023

@P1F @wengzhe has developed an infrastructure for iptable, you may port filter functionality less effort and more standardized now. Please reference the follow PR to learn the usage: apache/nuttx-apps#1479 and #7989.

@P1F @duduita please take a look on this infrastructure for iptable from @wengzhe when adapting the code

@P1F
Copy link
Author

P1F commented Jun 15, 2023

@P1F @wengzhe has developed an infrastructure for iptable, you may port filter functionality less effort and more standardized now. Please reference the follow PR to learn the usage: apache/nuttx-apps#1479 and #7989.

@P1F @duduita please take a look on this infrastructure for iptable from @wengzhe when adapting the code

Hey Alan! We'll take a look. Thank you for referencing the PR.

@xiaotailang
Copy link

@P1F @yamt @xiaoxiang781216 @acassis @jkivilin Hello everyone, if I want to support a firewall in the NuttX system, what should I do? Are there any corresponding development references or references for porting third-party software? I found this issue. Could you please confirm whether this merge request, once added, will be able to implement common firewall functionalities?

@xiaoxiang781216
Copy link
Contributor

@xiaotailang this patch expose functions to userspace directly which is forbidden, so the code need be modified to go through ioctl interface. apache/nuttx-apps#1479 and #7989 can help this.

@duduita
Copy link
Contributor

duduita commented Aug 23, 2023

@P1F and I were thinking about continuing to work on our packet filter contribution, but these PRs apache/nuttx-apps#1479, #7989 raised some questions @xiaoxiang781216.

It seems that the mentioned PRs implemented more sophisticated tables, and structures to be used in a packet filter, including the iptables, but the packet filter itself, in other words, the netfilter linux equivalent wasn't implemented, am I right @xiaoxiang781216?

So, if I understood well, if we decide to resume our contribution work, we wouldn't need the apache/nuttx-apps#1399, since the iptables seem to be already implemented in this one apache/nuttx-apps#1479, but we would still need to add the netfilter, something like this function that we have in this PR, but using the new structures, right?

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Aug 23, 2023

@P1F and I were thinking about continuing to work on our packet filter contribution, but these PRs apache/nuttx-apps#1479, #7989 raised some questions @xiaoxiang781216.

It seems that the mentioned PRs implemented more sophisticated tables, and structures to be used in a packet filter, including the iptables, but the packet filter itself, in other words, the netfilter linux equivalent wasn't implemented, am I right @xiaoxiang781216?

Yes, you are right. iptable is a complete framework, #7989 just implement the core framework and NAT functionality.

So, if I understood well, if we decide to resume our contribution work, we wouldn't need the apache/nuttx-apps#1399, since the iptables seem to be already implemented in this one apache/nuttx-apps#1479,

Yes, you can reuse the tool in userspace.

but we would still need to add the netfilter, something like this function that we have in this PR, but using the new structures, right?

Yes, the rest work is integrate your work to the new iptable framework and expose the new ipfilter through ioctl/struct which should be compatible with Linux.

@xiaotailang
Copy link

ok ! thank you very much.

@duduita
Copy link
Contributor

duduita commented Oct 22, 2023

@P1F and I are working on the suggested changes to merge this PR, but we have some questions @xiaoxiang781216 @acassis @wengzhe:

  1. In our implementation, we created our own functions, like nflite_addrule, nflite_listall, nflite_flushall, etc... to add/list/delete the rules in the table. However, after taking a look at the iptables NAT implementation, and your comments, it seems that we should use the ioctl or ioctl-like operations to do it, instead of using our own functions, am I right? I'm asking since we thought that we could just create some syscalls, to use our own functions, but I believe that we should use the ioctl/ioctl-like operations instead.

  2. I thought that we wouldn't have many things to do before merging the packet filter to NuttX, but considering what I said above, it seems that we'll have to basically rewrite all our functions to use ioctl-like operations, such as setsockopt, getsockopt, etc... Am I right, or maybe I'm seeing the things more complicated than they really are?

@xiaoxiang781216
Copy link
Contributor

@P1F and I are working on the suggested changes to merge this PR, but we have some questions @xiaoxiang781216 @acassis @wengzhe:

  1. In our implementation, we created our own functions, like nflite_addrule, nflite_listall, nflite_flushall, etc... to add/list/delete the rules in the table. However, after taking a look at the iptables NAT implementation, and your comments, it seems that we should use the ioctl or ioctl-like operations to do it, instead of using our own functions, am I right? I'm asking since we thought that we could just create some syscalls, to use our own functions, but I believe that we should use the ioctl/ioctl-like operations instead.

Normally, only the core APIs are exposed through syscall, the optional APIs are exposed through ioctl. But the key point is that the new interface is better to follow:

  1. The standard, e.g., POSIX, ISO C/C++
  2. Other POSIX compliant OS, e.g., *BSD, Linux
  1. I thought that we wouldn't have many things to do before merging the packet filter to NuttX, but considering what I said above, it seems that we'll have to basically rewrite all our functions to use ioctl-like operations, such as setsockopt, getsockopt, etc... Am I right, or maybe I'm seeing the things more complicated than they really are?

Yes, the ad hoc user space APIs should be avoided as much as possible, especially other OS already define a mature interface:
https://github.com/apache/nuttx/blob/master/INVIOLABLES.md#strict-posix-compliance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants