From nobody Thu Jan 04 22:19:17 2024 X-Original-To: dev-commits-src-main@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 4T5gxH29MPz55PYb for ; Thu, 4 Jan 2024 22:19:31 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4T5gxH0LK8z4Pdc for ; Thu, 4 Jan 2024 22:19:31 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-wm1-f41.google.com with SMTP id 5b1f17b1804b1-40d4a222818so181255e9.0 for ; Thu, 04 Jan 2024 14:19:30 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704406769; x=1705011569; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ab+GX5X0rrtJ0EWVODvgOslERzUG9b8UDw/9b/5qx5Y=; b=Bb1KC6uKnkgE+oo/bNRkVO6lIWPD3B92ldaD+7TzeBozAonT6ZN6ACTB8G3JBRapFr KR49UQRFoegOwAqRY2DxPON1dGins9TGNZEqb8P60ctIEXmj9dRJZ4JtdouymjOe94E7 0s/XtxoCAkemFALnpHoBtvpBL6Lu9ILc17eLju91Uucl3jkHFFk1ABaT0UQQ9SpmRp44 NYS4LfO/nSOV8bjEsejB00run6+4jPxfaJ92MiMxr/j8frAMJAPckyfgzcYCatvFt6wz o3oW2R3Muqo5n9DTIezZeQFEpZ2YR/9DLrs/QVa6cbYw6q8FPZkE9E6Brhs3mNjehPEn W2gw== X-Gm-Message-State: AOJu0Yw7Zv4+WLggjz7MyS4Cb4737YErJNqevNWVeKzwop1N9n83dNuI NEllDoJLeO3RCJKS9tZwf92C4+dWCPM3LQ== X-Google-Smtp-Source: AGHT+IGITUcO4MwGEKv/JkXqVrmfWryrdQFWQAx2XLJocapvP8IY5265FeS52xUTfkDP+vovZiUFJw== X-Received: by 2002:a05:600c:4f15:b0:40c:2a2b:4ea with SMTP id l21-20020a05600c4f1500b0040c2a2b04eamr718846wmq.155.1704406769249; Thu, 04 Jan 2024 14:19:29 -0800 (PST) Received: from smtpclient.apple ([131.111.5.246]) by smtp.gmail.com with ESMTPSA id l21-20020a056000023500b00336755f15b0sm186035wrz.68.2024.01.04.14.19.28 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 04 Jan 2024 14:19:28 -0800 (PST) Content-Type: text/plain; charset=us-ascii List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3774.200.91.1.1\)) Subject: Re: git: 324fd7ec4043 - main - libpfctl: introduce a handle-enabled variant of pfctl_add_rule() From: Jessica Clarke In-Reply-To: <202401042211.404MBC3D003204@gitrepo.freebsd.org> Date: Thu, 4 Jan 2024 22:19:17 +0000 Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Content-Transfer-Encoding: quoted-printable Message-Id: <38CDCAED-9DF7-467B-BEF9-84BE6D1E8085@freebsd.org> References: <202401042211.404MBC3D003204@gitrepo.freebsd.org> To: Kristof Provost X-Mailer: Apple Mail (2.3774.200.91.1.1) X-Rspamd-Queue-Id: 4T5gxH0LK8z4Pdc X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US] On 4 Jan 2024, at 22:11, Kristof Provost wrote: >=20 > The branch main has been updated by kp: >=20 > URL: = https://cgit.FreeBSD.org/src/commit/?id=3D324fd7ec40439e6b3916429a69956d7a= cf74eb19 >=20 > commit 324fd7ec40439e6b3916429a69956d7acf74eb19 > Author: Kristof Provost > AuthorDate: 2024-01-04 12:45:56 +0000 > Commit: Kristof Provost > CommitDate: 2024-01-04 22:10:44 +0000 >=20 > libpfctl: introduce a handle-enabled variant of pfctl_add_rule() >=20 > Introduce pfctl_add_rule_h(), which takes a pfctl_handle rather = than a > file descriptor (which it didn't use). This means that library = users can > open the handle while they're running as root, but later drop = privileges > and still add rules to pf. Given libpfctl is an INTERALLIB, why do we need to care about this compatibility (and live with this cruft) instead of just changing pfctl_add_rule to the new thing? Jess > Sponsored by: Rubicon Communications, LLC ("Netgate") > --- > contrib/pf/ftp-proxy/filter.c | 10 +++++++--- > contrib/pf/tftp-proxy/filter.c | 12 +++++++++--- > lib/libpfctl/libpfctl.c | 29 +++++++++++++++++++++++------ > lib/libpfctl/libpfctl.h | 3 +++ > 4 files changed, 42 insertions(+), 12 deletions(-) >=20 > diff --git a/contrib/pf/ftp-proxy/filter.c = b/contrib/pf/ftp-proxy/filter.c > index 4277e079f3be..612e35c4ac6e 100644 > --- a/contrib/pf/ftp-proxy/filter.c > +++ b/contrib/pf/ftp-proxy/filter.c > @@ -58,6 +58,7 @@ static uint32_t pfpool_ticket; > static struct pfioc_trans pft; > static struct pfioc_trans_e pfte[TRANS_SIZE]; > static int dev, rule_log; > +static struct pfctl_handle *pfh =3D NULL; > static const char *qname, *tagname; >=20 > int > @@ -73,7 +74,7 @@ add_filter(u_int32_t id, u_int8_t dir, struct = sockaddr *src, > return (-1); >=20 > pfrule.direction =3D dir; > - if (pfctl_add_rule(dev, &pfrule, pfanchor, pfanchor_call, > + if (pfctl_add_rule_h(pfh, &pfrule, pfanchor, pfanchor_call, > pfticket, pfpool_ticket)) > return (-1); >=20 > @@ -108,7 +109,7 @@ add_nat(u_int32_t id, struct sockaddr *src, struct = sockaddr *dst, >=20 > pfrule.rpool.proxy_port[0] =3D nat_range_low; > pfrule.rpool.proxy_port[1] =3D nat_range_high; > - if (pfctl_add_rule(dev, &pfrule, pfanchor, pfanchor_call, > + if (pfctl_add_rule_h(pfh, &pfrule, pfanchor, pfanchor_call, > pfticket, pfpool_ticket)) > return (-1); >=20 > @@ -141,7 +142,7 @@ add_rdr(u_int32_t id, struct sockaddr *src, struct = sockaddr *dst, > return (-1); >=20 > pfrule.rpool.proxy_port[0] =3D rdr_port; > - if (pfctl_add_rule(dev, &pfrule, pfanchor, pfanchor_call, > + if (pfctl_add_rule_h(pfh, &pfrule, pfanchor, pfanchor_call, > pfticket, pfpool_ticket)) > return (-1); >=20 > @@ -182,6 +183,9 @@ init_filter(const char *opt_qname, const char = *opt_tagname, int opt_verbose) > dev =3D open("/dev/pf", O_RDWR);=20 > if (dev =3D=3D -1) > err(1, "open /dev/pf"); > + pfh =3D pfctl_open(PF_DEVICE); > + if (pfh =3D=3D NULL) > + err(1, "pfctl_open"); > status =3D pfctl_get_status(dev); > if (status =3D=3D NULL) > err(1, "DIOCGETSTATUS"); > diff --git a/contrib/pf/tftp-proxy/filter.c = b/contrib/pf/tftp-proxy/filter.c > index 966628464d28..f372ddd0aeae 100644 > --- a/contrib/pf/tftp-proxy/filter.c > +++ b/contrib/pf/tftp-proxy/filter.c > @@ -62,6 +62,7 @@ static char pfanchor_call[PF_ANCHOR_NAME_SIZE]; > static struct pfioc_trans pft; > static struct pfioc_trans_e pfte[TRANS_SIZE]; > static int dev, rule_log; > +static struct pfctl_handle *pfh =3D NULL; > static char *qname; >=20 > int > @@ -77,7 +78,7 @@ add_filter(u_int32_t id, u_int8_t dir, struct = sockaddr *src, > return (-1); >=20 > pfrule.direction =3D dir; > - if (pfctl_add_rule(dev, &pfrule, pfanchor, pfanchor_call, > + if (pfctl_add_rule_h(pfh, &pfrule, pfanchor, pfanchor_call, > pfticket, pfpool_ticket)) > return (-1); >=20 > @@ -112,7 +113,7 @@ add_nat(u_int32_t id, struct sockaddr *src, struct = sockaddr *dst, >=20 > pfrule.rpool.proxy_port[0] =3D nat_range_low; > pfrule.rpool.proxy_port[1] =3D nat_range_high; > - if (pfctl_add_rule(dev, &pfrule, pfanchor, pfanchor_call, > + if (pfctl_add_rule_h(pfh, &pfrule, pfanchor, pfanchor_call, > pfticket, pfpool_ticket)) > return (-1); >=20 > @@ -145,7 +146,7 @@ add_rdr(u_int32_t id, struct sockaddr *src, struct = sockaddr *dst, > return (-1); >=20 > pfrule.rpool.proxy_port[0] =3D rdr_port; > - if (pfctl_add_rule(dev, &pfrule, pfanchor, pfanchor_call, > + if (pfctl_add_rule_h(pfh, &pfrule, pfanchor, pfanchor_call, > pfticket, pfpool_ticket)) > return (-1); >=20 > @@ -187,6 +188,11 @@ init_filter(char *opt_qname, int opt_verbose) > syslog(LOG_ERR, "can't open /dev/pf"); > exit(1); > } > + pfh =3D pfctl_open(PF_DEVICE); > + if (pfh =3D=3D NULL) { > + syslog(LOG_ERR, "can't pfctl_open()"); > + exit(1); > + } > status =3D pfctl_get_status(dev); > if (status =3D=3D NULL) { > syslog(LOG_ERR, "DIOCGETSTATUS"); > diff --git a/lib/libpfctl/libpfctl.c b/lib/libpfctl/libpfctl.c > index 94949a5a7337..2db3f0ede99f 100644 > --- a/lib/libpfctl/libpfctl.c > +++ b/lib/libpfctl/libpfctl.c > @@ -1116,20 +1116,37 @@ snl_add_msg_attr_pf_rule(struct snl_writer = *nw, uint32_t type, const struct pfct > int > pfctl_add_rule(int dev __unused, const struct pfctl_rule *r, const = char *anchor, > const char *anchor_call, uint32_t ticket, uint32_t pool_ticket) > +{ > + struct pfctl_handle *h; > + int ret; > + > + h =3D pfctl_open(PF_DEVICE); > + if (h =3D=3D NULL) > + return (ENODEV); > + > + ret =3D pfctl_add_rule_h(h, r, anchor, anchor_call, ticket, = pool_ticket); > + > + pfctl_close(h); > + > + return (ret); > +} > + > +int > +pfctl_add_rule_h(struct pfctl_handle *h, const struct pfctl_rule *r, > + const char *anchor, const char *anchor_call, uint32_t ticket, > + uint32_t pool_ticket) > { > struct snl_writer nw; > - struct snl_state ss =3D {}; > struct snl_errmsg_data e =3D {}; > struct nlmsghdr *hdr; > uint32_t seq_id; > int family_id; >=20 > - snl_init(&ss, NETLINK_GENERIC); > - family_id =3D snl_get_genl_family(&ss, PFNL_FAMILY_NAME); > + family_id =3D snl_get_genl_family(&h->ss, PFNL_FAMILY_NAME); > if (family_id =3D=3D 0) > return (ENOTSUP); >=20 > - snl_init_writer(&ss, &nw); > + snl_init_writer(&h->ss, &nw); > hdr =3D snl_create_genl_msg_request(&nw, family_id, PFNL_CMD_ADDRULE); > hdr->nlmsg_flags |=3D NLM_F_DUMP; > snl_add_msg_attr_u32(&nw, PF_ART_TICKET, ticket); > @@ -1144,10 +1161,10 @@ pfctl_add_rule(int dev __unused, const struct = pfctl_rule *r, const char *anchor, >=20 > seq_id =3D hdr->nlmsg_seq; >=20 > - if (! snl_send_message(&ss, hdr)) > + if (! snl_send_message(&h->ss, hdr)) > return (ENXIO); >=20 > - while ((hdr =3D snl_read_reply_multi(&ss, seq_id, &e)) !=3D NULL) { > + while ((hdr =3D snl_read_reply_multi(&h->ss, seq_id, &e)) !=3D NULL) = { > } >=20 > return (e.error); > diff --git a/lib/libpfctl/libpfctl.h b/lib/libpfctl/libpfctl.h > index f128e5340891..cd72d04d6715 100644 > --- a/lib/libpfctl/libpfctl.h > +++ b/lib/libpfctl/libpfctl.h > @@ -421,6 +421,9 @@ int pfctl_get_clear_rule(int dev, uint32_t nr, = uint32_t ticket, > int pfctl_add_rule(int dev, const struct pfctl_rule *r, > const char *anchor, const char *anchor_call, uint32_t ticket, > uint32_t pool_ticket); > +int pfctl_add_rule_h(struct pfctl_handle *h, const struct pfctl_rule = *r, > + const char *anchor, const char *anchor_call, uint32_t ticket, > + uint32_t pool_ticket); > int pfctl_set_keepcounters(int dev, bool keep); > int pfctl_get_creatorids(struct pfctl_handle *h, uint32_t *creators, = size_t *len); >=20