net: ipv6_fragment: improve reassembly condition

Currently net_ipv6_handle_fragment_hdr() performs 2 distinct tests: it
checks the M-bit of the most recent fragment to decide if we can proceed
with the reassembly. Then it performs some sanity checks which can lead
to dropping the whole packet if not successful.

The test on the M-bit assumes that fragments arrive in order. But this
will fail if packets arrive out-of-order, since the last fragment can
arrive before some other fragments. In that case, we proceed with the
reassembly but it will fail because not all the fragments have been
received.

We need a more complete check before proceeding with the reassembly:
- We received the first fragment (offset = 0)
- All intermediate fragments are contiguous
- The More bit of the last fragment is 0

Since these conditions can also detect a malformed fragmented packet, we
can replace the existing sanity check that is performed before
reassembly. As a bonus, we can now detect and rejected overlapping
fragments, since this can have some security issues (see RFC 5722).

Signed-off-by: Florian Vaussard <[email protected]>

subsys/net/ip/ipv6_fragment.c

@@ -365,39 +365,62 @@ void net_ipv6_frag_foreach(net_ipv6_frag_cb_t cb, void *user_data)
 }
 
 /* Verify that we have all the fragments received and in correct order.
+ * Return:
+ * - a negative value if the fragments are erroneous and must be dropped
+ * - zero if we are expecting more fragments
+ * - a positive value if we can proceed with the reassembly
  */
-static bool fragment_verify(struct net_ipv6_reassembly *reass)
+static int fragments_are_ready(struct net_ipv6_reassembly *reass)
 {
-	uint16_t offset;
-	int i, prev_len;
-
-	prev_len = net_pkt_get_len(reass->pkt[0]);
-	offset = net_pkt_ipv6_fragment_offset(reass->pkt[0]);
-
-	NET_DBG("pkt %p offset %u", reass->pkt[0], offset);
+	unsigned int expected_offset = 0;
+	bool more = true;
+	int i;
 
-	if (offset != 0U) {
-		return false;
-	}
+	/* Fragments can arrive in any order, for example in reverse order:
+	 *   1 -> Fragment3(M=0, offset=x2)
+	 *   2 -> Fragment2(M=1, offset=x1)
+	 *   3 -> Fragment1(M=1, offset=0)
+	 * We have to test several requirements before proceeding with the reassembly:
+	 * - We received the first fragment (Fragment Offset is 0)
+	 * - All intermediate fragments are contiguous
+	 * - The More bit of the last fragment is 0
+	 */
+	for (i = 0; i < CONFIG_NET_IPV6_FRAGMENT_MAX_PKT; i++) {
+		struct net_pkt *pkt = reass->pkt[i];
+		unsigned int offset;
+		int payload_len;
 
-	for (i = 1; i < CONFIG_NET_IPV6_FRAGMENT_MAX_PKT; i++) {
-		if (!reass->pkt[i]) {
+		if (!pkt) {
 			break;
 		}
-		offset = net_pkt_ipv6_fragment_offset(reass->pkt[i]);
 
-		NET_DBG("pkt %p offset %u prev_len %d", reass->pkt[i],
-			offset, prev_len);
+		offset = net_pkt_ipv6_fragment_offset(pkt);
 
-		if (prev_len < offset) {
-			/* Something wrong with the offset value */
-			return false;
+		if (offset < expected_offset) {
+			/* Overlapping or duplicated
+			 * According to RFC8200 we can drop it
+			 */
+			return -EBADMSG;
+		} else if (offset != expected_offset) {
+			/* Not contiguous, let's wait for fragments */
+			return 0;
+		}
+
+		payload_len = net_pkt_get_len(pkt) - net_pkt_ipv6_fragment_start(pkt);
+		payload_len -= sizeof(struct net_ipv6_frag_hdr);
+		if (payload_len < 0) {
+			return -EBADMSG;
 		}
 
-		prev_len = net_pkt_get_len(reass->pkt[i]);
+		expected_offset += payload_len;
+		more = net_pkt_ipv6_fragment_more(pkt);
+	}
+
+	if (more) {
+		return 0;
 	}
 
-	return true;
+	return 1;
 }
 
 static int shift_packets(struct net_ipv6_reassembly *reass, int pos)
@@ -438,6 +461,7 @@ enum net_verdict net_ipv6_handle_fragment_hdr(struct net_pkt *pkt,
 	bool found;
 	uint8_t more;
 	uint32_t id;
+	int ret;
 	int i;
 
 	if (!reassembly_init_done) {
@@ -517,18 +541,8 @@ enum net_verdict net_ipv6_handle_fragment_hdr(struct net_pkt *pkt,
 		goto drop;
 	}
 
-	if (more) {
-		}
-
-		reassembly_info("Reassembly nth pkt", reass);
-
-		NET_DBG("More fragments to be received");
-		goto accept;
-	}
-
-	reassembly_info("Reassembly last pkt", reass);
-
-	if (!fragment_verify(reass)) {
+	ret = fragments_are_ready(reass);
+	if (ret < 0) {
 		NET_DBG("Reassembled IPv6 verify failed, dropping id %u",
 			reass->id);
 
@@ -539,8 +553,15 @@ enum net_verdict net_ipv6_handle_fragment_hdr(struct net_pkt *pkt,
 
 		net_pkt_unref(pkt);
 		goto drop;
+	} else if (ret == 0) {
+		reassembly_info("Reassembly nth pkt", reass);
+
+		NET_DBG("More fragments to be received");
+		goto accept;
 	}
 
+	reassembly_info("Reassembly last pkt", reass);
+
 	/* The last fragment received, reassemble the packet */
 	reassemble_packet(reass);