Date: Fri, 6 Oct 2023 16:16:35 GMT From: Kristof Provost <kp@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 7ce98cf2f87a - main - pfctl: fix incorrect mask on dynamic address Message-ID: <202310061616.396GGZho077093@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=7ce98cf2f87a22240b66e4c38fd887431a25bf7d commit 7ce98cf2f87a22240b66e4c38fd887431a25bf7d Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2023-10-06 12:20:17 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2023-10-06 16:11:28 +0000 pfctl: fix incorrect mask on dynamic address A PF rule using an IPv4 address followed by an IPv6 address and then a dynamic address, e.g. "pass from {192.0.2.1 2001:db8::1} to (pppoe0)", will have an incorrect /32 mask applied to the dynamic address. MFC after: 3 weeks Obtained from: OpenBSD See also: https://ftp.openbsd.org/pub/OpenBSD/patches/5.6/common/007_pfctl.patch.sig Sponsored by: Rubicon Communications, LLC ("Netgate") Event: Oslo Hackathon at Modirum --- sbin/pfctl/parse.y | 39 +++++++++++++++++++++++++++++++++++- sbin/pfctl/pfctl_parser.c | 30 ++++++++++++++++++++++----- sbin/pfctl/pfctl_parser.h | 1 + sbin/pfctl/tests/files/pf0102.ok | 4 ++-- sbin/pfctl/tests/files/pf1018.in | 1 + sbin/pfctl/tests/files/pf1018.ok | 2 ++ sbin/pfctl/tests/pfctl_test_list.inc | 1 + 7 files changed, 70 insertions(+), 8 deletions(-) diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y index 041dcb0587b3..17227b674814 100644 --- a/sbin/pfctl/parse.y +++ b/sbin/pfctl/parse.y @@ -4720,6 +4720,10 @@ natrule : nataction interface af proto fromto tag tagged rtable remove_invalid_hosts(&$9->host, &r.af); if (invalid_redirect($9->host, r.af)) YYERROR; + if ($9->host->addr.type == PF_ADDR_DYNIFTL) { + if (($9->host = gen_dynnode($9->host, r.af)) == NULL) + err(1, "calloc"); + } if (check_netmask($9->host, r.af)) YYERROR; @@ -4936,6 +4940,10 @@ binatrule : no BINAT natpasslog interface af proto FROM ipspec toipspec tag yyerror("binat ip versions must match"); YYERROR; } + if ($8->addr.type == PF_ADDR_DYNIFTL) { + if (($8 = gen_dynnode($8, binat.af)) == NULL) + err(1, "calloc"); + } if (check_netmask($8, binat.af)) YYERROR; memcpy(&binat.src.addr, &$8->addr, @@ -4951,6 +4959,10 @@ binatrule : no BINAT natpasslog interface af proto FROM ipspec toipspec tag yyerror("binat ip versions must match"); YYERROR; } + if ($9->addr.type == PF_ADDR_DYNIFTL) { + if (($9 = gen_dynnode($9, binat.af)) == NULL) + err(1, "calloc"); + } if (check_netmask($9, binat.af)) YYERROR; memcpy(&binat.dst.addr, &$9->addr, @@ -4980,6 +4992,10 @@ binatrule : no BINAT natpasslog interface af proto FROM ipspec toipspec tag "a single address"); YYERROR; } + if ($13->host->addr.type == PF_ADDR_DYNIFTL) { + if (($13->host = gen_dynnode($13->host, binat.af)) == NULL) + err(1, "calloc"); + } if (check_netmask($13->host, binat.af)) YYERROR; @@ -5982,7 +5998,7 @@ expand_rule(struct pfctl_rule *r, char tagname[PF_TAG_NAME_SIZE]; char match_tagname[PF_TAG_NAME_SIZE]; struct pf_pooladdr *pa; - struct node_host *h; + struct node_host *h, *osrch, *odsth; u_int8_t flags, flagset, keep_state; memcpy(label, r->label, sizeof(r->label)); @@ -6043,6 +6059,18 @@ expand_rule(struct pfctl_rule *r, sizeof(r->match_tagname)) >= sizeof(r->match_tagname)) errx(1, "expand_rule: strlcpy"); + osrch = odsth = NULL; + if (src_host->addr.type == PF_ADDR_DYNIFTL) { + osrch = src_host; + if ((src_host = gen_dynnode(src_host, r->af)) == NULL) + err(1, "expand_rule: calloc"); + } + if (dst_host->addr.type == PF_ADDR_DYNIFTL) { + odsth = dst_host; + if ((dst_host = gen_dynnode(dst_host, r->af)) == NULL) + err(1, "expand_rule: calloc"); + } + error += check_netmask(src_host, r->af); error += check_netmask(dst_host, r->af); @@ -6121,6 +6149,15 @@ expand_rule(struct pfctl_rule *r, added++; } + if (osrch && src_host->addr.type == PF_ADDR_DYNIFTL) { + free(src_host); + src_host = osrch; + } + if (odsth && dst_host->addr.type == PF_ADDR_DYNIFTL) { + free(dst_host); + dst_host = odsth; + } + )))))))))); FREE_LIST(struct node_if, interfaces); diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c index 936c5ec53759..925848055bba 100644 --- a/sbin/pfctl/pfctl_parser.c +++ b/sbin/pfctl/pfctl_parser.c @@ -1306,16 +1306,12 @@ int check_netmask(struct node_host *h, sa_family_t af) { struct node_host *n = NULL; - struct pf_addr *m; + struct pf_addr *m; for (n = h; n != NULL; n = n->next) { if (h->addr.type == PF_ADDR_TABLE) continue; m = &h->addr.v.a.mask; - /* fix up netmask for dynaddr */ - if (af == AF_INET && h->addr.type == PF_ADDR_DYNIFTL && - unmask(m, AF_INET6) > 32) - set_ipmask(n, 32); /* netmasks > 32 bit are invalid on v4 */ if (af == AF_INET && (m->addr32[1] || m->addr32[2] || m->addr32[3])) { @@ -1327,6 +1323,30 @@ check_netmask(struct node_host *h, sa_family_t af) return (0); } +struct node_host * +gen_dynnode(struct node_host *h, sa_family_t af) +{ + struct node_host *n; + struct pf_addr *m; + + if (h->addr.type != PF_ADDR_DYNIFTL) + return (NULL); + + if ((n = calloc(1, sizeof(*n))) == NULL) + return (NULL); + bcopy(h, n, sizeof(*n)); + n->ifname = NULL; + n->next = NULL; + n->tail = NULL; + + /* fix up netmask */ + m = &n->addr.v.a.mask; + if (af == AF_INET && unmask(m, AF_INET6) > 32) + set_ipmask(n, 32); + + return (n); +} + /* interface lookup routines */ static struct node_host *iftab; diff --git a/sbin/pfctl/pfctl_parser.h b/sbin/pfctl/pfctl_parser.h index 83a4880106a8..d0f3bc3c303c 100644 --- a/sbin/pfctl/pfctl_parser.h +++ b/sbin/pfctl/pfctl_parser.h @@ -360,6 +360,7 @@ extern const struct pf_timeout pf_timeouts[]; void set_ipmask(struct node_host *, u_int8_t); int check_netmask(struct node_host *, sa_family_t); int unmask(struct pf_addr *, sa_family_t); +struct node_host *gen_dynnode(struct node_host *, sa_family_t); void ifa_load(void); int get_query_socket(void); struct node_host *ifa_exists(char *); diff --git a/sbin/pfctl/tests/files/pf0102.ok b/sbin/pfctl/tests/files/pf0102.ok index 3233ca5a2643..1c76ec2725ba 100644 --- a/sbin/pfctl/tests/files/pf0102.ok +++ b/sbin/pfctl/tests/files/pf0102.ok @@ -1,8 +1,8 @@ pass inet from 1.1.1.1 to (self) flags S/SA keep state -pass inet6 from 2002:: to (self)/32 flags S/SA keep state +pass inet6 from 2002:: to (self) flags S/SA keep state pass inet6 from 2002:: to (self) flags S/SA keep state pass inet from 1.1.1.1 to (self) flags S/SA keep state pass inet from 1.1.1.1 to (self) flags S/SA keep state -pass inet6 from 2002:: to (self)/32 flags S/SA keep state +pass inet6 from 2002:: to (self)/40 flags S/SA keep state pass inet6 from 2002:: to (self)/40 flags S/SA keep state pass inet from 1.1.1.1 to (self) flags S/SA keep state diff --git a/sbin/pfctl/tests/files/pf1018.in b/sbin/pfctl/tests/files/pf1018.in new file mode 100644 index 000000000000..90f0a3a0bab7 --- /dev/null +++ b/sbin/pfctl/tests/files/pf1018.in @@ -0,0 +1 @@ +pass from { 192.0.2.1 2001:db8::1 } to (pppoe0) diff --git a/sbin/pfctl/tests/files/pf1018.ok b/sbin/pfctl/tests/files/pf1018.ok new file mode 100644 index 000000000000..04950f0035b8 --- /dev/null +++ b/sbin/pfctl/tests/files/pf1018.ok @@ -0,0 +1,2 @@ +pass inet from 192.0.2.1 to (pppoe0) flags S/SA keep state +pass inet6 from 2001:db8::1 to (pppoe0) flags S/SA keep state diff --git a/sbin/pfctl/tests/pfctl_test_list.inc b/sbin/pfctl/tests/pfctl_test_list.inc index 98ea3257d492..4a63c0ba48b7 100644 --- a/sbin/pfctl/tests/pfctl_test_list.inc +++ b/sbin/pfctl/tests/pfctl_test_list.inc @@ -126,3 +126,4 @@ PFCTL_TEST(1014, "Ethernet rule with one label") PFCTL_TEST(1015, "Ethernet rule with several labels") PFCTL_TEST(1016, "Ethernet rule with ridentifier and one label") PFCTL_TEST(1017, "Ethernet rule with ridentifier and several labels") +PFCTL_TEST(1018, "Test dynamic address mask")
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202310061616.396GGZho077093>