Found some minor issues.
Please disregard for now.
On Tue, 2020-12-01 at 17:12 +0100, Martijn van Duren wrote:
> Hello tech@,
>
> Long story short: the traphandler process in snmpd annoys me a great
> deal and is in the way for overhauling the transport mapping section
> of snmpe, which is needed for implementing new agentx master support.
>
> The current traphandler process is also a joke, since all it does is
> receive a message, does a minimal parsing validation and then pass
> then entire buffer to the parent process, which forks the actual
> handlers. This means that the current pledgeset is also way too wide
> for what traphandler does.
>
> Since snmpe already does a similar packet parsing I see no reason to
> keep this initial verification in the traphandler process. The diff
> below drops the traphandler process and moves the receiving of the
> packets to snmpe, which then forwards the pdu (instead of the
> current whole packet) to the parent. I also extended the checking, so
> it should be a little more resilient to malformed packets.
>
> While here I also didn't like the fact that listening on a trap-port
> always implies that snmp itself is also listening. This diff adds the
> trap keyword (for udp) on the listen on line, so that we can setup a
> traphandler-only setup. This gives the added bonus that we can now
> also choose the port we listen on for traps. Adding a listen trap
> statement requires a trap handle and vice versa.
>
> It's not the pretties code, but it's a stopgap that allows me to move
> forward without creating even larger diffs and without (hopefully)
> breaking existing setups (apart from the keyword change).
>
> 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 1 Dec 2020 16:12:20 -0000
> @@ -94,11 +94,9 @@ 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);
>
> typedef struct {
> union {
> @@ -284,13 +282,16 @@ listenproto : UDP listen_udp
> listen_udp : STRING port {
> struct sockaddr_storage ss[16];
> int nhosts, i;
> + char *port = $2;
>
> - nhosts = host($1, $2, SOCK_DGRAM, ss, nitems(ss));
> + if (port == NULL)
> + port = SNMPD_PORT;
> +
> + 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))
> @@ -298,10 +299,38 @@ listen_udp : STRING port {
> $1, $2, nitems(ss));
>
> free($1);
> - if ($2 != snmpd_port)
> + free($2);
> + for (i = 0; i < nhosts; i++) {
> + if (listen_add(&(ss[i]), SOCK_DGRAM, 0) ==
> -1) {
> + yyerror("calloc");
> + YYERROR;
> + }
> + }
> + }
> + | STRING port TRAP {
> + struct sockaddr_storage ss[16];
> + int nhosts, i;
> + char *port = $2;
> +
> + if (port == NULL)
> + port = SNMPD_TRAPPORT;
> +
> + nhosts = host($1, port, SOCK_DGRAM, ss, nitems(ss));
> + if (nhosts < 1) {
> + yyerror("invalid address: %s", $1);
> + free($1);
> free($2);
> + YYERROR;
> + }
> + if (nhosts > (int)nitems(ss))
> + log_warn("%s:%s resolves to more than %zu
> hosts",
> + $1, $2, nitems(ss));
> +
> + free($1);
> + free($2);
> for (i = 0; i < nhosts; i++) {
> - if (listen_add(&(ss[i]), SOCK_DGRAM) == -1) {
> + if (listen_add(&(ss[i]), SOCK_DGRAM,
> + ADDRESS_FLAG_TRAP) == -1) {
> yyerror("calloc");
> YYERROR;
> }
> @@ -311,13 +340,16 @@ listen_udp : STRING port {
> listen_tcp : STRING port {
> struct sockaddr_storage ss[16];
> int nhosts, i;
> + char *port = $2;
> +
> + if (port == NULL)
> + port = SNMPD_PORT;
>
> - nhosts = host($1, $2, SOCK_STREAM, ss, nitems(ss));
> + 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))
> @@ -325,10 +357,10 @@ listen_tcp : STRING port {
> $1, $2, 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,
> + 0) == -1) {
> yyerror("calloc");
> YYERROR;
> }
> @@ -336,7 +368,7 @@ listen_tcp : STRING port {
> }
>
> port : /* empty */ {
> - $$ = snmpd_port;
> + $$ = NULL;
> }
> | PORT STRING {
> $$ = $2;
> @@ -1079,7 +1111,7 @@ parse_config(const char *filename, u_int
> {
> struct sockaddr_storage ss;
> struct sym *sym, *next;
> - struct address *h;
> + struct address *h, *htmp;
> int found;
>
> if ((conf = calloc(1, sizeof(*conf))) == NULL) {
> @@ -1108,15 +1140,28 @@ parse_config(const char *filename, u_int
>
> endservent();
>
> + found = 0;
> + TAILQ_FOREACH_SAFE(h, &conf->sc_addresses, entry, htmp) {
> + if (h->flags & ADDRESS_FLAG_TRAP) {
> + if (!conf->sc_traphandler) {
> + if (!found)
> + fatalx("No trap handle configured");
> + }
> + found = 1;
> + }
> + }
> + if (conf->sc_traphandler && !found)
> + fatalx("Trap handle configured without trap listener");
> +
> /* Setup default listen addresses */
> if (TAILQ_EMPTY(&conf->sc_addresses)) {
> if (host("0.0.0.0", SNMPD_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)
> 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) {
> @@ -1258,7 +1303,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 +1320,7 @@ listen_add(struct sockaddr_storage *ss,
> h->port = ntohs(sin6->sin6_port);
> }
> h->type = type;
> + h->flags = flags;
> 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 1 Dec 2020 16:12:20 -0000
> @@ -51,9 +51,7 @@ int check_child(pid_t, const char *);
> 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 }
> + { "snmpe", PROC_SNMPE, snmpd_dispatch_snmpe, snmpe, snmpe_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_ALERT:
> + 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 1 Dec 2020 16:12:20 -0000
> @@ -95,13 +95,33 @@ 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
> trap
> Specify the local address
> .Xr snmpd 8
> should listen on for incoming SNMP messages.
> Multiple
> .Ic listen on
> -statements are supported, the default is UDP.
> +statements are supported, the default is
> +.Ic udp .
> +The default
> +.Ar port
> +is 161.
> +.Pp
> +The
> +.Ic trap
> +keyword disables support for get and set queries and enables support for
> +receiving traps.
> +Currently only
> +.Ic udp
> +listeners support
> +.Ic trap .
> +At least one
> +.Ic trap handle
> +directive must be set.
> +The default
> +.Ic trap
> +.Ar port
> +port is 162.
> .It Ic read-only community Ar string
> Specify the name of the read-only community.
> The default value is
> @@ -193,9 +213,11 @@ the resolved hostname of the host sendin
> the IP address of the host sending the trap,
> and any variable bindings contained in the trap
> (the OID followed by the value, separated by a single space).
> -Traps will be accepted on all
> -.Ic listen on
> -UDP addresses.
> +Setting
> +.Ic trap handle
> +requires that at least one
> +.Ic listen on trap
> +command is set.
> .It Xo
> .Ic trap receiver Ar string
> .Op Ic oid Ar oid-string
> 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 1 Dec 2020 16:12:20 -0000
> @@ -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
> };
>
> @@ -386,6 +385,7 @@ struct snmp_message {
> int sm_sock_tcp;
> struct event sm_sockev;
> char sm_host[HOST_NAME_MAX+1];
> + int sm_addressflags;
>
> struct sockaddr_storage sm_local_ss;
> socklen_t sm_local_slen;
> @@ -483,11 +483,15 @@ struct address {
> in_port_t port;
> int type;
> int fd;
> + int flags;
> struct event ev;
> struct event evt;
>
> TAILQ_ENTRY(address) entry;
> };
> +
> +#define ADDRESS_FLAG_TRAP 0x01
> +
> TAILQ_HEAD(addresslist, address);
>
> struct trap_address {
> @@ -588,6 +592,7 @@ extern struct snmpd *snmpd_env;
> /* parse.y */
> struct snmpd *parse_config(const char *, u_int);
> int cmdline_symset(char *);
> +int listen_add(struct sockaddr_storage *, int, int);
>
> /* log.c */
> void log_init(int, int);
> @@ -761,9 +766,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 1 Dec 2020 16:12:20 -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,12 @@ snmpe_parse(struct snmp_message *msg)
> if (class != BER_CLASS_CONTEXT)
> goto parsefail;
>
> + if (msg->sm_addressflags & ADDRESS_FLAG_TRAP &&
> + type != SNMP_C_TRAP && type != SNMP_C_TRAPV2) {
> + msg->sm_errstr = "invalid request on trap port";
> + goto fail;
> + }
> +
> switch (type) {
> case SNMP_C_GETBULKREQ:
> if (msg->sm_version == SNMP_V1) {
> @@ -320,7 +326,18 @@ snmpe_parse(struct snmp_message *msg)
> goto parsefail;
>
> case SNMP_C_TRAP:
> + if (msg->sm_version != SNMP_V1) {
> + stats->snmp_inbadversions++;
> + msg->sm_errstr = "SNMPv1-trap on new protocol";
> + goto fail;
> + }
> case SNMP_C_TRAPV2:
> + if (type == SNMP_C_TRAPV2 &&
> + msg->sm_version != SNMP_V2 && msg->sm_version != SNMP_V3)
> {
> + stats->snmp_inbadversions++;
> + msg->sm_errstr = "SNMPv2-trap on wrong protocol";
> + goto fail;
> + }
> if (msg->sm_version != SNMP_V3 &&
> strcmp(env->sc_trcommunity, msg->sm_community) != 0) {
> stats->snmp_inbadcommunitynames++;
> @@ -328,8 +345,13 @@ snmpe_parse(struct snmp_message *msg)
> goto fail;
> }
> stats->snmp_intraps++;
> - msg->sm_errstr = "received trap";
> - goto fail;
> + if ((msg->sm_addressflags & ADDRESS_FLAG_TRAP) == 0) {
> + msg->sm_errstr = "received trap";
> + goto fail;
> + }
> + if (traphandler_parse(msg) == -1)
> + goto fail;
> + return (0);
>
> default:
> msg->sm_errstr = "invalid context";
> @@ -662,8 +684,8 @@ 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;
>
> @@ -672,6 +694,7 @@ snmpe_recvmsg(int fd, short sig, void *a
>
> msg->sm_sock = fd;
> msg->sm_slen = sizeof(msg->sm_ss);
> + msg->sm_addressflags = h->flags;
> 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) {
> Index: traphandler.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/traphandler.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 traphandler.c
> --- traphandler.c 6 Sep 2020 15:51:28 -0000 1.18
> +++ traphandler.c 1 Dec 2020 16:12:20 -0000
> @@ -42,16 +42,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_extract(struct snmp_message *, struct ber_element *,
> + u_int *, struct ber_oid *, struct ber_element **);
> int traphandler_priv_recvmsg(struct privsep_proc *, struct imsg *);
> int traphandler_fork_handler(struct privsep_proc *, struct imsg *);
> -int traphandler_parse(char *, size_t, struct ber_element **,
> - struct ber_element **, u_int *, struct ber_oid *);
> void traphandler_v1translate(struct ber_oid *, u_int, u_int);
>
> int trapcmd_cmp(struct trapcmd *, struct trapcmd *);
> @@ -65,208 +59,105 @@ 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, ps);
> - 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 privsep *ps = arg;
> - char buf[8196];
> - struct iovec iov[2];
> - struct sockaddr_storage ss;
> - socklen_t slen;
> - ssize_t n;
> - struct ber_element *req, *iter;
> - struct ber_oid trapoid;
> - u_int uptime;
> -
> - slen = sizeof(ss);
> - if ((n = recvfrom(fd, buf, sizeof(buf), 0, (struct sockaddr *)&ss,
> - &slen)) == -1)
> - return;
> -
> - if (traphandler_parse(buf, n, &req, &iter, &uptime, &trapoid) == -1)
> - goto done;
> -
> - iov[0].iov_base = &ss;
> - iov[0].iov_len = ss.ss_len;
> - iov[1].iov_base = buf;
> - iov[1].iov_len = n;
> -
> - /* Forward it to the parent process */
> - if (proc_composev(ps, PROC_PARENT, IMSG_ALERT, iov, 2) == -1)
> - goto done;
> -
> - done:
> - if (req != NULL)
> - ober_free_elements(req);
> - return;
> -}
> -
> /*
> - * Validate received message
> + * Validate received message let parent create a handler process.
> */
> int
> -traphandler_parse(char *buf, size_t n, struct ber_element **req,
> - struct ber_element **vbinds, u_int *uptime, struct ber_oid *trapoid)
> +traphandler_parse(struct snmp_message *msg)
> {
> struct ber ber;
> - struct ber_element *elm;
> - u_int vers, gtype, etype;
> + struct ber_oid oid;
> + struct ber_element *vb;
> + struct iovec iov[2];
> + u_int uptime;
> + ssize_t len;
> + char *pdu;
> +
> + if (traphandler_extract(msg, msg->sm_pdu, &uptime, &oid, &vb) == -1)
> + return -1;
>
> bzero(&ber, sizeof(ber));
> ober_set_application(&ber, smi_application);
> - ober_set_readbuf(&ber, buf, n);
>
> - if ((*req = ober_read_elements(&ber, NULL)) == NULL)
> - goto done;
> + if ((len = ober_write_elements(&ber, msg->sm_pdu)) == -1) {
> + /* ober_write_elements can only return ENOMEM on error */
> + msg->sm_errstr = "failed to handle trap: Cannot allocate
> memory";
> + ober_free(&ber);
> + return -1;
> + }
> + ober_get_writebuf(&ber, (void **)&pdu);
> +
> + iov[0].iov_base = &(msg->sm_ss);
> + iov[0].iov_len = msg->sm_ss.ss_len;
> + iov[1].iov_base = pdu;
> + iov[1].iov_len = (size_t)len;
> +
> + if (proc_composev(&(snmpd_env->sc_ps), PROC_PARENT, IMSG_ALERT, iov,
> + 2) == -1) {
> + ober_free(&ber);
> + /* We need a better error reporting system, no errno for now
> */
> + msg->sm_errstr = "Failed to handle trap";
> + return -1;
> + }
>
> - if (ober_scanf_elements(*req, "{dSe", &vers, &elm) == -1)
> - goto done;
> + ober_free(&ber);
> + return (0);
> +}
>
> - switch (vers) {
> - case SNMP_V1:
> - if (ober_scanf_elements(elm, "{oSddde",
> - trapoid, >ype, &etype, uptime, &elm) == -1)
> - goto done;
> - traphandler_v1translate(trapoid, gtype, etype);
> - if (elm->be_type != BER_TYPE_SEQUENCE)
> - goto done;
> - *vbinds = elm->be_sub;
> +int
> +traphandler_extract(struct snmp_message *msg, struct ber_element *pdu,
> + u_int *uptime, struct ber_oid *trapoid, struct ber_element **vb0)
> +{
> + struct ber_oid oid;
> + u_int class, gtype, etype;
> + unsigned int type;
> + struct ber_element *vb, *tmp;
> + /* OK to set stats twice, they only count in the snmpe process */
> + struct snmp_stats *stats = &(snmpd_env->sc_stats);
> + char *buf;
> +
> + if (ober_scanf_elements(pdu, "t", &class, &type) != 0) {
> + msg->sm_errstr = "Invalid trap format";
> + stats->snmp_inasnparseerrs++;
> + return -1;
> + }
> +
> + switch (type) {
> + case SNMP_C_TRAP:
> + if (ober_scanf_elements(pdu, "{oSddd{e", trapoid, >ype,
> &etype,
> + &uptime, vb0) == -1) {
> + msg->sm_errstr = "Invalid trap format";
> + stats->snmp_inasnparseerrs++;
> + return -1;
> + }
> break;
> -
> - case SNMP_V2:
> - if (ober_scanf_elements(elm, "{SSS{e}}", &elm) == -1 ||
> - ober_scanf_elements(elm, "{Sd}{So}",
> - uptime, trapoid) == -1)
> - goto done;
> - *vbinds = elm->be_next->be_next;
> + case SNMP_C_TRAPV2:
> + if (ober_scanf_elements(pdu, "{SSS{e", &vb) == -1 ||
> + ober_scanf_elements(vb, "{Sd}{So}",
> + &uptime, trapoid) == -1) {
> + msg->sm_errstr = "Invalid trapv2 format";
> + stats->snmp_inasnparseerrs++;
> + return -1;
> + }
> + *vb0 = vb->be_next->be_next;
> break;
> -
> default:
> - log_warnx("unsupported SNMP trap version '%d'", vers);
> - goto done;
> + msg->sm_errstr = "invalid pdu type";
> + stats->snmp_inasnparseerrs++;
> + return -1;
> }
>
> - ober_free(&ber);
> - return (0);
> + for (vb = *vb0; vb != NULL; vb = vb->be_next) {
> + if (ober_scanf_elements(vb, "{oe}", &oid, &tmp) == -1 ||
> + (buf = smi_print_element(tmp)) == NULL) {
> + msg->sm_errstr = "invalid varbind in trap";
> + stats->snmp_inasnparseerrs++;
> + return -1;
> + }
> + free(buf);
> + }
>
> - done:
> - ober_free(&ber);
> - if (*req)
> - ober_free_elements(*req);
> - *req = NULL;
> - return (-1);
> + return 0;
> }
>
> void
> @@ -312,7 +203,8 @@ traphandler_fork_handler(struct privsep_
> struct sockaddr *sa;
> char *buf;
> ssize_t n;
> - struct ber_element *req, *iter;
> + struct ber ber;
> + struct ber_element *iter, *pdu;
> struct trapcmd *cmd;
> struct ber_oid trapoid;
> u_int uptime;
> @@ -339,15 +231,21 @@ traphandler_fork_handler(struct privsep_
> n -= sa->sa_len;
> buf = (char *)imsg->data + sa->sa_len;
>
> - if (traphandler_parse(buf, n, &req, &iter, &uptime, &trapoid) == -1)
> + bzero(&ber, sizeof(ber));
> + ober_set_application(&ber, smi_application);
> + ober_set_readbuf(&ber, buf, n);
> + if ((pdu = ober_read_elements(&ber, NULL)) == NULL)
> + fatal("traphandler_fork_handler: ober_read_elements");
> +
> + if (traphandler_extract(NULL, pdu, &uptime, &trapoid, &iter) == -1)
> fatalx("couldn't parse SNMP trap message");
>
> smi_oid2string(&trapoid, oidbuf, sizeof(oidbuf), 0);
> if ((cmd = trapcmd_lookup(&trapoid)) != NULL)
> trapcmd_exec(cmd, sa, iter, oidbuf, uptime);
>
> - if (req != NULL)
> - ober_free_elements(req);
> + ober_free_elements(pdu);
> + ober_free(&ber);
>
> exit(0);
> }
>
>
