On Wed, Jun 29, 2022 at 01:35:37PM +0200, Martijn van Duren wrote: > On Tue, 2022-06-28 at 12:33 +0200, Martijn van Duren wrote: > > On Tue, 2022-06-28 at 12:21 +0200, Martijn van Duren wrote: > > > On Tue, 2022-06-28 at 10:21 +0200, Martijn van Duren wrote: > > > > Back in 2020 florian@ added the filter-pf-addresses keyword. > > > > Although useful, I always felt it was a bit too case-specific. The diff > > > > below adds a new blocklist feature/backend, which takes hold of an > > > > entire subtree, effectively removing it from the tree. > > > > > > > > With this I've deprecated the filter-pf-address case and should > > > > probably be removed somewhere after 7.4. The filter-routes case can't > > > > be removed unfortunately, since its behaviour is not identical, and > > > > instead adds filters to the routing socket, preventing updates being > > > > pushed to snmpd(8). > > > > > > > > Feedback/OK? > > > > > > > > martijn@ > > > > > > > Also clean up after ourselves if appl_exception fails. > > > > > *sigh* Missed a return. > > > And also some cleanup during shutdown.
Can't spot anything wrong in this one and works fine in some basic testing. ok tb > > Index: Makefile > =================================================================== > RCS file: /cvs/src/usr.sbin/snmpd/Makefile,v > retrieving revision 1.18 > diff -u -p -r1.18 Makefile > --- Makefile 19 Jan 2022 11:00:56 -0000 1.18 > +++ Makefile 29 Jun 2022 11:35:23 -0000 > @@ -3,6 +3,7 @@ > PROG= snmpd > MAN= snmpd.8 snmpd.conf.5 > SRCS= parse.y log.c snmpe.c application.c > application_legacy.c \ > + application_blocklist.c \ > mps.c trap.c mib.c smi.c kroute.c snmpd.c timer.c \ > pf.c proc.c usm.c traphandler.c util.c > > Index: application.c > =================================================================== > RCS file: /cvs/src/usr.sbin/snmpd/application.c,v > retrieving revision 1.5 > diff -u -p -r1.5 application.c > --- application.c 27 Jun 2022 10:31:17 -0000 1.5 > +++ application.c 29 Jun 2022 11:35:23 -0000 > @@ -148,6 +148,7 @@ RB_PROTOTYPE_STATIC(appl_requests, appl_ > void > appl_init(void) > { > + appl_blocklist_init(); > appl_legacy_init(); > } > > @@ -156,6 +157,7 @@ appl_shutdown(void) > { > struct appl_context *ctx, *tctx; > > + appl_blocklist_shutdown(); > appl_legacy_shutdown(); > > TAILQ_FOREACH_SAFE(ctx, &contexts, ac_entries, tctx) { > Index: application.h > =================================================================== > RCS file: /cvs/src/usr.sbin/snmpd/application.h,v > retrieving revision 1.1 > diff -u -p -r1.1 application.h > --- application.h 19 Jan 2022 10:59:35 -0000 1.1 > +++ application.h 29 Jun 2022 11:35:23 -0000 > @@ -133,3 +133,7 @@ struct ber_element *appl_exception(enum > /* application_legacy.c */ > void appl_legacy_init(void); > void appl_legacy_shutdown(void); > + > +/* application_blocklist.c */ > +void appl_blocklist_init(void); > +void appl_blocklist_shutdown(void); > Index: application_blocklist.c > =================================================================== > RCS file: application_blocklist.c > diff -N application_blocklist.c > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ application_blocklist.c 29 Jun 2022 11:35:23 -0000 > @@ -0,0 +1,141 @@ > +/* $OpenBSD: application.c,v 1.5 2022/06/27 10:31:17 martijn Exp $ */ > + > +/* > + * Copyright (c) 2022 Martijn van Duren <mart...@openbsd.org> > + * > + * Permission to use, copy, modify, and distribute this software for any > + * purpose with or without fee is hereby granted, provided that the above > + * copyright notice and this permission notice appear in all copies. > + * > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > + */ > + > +#include <event.h> > +#include <stdlib.h> > + > +#include "application.h" > +#include "snmpd.h" > + > +struct appl_varbind *appl_blocklist_response(size_t); > +void appl_blocklist_get(struct appl_backend *, int32_t, int32_t, const char > *, > + struct appl_varbind *); > +void appl_blocklist_getnext(struct appl_backend *, int32_t, int32_t, > + const char *, struct appl_varbind *); > + > +struct appl_backend_functions appl_blocklist_functions = { > + .ab_get = appl_blocklist_get, > + .ab_getnext = appl_blocklist_getnext, > + .ab_getbulk = NULL, > +}; > + > +struct appl_backend appl_blocklist = { > + .ab_name = "blocklist", > + .ab_cookie = NULL, > + .ab_retries = 0, > + .ab_fn = &appl_blocklist_functions > +}; > + > +static struct appl_varbind *response = NULL; > +static size_t responsesz = 0; > + > +struct appl_varbind * > +appl_blocklist_response(size_t nvarbind) > +{ > + struct appl_varbind *tmp; > + size_t i; > + > + if (responsesz < nvarbind) { > + if ((tmp = recallocarray(response, responsesz, nvarbind, > + sizeof(*response))) == NULL) { > + log_warn(NULL); > + return NULL; > + } > + responsesz = nvarbind; > + response = tmp; > + } > + for (i = 0; i < nvarbind; i++) > + response[i].av_next = i + 1 == nvarbind ? > + NULL : &(response[i + 1]); > + return response; > +} > + > +void > +appl_blocklist_init(void) > +{ > + extern struct snmpd *snmpd_env; > + size_t i; > + > + for (i = 0; i < snmpd_env->sc_nblocklist; i++) > + appl_register(NULL, 150, 1, &(snmpd_env->sc_blocklist[i]), > + 0, 1, 0, 0, &appl_blocklist); > +} > + > +void > +appl_blocklist_shutdown(void) > +{ > + appl_close(&appl_blocklist); > + free(response); > +} > + > +void > +appl_blocklist_get(struct appl_backend *backend, __unused int32_t > transactionid, > + int32_t requestid, __unused const char *ctx, struct appl_varbind *vblist) > +{ > + struct appl_varbind *vb, *rvb, *rvblist; > + size_t i; > + > + for (i = 0, vb = vblist; vb != NULL; vb = vb->av_next) > + i++; > + if ((rvblist = appl_blocklist_response(i)) == NULL) > + goto fail; > + rvb = rvblist; > + for (vb = vblist; vb != NULL; vb = vb->av_next, rvb = rvb->av_next) { > + rvb->av_oid = vb->av_oid; > + rvb->av_value = appl_exception(APPL_EXC_NOSUCHOBJECT); > + if (rvb->av_value == NULL) > + goto fail; > + } > + > + appl_response(backend, requestid, APPL_ERROR_NOERROR, 0, rvblist); > + return; > + fail: > + for (rvb = rvblist; rvb != NULL && rvb->av_value != NULL; > + rvb = rvb->av_next) > + ober_free_elements(rvb->av_value); > + appl_response(backend, requestid, APPL_ERROR_GENERR, 1, vb); > +} > + > +void > +appl_blocklist_getnext(struct appl_backend *backend, > + __unused int32_t transactionid, int32_t requestid, __unused const char > *ctx, > + struct appl_varbind *vblist) > +{ > + struct appl_varbind *vb, *rvb, *rvblist; > + size_t i; > + > + for (i = 0, vb = vblist; vb != NULL; vb = vb->av_next) > + i++; > + if ((rvblist = appl_blocklist_response(i)) == NULL) > + goto fail; > + rvb = rvblist; > + for (vb = vblist; vb != NULL; vb = vb->av_next, rvb = rvb->av_next) { > + rvb->av_oid = vb->av_oid; > + rvb->av_value = appl_exception(APPL_EXC_ENDOFMIBVIEW); > + if (rvb->av_value == NULL) > + goto fail; > + } > + > + appl_response(backend, requestid, APPL_ERROR_NOERROR, 0, rvblist); > + return; > + fail: > + for (rvb = rvblist; rvb != NULL && rvb->av_value != NULL; > + rvb = rvb->av_next) > + ober_free_elements(rvb->av_value); > + appl_response(backend, requestid, APPL_ERROR_GENERR, 1, vb); > +} > Index: mib.c > =================================================================== > RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v > retrieving revision 1.102 > diff -u -p -r1.102 mib.c > --- mib.c 1 Sep 2021 15:54:40 -0000 1.102 > +++ mib.c 29 Jun 2022 11:35:23 -0000 > @@ -2223,9 +2223,6 @@ mib_pftableaddrs(struct oid *oid, struct > struct pfr_astats as; > int tblidx; > > - if (snmpd_env->sc_pfaddrfilter) > - return (-1); > - > tblidx = o->bo_id[OIDIDX_pfTblAddr + 1]; > mps_decodeinaddr(o, &as.pfras_a.pfra_ip4addr, OIDIDX_pfTblAddr + 2); > as.pfras_a.pfra_net = o->bo_id[OIDIDX_pfTblAddr + 6]; > @@ -2313,9 +2310,6 @@ mib_pftableaddrstable(struct oid *oid, s > struct pfr_astats as; > struct oid a, b; > u_int32_t id, tblidx; > - > - if (snmpd_env->sc_pfaddrfilter) > - return (NULL); > > bcopy(&oid->o_id, no, sizeof(*no)); > id = oid->o_oidlen - 1; > Index: parse.y > =================================================================== > RCS file: /cvs/src/usr.sbin/snmpd/parse.y,v > retrieving revision 1.74 > diff -u -p -r1.74 parse.y > --- parse.y 28 Jun 2022 09:11:33 -0000 1.74 > +++ parse.y 29 Jun 2022 11:35:23 -0000 > @@ -130,7 +130,7 @@ typedef struct { > %token SYSTEM CONTACT DESCR LOCATION NAME OBJECTID SERVICES RTFILTER > %token READONLY READWRITE OCTETSTRING INTEGER COMMUNITY TRAP RECEIVER > %token SECLEVEL NONE AUTH ENC USER AUTHKEY ENCKEY ERROR > -%token HANDLE DEFAULT SRCADDR TCP UDP PFADDRFILTER PORT > +%token HANDLE DEFAULT SRCADDR TCP UDP PFADDRFILTER BLOCKLIST PORT > %token <v.string> STRING > %token <v.number> NUMBER > %type <v.string> usmuser community optcommunity > @@ -255,6 +255,20 @@ main : LISTEN ON listen_udptcp > } > conf->sc_traphandler = 1; > } > + | BLOCKLIST oid { > + struct ber_oid *blocklist; > + > + blocklist = recallocarray(conf->sc_blocklist, > + conf->sc_nblocklist, conf->sc_nblocklist + 1, > + sizeof(*blocklist)); > + if (blocklist == NULL) { > + yyerror("malloc"); > + YYERROR; > + } > + conf->sc_blocklist = blocklist; > + blocklist[conf->sc_nblocklist++] = *$2; > + free($2); > + } > | RTFILTER yesno { > if ($2 == 1) > conf->sc_rtfilter = ROUTE_FILTER(RTM_NEWADDR) | > @@ -264,8 +278,25 @@ main : LISTEN ON listen_udptcp > else > conf->sc_rtfilter = 0; > } > + /* XXX Remove after 7.4 */ > | PFADDRFILTER yesno { > - conf->sc_pfaddrfilter = $2; > + struct ber_oid *blocklist; > + > + log_warnx("filter-pf-addresses is deprecated. " > + "Please use blocklist instead."); > + if ($2) { > + blocklist = recallocarray(conf->sc_blocklist, > + conf->sc_nblocklist, > + conf->sc_nblocklist + 1, > + sizeof(*blocklist)); > + if (blocklist == NULL) { > + yyerror("malloc"); > + YYERROR; > + } > + conf->sc_blocklist = blocklist; > + smi_string2oid("pfTblAddrTable", > + &(blocklist[conf->sc_nblocklist++])); > + } > } > | seclevel { > conf->sc_min_seclevel = $1; > @@ -995,6 +1026,7 @@ lookup(char *s) > { "agentid", AGENTID }, > { "auth", AUTH }, > { "authkey", AUTHKEY }, > + { "blocklist", BLOCKLIST }, > { "community", COMMUNITY }, > { "contact", CONTACT }, > { "default", DEFAULT }, > Index: snmpd.conf.5 > =================================================================== > RCS file: /cvs/src/usr.sbin/snmpd/snmpd.conf.5,v > retrieving revision 1.59 > diff -u -p -r1.59 snmpd.conf.5 > --- snmpd.conf.5 31 Mar 2022 17:27:31 -0000 1.59 > +++ snmpd.conf.5 29 Jun 2022 11:35:23 -0000 > @@ -78,15 +78,13 @@ listen on $ext_addr > .Sh GLOBAL CONFIGURATION > The following options can be set globally: > .Bl -tag -width Ds > -.It Ic filter-pf-addresses Pq Ic yes | no > -If set to > -.Ic yes , > -.Xr snmpd 8 > -will filter out the OPENBSD-PF-MIB::pfTblAddrTable tree. > -Addresses stored in PF tables will not be available, but CPU use will be > -reduced during bulk walks. > -The default is > -.Ic no . > +.It Ic blocklist Ar oid > +Remove the > +.Ar oid > +subtree from view. > +Multiple > +.Ic blocklist > +statements are supported. > .It Ic filter-routes Pq Ic yes | no > If set to > .Ic yes , > Index: snmpd.h > =================================================================== > RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v > retrieving revision 1.102 > diff -u -p -r1.102 snmpd.h > --- snmpd.h 19 Jan 2022 10:25:04 -0000 1.102 > +++ snmpd.h 29 Jun 2022 11:35:23 -0000 > @@ -591,8 +591,9 @@ struct snmpd { > > int sc_ncpu; > int64_t *sc_cpustates; > + struct ber_oid *sc_blocklist; > + size_t sc_nblocklist; > int sc_rtfilter; > - int sc_pfaddrfilter; > > int sc_min_seclevel; > int sc_traphandler; >