ping
On Tue, 2021-01-05 at 22:24 +0100, Martijn van Duren wrote:
> With the traphandler code beaten into submission I was able to keep the
> traphandler process removal diff more manageable. This diff does a
> couple things.
> - "listen on" now accepts any combination of read, write and notify.
> This combination removes the traphandler dependency on the generic
> listen on statement, which allows an admin to run snmpd in
> traphandler-only mode. This will break existing traphandler setups,
> but considering the benefit of not having the two functionalities
> mixed is worth it to me and is fully intentional.
> The names are choosen based on viewType names from RFC 3415 (VACM)
> - The traphandler process is gone, resulting in a single dispatcher
> responsible for the initial parsing of the packets, which allows for
> better msg checking in the future. The current traphandler process
> does nothing more then what snmpe already does, but a lot crappier:
> the pledge was also too wide.
> - We now check incoming trap packets against "trap community". The
> current code does no community verification.
> - trap parsing errors are now shown in a similar fashion to the read
> and write requests.
> - traphandler now support tcp!!!
> - Minus 65 LoC
>
> With this change regress requires the following diff:
> Index: snmpd.sh
> ===================================================================
> RCS file: /cvs/src/regress/usr.sbin/snmpd/snmpd.sh,v
> retrieving revision 1.12
> diff -u -p -r1.12 snmpd.sh
> --- snmpd.sh 8 Aug 2020 11:06:40 -0000 1.12
> +++ snmpd.sh 5 Jan 2021 20:46:19 -0000
> @@ -65,6 +65,7 @@ cat > ${OBJDIR}/snmpd.conf <<EOF
> # This is config template (1) for snmpd regression testing
> # Restrict daemon to listen on localhost only
> listen on 127.0.0.1
> +listen on 127.0.0.1 notify
> listen on ::1
>
> # Specify a number of trap receivers
>
> OK?
>
> martijn@
>
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/parse.y,v
> retrieving revision 1.62
> diff -u -p -r1.62 parse.y
> --- parse.y 30 Oct 2020 07:43:48 -0000 1.62
> +++ parse.y 5 Jan 2021 21:22:12 -0000
> @@ -94,11 +94,10 @@ char *symget(const char *);
> struct snmpd *conf = NULL;
> static int errors = 0;
> static struct usmuser *user = NULL;
> -static char *snmpd_port = SNMPD_PORT;
>
> int host(const char *, const char *, int,
> struct sockaddr_storage *, int);
> -int listen_add(struct sockaddr_storage *, int);
> +int listen_add(struct sockaddr_storage *, int, int);
>
> typedef struct {
> union {
> @@ -121,7 +120,7 @@ typedef struct {
> %}
>
> %token INCLUDE
> -%token LISTEN ON
> +%token LISTEN ON READ WRITE NOTIFY
> %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 DISABLED
> @@ -130,7 +129,7 @@ typedef struct {
> %token <v.number> NUMBER
> %type <v.string> hostcmn
> %type <v.string> srcaddr port
> -%type <v.number> optwrite yesno seclevel
> +%type <v.number> optwrite yesno seclevel listenopt listenopts
> %type <v.data> objtype cmd
> %type <v.oid> oid hostoid trapoid
> %type <v.auth> auth
> @@ -281,54 +280,74 @@ listenproto : UDP listen_udp
> | TCP listen_tcp
> | listen_udp
>
> -listen_udp : STRING port {
> +listenopts : /* empty */ { $$ = 0; }
> + | listenopts listenopt { $$ |= $2; }
> + ;
> +
> +listenopt : READ { $$ = ADDRESS_FLAG_READ; }
> + | WRITE { $$ = ADDRESS_FLAG_WRITE; }
> + | NOTIFY { $$ = ADDRESS_FLAG_NOTIFY; }
> + ;
> +
> +listen_udp : STRING port listenopts {
> struct sockaddr_storage ss[16];
> int nhosts, i;
> + char *port = $2;
> +
> + if (port == NULL) {
> + if ($3 == ADDRESS_FLAG_NOTIFY)
> + port = SNMPTRAP_PORT;
> + else
> + port = SNMP_PORT;
> + }
>
> - nhosts = host($1, $2, SOCK_DGRAM, ss, nitems(ss));
> + nhosts = host($1, port, SOCK_DGRAM, ss, nitems(ss));
> if (nhosts < 1) {
> yyerror("invalid address: %s", $1);
> free($1);
> - if ($2 != snmpd_port)
> - free($2);
> + free($2);
> YYERROR;
> }
> if (nhosts > (int)nitems(ss))
> log_warn("%s:%s resolves to more than %zu
> hosts",
> - $1, $2, nitems(ss));
> + $1, port, nitems(ss));
>
> free($1);
> - if ($2 != snmpd_port)
> - free($2);
> + free($2);
> for (i = 0; i < nhosts; i++) {
> - if (listen_add(&(ss[i]), SOCK_DGRAM) == -1) {
> + if (listen_add(&(ss[i]), SOCK_DGRAM, $3) ==
> -1) {
> yyerror("calloc");
> YYERROR;
> }
> }
> }
>
> -listen_tcp : STRING port {
> +listen_tcp : STRING port listenopts {
> struct sockaddr_storage ss[16];
> int nhosts, i;
> + char *port = $2;
>
> - nhosts = host($1, $2, SOCK_STREAM, ss, nitems(ss));
> + if (port == NULL) {
> + if ($3 == ADDRESS_FLAG_NOTIFY)
> + port = SNMPTRAP_PORT;
> + else
> + port = SNMP_PORT;
> + }
> + nhosts = host($1, port, SOCK_STREAM, ss, nitems(ss));
> if (nhosts < 1) {
> yyerror("invalid address: %s", $1);
> free($1);
> - if ($2 != snmpd_port)
> - free($2);
> + free($2);
> YYERROR;
> }
> if (nhosts > (int)nitems(ss))
> log_warn("%s:%s resolves to more than %zu
> hosts",
> - $1, $2, nitems(ss));
> + $1, port, nitems(ss));
>
> free($1);
> - if ($2 != snmpd_port)
> - free($2);
> + free($2);
> for (i = 0; i < nhosts; i++) {
> - if (listen_add(&(ss[i]), SOCK_STREAM) == -1) {
> + if (listen_add(&(ss[i]), SOCK_STREAM, $3) ==
> -1) {
> yyerror("calloc");
> YYERROR;
> }
> @@ -336,7 +355,7 @@ listen_tcp : STRING port {
> }
>
> port : /* empty */ {
> - $$ = snmpd_port;
> + $$ = NULL;
> }
> | PORT STRING {
> $$ = $2;
> @@ -497,7 +516,7 @@ hostdef : STRING hostoid hostcmn srcadd
> YYERROR;
> }
>
> - if (host($1, SNMPD_TRAPPORT, SOCK_DGRAM, &ss, 1) <=
> 0) {
> + if (host($1, SNMPTRAP_PORT, SOCK_DGRAM, &ss, 1) <= 0)
> {
> yyerror("invalid host: %s", $1);
> free($1);
> free($2);
> @@ -704,9 +723,11 @@ lookup(char *s)
> { "location", LOCATION },
> { "name", NAME },
> { "none", NONE },
> + { "notify", NOTIFY },
> { "oid", OBJECTID },
> { "on", ON },
> { "port", PORT },
> + { "read", READ },
> { "read-only", READONLY },
> { "read-write", READWRITE },
> { "receiver", RECEIVER },
> @@ -718,7 +739,8 @@ lookup(char *s)
> { "tcp", TCP },
> { "trap", TRAP },
> { "udp", UDP },
> - { "user", USER }
> + { "user", USER },
> + { "write", WRITE }
> };
> const struct keywords *p;
>
> @@ -1110,27 +1132,29 @@ parse_config(const char *filename, u_int
>
> /* Setup default listen addresses */
> if (TAILQ_EMPTY(&conf->sc_addresses)) {
> - if (host("0.0.0.0", SNMPD_PORT, SOCK_DGRAM, &ss, 1) != 1)
> + if (host("0.0.0.0", SNMP_PORT, SOCK_DGRAM, &ss, 1) != 1)
> fatal("Unexpected resolving of 0.0.0.0");
> - if (listen_add(&ss, SOCK_DGRAM) == -1)
> + if (listen_add(&ss, SOCK_DGRAM, 0) == -1)
> fatal("calloc");
> - if (host("::", SNMPD_PORT, SOCK_DGRAM, &ss, 1) != 1)
> + if (host("::", SNMP_PORT, SOCK_DGRAM, &ss, 1) != 1)
> fatal("Unexpected resolving of ::");
> - if (listen_add(&ss, SOCK_DGRAM) == -1)
> + if (listen_add(&ss, SOCK_DGRAM, 0) == -1)
> fatal("calloc");
> }
> - if (conf->sc_traphandler) {
> - found = 0;
> - TAILQ_FOREACH(h, &conf->sc_addresses, entry) {
> - if (h->type == SOCK_DGRAM)
> - found = 1;
> - }
> - if (!found) {
> - log_warnx("trap handler needs at least one "
> - "udp listen address");
> - free(conf);
> - return (NULL);
> - }
> + found = 0;
> + TAILQ_FOREACH(h, &conf->sc_addresses, entry) {
> + if (h->flags & ADDRESS_FLAG_NOTIFY)
> + found = 1;
> + }
> + if (conf->sc_traphandler && !found) {
> + log_warnx("trap handler needs at least one notify listener");
> + free(conf);
> + return (NULL);
> + }
> + if (!conf->sc_traphandler && found) {
> + log_warnx("notify listener needs at least one trap handler");
> + free(conf);
> + return (NULL);
> }
>
> /* Free macros and check which have not been used. */
> @@ -1258,7 +1282,7 @@ host(const char *s, const char *port, in
> }
>
> int
> -listen_add(struct sockaddr_storage *ss, int type)
> +listen_add(struct sockaddr_storage *ss, int type, int flags)
> {
> struct sockaddr_in *sin;
> struct sockaddr_in6 *sin6;
> @@ -1275,6 +1299,12 @@ listen_add(struct sockaddr_storage *ss,
> h->port = ntohs(sin6->sin6_port);
> }
> h->type = type;
> + if ((h->flags = flags) == 0) {
> + if (h->port == 162)
> + h->flags = ADDRESS_FLAG_NOTIFY;
> + else
> + h->flags = ADDRESS_FLAG_READ | ADDRESS_FLAG_WRITE;
> + }
> TAILQ_INSERT_TAIL(&(conf->sc_addresses), h, entry);
>
> return 0;
> Index: snmpd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/snmpd.c,v
> retrieving revision 1.42
> diff -u -p -r1.42 snmpd.c
> --- snmpd.c 6 Sep 2020 15:51:28 -0000 1.42
> +++ snmpd.c 5 Jan 2021 21:22:12 -0000
> @@ -52,8 +52,6 @@ struct snmpd *snmpd_env;
>
> static struct privsep_proc procs[] = {
> { "snmpe", PROC_SNMPE, snmpd_dispatch_snmpe, snmpe, snmpe_shutdown },
> - { "traphandler", PROC_TRAP, snmpd_dispatch_traphandler, traphandler,
> - traphandler_shutdown }
> };
>
> void
> @@ -300,6 +298,8 @@ int
> snmpd_dispatch_snmpe(int fd, struct privsep_proc *p, struct imsg *imsg)
> {
> switch (imsg->hdr.type) {
> + case IMSG_TRAP_EXEC:
> + return (traphandler_priv_recvmsg(p, imsg));
> case IMSG_CTL_RELOAD:
> /* XXX notyet */
> default:
> Index: snmpd.conf.5
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/snmpd.conf.5,v
> retrieving revision 1.45
> diff -u -p -r1.45 snmpd.conf.5
> --- snmpd.conf.5 24 Oct 2020 14:52:11 -0000 1.45
> +++ snmpd.conf.5 5 Jan 2021 21:22:12 -0000
> @@ -95,13 +95,37 @@ Routing table information will not be av
> reduced during bulk updates.
> The default is
> .Ic no .
> -.It Ic listen on Oo Ic tcp | udp Oc Ar address Op Ic port Ar port
> +.It Ic listen on Oo Ic tcp | udp Oc Ar address Oo Ic port Ar port Oc Op Ic
> read | Ic write | Ic notify
> Specify the local address
> .Xr snmpd 8
> should listen on for incoming SNMP messages.
> +The
> +.Ic read
> +flag specifies if the listen statement accepts get, getnext and bulkget
> +requests.
> +The
> +.Ic write
> +flag specifies if the listen statement accepts set requests
> +and
> +.Ic notify
> +flags specifes if the listen statements accepts trapv1 and trapv2 requests.
> Multiple
> .Ic listen on
> -statements are supported, the default is UDP.
> +statements are supported.
> +The default protocol is
> +.Ic udp.
> +The default
> +.Ar port
> +is 161, unless the only listen flags contains
> +.Ic notify
> +which sets it to 162.
> +If no flags are specified it defaults to
> +.Dq Ic read Ic write ,
> +or
> +.Ic notify
> +when
> +.Ar port
> +is 162.
> .It Ic read-only community Ar string
> Specify the name of the read-only community.
> The default value is
> Index: snmpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v
> retrieving revision 1.90
> diff -u -p -r1.90 snmpd.h
> --- snmpd.h 6 Sep 2020 15:51:28 -0000 1.90
> +++ snmpd.h 5 Jan 2021 21:22:12 -0000
> @@ -48,8 +48,8 @@
> #define CONF_FILE "/etc/snmpd.conf"
> #define SNMPD_SOCKET "/var/run/snmpd.sock"
> #define SNMPD_USER "_snmpd"
> -#define SNMPD_PORT "161"
> -#define SNMPD_TRAPPORT "162"
> +#define SNMP_PORT "161"
> +#define SNMPTRAP_PORT "162"
>
> #define SNMPD_MAXSTRLEN 484
> #define SNMPD_MAXCOMMUNITYLEN SNMPD_MAXSTRLEN
> @@ -88,7 +88,7 @@ enum imsg_type {
> IMSG_CTL_VERBOSE,
> IMSG_CTL_RELOAD,
> IMSG_CTL_PROCFD,
> - IMSG_ALERT
> + IMSG_TRAP_EXEC
> };
>
> struct imsgev {
> @@ -110,7 +110,6 @@ struct imsgev {
> enum privsep_procid {
> PROC_PARENT, /* Parent process and application interface */
> PROC_SNMPE, /* SNMP engine */
> - PROC_TRAP, /* SNMP trap receiver */
> PROC_MAX
> };
>
> @@ -384,8 +383,11 @@ struct snmp_message {
> struct sockaddr_storage sm_ss;
> socklen_t sm_slen;
> int sm_sock_tcp;
> + int sm_aflags;
> + int sm_type;
> struct event sm_sockev;
> char sm_host[HOST_NAME_MAX+1];
> + in_port_t sm_port;
>
> struct sockaddr_storage sm_local_ss;
> socklen_t sm_local_slen;
> @@ -482,6 +484,7 @@ struct address {
> struct sockaddr_storage ss;
> in_port_t port;
> int type;
> + int flags;
> int fd;
> struct event ev;
> struct event evt;
> @@ -490,6 +493,10 @@ struct address {
> };
> TAILQ_HEAD(addresslist, address);
>
> +#define ADDRESS_FLAG_READ 0x1
> +#define ADDRESS_FLAG_WRITE 0x2
> +#define ADDRESS_FLAG_NOTIFY 0x4
> +
> struct trap_address {
> struct sockaddr_storage ss;
> struct sockaddr_storage ss_local;
> @@ -761,9 +768,8 @@ struct imsgev *
> int proc_flush_imsg(struct privsep *, enum privsep_procid, int);
>
> /* traphandler.c */
> -void traphandler(struct privsep *, struct privsep_proc *);
> -void traphandler_shutdown(void);
> -int snmpd_dispatch_traphandler(int, struct privsep_proc *, struct imsg
> *);
> +int traphandler_parse(struct snmp_message *);
> +int traphandler_priv_recvmsg(struct privsep_proc *, struct imsg *);
> void trapcmd_free(struct trapcmd *);
> int trapcmd_add(struct trapcmd *);
> struct trapcmd *
> Index: snmpe.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> retrieving revision 1.67
> diff -u -p -r1.67 snmpe.c
> --- snmpe.c 6 Sep 2020 17:29:35 -0000 1.67
> +++ snmpe.c 5 Jan 2021 21:22:12 -0000
> @@ -108,7 +108,7 @@ snmpe_init(struct privsep *ps, struct pr
> evtimer_set(&h->evt, snmpe_acceptcb, h);
> } else {
> event_set(&h->ev, h->fd, EV_READ|EV_PERSIST,
> - snmpe_recvmsg, env);
> + snmpe_recvmsg, h);
> }
> event_add(&h->ev, NULL);
> }
> @@ -271,6 +271,7 @@ snmpe_parse(struct snmp_message *msg)
> if (class != BER_CLASS_CONTEXT)
> goto parsefail;
>
> + msg->sm_type = type;
> switch (type) {
> case SNMP_C_GETBULKREQ:
> if (msg->sm_version == SNMP_V1) {
> @@ -288,6 +289,10 @@ snmpe_parse(struct snmp_message *msg)
> case SNMP_C_GETNEXTREQ:
> if (type == SNMP_C_GETNEXTREQ)
> stats->snmp_ingetnexts++;
> + if (!(msg->sm_aflags & ADDRESS_FLAG_READ)) {
> + msg->sm_errstr = "read requests disabled";
> + goto fail;
> + }
> if (msg->sm_version != SNMP_V3 &&
> strcmp(env->sc_rdcommunity, msg->sm_community) != 0 &&
> (env->sc_readonly ||
> @@ -301,6 +306,10 @@ snmpe_parse(struct snmp_message *msg)
>
> case SNMP_C_SETREQ:
> stats->snmp_insetrequests++;
> + if (!(msg->sm_aflags & ADDRESS_FLAG_WRITE)) {
> + msg->sm_errstr = "write requests disabled";
> + goto fail;
> + }
> if (msg->sm_version != SNMP_V3 &&
> (env->sc_readonly ||
> strcmp(env->sc_rwcommunity, msg->sm_community) != 0)) {
> @@ -320,16 +329,40 @@ snmpe_parse(struct snmp_message *msg)
> goto parsefail;
>
> case SNMP_C_TRAP:
> + if (msg->sm_version != SNMP_V1) {
> + msg->sm_errstr = "trapv1 request on !SNMPv1 message";
> + goto parsefail;
> + }
> case SNMP_C_TRAPV2:
> - if (msg->sm_version != SNMP_V3 &&
> - strcmp(env->sc_trcommunity, msg->sm_community) != 0) {
> + if (type == SNMP_C_TRAPV2 &&
> + !(msg->sm_version == SNMP_V2 ||
> + msg->sm_version != SNMP_V3)) {
> + msg->sm_errstr = "trapv2 request on !SNMPv2C or "
> + "!SNMPv3 message";
> + goto parsefail;
> + }
> + if (!(msg->sm_aflags & ADDRESS_FLAG_NOTIFY)) {
> + msg->sm_errstr = "notify requests disabled";
> + goto fail;
> + }
> + if (msg->sm_version == SNMP_V3) {
> + msg->sm_errstr = "SNMPv3 doesn't support traps yet";
> + goto fail;
> + }
> + if (strcmp(env->sc_trcommunity, msg->sm_community) != 0) {
> stats->snmp_inbadcommunitynames++;
> msg->sm_errstr = "wrong trap community";
> goto fail;
> }
> stats->snmp_intraps++;
> - msg->sm_errstr = "received trap";
> - goto fail;
> + /*
> + * This should probably go into parsevarbinds, but that's for
> a
> + * next refactor
> + */
> + if (traphandler_parse(msg) == -1)
> + goto fail;
> + /* Shortcircuit */
> + return 0;
>
> default:
> msg->sm_errstr = "invalid context";
> @@ -356,15 +389,15 @@ snmpe_parse(struct snmp_message *msg)
>
> print_host(ss, msg->sm_host, sizeof(msg->sm_host));
> if (msg->sm_version == SNMP_V3)
> - log_debug("%s: %s: SNMPv3 context %d, flags %#x, "
> + log_debug("%s: %s:%hd: SNMPv3 context %d, flags %#x, "
> "secmodel %lld, user '%s', ctx-engine %s, ctx-name '%s', "
> - "request %lld", __func__, msg->sm_host, msg->sm_context,
> - msg->sm_flags, msg->sm_secmodel, msg->sm_username,
> - tohexstr(msg->sm_ctxengineid, msg->sm_ctxengineid_len),
> - msg->sm_ctxname, msg->sm_request);
> + "request %lld", __func__, msg->sm_host, msg->sm_port,
> + msg->sm_context, msg->sm_flags, msg->sm_secmodel,
> + msg->sm_username, tohexstr(msg->sm_ctxengineid,
> + msg->sm_ctxengineid_len), msg->sm_ctxname,
> msg->sm_request);
> else
> - log_debug("%s: %s: SNMPv%d '%s' context %d request %lld",
> - __func__, msg->sm_host, msg->sm_version + 1,
> + log_debug("%s: %s:%hd: SNMPv%d '%s' context %d request %lld",
> + __func__, msg->sm_host, msg->sm_port, msg->sm_version + 1,
> msg->sm_community, msg->sm_context, msg->sm_request);
>
> return (0);
> @@ -373,7 +406,8 @@ snmpe_parse(struct snmp_message *msg)
> stats->snmp_inasnparseerrs++;
> fail:
> print_host(ss, msg->sm_host, sizeof(msg->sm_host));
> - log_debug("%s: %s: %s", __func__, msg->sm_host, msg->sm_errstr);
> + log_debug("%s: %s:%hd: %s", __func__, msg->sm_host, msg->sm_port,
> + msg->sm_errstr);
> return (-1);
> }
>
> @@ -403,8 +437,8 @@ snmpe_parsevarbinds(struct snmp_message
> if (o.bo_n < BER_MIN_OID_LEN || o.bo_n > BER_MAX_OID_LEN)
> goto varfail;
>
> - log_debug("%s: %s: oid %s", __func__, msg->sm_host,
> - smi_oid2string(&o, buf, sizeof(buf), 0));
> + log_debug("%s: %s:%hd: oid %s", __func__, msg->sm_host,
> + msg->sm_port, smi_oid2string(&o, buf, sizeof(buf), 0));
>
> /*
> * XXX intotalreqvars should only be incremented after all are
> @@ -479,8 +513,8 @@ snmpe_parsevarbinds(struct snmp_message
>
> return 0;
> varfail:
> - log_debug("%s: %s: %s, error index %d", __func__,
> - msg->sm_host, msg->sm_errstr, i);
> + log_debug("%s: %s:%hd: %s, error index %d", __func__,
> + msg->sm_host, msg->sm_port, msg->sm_errstr, i);
> if (msg->sm_error == 0)
> msg->sm_error = SNMP_ERROR_GENERR;
> msg->sm_errorindex = i;
> @@ -515,6 +549,10 @@ snmpe_acceptcb(int fd, short type, void
> if ((msg = calloc(1, sizeof(*msg))) == NULL)
> goto fail;
>
> + memcpy(&(msg->sm_ss), &ss, len);
> + msg->sm_slen = len;
> + msg->sm_aflags = h->flags;
> + msg->sm_port = h->port;
> snmpe_prepare_read(msg, afd);
> return;
> fail:
> @@ -635,6 +673,10 @@ snmpe_writecb(int fd, short type, void *
>
> if ((nmsg = calloc(1, sizeof(*nmsg))) == NULL)
> goto fail;
> + memcpy(&(nmsg->sm_ss), &(msg->sm_ss), msg->sm_slen);
> + nmsg->sm_slen = msg->sm_slen;
> + nmsg->sm_aflags = msg->sm_aflags;
> + nmsg->sm_port = msg->sm_port;
>
> /*
> * Reuse the connection.
> @@ -662,16 +704,18 @@ snmpe_writecb(int fd, short type, void *
> void
> snmpe_recvmsg(int fd, short sig, void *arg)
> {
> - struct snmpd *env = arg;
> - struct snmp_stats *stats = &env->sc_stats;
> + struct address *h = arg;
> + struct snmp_stats *stats = &snmpd_env->sc_stats;
> ssize_t len;
> struct snmp_message *msg;
>
> if ((msg = calloc(1, sizeof(*msg))) == NULL)
> return;
>
> + msg->sm_aflags = h->flags;
> msg->sm_sock = fd;
> msg->sm_slen = sizeof(msg->sm_ss);
> + msg->sm_port = h->port;
> if ((len = recvfromto(fd, msg->sm_data, sizeof(msg->sm_data), 0,
> (struct sockaddr *)&msg->sm_ss, &msg->sm_slen,
> (struct sockaddr *)&msg->sm_local_ss, &msg->sm_local_slen)) < 1) {
> @@ -715,6 +759,11 @@ snmpe_recvmsg(int fd, short sig, void *a
> void
> snmpe_dispatchmsg(struct snmp_message *msg)
> {
> + if (msg->sm_type == SNMP_C_TRAP ||
> + msg->sm_type == SNMP_C_TRAPV2) {
> + snmp_msgfree(msg);
> + return;
> + }
> /* dispatched to subagent */
> /* XXX Do proper error handling */
> (void) snmpe_parsevarbinds(msg);
> Index: traphandler.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/traphandler.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 traphandler.c
> --- traphandler.c 5 Jan 2021 18:12:15 -0000 1.19
> +++ traphandler.c 5 Jan 2021 21:22:12 -0000
> @@ -43,17 +43,10 @@
> #include "snmpd.h"
> #include "mib.h"
>
> -char trap_path[PATH_MAX];
> -
> -void traphandler_init(struct privsep *, struct privsep_proc *, void *arg);
> -int traphandler_dispatch_parent(int, struct privsep_proc *, struct imsg
> *);
> -int traphandler_bind(struct address *);
> -void traphandler_recvmsg(int, short, void *);
> int traphandler_priv_recvmsg(struct privsep_proc *, struct imsg *);
> int traphandler_fork_handler(struct privsep_proc *, struct imsg *);
> -int traphandler_parse(struct ber_element *, char *, struct sockaddr *);
> struct ber_element *
> - traphandler_v1translate(struct ber_element *, char *, int);
> + traphandler_v1translate(struct snmp_message *, int);
> int trapcmd_cmp(struct trapcmd *, struct trapcmd *);
> void trapcmd_exec(struct trapcmd *, struct sockaddr *,
> struct ber_element *);
> @@ -65,172 +58,14 @@ RB_GENERATE(trapcmd_tree, trapcmd, cmd_e
>
> struct trapcmd_tree trapcmd_tree = RB_INITIALIZER(&trapcmd_tree);
>
> -static struct privsep_proc procs[] = {
> - { "parent", PROC_PARENT, traphandler_dispatch_parent }
> -};
> -
> -void
> -traphandler(struct privsep *ps, struct privsep_proc *p)
> -{
> - struct snmpd *env = ps->ps_env;
> - struct address *h;
> -
> - if (env->sc_traphandler) {
> - TAILQ_FOREACH(h, &env->sc_addresses, entry) {
> - if (h->type != SOCK_DGRAM)
> - continue;
> - if ((h->fd = traphandler_bind(h)) == -1)
> - fatal("could not create trap listener
> socket");
> - }
> - }
> -
> - proc_run(ps, p, procs, nitems(procs), traphandler_init, NULL);
> -}
> -
> -void
> -traphandler_init(struct privsep *ps, struct privsep_proc *p, void *arg)
> -{
> - struct snmpd *env = ps->ps_env;
> - struct address *h;
> -
> - if (pledge("stdio id proc recvfd exec", NULL) == -1)
> - fatal("pledge");
> -
> - if (!env->sc_traphandler)
> - return;
> -
> - /* listen for SNMP trap messages */
> - TAILQ_FOREACH(h, &env->sc_addresses, entry) {
> - event_set(&h->ev, h->fd, EV_READ|EV_PERSIST,
> - traphandler_recvmsg, NULL);
> - event_add(&h->ev, NULL);
> - }
> -}
> -
> -int
> -traphandler_bind(struct address *addr)
> -{
> - int s;
> - char buf[512];
> - struct sockaddr_in *sin;
> - struct sockaddr_in6 *sin6;
> -
> - if (addr->ss.ss_family == AF_INET) {
> - sin = (struct sockaddr_in *)&(addr->ss);
> - sin->sin_port = htons(162);
> - } else {
> - sin6 = (struct sockaddr_in6 *)&(addr->ss);
> - sin6->sin6_port = htons(162);
> - }
> - if ((s = snmpd_socket_af(&addr->ss, SOCK_DGRAM)) == -1)
> - return (-1);
> -
> - if (fcntl(s, F_SETFL, O_NONBLOCK) == -1)
> - goto bad;
> -
> - if (bind(s, (struct sockaddr *)&addr->ss, addr->ss.ss_len) == -1)
> - goto bad;
> -
> - if (print_host(&addr->ss, buf, sizeof(buf)) == NULL)
> - goto bad;
> -
> - log_info("traphandler: listening on %s:%s", buf, SNMPD_TRAPPORT);
> -
> - return (s);
> - bad:
> - close (s);
> - return (-1);
> -}
> -
> -void
> -traphandler_shutdown(void)
> -{
> - struct address *h;
> -
> - TAILQ_FOREACH(h, &snmpd_env->sc_addresses, entry) {
> - event_del(&h->ev);
> - close(h->fd);
> - }
> -}
> -
> -int
> -traphandler_dispatch_parent(int fd, struct privsep_proc *p, struct imsg
> *imsg)
> -{
> - switch (imsg->hdr.type) {
> - default:
> - break;
> - }
> -
> - return (-1);
> -}
> -
> -int
> -snmpd_dispatch_traphandler(int fd, struct privsep_proc *p, struct imsg *imsg)
> -{
> - switch (imsg->hdr.type) {
> - case IMSG_ALERT:
> - return (traphandler_priv_recvmsg(p, imsg));
> - default:
> - break;
> - }
> -
> - return (-1);
> -}
> -
> -void
> -traphandler_recvmsg(int fd, short events, void *arg)
> -{
> - struct ber ber = {0};
> - struct ber_element *msg = NULL, *pdu;
> - char buf[8196];
> - struct sockaddr_storage ss;
> - socklen_t slen;
> - ssize_t n;
> - int vers;
> - char *community;
> -
> - slen = sizeof(ss);
> - if ((n = recvfrom(fd, buf, sizeof(buf), 0, (struct sockaddr *)&ss,
> - &slen)) == -1)
> - return;
> -
> - ober_set_application(&ber, smi_application);
> - ober_set_readbuf(&ber, buf, n);
> -
> - if ((msg = ober_read_elements(&ber, NULL)) == NULL)
> - goto parsefail;
> -
> - if (ober_scanf_elements(msg, "{dse", &vers, &community, &pdu) == -1)
> - goto parsefail;
> -
> - switch (vers) {
> - case SNMP_V1:
> - if (pdu->be_type != SNMP_C_TRAP)
> - goto parsefail;
> - break;
> - case SNMP_V2:
> - if (pdu->be_type != SNMP_C_TRAPV2)
> - goto parsefail;
> - break;
> - default:
> - goto parsefail;
> - }
> -
> - (void)traphandler_parse(pdu, community, (struct sockaddr *)&ss);
> -
> -parsefail:
> - ober_free(&ber);
> - if (msg != NULL)
> - ober_free_elements(msg);
> -}
> -
> /*
> * Validate received message
> */
> int
> -traphandler_parse(struct ber_element *pdu, char *community, struct sockaddr
> *sa)
> +traphandler_parse(struct snmp_message *msg)
> {
> struct privsep *ps = &snmpd_env->sc_ps;
> + struct snmp_stats *stats = &snmpd_env->sc_stats;
> struct ber ber = {0};
> struct ber_element *vblist = NULL, *elm, *elm2;
> struct ber_oid o1, o2, snmpTrapOIDOID;
> @@ -241,22 +76,24 @@ traphandler_parse(struct ber_element *pd
> ssize_t buflen;
> int ret = -1;
>
> - switch (pdu->be_type) {
> + switch (msg->sm_pdu->be_type) {
> case SNMP_C_TRAP:
> - vblist = traphandler_v1translate(pdu, community, 0);
> - if (vblist == NULL)
> + if ((vblist = traphandler_v1translate(msg, 0)) == NULL)
> goto done;
> break;
> case SNMP_C_TRAPV2:
> - if (ober_scanf_elements(pdu, "{SSe}", &elm) == -1)
> + if (ober_scanf_elements(msg->sm_pdu, "{SSe}", &elm) == -1) {
> + stats->snmp_inasnparseerrs++;
> goto done;
> - if (elm->be_type != BER_TYPE_INTEGER)
> + }
> + if (elm->be_type != BER_TYPE_INTEGER) {
> + stats->snmp_inasnparseerrs++;
> goto done;
> + }
> vblist = ober_unlink_elements(elm);
> break;
> default:
> - log_warnx("unsupported SNMP trap version '%d'", pdu->be_type);
> - goto done;
> + fatalx("%s called without proper context", __func__);
> }
>
> (void)ober_string2oid("1.3.6.1.2.1.1.3.0", &sysUpTimeOID);
> @@ -265,29 +102,36 @@ traphandler_parse(struct ber_element *pd
> &snmpTrapOID) == -1 ||
> ober_oid_cmp(&o1, &sysUpTimeOID) != 0 ||
> ober_oid_cmp(&o2, &snmpTrapOIDOID) != 0) {
> + stats->snmp_inasnparseerrs++;
> goto done;
> }
> (void)ober_scanf_elements(vblist, "{Se", &elm);
> for (elm = elm->be_next; elm != NULL; elm = elm->be_next) {
> if (ober_scanf_elements(elm, "{oe}", &o1, &elm2) == -1 ||
> - elm2->be_next != NULL)
> + elm2->be_next != NULL) {
> + stats->snmp_inasnparseerrs++;
> goto done;
> + }
> }
>
> ober_set_application(&ber, smi_application);
>
> if ((buflen = ober_write_elements(&ber, vblist)) == -1 ||
> - ober_get_writebuf(&ber, &buf) == -1)
> + ober_get_writebuf(&ber, &buf) == -1) {
> + msg->sm_errstr = "failed to handle trap";
> goto done;
> + }
>
> - iov[0].iov_base = sa;
> - iov[0].iov_len = sa->sa_len;
> + iov[0].iov_base = &(msg->sm_ss);
> + iov[0].iov_len = msg->sm_slen;
> iov[1].iov_base = buf;
> iov[1].iov_len = buflen;
>
> /* Forward it to the parent process */
> - if (proc_composev(ps, PROC_PARENT, IMSG_ALERT, iov, 2) == -1)
> + if (proc_composev(ps, PROC_PARENT, IMSG_TRAP_EXEC, iov, 2) == -1) {
> + msg->sm_errstr = "failed to handle trap";
> goto done;
> + }
>
> ret = 0;
> done:
> @@ -298,8 +142,9 @@ done:
> }
>
> struct ber_element *
> -traphandler_v1translate(struct ber_element *pdu, char *community, int proxy)
> +traphandler_v1translate(struct snmp_message *msg, int proxy)
> {
> + struct snmp_stats *stats = &snmpd_env->sc_stats;
> struct ber_oid trapoid, enterprise, oid, snmpTrapAddressOid;
> struct ber_oid snmpTrapCommunityOid, snmpTrapEnterpriseOid;
> struct ber_element *elm, *last, *vblist, *vb0 = NULL;
> @@ -308,12 +153,12 @@ traphandler_v1translate(struct ber_eleme
> int generic_trap, specific_trap, time_stamp;
> int hasaddress = 0, hascommunity = 0, hasenterprise = 0;
>
> - if (ober_scanf_elements(pdu, "{oxddde", &enterprise, &agent_addr,
> - &agent_addrlen, &generic_trap, &specific_trap, &time_stamp,
> - &vblist) == -1 ||
> + if (ober_scanf_elements(msg->sm_pdu, "{oxddde", &enterprise,
> + &agent_addr, &agent_addrlen, &generic_trap, &specific_trap,
> + &time_stamp, &vblist) == -1 ||
> agent_addrlen != 4 ||
> vblist->be_type != BER_TYPE_SEQUENCE) {
> - errno = EINVAL;
> + stats->snmp_inasnparseerrs++;
> return NULL;
> }
> switch (generic_trap) {
> @@ -337,15 +182,16 @@ traphandler_v1translate(struct ber_eleme
> break;
> case 6:
> trapoid = enterprise;
> + /* Officially this should be 128, but BER_MAX_OID_LEN is 64 */
> if (trapoid.bo_n + 2 > BER_MAX_OID_LEN) {
> - errno = EINVAL;
> + stats->snmp_inasnparseerrs++;
> return NULL;
> }
> trapoid.bo_id[trapoid.bo_n++] = 0;
> trapoid.bo_id[trapoid.bo_n++] = specific_trap;
> break;
> default:
> - errno = EINVAL;
> + stats->snmp_inasnparseerrs++;
> return NULL;
> }
>
> @@ -354,12 +200,14 @@ traphandler_v1translate(struct ber_eleme
> vb0 = ober_unlink_elements(vblist);
>
> if ((vblist = ober_add_sequence(NULL)) == NULL) {
> + msg->sm_errstr = strerror(errno);
> if (vb0 != NULL)
> ober_free_elements(vb0);
> return NULL;
> }
> if (ober_printf_elements(vblist, "{od}{oO}e", "1.3.6.1.2.1.1.3.0",
> time_stamp, "1.3.6.1.6.3.1.1.4.1.0", &trapoid, vb0) == NULL) {
> + msg->sm_errstr = strerror(errno);
> if (vb0 != 0)
> ober_free_elements(vb0);
> ober_free_elements(vblist);
> @@ -375,6 +223,7 @@ traphandler_v1translate(struct ber_eleme
> &snmpTrapEnterpriseOid);
> for (elm = vblist->be_sub; elm != NULL; elm = elm->be_next) {
> if (ober_get_oid(elm->be_sub, &oid) == -1) {
> + msg->sm_errstr = "failed to read oid";
> ober_free_elements(vblist);
> return NULL;
> }
> @@ -391,8 +240,9 @@ traphandler_v1translate(struct ber_eleme
> if (ober_printf_elements(last, "{Oxt}{Os}{OO}",
> &snmpTrapAddressOid, agent_addr, 4,
> BER_CLASS_APPLICATION, SNMP_T_IPADDR,
> - &snmpTrapCommunityOid, community,
> + &snmpTrapCommunityOid, msg->sm_community,
> &snmpTrapEnterpriseOid, &enterprise) == NULL) {
> + msg->sm_errstr = strerror(errno);
> ober_free_elements(vblist);
> return NULL;
> }
>
