netdev - Re: [PATCH nf] netfilter: seqadj: Fix the wrong ack adjust for the RST packet without ack

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: <CA+6hz4r+g-s_h=FiV4axCuwT_JQLiztyN2oDQ_iyz_20Mp2j+A@mail.gmail.com>
Date:   Tue, 6 Sep 2016 10:10:41 +0800
From:   Feng Gao <gfree.wind@...il.com>
To:     Pablo Neira Ayuso <pablo@...filter.org>,
        Netfilter Developer Mailing List 
        <netfilter-devel@...r.kernel.org>, Florian Westphal <fw@...len.de>,
        coreteam@...filter.org,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Cc:     Feng Gao <gfree.wind@...il.com>, Gao Feng <fgao@...ai8.com>
Subject: Re: [PATCH nf] netfilter: seqadj: Fix the wrong ack adjust for the
 RST packet without ack

On Tue, Sep 6, 2016 at 10:06 AM,  <fgao@...ai8.com> wrote:
> From: Gao Feng <fgao@...ai8.com>
>
> It is valid that the TCP RST packet which does not set ack flag, and bytes
> of ack number are zero. For these RST packets, seqadj could not adjust the
> ack number.
>
> Signed-off-by: Gao Feng <fgao@...ai8.com>
> ---
>  v2: Regenerate because the first patch is removed
>  v1: Initial patch
>
>  net/netfilter/nf_conntrack_seqadj.c | 34 +++++++++++++++++++---------------
>  1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_seqadj.c b/net/netfilter/nf_conntrack_seqadj.c
> index dff0f0c..3bd9c7e 100644
> --- a/net/netfilter/nf_conntrack_seqadj.c
> +++ b/net/netfilter/nf_conntrack_seqadj.c
> @@ -179,30 +179,34 @@ int nf_ct_seq_adjust(struct sk_buff *skb,
>
>         tcph = (void *)skb->data + protoff;
>         spin_lock_bh(&ct->lock);
> +
>         if (after(ntohl(tcph->seq), this_way->correction_pos))
>                 seqoff = this_way->offset_after;
>         else
>                 seqoff = this_way->offset_before;
>
> -       if (after(ntohl(tcph->ack_seq) - other_way->offset_before,
> -                 other_way->correction_pos))
> -               ackoff = other_way->offset_after;
> -       else
> -               ackoff = other_way->offset_before;
> -
>         newseq = htonl(ntohl(tcph->seq) + seqoff);
> -       newack = htonl(ntohl(tcph->ack_seq) - ackoff);
> -
>         inet_proto_csum_replace4(&tcph->check, skb, tcph->seq, newseq, false);
> -       inet_proto_csum_replace4(&tcph->check, skb, tcph->ack_seq, newack,
> -                                false);
> -
> -       pr_debug("Adjusting sequence number from %u->%u, ack from %u->%u\n",
> -                ntohl(tcph->seq), ntohl(newseq), ntohl(tcph->ack_seq),
> -                ntohl(newack));
>
> +       pr_debug("Adjusting sequence number from %u->%u\n",
> +                ntohl(tcph->seq), ntohl(newseq));
>         tcph->seq = newseq;
> -       tcph->ack_seq = newack;
> +
> +       if (likely(tcph->ack)) {
> +               if (after(ntohl(tcph->ack_seq) - other_way->offset_before,
> +                         other_way->correction_pos))
> +                       ackoff = other_way->offset_after;
> +               else
> +                       ackoff = other_way->offset_before;
> +
> +               newack = htonl(ntohl(tcph->ack_seq) - ackoff);
> +               inet_proto_csum_replace4(&tcph->check, skb, tcph->ack_seq,
> +                                        newack, false);
> +
> +               pr_debug("Adjusting ack number from %u->%u\n",
> +                        ntohl(tcph->ack_seq), ntohl(newack));
> +               tcph->ack_seq = newack;
> +       }
>
>         res = nf_ct_sack_adjust(skb, protoff, tcph, ct, ctinfo);
>         spin_unlock_bh(&ct->lock);
> --
> 1.9.1
>
>

Sorry, I forget to add the v2 in the subject.

Best Regards
Feng