From nobody Mon Aug 19 14:37:40 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 4WnZv84QT6z5T8Bk; Mon, 19 Aug 2024 14:37:40 +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 4WnZv82DSvz4tv2; Mon, 19 Aug 2024 14:37:40 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1724078260; 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=gyj6DZX0JWhknLtbFvZ7JEDBDEqTjSrbEhfA3ZhrLCw=; b=u8RUE5nT2UTTzG5MLitsdl7oTt+Z0WgrmF4+o8xp5z6UwM8cO7BOuTJFD7UsbZ65bE9/Wt 5k2ffeQIRWSdLTjh6Kptm+KJbuBWE0hfYGLz6d2rcVT6kHBq+j3Vft46B0TCm/uBvo9/hN 2Q2v2vuTWm9tllkllG+bdrj32qWF1XIyUFQF6/BjMqL8zJLIBIUhkuYyPlrAykDsDgdvIk lff3oGC0uShqEY9kPEP4otFOWnnx1FHIq904VBI4MxWXphAh2FmX67NCm24SwFPk5dsq/Q 9s3NkOCUC8qkaEV0uCwu/CWUzoR5gjKVMiQYQ69qb1KJuJEDu/kpBa1Q03/lDw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1724078260; a=rsa-sha256; cv=none; b=k89ble7nt6r+3pzxDbZsaaxDPWrYs7qmtOAuMwbLOkHTZuY8DYicN59Fjx5bcR+SkNIZ+6 ewdbIlL+ZlUkJlIFhal6TFTwL1Eun7TnlpLQpD027hKNr6fqmMZ/YfBQAyL+wx6PRcSmAd 3NfM/t4z/gC1OSbYl6KQnpeOTXZuK+JmLRM2jhOmwFcHPfbakMfdTF8HDtO4AfjOsC+IZa uhbrfr/46uFyD7OdVS8nrfoLZF+C7qal06I42giotF5pAMgFrXpzjOg/YQ6HzhGFAeS6ov KkKBiE+xIk3CtPFCCSozIYY1RmbvCrWjPd61OFUC/TpmVuIKfhWqZgrQ8Qf6RQ== 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=1724078260; 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=gyj6DZX0JWhknLtbFvZ7JEDBDEqTjSrbEhfA3ZhrLCw=; b=NobiXsyJCNfk0fcendJMa43Pw/rqJehFcT0XjchJqIKPMAbGiafvHv6/HI1o5ki7HuI20/ 0pz5rBHIspaQ/NNaCBtKlFDep27iM2AaeEYP/+L3l6d+Tae3G5cus/LUnHBgrkzPatP08f 8CxF7SXf+ffbS5Fud1aPIgc3mGqiNEdKhmR1nlmeFJ3jSU7uZGGuWDiLUTgMApk0H+J6s5 yE2nr/mFPT1l9lb1lTX5ad6gRjIygoE4+aF9rbC5gsRvJLvzY+F1rtiSxeU6Uz92JVx4V9 V2+U5RSBpdasTJdEI8NZTCPJ7qkIpWu9rWZ75XX+elIUFOqKXoZ+19Z6HfP1CA== 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 4WnZv81KsBzLRV; Mon, 19 Aug 2024 14:37:40 +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 47JEbeid094141; Mon, 19 Aug 2024 14:37:40 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 47JEbeaf094138; Mon, 19 Aug 2024 14:37:40 GMT (envelope-from git) Date: Mon, 19 Aug 2024 14:37:40 GMT Message-Id: <202408191437.47JEbeaf094138@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mark Johnston Subject: git: 7e65cfc9bbe5 - main - pf: Make pf_get_translation() more expressive 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: markj X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 7e65cfc9bbe5a9d735ef38f7ed49965b234b8a20 Auto-Submitted: auto-generated The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=7e65cfc9bbe5a9d735ef38f7ed49965b234b8a20 commit 7e65cfc9bbe5a9d735ef38f7ed49965b234b8a20 Author: Mark Johnston AuthorDate: 2024-08-19 14:14:30 +0000 Commit: Mark Johnston CommitDate: 2024-08-19 14:37:27 +0000 pf: Make pf_get_translation() more expressive Currently pf_get_translation() returns a pointer to a matching nat/rdr/binat rule, or NULL if no rule was matched or an error occurred while applying the translation. That is, we don't distinguish between errors and the lack of a matching rule. This, if an error (e.g., a memory allocation failure or a state conflict) occurs, we simply handle the packet as if no translation rule was present. This is not desireable. Make pf_get_translation() return the matching rule as an out-param and instead return a reason code which indicates whether there was no translation rule, or there was a translation rule and we failed to apply it, or there was a translation rule and we applied it successfully. Reviewed by: kp, allanjude MFC after: 3 months Sponsored by: Klara, Inc. Sponsored by: Modirum Differential Revision: https://reviews.freebsd.org/D45672 --- sys/net/pfvar.h | 5 +++-- sys/netpfil/pf/pf.c | 18 ++++++++++++---- sys/netpfil/pf/pf_lb.c | 57 +++++++++++++++++++++++++++++++++----------------- 3 files changed, 55 insertions(+), 25 deletions(-) diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index 863883c2d61e..d66e6f799761 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -2569,11 +2569,12 @@ u_short pf_map_addr(u_int8_t, struct pf_krule *, struct pf_addr *, struct pf_addr *, struct pfi_kkif **nkif, struct pf_addr *, struct pf_ksrc_node **); -struct pf_krule *pf_get_translation(struct pf_pdesc *, struct mbuf *, +u_short pf_get_translation(struct pf_pdesc *, struct mbuf *, int, struct pfi_kkif *, struct pf_ksrc_node **, struct pf_state_key **, struct pf_state_key **, struct pf_addr *, struct pf_addr *, - uint16_t, uint16_t, struct pf_kanchor_stackframe *); + uint16_t, uint16_t, struct pf_kanchor_stackframe *, + struct pf_krule **); struct pf_state_key *pf_state_key_setup(struct pf_pdesc *, struct pf_addr *, struct pf_addr *, u_int16_t, u_int16_t); diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index 0547e29e04c2..2bbd231b3ee9 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -4605,7 +4605,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, struct pfi_kkif *kif, struct pf_ksrc_node *nsn = NULL; struct tcphdr *th = &pd->hdr.tcp; struct pf_state_key *sk = NULL, *nk = NULL; - u_short reason; + u_short reason, transerror; int rewrite = 0, hdrlen = 0; int tag = -1; int asd = 0; @@ -4618,6 +4618,8 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, struct pfi_kkif *kif, PF_RULES_RASSERT(); + SLIST_INIT(&match_rules); + if (inp != NULL) { INP_LOCK_ASSERT(inp); pd->lookup.uid = inp->inp_cred->cr_uid; @@ -4686,8 +4688,17 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, struct pfi_kkif *kif, r = TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_FILTER].active.ptr); /* check packet for BINAT/NAT/RDR */ - if ((nr = pf_get_translation(pd, m, off, kif, &nsn, &sk, - &nk, saddr, daddr, sport, dport, anchor_stack)) != NULL) { + transerror = pf_get_translation(pd, m, off, kif, &nsn, &sk, + &nk, saddr, daddr, sport, dport, anchor_stack, &nr); + switch (transerror) { + default: + /* A translation error occurred. */ + REASON_SET(&reason, transerror); + goto cleanup; + case PFRES_MAX: + /* No match. */ + break; + case PFRES_MATCH: KASSERT(sk != NULL, ("%s: null sk", __func__)); KASSERT(nk != NULL, ("%s: null nk", __func__)); @@ -4836,7 +4847,6 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, struct pfi_kkif *kif, pd->nat_rule = nr; } - SLIST_INIT(&match_rules); while (r != NULL) { pf_counter_u64_add(&r->evaluations, 1); if (pfi_kkif_match(r->kif, kif) == r->ifnot) diff --git a/sys/netpfil/pf/pf_lb.c b/sys/netpfil/pf/pf_lb.c index 4b703d3d02da..68fc76233dab 100644 --- a/sys/netpfil/pf/pf_lb.c +++ b/sys/netpfil/pf/pf_lb.c @@ -591,22 +591,26 @@ done: return (reason); } -struct pf_krule * +u_short pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, struct pfi_kkif *kif, struct pf_ksrc_node **sn, struct pf_state_key **skp, struct pf_state_key **nkp, struct pf_addr *saddr, struct pf_addr *daddr, - uint16_t sport, uint16_t dport, struct pf_kanchor_stackframe *anchor_stack) + uint16_t sport, uint16_t dport, struct pf_kanchor_stackframe *anchor_stack, + struct pf_krule **rp) { struct pf_krule *r = NULL; struct pf_addr *naddr; uint16_t *nportp; uint16_t low, high; + u_short reason; PF_RULES_RASSERT(); KASSERT(*skp == NULL, ("*skp not NULL")); KASSERT(*nkp == NULL, ("*nkp not NULL")); + *rp = NULL; + if (pd->dir == PF_OUT) { r = pf_match_translation(pd, m, off, kif, saddr, sport, daddr, dport, PF_RULESET_BINAT, anchor_stack); @@ -624,23 +628,23 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, } if (r == NULL) - return (NULL); + return (PFRES_MAX); switch (r->action) { case PF_NONAT: case PF_NOBINAT: case PF_NORDR: - return (NULL); + return (PFRES_MAX); } *skp = pf_state_key_setup(pd, saddr, daddr, sport, dport); if (*skp == NULL) - return (NULL); + return (PFRES_MEMORY); *nkp = pf_state_key_clone(*skp); if (*nkp == NULL) { uma_zfree(V_pf_state_key_z, *skp); *skp = NULL; - return (NULL); + return (PFRES_MEMORY); } naddr = &(*nkp)->addr[1]; @@ -664,6 +668,7 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, r->rpool.mape.offset, r->rpool.mape.psidlen, r->rpool.mape.psid)); + reason = PFRES_MAPFAILED; goto notrans; } } else if (pf_get_sport(pd->af, pd->proto, r, saddr, sport, @@ -671,6 +676,7 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, DPFPRINTF(PF_DEBUG_MISC, ("pf: NAT proxy port allocation (%u-%u) failed\n", r->rpool.proxy_port[0], r->rpool.proxy_port[1])); + reason = PFRES_MAPFAILED; goto notrans; } break; @@ -682,8 +688,10 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, #ifdef INET case AF_INET: if (r->rpool.cur->addr.p.dyn-> - pfid_acnt4 < 1) + pfid_acnt4 < 1) { + reason = PFRES_MAPFAILED; goto notrans; + } PF_POOLMASK(naddr, &r->rpool.cur->addr.p.dyn-> pfid_addr4, @@ -694,8 +702,10 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, #ifdef INET6 case AF_INET6: if (r->rpool.cur->addr.p.dyn-> - pfid_acnt6 < 1) + pfid_acnt6 < 1) { + reason = PFRES_MAPFAILED; goto notrans; + } PF_POOLMASK(naddr, &r->rpool.cur->addr.p.dyn-> pfid_addr6, @@ -715,8 +725,10 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, switch (pd->af) { #ifdef INET case AF_INET: - if (r->src.addr.p.dyn-> pfid_acnt4 < 1) + if (r->src.addr.p.dyn->pfid_acnt4 < 1) { + reason = PFRES_MAPFAILED; goto notrans; + } PF_POOLMASK(naddr, &r->src.addr.p.dyn->pfid_addr4, &r->src.addr.p.dyn->pfid_mask4, @@ -725,8 +737,10 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, #endif /* INET */ #ifdef INET6 case AF_INET6: - if (r->src.addr.p.dyn->pfid_acnt6 < 1) + if (r->src.addr.p.dyn->pfid_acnt6 < 1) { + reason = PFRES_MAPFAILED; goto notrans; + } PF_POOLMASK(naddr, &r->src.addr.p.dyn->pfid_addr6, &r->src.addr.p.dyn->pfid_mask6, @@ -744,7 +758,8 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, struct pf_state_key_cmp key; uint16_t cut, low, high, nport; - if (pf_map_addr(pd->af, r, saddr, naddr, NULL, NULL, sn)) + reason = pf_map_addr(pd->af, r, saddr, naddr, NULL, NULL, sn); + if (reason != 0) goto notrans; if ((r->rpool.opts & PF_POOL_TYPEMASK) == PF_POOL_BITMASK) PF_POOLMASK(naddr, naddr, &r->rpool.cur->addr.v.a.mask, @@ -815,12 +830,13 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, DPFPRINTF(PF_DEBUG_MISC, ("pf: RDR source port allocation failed\n")); - if (0) { + reason = PFRES_MAPFAILED; + goto notrans; + out: - DPFPRINTF(PF_DEBUG_MISC, - ("pf: RDR source port allocation %u->%u\n", - ntohs(sport), ntohs((*nkp)->port[0]))); - } + DPFPRINTF(PF_DEBUG_MISC, + ("pf: RDR source port allocation %u->%u\n", + ntohs(sport), ntohs((*nkp)->port[0]))); break; } default: @@ -828,14 +844,17 @@ out: } /* Return success only if translation really happened. */ - if (bcmp(*skp, *nkp, sizeof(struct pf_state_key_cmp))) - return (r); + if (bcmp(*skp, *nkp, sizeof(struct pf_state_key_cmp))) { + *rp = r; + return (PFRES_MATCH); + } + reason = PFRES_MAX; notrans: uma_zfree(V_pf_state_key_z, *nkp); uma_zfree(V_pf_state_key_z, *skp); *skp = *nkp = NULL; *sn = NULL; - return (NULL); + return (reason); }