Bug 1749698

Summary:	TestOnly: iptables-restore fails if comment contains '-' and 't'
Product:	Red Hat Enterprise Linux 8	Reporter:	Phil Sutter <psutter>
Component:	iptables	Assignee:	Phil Sutter <psutter>
Status:	CLOSED ERRATA	QA Contact:	Jiri Peska <jpeska>
Severity:	high	Docs Contact:
Priority:	high
Version:	8.2	CC:	ajohn, iptables-maint-list, jpeska, todoleza
Target Milestone:	rc	Keywords:	TestOnly
Target Release:	8.0
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	iptables-1.8.4-4.el8	Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:
Clones:	1749700 (view as bug list)		Environment:
Last Closed:	2020-04-28 17:00:25 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	1749700

Description Phil Sutter 2019-09-06 09:14:59 UTC

Seems like iptables-restore parser is buggy:

# iptables -A FORWARD -m comment --comment "-t foo bar" -j ACCEPT
# iptables-save >/tmp/ipt.dump
# grep -v '^#' /tmp/ipt.dump
*filter
:INPUT ACCEPT [0:0]
:FORWARD ACCEPT [0:0]
:OUTPUT ACCEPT [0:0]
-A FORWARD -m comment --comment "-t foo bar" -j ACCEPT
COMMIT
# iptables-restore </tmp/ipt.dump
iptables-nft-restore v1.8.3 (nf_tables): The -t option (seen in line 7) cannot be used in iptables-nft-restore.

Error occurred at line: 7
Try `iptables-nft-restore -h' or 'iptables-nft-restore --help' for more information.

The Problem is consistent with upstream and happens with legacy as well as nft
variant. RHEL7 iptables-1.4.21-28.el7.x86_64 is reported to work correctly, so
this is a regression.

Comment 1 Phil Sutter 2019-09-20 15:52:22 UTC

Correction: The actual bug (and regression) is rejecting of comments starting
with dash and having a 't' somewhere, not '-t' itself (this was never allowed).

A testcase for this is:

| *filter
| -A FORWARD -m comment --comment "- allow this" -j ACCEPT
| COMMIT

Fix sent upstream: https://lore.kernel.org/netfilter-devel/20190920154920.7927-1-phil@nwl.cc/T/#u

Comment 2 Phil Sutter 2019-10-21 17:17:16 UTC

Upstream commit to backport:

commit 3dc433b55bbfaf9df3ee408aaa6282742f377864
Author: Phil Sutter <phil>
Date:   Fri Sep 20 17:31:58 2019 +0200

    xtables-restore: Fix --table parameter check
    
    Xtables-restore tries to reject rule commands in input which contain a
    --table parameter (since it is adding this itself based on the previous
    table line). The manual check was not perfect though as it caught any
    parameter starting with a dash and containing a 't' somewhere, even in
    rule comments:
    
    | *filter
    | -A FORWARD -m comment --comment "- allow this one" -j ACCEPT
    | COMMIT
    
    Instead of error-prone manual checking, go a much simpler route: All
    do_command callbacks are passed a boolean indicating they're called from
    *tables-restore. React upon this when handling a table parameter and
    error out if it's not the first one.
    
    Fixes: f8e5ebc5986bf ("iptables: Fix crash on malformed iptables-restore")
    Signed-off-by: Phil Sutter <phil>
    Acked-by: Florian Westphal <fw>

Comment 3 Phil Sutter 2019-10-28 07:00:12 UTC

Follow-up of above commit which also needs backporting:

commit 4e470fa34761085144640fb561a9ad26b2cde382
Author: Phil Sutter <phil>
Date:   Tue Oct 22 12:25:28 2019 +0200

    xtables-restore: Unbreak *tables-restore
    
    Commit 3dc433b55bbfa ("xtables-restore: Fix --table parameter check")
    installed an error check which evaluated true in all cases as all
    callers of do_command callbacks pass a pointer to a table name already.
    Attached test case passed as it tested error condition only.
    
    Fix the whole mess by introducing a boolean to indicate whether a table
    parameter was seen already. Extend the test case to cover positive as
    well as negative behaviour and to test ebtables-restore and
    ip6tables-restore as well. Also add the required checking code to the
    latter since the original commit missed it.
    
    Fixes: 3dc433b55bbfa ("xtables-restore: Fix --table parameter check")
    Signed-off-by: Phil Sutter <phil>
    Acked-by: Pablo Neira Ayuso <pablo>

Comment 4 Phil Sutter 2019-12-05 10:13:47 UTC

Fixed implicitly by rebase to 1.8.4 release.

Comment 7 errata-xmlrpc 2020-04-28 17:00:25 UTC

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHEA-2020:1889