Hello,
On Sat, 24 Feb 2018, Florian Westphal wrote:
> FTP passive mode got broken by this change:
> - if (.. && nfct_nat(ct)) {
> + if (.. (ct->status & IPS_NAT_MASK)) {
Looks like this check was needed to avoid crash in 2.6.35,
see commit 7bcbf81a2296a8 ("ipvs: avoid oops for passive FTP").
I just tested such fix and it should be enough
to fix the passive FTP:
diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index 3e17d32..58d5d05 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -260,7 +260,7 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
buf_len = strlen(buf);
ct = nf_ct_get(skb, &ctinfo);
- if (ct && (ct->status & IPS_NAT_MASK)) {
+ if (ct) {
bool mangled;
/* If mangling fails this function will return 0
If it looks ok to you and if you prefer you can submit it
as patch, otherwise I'll do it in the following days...
> The PASV reply sent by real server need to be translated to contain
> the load balancers address, but they are passed unchanged.
>
> IPS_NAT_MASK should be true for connections where reverse
> of reply tuple isn't the original tupe (i.e. connection had
> its source/destination changed), but ipvs uses a different function
> to replace the reply tuple address, and did not set SRC/DST NAT bit.
>
> Reported-by: Li Shuang <shuali@xxxxxxxxxx>
> Fixes: 41390895e50bc4 ("netfilter: ipvs: don't check for presence of nat extension")
> Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
> ---
> net/netfilter/ipvs/ip_vs_nfct.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_nfct.c b/net/netfilter/ipvs/ip_vs_nfct.c
> index 6cf3fd81a5ec..0ceac36819d9 100644
> --- a/net/netfilter/ipvs/ip_vs_nfct.c
> +++ b/net/netfilter/ipvs/ip_vs_nfct.c
> @@ -119,13 +119,17 @@ ip_vs_update_conntrack(struct sk_buff *skb, struct ip_vs_conn *cp, int outin)
> if (outin) {
> new_tuple.src.u3 = cp->daddr;
> if (new_tuple.dst.protonum != IPPROTO_ICMP &&
> - new_tuple.dst.protonum != IPPROTO_ICMPV6)
> + new_tuple.dst.protonum != IPPROTO_ICMPV6) {
> new_tuple.src.u.tcp.port = cp->dport;
> + ct->status |= IPS_SRC_NAT;
These bits should be reversed, 'outin' means 'from client
towards real server', i.e. like DNAT rule in original direction:
ct->status |= IPS_DST_NAT;
> + }
> } else {
> new_tuple.dst.u3 = cp->vaddr;
> if (new_tuple.dst.protonum != IPPROTO_ICMP &&
> - new_tuple.dst.protonum != IPPROTO_ICMPV6)
> + new_tuple.dst.protonum != IPPROTO_ICMPV6) {
> new_tuple.dst.u.tcp.port = cp->vport;
> + ct->status |= IPS_DST_NAT;
here too:
ct->status |= IPS_SRC_NAT;
But for now we do not need to set them. If your patch
works, it was because the bits helped for the IPS_NAT_MASK
check to allow the nf_nat_mangle_tcp_packet call.
Regards
--
Julian Anastasov <ja@xxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
