Skip to content

Commit

Permalink
icmp: randomize the global rate limiter
Browse files Browse the repository at this point in the history
[ Upstream commit b38e781 ]

Keyu Man reported that the ICMP rate limiter could be used
by attackers to get useful signal. Details will be provided
in an upcoming academic publication.

Our solution is to add some noise, so that the attackers
no longer can get help from the predictable token bucket limiter.

Fixes: 4cdf507 ("icmp: add a global rate limitation")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Keyu Man <kman001@ucr.edu>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Eric Dumazet authored and gregkh committed Oct 29, 2020
1 parent 9fa95d1 commit 8df0ffe
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
4 changes: 3 additions & 1 deletion Documentation/networking/ip-sysctl.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1000,12 +1000,14 @@ icmp_ratelimit - INTEGER
icmp_msgs_per_sec - INTEGER
Limit maximal number of ICMP packets sent per second from this host.
Only messages whose type matches icmp_ratemask (see below) are
controlled by this limit.
controlled by this limit. For security reasons, the precise count
of messages per second is randomized.
Default: 1000

icmp_msgs_burst - INTEGER
icmp_msgs_per_sec controls number of ICMP packets sent per second,
while icmp_msgs_burst controls the burst size of these packets.
For security reasons, the precise burst size is randomized.
Default: 50

icmp_ratemask - INTEGER
Expand Down
7 changes: 5 additions & 2 deletions net/ipv4/icmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ static struct {
/**
* icmp_global_allow - Are we allowed to send one more ICMP message ?
*
* Uses a token bucket to limit our ICMP messages to sysctl_icmp_msgs_per_sec.
* Uses a token bucket to limit our ICMP messages to ~sysctl_icmp_msgs_per_sec.
* Returns false if we reached the limit and can not send another packet.
* Note: called with BH disabled
*/
Expand Down Expand Up @@ -267,7 +267,10 @@ bool icmp_global_allow(void)
}
credit = min_t(u32, icmp_global.credit + incr, sysctl_icmp_msgs_burst);
if (credit) {
credit--;
/* We want to use a credit of one in average, but need to randomize
* it for security reasons.
*/
credit = max_t(int, credit - prandom_u32_max(3), 0);
rc = true;
}
WRITE_ONCE(icmp_global.credit, credit);
Expand Down

0 comments on commit 8df0ffe

Please sign in to comment.