Page MenuHomeFreeBSD

sbin/ipfw: Fix parsing error in table based forward
ClosedPublic

Authored by donner on May 7 2021, 7:07 PM.

Details

Summary

The argument parser does not recognise the optional port for an
"tablearg" argument. Fix simplifies the code by make the internal
representation expicit for the parser.

PR: 252744
MFC: 1 week

Test Plan
# ipfw add 7110 fwd tablearg,8000 tcp from 'table(5)' to any dst-port 80,8000,8080
07110 fwd tablearg,8000 tcp from table(5) to any 80,8000,8080

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

donner requested review of this revision.May 7 2021, 7:07 PM

LGTM.

Keep in mind that I'm a ports committer. I'm not a src committer, and haven't really been doing much work on ipfw lately. I'm more focused on ports nowadays and if not that, then drm-kmod/linuxkpi (which I only started with). It's just that ports committers can commit to src with approval of a src committer, and vice versa.

I'd recommend getting a src committer to approve as well.

This revision is now accepted and ready to land.May 8 2021, 7:34 PM

Replacing _substrcmp() with strncmp(,,8) breaks the case, when "tablearg" is part of hostname:port syntax.

In D30164#678109, @ae wrote:

Replacing _substrcmp() with strncmp(,,8) breaks the case, when "tablearg" is part of hostname:port syntax.

No, it _substrcmp() is used to match any shorter string instead of "tablearg", and prints a depreciation warning.
It will never match "tablearg,80" to "tablearg", because the argument is longer than the searched string.
This behavior caused the initial problem report.

strncmp(,,8) does match "tablearg" exactly regardless of an optional port part.

I meant the case fwd tableargs.home.lan:8000.

In D30164#678274, @ae wrote:

I meant the case fwd tableargs.home.lan:8000.

Interesting case. Thank you.
This will be rejected.