From nobody Sun Mar 24 16:19:30 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4V2h8y4Tqvz5G0JB; Sun, 24 Mar 2024 16:19:30 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4V2h8y2XC1z4GFb; Sun, 24 Mar 2024 16:19:30 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1711297170; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=M8ISP+AdZ1G83W+GxY+j+N7E4dte/lGe1iun2o2MoLw=; b=gLjXxH8viC3brnSTpcD0i2XflyObeNZfYUzd2MUf7pssaMYcYuHOiwTdAXJsC4fhcmd1m+ gHZGLgt/fnpnwn+Zy4nXyM8VjUPlt6LS1bWHSthueDM2UF0q0CE92vUWFn5v+Z/xed4e+W v+aAkZCb+4AWzTUBbv3SGR2Tr0pA7lTBNNafMD+RGqrLrqzKCmc+oPYVhbZ4Rwf8JylLlg 5EKNcwC74k97/ZAUH3SPV9RKL4YwP0WRGJWmJOLCmnGdO96BxT3H9BqahcK/GY08rvJOoh +vu0OPHQAjO3H3HxNgYPbkVFDFrv1bakOTDP8Jt8XuIcRb2GKHXs04QlUrOvdw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1711297170; a=rsa-sha256; cv=none; b=RVl84mTpZ/jw26MHTgGk2OWtQ4UfqAOPOWSu0K2glZiceveVO0Virinw0Faol1H9wz2BOl m24UovAZNkekEUZ6SgQkG4Ubi8MCY1gx4A5m2tXLuIM6Zab55xoBA9i/vOr4yiArRPI6Ya 8CzxLxYPovS3JFZVPJxj3CwHoYoliPFt0O2bJoASErppXhoITwAD4L74wus46LhNeYhZu+ 1HAm+rmzMY7eGnic35JJ33tdaquXxu5MYJCfr7pLA7ktZu/Epz+xyG5+KeHo4yUkhXniVd 0Y0/49PImCCFxwxCpjl6XQwayfIHAdlpLhzYchiwlszXcxjB12VgSCPKE0xg4w== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1711297170; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=M8ISP+AdZ1G83W+GxY+j+N7E4dte/lGe1iun2o2MoLw=; b=kcMtLvJ0zjKh8ay2zYCGUf4T6ST6e7medi89deSaF9pztCbz4mAjZg8zCg+jL6i5f4jdGO p2Eqxc/l+EnItZ/IVwFAk9o4W4Znz9MJ3KzyBd/rpMdb07sEd8r0tyYIhz5XyDjBzsokCj 88fWGLS46Fu/60KWmnwOSE/9NpvPe8exJoGSkZ61E8Jb/B13g1ALsr1hmmVurq8102skjj uhxsRTLKgdJxY/tq1giHsdvgH44zGGDjGQ4rcnLw6Qo+vPbq/utGBUV7Gn5M+1KeELaKis Iv730jnqOQtdpujDs8YL31VtMEYuI4xRMlaFiblKXUug+RPNYiuiw57RyeSHlw== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4V2h8y28F3zfwN; Sun, 24 Mar 2024 16:19:30 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 42OGJUGo079454; Sun, 24 Mar 2024 16:19:30 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 42OGJUuJ079451; Sun, 24 Mar 2024 16:19:30 GMT (envelope-from git) Date: Sun, 24 Mar 2024 16:19:30 GMT Message-Id: <202403241619.42OGJUuJ079451@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Gleb Smirnoff Subject: git: ac44739fd834 - main - icmp: improve ICMP limit jitter List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: glebius X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: ac44739fd834f51cacb26485a4140fd482e20150 Auto-Submitted: auto-generated The branch main has been updated by glebius: URL: https://cgit.FreeBSD.org/src/commit/?id=ac44739fd834f51cacb26485a4140fd482e20150 commit ac44739fd834f51cacb26485a4140fd482e20150 Author: Gleb Smirnoff AuthorDate: 2024-03-24 16:13:23 +0000 Commit: Gleb Smirnoff CommitDate: 2024-03-24 16:13:23 +0000 icmp: improve ICMP limit jitter Instead of fixing up invalid values set by a user in badport_bandlim() which is a fast path function, provide a sysctl handler sysctl_icmplim_and_jitter(), that will check that jitter is less than the limit. Provide jitter initilization function icmplim_new_jitter() used at boot, in the sysctl handler and when we actually hit the limit. This also fixes no jitter on a fresh booted system until first limit hit. Instead of CVE number provide link the the actual paper that explains what and why we are doing here. The CVE number isn't very informative, it will just tell you what RedHat version you need to upgrade to. Reviewed by: kp, tuexen, zlei Differential Revision: https://reviews.freebsd.org/D44478 --- sys/netinet/ip_icmp.c | 81 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 59 insertions(+), 22 deletions(-) diff --git a/sys/netinet/ip_icmp.c b/sys/netinet/ip_icmp.c index 245e1c8040a4..acb707b1d192 100644 --- a/sys/netinet/ip_icmp.c +++ b/sys/netinet/ip_icmp.c @@ -80,19 +80,22 @@ extern ipproto_ctlinput_t *ip_ctlprotox[]; * routines to turnaround packets back to the originator, and * host table maintenance routines. */ -VNET_DEFINE_STATIC(int, icmplim) = 200; +static int sysctl_icmplim_and_jitter(SYSCTL_HANDLER_ARGS); +VNET_DEFINE_STATIC(u_int, icmplim) = 200; #define V_icmplim VNET(icmplim) -SYSCTL_INT(_net_inet_icmp, ICMPCTL_ICMPLIM, icmplim, CTLFLAG_VNET | CTLFLAG_RW, - &VNET_NAME(icmplim), 0, - "Maximum number of ICMP responses per second"); +SYSCTL_PROC(_net_inet_icmp, ICMPCTL_ICMPLIM, icmplim, CTLTYPE_UINT | + CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(icmplim), 0, + &sysctl_icmplim_and_jitter, "IU", + "Maximum number of ICMP responses per second"); VNET_DEFINE_STATIC(int, icmplim_curr_jitter) = 0; #define V_icmplim_curr_jitter VNET(icmplim_curr_jitter) -VNET_DEFINE_STATIC(int, icmplim_jitter) = 16; +VNET_DEFINE_STATIC(u_int, icmplim_jitter) = 16; #define V_icmplim_jitter VNET(icmplim_jitter) -SYSCTL_INT(_net_inet_icmp, OID_AUTO, icmplim_jitter, CTLFLAG_VNET | CTLFLAG_RW, - &VNET_NAME(icmplim_jitter), 0, - "Random icmplim jitter adjustment limit"); +SYSCTL_PROC(_net_inet_icmp, OID_AUTO, icmplim_jitter, CTLTYPE_UINT | + CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(icmplim_jitter), 0, + &sysctl_icmplim_and_jitter, "IU", + "Random icmplim jitter adjustment limit"); VNET_DEFINE_STATIC(int, icmplim_output) = 1; #define V_icmplim_output VNET(icmplim_output) @@ -1104,6 +1107,52 @@ static const char *icmp_rate_descrs[BANDLIM_MAX] = { [BANDLIM_SCTP_OOTB] = "sctp ootb", }; +static void +icmplim_new_jitter(void) +{ + /* + * Adjust limit +/- to jitter the measurement to deny a side-channel + * port scan as in https://dl.acm.org/doi/10.1145/3372297.3417280 + */ + if (V_icmplim_jitter > 0) + V_icmplim_curr_jitter = + arc4random_uniform(V_icmplim_jitter * 2 + 1) - + V_icmplim_jitter; +} + +static int +sysctl_icmplim_and_jitter(SYSCTL_HANDLER_ARGS) +{ + uint32_t new; + int error; + bool lim; + + MPASS(oidp->oid_arg1 == &VNET_NAME(icmplim) || + oidp->oid_arg1 == &VNET_NAME(icmplim_jitter)); + + lim = (oidp->oid_arg1 == &VNET_NAME(icmplim)); + new = lim ? V_icmplim : V_icmplim_jitter; + error = sysctl_handle_int(oidp, &new, 0, req); + if (error == 0 && req->newptr) { + if (lim) { + if (new <= V_icmplim_jitter) + error = EINVAL; + else + V_icmplim = new; + } else { + if (new >= V_icmplim) + error = EINVAL; + else { + V_icmplim_jitter = new; + icmplim_new_jitter(); + } + } + } + MPASS(V_icmplim + V_icmplim_curr_jitter > 0); + + return (error); +} + static void icmp_bandlimit_init(void) { @@ -1112,6 +1161,7 @@ icmp_bandlimit_init(void) V_icmp_rates[i].cr_rate = counter_u64_alloc(M_WAITOK); V_icmp_rates[i].cr_ticks = ticks; } + icmplim_new_jitter(); } VNET_SYSINIT(icmp_bandlimit, SI_SUB_PROTO_DOMAIN, SI_ORDER_ANY, icmp_bandlimit_init, NULL); @@ -1139,9 +1189,6 @@ badport_bandlim(int which) KASSERT(which >= 0 && which < BANDLIM_MAX, ("%s: which %d", __func__, which)); - if ((V_icmplim + V_icmplim_curr_jitter) <= 0) - V_icmplim_curr_jitter = -V_icmplim + 1; - pps = counter_ratecheck(&V_icmp_rates[which], V_icmplim + V_icmplim_curr_jitter); if (pps > 0) { @@ -1150,17 +1197,7 @@ badport_bandlim(int which) "Limiting %s response from %jd to %d packets/sec\n", icmp_rate_descrs[which], (intmax_t )pps, V_icmplim + V_icmplim_curr_jitter); - /* - * Adjust limit +/- to jitter the measurement to deny a - * side-channel port scan as in CVE-2020-25705 - */ - if (V_icmplim_jitter > 0) { - int32_t inc = - arc4random_uniform(V_icmplim_jitter * 2 +1) - - V_icmplim_jitter; - - V_icmplim_curr_jitter = inc; - } + icmplim_new_jitter(); } if (pps == -1) return (-1);