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;
>
