| Message ID | 20210222143550.28728-2-tim.gardner@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | icmp: randomize the global rate limiter | expand |
On Mon, Feb 22, 2021 at 07:35:50AM -0700, Tim Gardner wrote: > From: Eric Dumazet <edumazet@google.com> > > 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: 4cdf507d5452 ("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> > (backported from commit b38e7819cae946e2edf869e604af1e65a5d241c5) > CVE-2020-25705 > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> > > Back port notes: dropped edits to Documentation/networking/ip-sysctl.rst > as that file does not yet exist. Well, it is a txt instead of rst file, and 5.4 backport modifies the old version just fine. Can you resend picking up the same change for that file as the 5.4 one? Cascardo. > --- > net/ipv4/icmp.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c > index f369e7ce685b..dcffda472585 100644 > --- a/net/ipv4/icmp.c > +++ b/net/ipv4/icmp.c > @@ -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 > */ > @@ -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); > -- > 2.17.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
[Impact] A flaw in the way reply ICMP packets are limited in the Linux kernel functionality was found that allows to quickly scan open UDP ports. This flaw allows an off-path remote user to effectively bypassing source port UDP randomization. The highest threat from this vulnerability is to confidentiality and possibly integrity, because software that relies on UDP source port randomization are indirectly affected as well. Kernel versions before 5.10 may be vulnerable to this issue. From the Ubuntu security team: Keyu Man discovered that the ICMP global rate limiter in the Linux kernel could be used to assist in scanning open UDP ports. A remote attacker could use to facilitate attacks on UDP based services that depend on source port randomization. [Test Case] Given the nature of the exploit, a test case is not feasible. [Potential regression] This is a simple one line code change that has been released in all other Focal kernels without regression.
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index f369e7ce685b..dcffda472585 100644 --- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c @@ -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 */ @@ -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);