Page MenuHomeFreeBSD

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

Authored by neel_neelc.org on Mar 31 2020, 1:30 AM.

Details

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.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

bcr added a subscriber: bcr.

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

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

This revision is now accepted and ready to land.Mar 31 2020, 8:18 AM

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.

neel_neelc.org retitled this revision from ipfw(8): Introduce src-ip4/dst-ip4 aliases for src-ip/dst-ip to ipfw(8): Introduce src-ip4/dst-ip4 and src-ipv4/dst-ipv4 aliases for src-ip/dst-ip.
neel_neelc.org edited the summary of this revision. (Show Details)
neel_neelc.org edited the test plan for this revision. (Show Details)
This revision now requires review to proceed.Mar 31 2020, 5:14 PM
This revision is now accepted and ready to land.Apr 1 2020, 7:58 AM
neel_neelc.org retitled this revision from ipfw(8): Introduce src-ip4/dst-ip4 and src-ipv4/dst-ipv4 aliases for src-ip/dst-ip to ipfw(8): Introduce src-ip4/dst-ip4 and src-ipv4/dst-ipv4 specifiers, make src-ip/dst-ip dual-stack.
neel_neelc.org edited the summary of this revision. (Show Details)
neel_neelc.org edited the test plan for this revision. (Show Details)

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.

This revision now requires review to proceed.Apr 11 2020, 3:34 AM

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.

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.

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).

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.

Makes sense.

Abandoning, for now at least (maybe forever).