Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Willy Tarreau <w@1wt.eu>, Amit Klein <aksecurity@gmail.com>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Sasha Levin <sashal@kernel.org>,
	netdev@vger.kernel.org
Subject: [PATCH AUTOSEL 4.9 13/35] ipv6: use prandom_u32() for ID generation
Date: Tue,  6 Jul 2021 07:28:25 -0400	[thread overview]
Message-ID: <20210706112848.2066036-13-sashal@kernel.org> (raw)
In-Reply-To: <20210706112848.2066036-1-sashal@kernel.org>

From: Willy Tarreau <w@1wt.eu>

[ Upstream commit 62f20e068ccc50d6ab66fdb72ba90da2b9418c99 ]

This is a complement to commit aa6dd211e4b1 ("inet: use bigger hash
table for IP ID generation"), but focusing on some specific aspects
of IPv6.

Contary to IPv4, IPv6 only uses packet IDs with fragments, and with a
minimum MTU of 1280, it's much less easy to force a remote peer to
produce many fragments to explore its ID sequence. In addition packet
IDs are 32-bit in IPv6, which further complicates their analysis. On
the other hand, it is often easier to choose among plenty of possible
source addresses and partially work around the bigger hash table the
commit above permits, which leaves IPv6 partially exposed to some
possibilities of remote analysis at the risk of weakening some
protocols like DNS if some IDs can be predicted with a good enough
probability.

Given the wide range of permitted IDs, the risk of collision is extremely
low so there's no need to rely on the positive increment algorithm that
is shared with the IPv4 code via ip_idents_reserve(). We have a fast
PRNG, so let's simply call prandom_u32() and be done with it.

Performance measurements at 10 Gbps couldn't show any difference with
the previous code, even when using a single core, because due to the
large fragments, we're limited to only ~930 kpps at 10 Gbps and the cost
of the random generation is completely offset by other operations and by
the network transfer time. In addition, this change removes the need to
update a shared entry in the idents table so it may even end up being
slightly faster on large scale systems where this matters.

The risk of at least one collision here is about 1/80 million among
10 IDs, 1/850k among 100 IDs, and still only 1/8.5k among 1000 IDs,
which remains very low compared to IPv4 where all IDs are reused
every 4 to 80ms on a 10 Gbps flow depending on packet sizes.

Reported-by: Amit Klein <aksecurity@gmail.com>
Signed-off-by: Willy Tarreau <w@1wt.eu>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Link: https://lore.kernel.org/r/20210529110746.6796-1-w@1wt.eu
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 net/ipv6/output_core.c | 28 +++++-----------------------
 1 file changed, 5 insertions(+), 23 deletions(-)

diff --git a/net/ipv6/output_core.c b/net/ipv6/output_core.c
index 6a6d01cb1ace..9c25e8b09306 100644
--- a/net/ipv6/output_core.c
+++ b/net/ipv6/output_core.c
@@ -14,29 +14,11 @@ static u32 __ipv6_select_ident(struct net *net,
 			       const struct in6_addr *dst,
 			       const struct in6_addr *src)
 {
-	const struct {
-		struct in6_addr dst;
-		struct in6_addr src;
-	} __aligned(SIPHASH_ALIGNMENT) combined = {
-		.dst = *dst,
-		.src = *src,
-	};
-	u32 hash, id;
-
-	/* Note the following code is not safe, but this is okay. */
-	if (unlikely(siphash_key_is_zero(&net->ipv4.ip_id_key)))
-		get_random_bytes(&net->ipv4.ip_id_key,
-				 sizeof(net->ipv4.ip_id_key));
-
-	hash = siphash(&combined, sizeof(combined), &net->ipv4.ip_id_key);
-
-	/* Treat id of 0 as unset and if we get 0 back from ip_idents_reserve,
-	 * set the hight order instead thus minimizing possible future
-	 * collisions.
-	 */
-	id = ip_idents_reserve(hash, 1);
-	if (unlikely(!id))
-		id = 1 << 31;
+	u32 id;
+
+	do {
+		id = prandom_u32();
+	} while (!id);
 
 	return id;
 }
-- 
2.30.2


  parent reply	other threads:[~2021-07-06 11:40 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-06 11:28 [PATCH AUTOSEL 4.9 01/35] net: pch_gbe: Use proper accessors to BE data in pch_ptp_match() Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 03/35] atm: iphase: fix possible use-after-free in ia_module_exit() Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 04/35] mISDN: fix possible use-after-free in HFC_cleanup() Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 05/35] atm: nicstar: Fix possible use-after-free in nicstar_cleanup() Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 06/35] net: Treat __napi_schedule_irqoff() as __napi_schedule() on PREEMPT_RT Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 11/35] e100: handle eeprom as little endian Sasha Levin
2021-07-06 11:28 ` Sasha Levin [this message]
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 16/35] net: micrel: check return value after calling platform_get_resource() Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 17/35] net: moxa: Use devm_platform_get_and_ioremap_resource() Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 18/35] fjes: check return value after calling platform_get_resource() Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 20/35] xfrm: Fix error reporting in xfrm_state_construct Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 21/35] wlcore/wl12xx: Fix wl12xx get_mac error if device is in ELP Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 22/35] wl1251: Fix possible buffer overflow in wl1251_cmd_scan Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 23/35] cw1200: add missing MODULE_DEVICE_TABLE Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 25/35] atm: nicstar: use 'dma_free_coherent' instead of 'kfree' Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 26/35] atm: nicstar: register the interrupt handler in the right place Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 28/35] sfc: avoid double pci_remove of VFs Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 29/35] sfc: error code if SRIOV cannot be disabled Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 30/35] wireless: wext-spy: Fix out-of-bounds warning Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 32/35] Bluetooth: Fix the HCI to MGMT status conversion table Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 33/35] Bluetooth: Shutdown controller after workqueues are flushed or cancelled Sasha Levin
2021-07-06 11:28 ` [PATCH AUTOSEL 4.9 35/35] sctp: add size validation when walking chunks Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210706112848.2066036-13-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=aksecurity@gmail.com \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=w@1wt.eu \
    --subject='Re: [PATCH AUTOSEL 4.9 13/35] ipv6: use prandom_u32() for ID generation' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).