The branch releng/13.5 has been updated by tuexen:
URL:
https://cgit.FreeBSD.org/src/commit/?id=cd91df3292bf9f7a46ff9b8a49c87468844d90e3
commit cd91df3292bf9f7a46ff9b8a49c87468844d90e3
Author: Michael Tuexen <tue...@freebsd.org>
AuthorDate: 2025-02-10 21:16:20 +0000
Commit: Michael Tuexen <tue...@freebsd.org>
CommitDate: 2025-02-13 14:04:39 +0000
icmp: use per rate limit randomized jitter
Using the same random jitter for multiple rate limits allows an
attacker to use one rate limiter to figure out the current jitter
and then use this knowledge to de-randomize the other rate limiters.
This can be mitigated by using a separate randomized jitter for each
rate limiter.
This issue was reported as issue number 10 in Keyu Man et al.:
SCAD: Towards a Universal and Automated Network Side-Channel
Vulnerability Detection
Reviewed by: rrs, Peter Lei, glebius
Sponsored by: Netflix, Inc.
Differential Revision: https://reviews.freebsd.org/D48804
Approved by: re (cperciva)
(cherry picked from commit 923c223f27e792e51ca13c476428adbbf6887551)
(cherry picked from commit e1dd07ede92382bdcc52b3093e8f2ec5d9c88467)
---
sys/netinet/ip_icmp.c | 20 ++++++++++++--------
sys/netinet6/icmp6.c | 50 +++++++++++++++++++++++++++-----------------------
2 files changed, 39 insertions(+), 31 deletions(-)
diff --git a/sys/netinet/ip_icmp.c b/sys/netinet/ip_icmp.c
index f4b697f30eee..417a86672d6f 100644
--- a/sys/netinet/ip_icmp.c
+++ b/sys/netinet/ip_icmp.c
@@ -89,7 +89,7 @@ SYSCTL_PROC(_net_inet_icmp, ICMPCTL_ICMPLIM, icmplim,
CTLTYPE_UINT |
&sysctl_icmplim_and_jitter, "IU",
"Maximum number of ICMP responses per second");
-VNET_DEFINE_STATIC(int, icmplim_curr_jitter) = 0;
+VNET_DEFINE_STATIC(int, icmplim_curr_jitter[BANDLIM_MAX]) = {0};
#define V_icmplim_curr_jitter VNET(icmplim_curr_jitter)
VNET_DEFINE_STATIC(u_int, icmplim_jitter) = 16;
#define V_icmplim_jitter VNET(icmplim_jitter)
@@ -1102,14 +1102,16 @@ static const char *icmp_rate_descrs[BANDLIM_MAX] = {
};
static void
-icmplim_new_jitter(void)
+icmplim_new_jitter(int which)
{
/*
* Adjust limit +/- to jitter the measurement to deny a side-channel
* port scan as in https://dl.acm.org/doi/10.1145/3372297.3417280
*/
+ KASSERT(which >= 0 && which < BANDLIM_MAX,
+ ("%s: which %d", __func__, which));
if (V_icmplim_jitter > 0)
- V_icmplim_curr_jitter =
+ V_icmplim_curr_jitter[which] =
arc4random_uniform(V_icmplim_jitter * 2 + 1) -
V_icmplim_jitter;
}
@@ -1138,7 +1140,9 @@ sysctl_icmplim_and_jitter(SYSCTL_HANDLER_ARGS)
error = EINVAL;
else {
V_icmplim_jitter = new;
- icmplim_new_jitter();
+ for (int i = 0; i < BANDLIM_MAX; i++) {
+ icmplim_new_jitter(i);
+ }
}
}
}
@@ -1154,8 +1158,8 @@ icmp_bandlimit_init(void)
for (int i = 0; i < BANDLIM_MAX; i++) {
V_icmp_rates[i].cr_rate = counter_u64_alloc(M_WAITOK);
V_icmp_rates[i].cr_ticks = ticks;
+ icmplim_new_jitter(i);
}
- icmplim_new_jitter();
}
VNET_SYSINIT(icmp_bandlimit, SI_SUB_PROTO_DOMAIN, SI_ORDER_ANY,
icmp_bandlimit_init, NULL);
@@ -1184,14 +1188,14 @@ badport_bandlim(int which)
("%s: which %d", __func__, which));
pps = counter_ratecheck(&V_icmp_rates[which], V_icmplim +
- V_icmplim_curr_jitter);
+ V_icmplim_curr_jitter[which]);
if (pps > 0) {
if (V_icmplim_output)
log(LOG_NOTICE,
"Limiting %s response from %jd to %d packets/sec\n",
icmp_rate_descrs[which], (intmax_t )pps,
- V_icmplim + V_icmplim_curr_jitter);
- icmplim_new_jitter();
+ V_icmplim + V_icmplim_curr_jitter[which]);
+ icmplim_new_jitter(which);
}
if (pps == -1)
return (-1);
diff --git a/sys/netinet6/icmp6.c b/sys/netinet6/icmp6.c
index 52430f9146ae..dd0044734c4b 100644
--- a/sys/netinet6/icmp6.c
+++ b/sys/netinet6/icmp6.c
@@ -2778,22 +2778,6 @@ SYSCTL_PROC(_net_inet6_icmp6, ICMPV6CTL_ERRPPSLIMIT,
errppslimit,
&sysctl_icmp6lim_and_jitter, "IU",
"Maximum number of ICMPv6 error/reply messages per second");
-VNET_DEFINE_STATIC(int, icmp6lim_curr_jitter) = 0;
-#define V_icmp6lim_curr_jitter VNET(icmp6lim_curr_jitter)
-
-VNET_DEFINE_STATIC(u_int, icmp6lim_jitter) = 8;
-#define V_icmp6lim_jitter VNET(icmp6lim_jitter)
-SYSCTL_PROC(_net_inet6_icmp6, OID_AUTO, icmp6lim_jitter, CTLTYPE_UINT |
- CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(icmp6lim_jitter), 0,
- &sysctl_icmp6lim_and_jitter, "IU",
- "Random errppslimit jitter adjustment limit");
-
-VNET_DEFINE_STATIC(int, icmp6lim_output) = 1;
-#define V_icmp6lim_output VNET(icmp6lim_output)
-SYSCTL_INT(_net_inet6_icmp6, OID_AUTO, icmp6lim_output,
- CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(icmp6lim_output), 0,
- "Enable logging of ICMPv6 response rate limiting");
-
typedef enum {
RATELIM_PARAM_PROB = 0,
RATELIM_TOO_BIG,
@@ -2815,15 +2799,33 @@ static const char *icmp6_rate_descrs[RATELIM_MAX] = {
[RATELIM_OTHER] = "(other)",
};
+VNET_DEFINE_STATIC(int, icmp6lim_curr_jitter[RATELIM_MAX]) = {0};
+#define V_icmp6lim_curr_jitter VNET(icmp6lim_curr_jitter)
+
+VNET_DEFINE_STATIC(u_int, icmp6lim_jitter) = 8;
+#define V_icmp6lim_jitter VNET(icmp6lim_jitter)
+SYSCTL_PROC(_net_inet6_icmp6, OID_AUTO, icmp6lim_jitter, CTLTYPE_UINT |
+ CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(icmp6lim_jitter), 0,
+ &sysctl_icmp6lim_and_jitter, "IU",
+ "Random errppslimit jitter adjustment limit");
+
+VNET_DEFINE_STATIC(int, icmp6lim_output) = 1;
+#define V_icmp6lim_output VNET(icmp6lim_output)
+SYSCTL_INT(_net_inet6_icmp6, OID_AUTO, icmp6lim_output,
+ CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(icmp6lim_output), 0,
+ "Enable logging of ICMPv6 response rate limiting");
+
static void
-icmp6lim_new_jitter(void)
+icmp6lim_new_jitter(int which)
{
/*
* Adjust limit +/- to jitter the measurement to deny a side-channel
* port scan as in https://dl.acm.org/doi/10.1145/3372297.3417280
*/
+ KASSERT(which >= 0 && which < RATELIM_MAX,
+ ("%s: which %d", __func__, which));
if (V_icmp6lim_jitter > 0)
- V_icmp6lim_curr_jitter =
+ V_icmp6lim_curr_jitter[which] =
arc4random_uniform(V_icmp6lim_jitter * 2 + 1) -
V_icmp6lim_jitter;
}
@@ -2852,7 +2854,9 @@ sysctl_icmp6lim_and_jitter(SYSCTL_HANDLER_ARGS)
error = EINVAL;
else {
V_icmp6lim_jitter = new;
- icmp6lim_new_jitter();
+ for (int i = 0; i < RATELIM_MAX; i++) {
+ icmp6lim_new_jitter(i);
+ }
}
}
}
@@ -2872,8 +2876,8 @@ icmp6_ratelimit_init(void)
for (int i = 0; i < RATELIM_MAX; i++) {
V_icmp6_rates[i].cr_rate = counter_u64_alloc(M_WAITOK);
V_icmp6_rates[i].cr_ticks = ticks;
+ icmp6lim_new_jitter(i);
}
- icmp6lim_new_jitter();
}
VNET_SYSINIT(icmp6_ratelimit, SI_SUB_PROTO_DOMAIN, SI_ORDER_ANY,
icmp6_ratelimit_init, NULL);
@@ -2935,14 +2939,14 @@ icmp6_ratelimit(const struct in6_addr *dst, const int
type, const int code)
};
pps = counter_ratecheck(&V_icmp6_rates[which], V_icmp6errppslim +
- V_icmp6lim_curr_jitter);
+ V_icmp6lim_curr_jitter[which]);
if (pps > 0) {
if (V_icmp6lim_output)
log(LOG_NOTICE, "Limiting ICMPv6 %s output from %jd "
"to %d packets/sec\n", icmp6_rate_descrs[which],
(intmax_t )pps, V_icmp6errppslim +
- V_icmp6lim_curr_jitter);
- icmp6lim_new_jitter();
+ V_icmp6lim_curr_jitter[which]);
+ icmp6lim_new_jitter(which);
}
if (pps == -1) {
ICMP6STAT_INC(icp6s_toofreq);
