Michal Kubecek 5acc8a
From: Eric Dumazet <edumazet@google.com>
Michal Kubecek 5acc8a
Date: Thu, 15 Oct 2020 11:42:00 -0700
Michal Kubecek 5acc8a
Subject: icmp: randomize the global rate limiter
Michal Kubecek 5acc8a
Patch-mainline: v5.10-rc1
Michal Kubecek 5acc8a
Git-commit: b38e7819cae946e2edf869e604af1e65a5d241c5
Michal Kubecek 5acc8a
References: CVE-2020-25705 bsc#1175721 git-fixes
Michal Kubecek 5acc8a
Michal Kubecek 5acc8a
Keyu Man reported that the ICMP rate limiter could be used
Michal Kubecek 5acc8a
by attackers to get useful signal. Details will be provided
Michal Kubecek 5acc8a
in an upcoming academic publication.
Michal Kubecek 5acc8a
Michal Kubecek 5acc8a
Our solution is to add some noise, so that the attackers
Michal Kubecek 5acc8a
no longer can get help from the predictable token bucket limiter.
Michal Kubecek 5acc8a
Michal Kubecek 5acc8a
Fixes: 4cdf507d5452 ("icmp: add a global rate limitation")
Michal Kubecek 5acc8a
Signed-off-by: Eric Dumazet <edumazet@google.com>
Michal Kubecek 5acc8a
Reported-by: Keyu Man <kman001@ucr.edu>
Michal Kubecek 5acc8a
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Michal Kubecek 5acc8a
Acked-by: Michal Kubecek <mkubecek@suse.cz>
Michal Kubecek 5acc8a
Michal Kubecek 5acc8a
---
Michal Kubecek 5acc8a
 Documentation/networking/ip-sysctl.txt | 4 +++-
Michal Kubecek 5acc8a
 net/ipv4/icmp.c                        | 7 +++++--
Michal Kubecek 5acc8a
 2 files changed, 8 insertions(+), 3 deletions(-)
Michal Kubecek 5acc8a
Michal Kubecek 5acc8a
--- a/Documentation/networking/ip-sysctl.txt
Michal Kubecek 5acc8a
+++ b/Documentation/networking/ip-sysctl.txt
Michal Kubecek 5acc8a
@@ -903,12 +903,14 @@ icmp_ratelimit - INTEGER
Michal Kubecek 5acc8a
 icmp_msgs_per_sec - INTEGER
Michal Kubecek 5acc8a
 	Limit maximal number of ICMP packets sent per second from this host.
Michal Kubecek 5acc8a
 	Only messages whose type matches icmp_ratemask (see below) are
Michal Kubecek 5acc8a
-	controlled by this limit.
Michal Kubecek 5acc8a
+	controlled by this limit. For security reasons, the precise count
Michal Kubecek 5acc8a
+	of messages per second is randomized.
Michal Kubecek 5acc8a
 	Default: 1000
Michal Kubecek 5acc8a
 
Michal Kubecek 5acc8a
 icmp_msgs_burst - INTEGER
Michal Kubecek 5acc8a
 	icmp_msgs_per_sec controls number of ICMP packets sent per second,
Michal Kubecek 5acc8a
 	while icmp_msgs_burst controls the burst size of these packets.
Michal Kubecek 5acc8a
+	For security reasons, the precise burst size is randomized.
Michal Kubecek 5acc8a
 	Default: 50
Michal Kubecek 5acc8a
 
Michal Kubecek 5acc8a
 icmp_ratemask - INTEGER
Michal Kubecek 5acc8a
--- a/net/ipv4/icmp.c
Michal Kubecek 5acc8a
+++ b/net/ipv4/icmp.c
Michal Kubecek 5acc8a
@@ -244,7 +244,7 @@ static struct {
Michal Kubecek 5acc8a
 /**
Michal Kubecek 5acc8a
  * icmp_global_allow - Are we allowed to send one more ICMP message ?
Michal Kubecek 5acc8a
  *
Michal Kubecek 5acc8a
- * Uses a token bucket to limit our ICMP messages to sysctl_icmp_msgs_per_sec.
Michal Kubecek 5acc8a
+ * Uses a token bucket to limit our ICMP messages to ~sysctl_icmp_msgs_per_sec.
Michal Kubecek 5acc8a
  * Returns false if we reached the limit and can not send another packet.
Michal Kubecek 5acc8a
  * Note: called with BH disabled
Michal Kubecek 5acc8a
  */
Michal Kubecek 5acc8a
@@ -271,7 +271,10 @@ bool icmp_global_allow(void)
Michal Kubecek 5acc8a
 	}
Michal Kubecek 5acc8a
 	credit = min_t(u32, icmp_global.credit + incr, sysctl_icmp_msgs_burst);
Michal Kubecek 5acc8a
 	if (credit) {
Michal Kubecek 5acc8a
-		credit--;
Michal Kubecek 5acc8a
+		/* We want to use a credit of one in average, but need to randomize
Michal Kubecek 5acc8a
+		 * it for security reasons.
Michal Kubecek 5acc8a
+		 */
Michal Kubecek 5acc8a
+		credit = max_t(int, credit - prandom_u32_max(3), 0);
Michal Kubecek 5acc8a
 		rc = true;
Michal Kubecek 5acc8a
 	}
Michal Kubecek 5acc8a
 	icmp_global.credit = credit;