| Message ID | 20210223135209.3936-2-tim.gardner@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | icmp: randomize the global rate limiter | expand |
On 23.02.21 14:52, Tim Gardner wrote: > From: Eric Dumazet <edumazet@google.com> > > BugLink: https://bugs.launchpad.net/bugs/1902115 > > [ Upstream commit b38e7819cae946e2edf869e604af1e65a5d241c5 ] > > 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> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Kamal Mostafa <kamal@canonical.com> > Signed-off-by: Ian May <ian.may@canonical.com> > (cherry picked from commit fc3d57b1e860f9bdd97bbbf14f1617e718fae7f3 bionic/linux-oracle-5.4) > CVE-2020-25705 > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> -- cut -- > > I chose bionic/linux-oracle-5.4 for this cherry-pick since the patch had already > been backported, regression tested, and released; thereby reducing the chances of backporting > error or regression. -- cut -- > --- When this gets applied, lets cut off above verbatim section. -Stefan > Documentation/networking/ip-sysctl.txt | 4 +++- > net/ipv4/icmp.c | 7 +++++-- > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt > index 5f53faff4e25..151d6ad0e0a7 100644 > --- a/Documentation/networking/ip-sysctl.txt > +++ b/Documentation/networking/ip-sysctl.txt > @@ -1005,12 +1005,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 > 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); >
On 23.02.21 14:52, Tim Gardner wrote: > From: Eric Dumazet <edumazet@google.com> > > BugLink: https://bugs.launchpad.net/bugs/1902115 > > [ Upstream commit b38e7819cae946e2edf869e604af1e65a5d241c5 ] > > 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> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Kamal Mostafa <kamal@canonical.com> > Signed-off-by: Ian May <ian.may@canonical.com> > (cherry picked from commit fc3d57b1e860f9bdd97bbbf14f1617e718fae7f3 bionic/linux-oracle-5.4) > CVE-2020-25705 > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> > > I chose bionic/linux-oracle-5.4 for this cherry-pick since the patch had already > been backported, regression tested, and released; thereby reducing the chances of backporting > error or regression. Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > Documentation/networking/ip-sysctl.txt | 4 +++- > net/ipv4/icmp.c | 7 +++++-- > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt > index 5f53faff4e25..151d6ad0e0a7 100644 > --- a/Documentation/networking/ip-sysctl.txt > +++ b/Documentation/networking/ip-sysctl.txt > @@ -1005,12 +1005,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 > 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); >
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt index 5f53faff4e25..151d6ad0e0a7 100644 --- a/Documentation/networking/ip-sysctl.txt +++ b/Documentation/networking/ip-sysctl.txt @@ -1005,12 +1005,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 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);