ipfw(8): Introduce src-ip4/dst-ip4 and src-ipv4/dst-ipv4 specifiers, make src-ip/dst-ip dual-stack

[nc]

Summary

ipfw(8): Introduce src-ip4/dst-ip4 and src-ipv4/dst-ipv4 specifiers and making src-ip/dst-ip dual-stack. This is consistent with the behavior of me/me6 and D24021.

Also, while we're here, add mention of src-ipv6 in man page.

Submitted by: Neel Chauhan <neel AT neelc DOT org>

Test Plan

Add a rule like this:

ipfw add deny src-ip4 1.0.0.0/24
ipfw add deny src-ip 1.0.0.0/24
ipfw add deny src-ip 2600::0

It should work.

[bcr]

OK from manpages. Don't forget to bump the .Dd when you commit this change. Thank you!

[donner]

What about src-ipv4? (for the sake of symmetry)
Somebody may think about "*ip" to accept both address families.

[nc]

In D24234#533263, @lutz_donnerhacke.de wrote:

What about src-ipv4? (for the sake of symmetry)
Somebody may think about "*ip" to accept both address families.

Great idea. I will post a revised patch with src-ipv4.

[nc]

This patch was discussed on freebsd-hackers@ here: https://lists.freebsd.org/pipermail/freebsd-hackers/2020-April/055818.html

This patch has changed significantly, making src-ip4/dst-ip4 and src-ipv4/dst-ipv4 deal with IPv4-only addresses and making src-ip/dst-ip dual-stack.

[rgrimes]

How is any of this being implemented in the kernel filter code itself? If your not adding opcodes I do not see how this can work properly, other than they are purely aliases for what is already in place.

[nc]

In D24234#536051, @rgrimes wrote:

How is any of this being implemented in the kernel filter code itself? If your not adding opcodes I do not see how this can work properly, other than they are purely aliases for what is already in place.

No kernel code is touched. The dual-stack part happens in ipfw2.c between lines 4842-4847 and 4853-4860. However, this is more of a bruteforce method and is ugly, and also breaks IPv4-based syntax using addr:mask (I realized this after posting) so it shouldn't be accepted.

[nc]

I have an updated diff which should work with almost all use cases (including 1.2.3.4:255.255.255.0 which was broken in an earlier diff). I believe this is backwards-compatible with today's src-ip.

However, if the dual-stack src-ip is used, IPv4 and IPv6 addresses cannot be mixed if a comma is used for multiple addresses. Commas are for multiple IPv4 or multiple IPv6 only. Being able to mix IPv4 and IPv6 addresses probably requires a separate opcode, or at least a new fill_ip() type function.

If a dual-stack src-ip is not required/desired, and src-ip should remain IPv4-only (like it is today), let me know and I will revert to the older patch (which just adds src-ip4/dst-ip4 and src-ipv4/dst-ipv4 aliases).

[rgrimes]

The more I think about this the more I think we may want to step back and take a bigger picture view of how ipfw2 deals with ipv4 vs ipv6 and the fact that it originally had no ipv6 support, and the grafting on of that feature left some edges that are not very clean and rather than adhoc try to shove this type of stuff in we should take a look at how to possibly clean up those edges. The basic "proto" match is ip, ip4|ipv4, ip6|ipv6. Reading the man page leads me to a missing bit, src and dst are specified as {addr | .... but I see no path to addr6-list or for that mater a clean path to an ipv6 address specifier. I think if one stepped back and tried to write a formal BNF syntax we would uncover some of the issues that are leading to the poor clarity in syntax today. I avoid most of these issues as my firewalls are written in a way that does protocol classification and dispatch very early so once I decide if a packet is ospf/ipv4/ipv6/icmp/icmp6/etc I no longer care about protocol field at all and know I am dealing with v4 or v6 in each section.

[nc]

Makes sense.

Abandoning, for now at least (maybe forever).