linux-kernel - Re: [PATCH v2 1/1] nf_conntrack_ftp: Added nfct_seqadj_ext

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aPZFNBNXlyq0Q5dM@strlen.de>
Date: Mon, 20 Oct 2025 16:20:36 +0200
From: Florian Westphal <fw@...len.de>
To: Andrii Melnychenko <a.melnychenko@...s.io>
Cc: pablo@...filter.org, kadlec@...filter.org, phil@....cc,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, horms@...nel.org,
	netfilter-devel@...r.kernel.org, coreteam@...filter.org,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] nf_conntrack_ftp: Added nfct_seqadj_ext_add() for
 ftp's conntrack.

Andrii Melnychenko <a.melnychenko@...s.io> wrote:
> I've researched the issue a bit. Despite the fact that in `nf_nat_ftp()`
> the helper for the expected connection is installed, it isn't executed in
> the following functions - `nf_nat_mangle_tcp_packet()`. Also, shouldn't the
> logic of `nf_nat_follow_master` affect the "upcoming" passive FTP
> connection?

Yes, but we need the seqadj extension on the control connection to
rewrite the announced address to connect to/from.

nf_nat_setup_info() takes care of this but only for template-based
helper assignment, not for the explicit assign done via
nft_ct_helper_obj_eval().

> I've also checked the setup of `nfct_seqadj_ext_add()` in the
> `ft_ct_helper_obj_eval()` routine - it works. However, now the seqadj would
> be added to all "NATed" conntrack helpers.

Yes.

> Maybe it's better to leave the
> seqadj setup in `nf_conntrack_ftp`, so it would apply explicitly to FTP
> traffic, but with an additional `(ct->status & IPS_NAT_MASK)` check?

As-is, almost all the helpers are broken when used with nat and assignment
via nft objref infra.  We could add some annotation to those that don't
need seqadj, but afaics thats just the netbios helper.

> I can prepare a new patch with changes in either `nft_ct` or
> `nf_conntrack_ftp`.
> Any suggestions?

Thanks, please fix nft_ct infra.  Does the above make sense to you?