From nobody Thu Jan 30 11:02:03 2025 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 4YkGLg4rkVz5lybT; Thu, 30 Jan 2025 11:02:03 +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 "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4YkGLg2tNDz3gpk; Thu, 30 Jan 2025 11:02:03 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1738234923; 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=Ep6AfEjNyxC4jIYPl3md/z1agIKgNsCsjQtd5gqwEFE=; b=iduALwqbY7Xs0a3KpXE/UfLrBl6FSdB7i2TqmoXDYppI2UC4RgjjFT4FkoF5MtfvyIqnxF bVMsPx4ddchKpLNxt9lOhp7PPLqtFcGCmS5lNgHxQW18G7+il6iMvbUoAJERktT2XHiomt Alb0nXBd81X4E3341iWz2mezQX0WdjxNsXybHuj9o5mExTUdFPYsMr52dnKflLlQKcWglA fSWIwx7vDThN+dFxfOpIXZTKZTPxKJDpf3FTuRNG1zMyGXzoxj/QuJpV59MMZ0oxzhSuuu thbgSL5lKMoU2AjCkywC0chDJ6pNGWMw2b5SknLESlTXJwHTToYONrq7hAja0w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1738234923; 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=Ep6AfEjNyxC4jIYPl3md/z1agIKgNsCsjQtd5gqwEFE=; b=pLi/mzdw0nO+5sMsBKCaK0+mtfcf4lPXcAtm9iQbIqoJygoHWmGGDeC33auZFHdFmIxcTw z2+z2hYERPWiBk7QytGwT9Qrl/jLETgbvYxur1halNxzDKv15I+G68gxLM0g3oYFlUkuLs RQmYefxF9JT0l4woDfCsJbUWu6TfTn2T/pwQi9Hs3uRcH1igZPsd4Hz0xvY1C0Eth55YGw XnVSRZkO8TPImg6Vz+RX+OLLMLSGMNYAUgfkiiLpmkGeHVOeAWe5EkQO4BPtkGZsDRi4r0 74WCA1Dsy6AhwOJUqVr+XxXMAh0jauRqJX+Q60ppUXIgkiceM9nbujNMIUvaoQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1738234923; a=rsa-sha256; cv=none; b=V4ZvbcsDXV3Z+a/7/1dmuoDviFSjz7nPH7Q955HamdYD082PL2bnG7uh0kaaiSNMGel+r+ 8uAg7mfS2JnoVDxH7eINQJZLa/r16fZBPqwLWWph2QuPpNyni4M4XzfaNMe/3wdPWi82QG mK+1L1Z1aA0VR34/Qc0LT7MfqpgyrZuQc6dNrX0rBtJHBwFFFyF91UqFOo5SfYUD3dpBPZ wQEyo0Y0EcZMLFbbx9uHff+qyiStE7Lj01h3ww6Nqkd3VAwz1hUaGaTCCeUfn7iHxwy9qr 9cBZswt/BBD436WhPELrDSv0WgtBI1NpQiA0f1d82VKAx7ISjeYfQAy4Ol8TNQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none 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 4YkGLg2T5qzkYQ; Thu, 30 Jan 2025 11:02:03 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 50UB23cW077987; Thu, 30 Jan 2025 11:02:03 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 50UB23lQ077984; Thu, 30 Jan 2025 11:02:03 GMT (envelope-from git) Date: Thu, 30 Jan 2025 11:02:03 GMT Message-Id: <202501301102.50UB23lQ077984@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Kristof Provost Subject: git: 491f5e37ae45 - stable/14 - pf: add 'allow-related' to always allow SCTP multihome extra connections 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: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/stable/14 X-Git-Reftype: branch X-Git-Commit: 491f5e37ae45deae613b52729bf521a91dc38292 Auto-Submitted: auto-generated The branch stable/14 has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=491f5e37ae45deae613b52729bf521a91dc38292 commit 491f5e37ae45deae613b52729bf521a91dc38292 Author: Kristof Provost AuthorDate: 2025-01-09 20:28:46 +0000 Commit: Kristof Provost CommitDate: 2025-01-30 11:00:31 +0000 pf: add 'allow-related' to always allow SCTP multihome extra connections Allow users to choose to allow permitted SCTP connections to set up additional multihomed connections regardless of the ruleset. That is, allow an already established connection to set up flows that would otherwise be disallowed. In case of if-bound connections we initially set the extra associations to be floating, because we don't know what path they'll be taking when they're created. Once we see the first traffic we can bind them. MFC after: 2 weeks Sponsored by: Orange Business Services Differential Revision: https://reviews.freebsd.org/D48453 (cherry picked from commit e4f2733df8c9d2fd0c5e8fdc8bec002bf39811f3) --- sbin/pfctl/parse.y | 22 +++++++++++-- share/man/man5/pf.conf.5 | 6 +++- sys/net/pfvar.h | 1 + sys/netpfil/pf/pf.c | 37 +++++++++++++++++++-- sys/netpfil/pf/pf.h | 1 + tests/sys/netpfil/pf/sctp.py | 76 ++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 137 insertions(+), 6 deletions(-) diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y index 17227b674814..ddee0bda8b9a 100644 --- a/sbin/pfctl/parse.y +++ b/sbin/pfctl/parse.y @@ -172,7 +172,8 @@ enum { PF_STATE_OPT_MAX, PF_STATE_OPT_NOSYNC, PF_STATE_OPT_SRCTRACK, PF_STATE_OPT_MAX_SRC_STATES, PF_STATE_OPT_MAX_SRC_CONN, PF_STATE_OPT_MAX_SRC_CONN_RATE, PF_STATE_OPT_MAX_SRC_NODES, PF_STATE_OPT_OVERLOAD, PF_STATE_OPT_STATELOCK, - PF_STATE_OPT_TIMEOUT, PF_STATE_OPT_SLOPPY, }; + PF_STATE_OPT_TIMEOUT, PF_STATE_OPT_SLOPPY, + PF_STATE_OPT_ALLOW_RELATED }; enum { PF_SRCTRACK_NONE, PF_SRCTRACK, PF_SRCTRACK_GLOBAL, PF_SRCTRACK_RULE }; @@ -512,7 +513,7 @@ int parseport(char *, struct range *r, int); %token DNPIPE DNQUEUE RIDENTIFIER %token LOAD RULESET_OPTIMIZATION PRIO %token STICKYADDRESS MAXSRCSTATES MAXSRCNODES SOURCETRACK GLOBAL RULE -%token MAXSRCCONN MAXSRCCONNRATE OVERLOAD FLUSH SLOPPY +%token MAXSRCCONN MAXSRCCONNRATE OVERLOAD FLUSH SLOPPY ALLOW_RELATED %token TAGGED TAG IFBOUND FLOATING STATEPOLICY STATEDEFAULTS ROUTE SETTOS %token DIVERTTO DIVERTREPLY BRIDGE_TO %token STRING @@ -2615,6 +2616,14 @@ pfrule : action dir logquick interface route af proto fromto } r.rule_flag |= PFRULE_STATESLOPPY; break; + case PF_STATE_OPT_ALLOW_RELATED: + if (r.rule_flag & PFRULE_ALLOW_RELATED) { + yyerror("state allow-related option: " + "multiple definitions"); + YYERROR; + } + r.rule_flag |= PFRULE_ALLOW_RELATED; + break; case PF_STATE_OPT_TIMEOUT: if (o->data.timeout.number == PFTM_ADAPTIVE_START || @@ -4368,6 +4377,14 @@ state_opt_item : MAXIMUM NUMBER { $$->next = NULL; $$->tail = $$; } + | ALLOW_RELATED { + $$ = calloc(1, sizeof(struct node_state_opt)); + if ($$ == NULL) + err(1, "state_opt_item: calloc"); + $$->type = PF_STATE_OPT_ALLOW_RELATED; + $$->next = NULL; + $$->tail = $$; + } | STRING NUMBER { int i; @@ -6240,6 +6257,7 @@ lookup(char *s) static const struct keywords keywords[] = { { "all", ALL}, { "allow-opts", ALLOWOPTS}, + { "allow-related", ALLOW_RELATED}, { "altq", ALTQ}, { "anchor", ANCHOR}, { "antispoof", ANTISPOOF}, diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5 index 935a70301d88..96deca5788c8 100644 --- a/share/man/man5/pf.conf.5 +++ b/share/man/man5/pf.conf.5 @@ -27,7 +27,7 @@ .\" ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE .\" POSSIBILITY OF SUCH DAMAGE. .\" -.Dd June 24, 2024 +.Dd January 10, 2025 .Dt PF.CONF 5 .Os .Sh NAME @@ -2438,6 +2438,10 @@ easier. This is intended to be used in situations where one does not see all packets of a connection, e.g. in asymmetric routing situations. Cannot be used with modulate or synproxy state. +.It Ar allow-related +Automatically allow connections related to this one, regardless of rules that +might otherwise affect them. +This currently only applies to SCTP multihomed connection. .El .Pp Multiple options can be specified, separated by commas: diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index 465e47b940b3..30050896b44e 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -1592,6 +1592,7 @@ struct pf_pdesc { #define PFDESC_SCTP_ADD_IP 0x1000 u_int16_t sctp_flags; u_int32_t sctp_initiate_tag; + struct pf_krule *related_rule; struct pf_sctp_multihome_jobs sctp_multihome_jobs; }; diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index 15569a294f98..81e942085ad2 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -434,8 +434,23 @@ enum { PF_ICMP_MULTI_NONE, PF_ICMP_MULTI_LINK }; return (PF_PASS); \ } while (0) -#define BOUND_IFACE(r, k) \ - ((r)->rule_flag & PFRULE_IFBOUND) ? (k) : V_pfi_all +static struct pfi_kkif * +BOUND_IFACE(struct pf_krule *rule, struct pfi_kkif *k, struct pf_pdesc *pd) +{ + /* Floating unless otherwise specified. */ + if (! (rule->rule_flag & PFRULE_IFBOUND)) + return (V_pfi_all); + + /* + * If this state is created based on another state (e.g. SCTP + * multihome) always set it floating initially. We can't know for sure + * what interface the actual traffic for this state will come in on. + */ + if (pd->related_rule) + return (V_pfi_all); + + return (k); +} #define STATE_INC_COUNTERS(s) \ do { \ @@ -4895,6 +4910,10 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, struct pfi_kkif *kif, } while (r != NULL) { + if (pd->related_rule) { + *rm = pd->related_rule; + break; + } pf_counter_u64_add(&r->evaluations, 1); if (pfi_kkif_match(r->kif, kif) == r->ifnot) r = r->skip[PF_SKIP_IFP].ptr; @@ -5257,7 +5276,7 @@ pf_create_state(struct pf_krule *r, struct pf_krule *nr, struct pf_krule *a, __func__, nr, sk, nk)); /* Swap sk/nk for PF_OUT. */ - if (pf_state_insert(BOUND_IFACE(r, kif), kif, + if (pf_state_insert(BOUND_IFACE(r, kif, pd), kif, (pd->dir == PF_IN) ? sk : nk, (pd->dir == PF_IN) ? nk : sk, s)) { REASON_SET(&reason, PFRES_STATEINS); @@ -6220,6 +6239,15 @@ pf_test_state_sctp(struct pf_kstate **state, struct pfi_kkif *kif, dst->scrub->pfss_v_tag = pd->sctp_initiate_tag; } + /* + * Bind to the correct interface if we're if-bound. For multihomed + * extra associations we don't know which interface that will be until + * here, so we've inserted the state on V_pf_all. Fix that now. + */ + if ((*state)->kif == V_pfi_all && + (*state)->rule.ptr->rule_flag & PFRULE_IFBOUND) + (*state)->kif = kif; + if (pd->sctp_flags & (PFDESC_SCTP_COOKIE | PFDESC_SCTP_HEARTBEAT_ACK)) { if (src->state < SCTP_ESTABLISHED) { pf_set_protostate(*state, psrc, SCTP_ESTABLISHED); @@ -6416,6 +6444,9 @@ again: j->pd.sctp_flags |= PFDESC_SCTP_ADD_IP; PF_RULES_RLOCK(); sm = NULL; + if (s->rule.ptr->rule_flag & PFRULE_ALLOW_RELATED) { + j->pd.related_rule = s->rule.ptr; + } ret = pf_test_rule(&r, &sm, kif, j->m, off, &j->pd, &ra, &rs, NULL); PF_RULES_RUNLOCK(); diff --git a/sys/netpfil/pf/pf.h b/sys/netpfil/pf/pf.h index dd9796b59ce9..cb122763b4a1 100644 --- a/sys/netpfil/pf/pf.h +++ b/sys/netpfil/pf/pf.h @@ -614,6 +614,7 @@ struct pf_rule { #define PFRULE_SET_TOS 0x00002000 #define PFRULE_IFBOUND 0x00010000 /* if-bound */ #define PFRULE_STATESLOPPY 0x00020000 /* sloppy state tracking */ +#define PFRULE_ALLOW_RELATED 0x00080000 #ifdef _KERNEL #define PFRULE_REFS 0x0080 /* rule has references */ diff --git a/tests/sys/netpfil/pf/sctp.py b/tests/sys/netpfil/pf/sctp.py index 38bb9f2dff74..230dbae0d327 100644 --- a/tests/sys/netpfil/pf/sctp.py +++ b/tests/sys/netpfil/pf/sctp.py @@ -426,6 +426,82 @@ class TestSCTP(VnetTestTemplate): assert re.search(r"all sctp 192.0.2.4:.*192.0.2.3:1234", states) assert re.search(r"all sctp 192.0.2.4:.*192.0.2.2:1234", states) + @pytest.mark.require_user("root") + def test_disallow_related(self): + srv_vnet = self.vnet_map["vnet2"] + + ToolsHelper.print_output("/sbin/pfctl -e") + ToolsHelper.pf_rules([ + "block proto sctp", + "pass inet proto sctp to 192.0.2.3", + "pass on lo"]) + + # Sanity check, we can communicate with the primary address. + client = SCTPClient("192.0.2.3", 1234) + client.send(b"hello", 0) + rcvd = self.wait_object(srv_vnet.pipe) + print(rcvd) + assert rcvd['ppid'] == 0 + assert rcvd['data'] == "hello" + + # This shouldn't work + success=False + try: + client.newpeer("192.0.2.2") + client.send(b"world", 0) + rcvd = self.wait_object(srv_vnet.pipe) + print(rcvd) + assert rcvd['ppid'] == 0 + assert rcvd['data'] == "world" + success=True + except: + success=False + assert not success + + # Check that we have a state for 192.0.2.3, but not 192.0.2.2 to 192.0.2.1 + states = ToolsHelper.get_output("/sbin/pfctl -ss") + assert re.search(r"all sctp 192.0.2.1:.*192.0.2.3:1234", states) + assert not re.search(r"all sctp 192.0.2.1:.*192.0.2.2:1234", states) + + @pytest.mark.require_user("root") + def test_allow_related(self): + srv_vnet = self.vnet_map["vnet2"] + + ToolsHelper.print_output("/sbin/pfctl -e") + ToolsHelper.pf_rules([ + "set state-policy if-bound", + "block proto sctp", + "pass inet proto sctp to 192.0.2.3 keep state (allow-related)", + "pass on lo"]) + + # Sanity check, we can communicate with the primary address. + client = SCTPClient("192.0.2.3", 1234) + client.send(b"hello", 0) + rcvd = self.wait_object(srv_vnet.pipe) + print(rcvd) + assert rcvd['ppid'] == 0 + assert rcvd['data'] == "hello" + + success=False + try: + client.newpeer("192.0.2.2") + client.send(b"world", 0) + rcvd = self.wait_object(srv_vnet.pipe) + print(rcvd) + assert rcvd['ppid'] == 0 + assert rcvd['data'] == "world" + success=True + finally: + # Debug output + ToolsHelper.print_output("/sbin/pfctl -ss") + ToolsHelper.print_output("/sbin/pfctl -sr -vv") + assert success + + # Check that we have a state for 192.0.2.3 and 192.0.2.2 to 192.0.2.1 + states = ToolsHelper.get_output("/sbin/pfctl -ss") + assert re.search(r"epair.*sctp 192.0.2.1:.*192.0.2.3:1234", states) + assert re.search(r"epair.*sctp 192.0.2.1:.*192.0.2.2:1234", states) + class TestSCTPv6(VnetTestTemplate): REQUIRED_MODULES = ["sctp", "pf"] TOPOLOGY = {