netdev - Re: [PATCH] fragment: Improved handling of incorrect IP fragments

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Kzdjj3swtF0OuPXyJNmMt2qALVQ3Hob3fvSXhMkyqTbp9qUer0B1s_Lj1uYJJV4hEXp2v1M6bJ0XCS3zbYWOrZrzyydP-Bb4P15pDPFj0FY=@protonmail.com>
Date:   Tue, 07 Jan 2020 00:37:51 +0000
From:   Ttttabcd <ttttabcd@...tonmail.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     Netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        "kuznet@....inr.ac.ru" <kuznet@....inr.ac.ru>,
        "yoshfuji@...ux-ipv6.org" <yoshfuji@...ux-ipv6.org>
Subject: Re: [PATCH] fragment: Improved handling of incorrect IP fragments

> With current (correct) Linux kernel code this gets reassembled and dropped.
> As seen in dmesg log and statistics.
>
> With your Ipv4 patch the oversize packet gets passed on up the stack.
>
> Testing this stuff is hard, it requires packet hacker tools.

I know what you mean. What you are talking about is a "ping of death" attack. The use of fragments to construct packets longer than 65535 made the system buffer overflow and crash.

This situation has been considered in my code. In the original logic of IPv6, the judgment of data packets exceeding 65535 is duplicated, and the judgment in IPv4 is too late.

I have improved this situation, you can see my explanation of the patch at the beginning.

> In both ip6_frag_queue and ip6_frag_reasm, it is checked whether it is an
> Oversized IPv6 packet, which is duplicated. The original code logic will
> only be processed in ip6_frag_queue. The code of ip6_frag_reasm will not
> be executed. Now change it to only process in ip6_frag_queue and output
> the prompt information.

> I also made similar changes in IPv4 fragmentation processing.
>
> It is not good to use 65535 values directly,
> I added the IPV4_MAX_TOT_LEN macro.
>
> The oversized check in IPv4 fragment processing is in the ip_frag_reasm
> of the reassembly fragment. This is too late. The incorrect IP fragment
> has been inserted into the fragment queue. I modified it in ip_frag_queue.
> I changed the original net_info_ratelimited to net_dbg_ratelimited to make
> the debugging information more controllable.


@@ -300,6 +300,12 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 	end = offset + skb->len - skb_network_offset(skb) - ihl;
 	err = -EINVAL;

+	if ((unsigned int)end + ihl > IPV4_MAX_TOT_LEN) {
+		net_dbg_ratelimited("ip_frag_queue: Oversized IP packet from %pI4, end = %d\n",
+				    &qp->q.key.v4.saddr, end);
+		goto discard_qp;
+	}
+

@@ -121,11 +121,10 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
 			((u8 *)(fhdr + 1) - (u8 *)(ipv6_hdr(skb) + 1)));

 	if ((unsigned int)end > IPV6_MAXPLEN) {
-		*prob_offset = (u8 *)&fhdr->frag_off - skb_network_header(skb);
-		/* note that if prob_offset is set, the skb is freed elsewhere,
-		 * we do not free it here.
-		 */
-		return -1;
+		prob_offset = (u8 *)&fhdr->frag_off - skb_network_header(skb);
+		net_dbg_ratelimited("ip6_frag_queue: Oversized IPv6 packet from %pI6c, end = %d\n",
+				    &fq->q.key.v6.saddr, end);
+		goto send_param_prob;
 	}

As long as the IP fragment length exceeds 65535, I will discard the entire fragment queue.