Bug 222705 - ipfw "proto udp" parsing problem
Summary: ipfw "proto udp" parsing problem
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 11.1-RELEASE
Hardware: amd64 Any
: --- Affects Only Me
Assignee: Andrey V. Elsukov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-30 21:02 UTC by Jason Mader
Modified: 2018-04-11 10:47 UTC (History)
2 users (show)

See Also:


Attachments
Proposed patch for unreach opcode (1.43 KB, patch)
2017-10-18 08:30 UTC, Andrey V. Elsukov
no flags Details | Diff
Proposed patch (22.30 KB, patch)
2018-03-24 09:43 UTC, Andrey V. Elsukov
no flags Details | Diff
Proposed patch (22.60 KB, patch)
2018-03-24 10:35 UTC, Andrey V. Elsukov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jason Mader 2017-09-30 21:02:05 UTC
Output of ipfw list displays a rule that cannot be inserted.

Rule:
unreach6 port ip6 from any to me6 proto udp dst-port 33434-33449 in

Is displayed as:
unreach6 port ip6 from any to me6 udp dst-port 33434-33449 in

missing the option "proto". Trying to add the rule without "proto udp" results in:

ipfw: unrecognised option [-1] udp
Comment 1 Jason Mader 2017-10-02 17:39:04 UTC
There's another problem on this rule,

"unreach6 port"

is generating an Address unreachable response, ICMPv6 (1,3) instead of Port unreachable, ICMPv6 (1,4).

It can be worked around with "unreach6 4"

But then the rule is displayed as, "unreach needfrag".
Comment 2 Jason Mader 2017-10-04 14:59:28 UTC
if the rule is,

unreach needfrag log ip from any to me udp dst-port 33434-33449 in

where "ip" instead of "ip6" then, IPv4 will receive a need to fragment response, and IPv6 will receive port closed.
Comment 3 Andrey V. Elsukov freebsd_committer 2017-10-18 08:30:16 UTC
Created attachment 187264 [details]
Proposed patch for unreach opcode

This patch adds mapping of IPv4 unreach codes into IPv6 codes that is also used in NAT64. The only difference, that "unreach proto" is not converted into "paramater problem", because unreach6 opcode must send ICMPv6 destination unreachable message. I'm unable to reproduce the problem described in comment #1.
This patch should only affect the case, when you have "unreach port" and IPv6 packet is matched by rule.
Comment 4 Andrey V. Elsukov freebsd_committer 2018-03-01 02:56:10 UTC
Fixed in r328326 and r328968.
Comment 5 Andrey V. Elsukov freebsd_committer 2018-03-01 03:18:08 UTC
Reopen, since was fixed only one part of bug report.
Comment 6 Andrey V. Elsukov freebsd_committer 2018-03-24 09:43:08 UTC
Created attachment 191775 [details]
Proposed patch

I reworked opcode parsing, can you test this patch with your rules?
Comment 7 Andrey V. Elsukov freebsd_committer 2018-03-24 10:01:56 UTC
Comment on attachment 191775 [details]
Proposed patch

The patch still have problems, I'll update it a bit later.
Comment 8 Andrey V. Elsukov freebsd_committer 2018-03-24 10:35:56 UTC
Created attachment 191779 [details]
Proposed patch
Comment 9 commit-hook freebsd_committer 2018-03-28 12:45:22 UTC
A commit references this bug:

Author: ae
Date: Wed Mar 28 12:44:29 UTC 2018
New revision: 331668
URL: https://svnweb.freebsd.org/changeset/base/331668

Log:
  Rework ipfw rules parsing and printing code.

  Introduce show_state structure to keep information about printed opcodes.
  Split show_static_rule() function into several smaller functions. Make
  parsing and printing opcodes into several passes. Each printed opcode
  is marked in show_state structure and will be skipped in next passes.
  Now show_static_rule() function is simple, it just prints each part
  of rule separately: action, modifiers, proto, src and dst addresses,
  options. The main goal of this change is avoiding occurrence of wrong
  result of `ifpw show` command, that can not be parsed by ipfw(8).
  Also now it is possible to make some simple static optimizations
  by reordering of opcodes in the rule.

  PR:		222705
  Discussed with:	melifaro
  MFC after:	2 weeks
  Sponsored by:	Yandex LLC

Changes:
  head/sbin/ipfw/ipfw2.c
  head/sbin/ipfw/ipfw2.h
  head/sbin/ipfw/main.c
Comment 10 commit-hook freebsd_committer 2018-04-11 10:25:45 UTC
A commit references this bug:

Author: ae
Date: Wed Apr 11 10:24:47 UTC 2018
New revision: 332400
URL: https://svnweb.freebsd.org/changeset/base/332400

Log:
  MFC r331668:
    Rework ipfw rules parsing and printing code.

    Introduce show_state structure to keep information about printed opcodes.
    Split show_static_rule() function into several smaller functions. Make
    parsing and printing opcodes into several passes. Each printed opcode
    is marked in show_state structure and will be skipped in next passes.
    Now show_static_rule() function is simple, it just prints each part
    of rule separately: action, modifiers, proto, src and dst addresses,
    options. The main goal of this change is avoiding occurrence of wrong
    result of `ifpw show` command, that can not be parsed by ipfw(8).
    Also now it is possible to make some simple static optimizations
    by reordering of opcodes in the rule.

    PR:		222705

Changes:
_U  stable/11/
  stable/11/sbin/ipfw/ipfw2.c
  stable/11/sbin/ipfw/ipfw2.h
  stable/11/sbin/ipfw/main.c
Comment 11 Andrey V. Elsukov freebsd_committer 2018-04-11 10:47:23 UTC
Fixed in head/ and stable/11. Thanks!