Right now snmpd's traphandler_parse and traphandler_v1translate are just
weird to me and do very little ASN1 checking. Since I want to remove the
traphandler process anyway and the current code structue is in the way,
this diff is a logical first step towards that goal.
The new traphandler_v1translate should now be properly build around
RFC3584 section 3.1 and I've choosen to disable the proxy addition,
since we don't forward it to another host, but it does work for me and
can be enabled for this scenario if people feel strong about it.
Another major change people should be aware of is when running snmpd
with -N the sysUpTime and snmpTrapOID parameters will now be numerical
(where these were hardcode printed before).
A minor thing people should be aware of is that the new code is a lot
more strict on its ASN1 validation, so if you use it with crappy
SNMP entities that send broken packets, you're out of luck.
Also, traphandler_parse is now called only once, which should help in my
next step in removing the traphandler process altogether.
OK?
martijn@
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 3 Jan 2021 14:20:17 -0000
@@ -28,6 +28,7 @@
#include <arpa/inet.h>
#include <ber.h>
+#include <errno.h>
#include <event.h>
#include <fcntl.h>
#include <imsg.h>
@@ -50,13 +51,12 @@ 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(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 traphandler_parse(struct ber_element *, char *, struct sockaddr *);
+struct ber_element *
+ traphandler_v1translate(struct ber_element *, char *, int);
int trapcmd_cmp(struct trapcmd *, struct trapcmd *);
void trapcmd_exec(struct trapcmd *, struct sockaddr *,
- struct ber_element *, char *, u_int);
+ struct ber_element *);
char *traphandler_hostname(struct sockaddr *, int);
@@ -102,7 +102,7 @@ traphandler_init(struct privsep *ps, str
/* 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);
+ traphandler_recvmsg, NULL);
event_add(&h->ev, NULL);
}
}
@@ -180,104 +180,216 @@ snmpd_dispatch_traphandler(int fd, struc
void
traphandler_recvmsg(int fd, short events, void *arg)
{
- struct privsep *ps = arg;
+ struct ber ber = {0};
+ struct ber_element *msg = NULL, *pdu;
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;
+ int vers;
+ char *community;
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;
+ ober_set_application(&ber, smi_application);
+ ober_set_readbuf(&ber, buf, n);
- iov[0].iov_base = &ss;
- iov[0].iov_len = ss.ss_len;
- iov[1].iov_base = buf;
- iov[1].iov_len = n;
+ if ((msg = ober_read_elements(&ber, NULL)) == NULL)
+ goto parsefail;
- /* Forward it to the parent process */
- if (proc_composev(ps, PROC_PARENT, IMSG_ALERT, iov, 2) == -1)
- goto done;
+ if (ober_scanf_elements(msg, "{dse", &vers, &community, &pdu) == -1)
+ goto parsefail;
- done:
- if (req != NULL)
- ober_free_elements(req);
- return;
+ 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(char *buf, size_t n, struct ber_element **req,
- struct ber_element **vbinds, u_int *uptime, struct ber_oid *trapoid)
+traphandler_parse(struct ber_element *pdu, char *community, struct sockaddr
*sa)
{
- struct ber ber;
- struct ber_element *elm;
- u_int vers, gtype, etype;
+ struct privsep *ps = &snmpd_env->sc_ps;
+ struct ber ber = {0};
+ struct ber_element *vblist = NULL, *elm, *elm2;
+ struct ber_oid o1, o2, snmpTrapOIDOID;
+ struct ber_oid snmpTrapOID, sysUpTimeOID;
+ int sysUpTime;
+ struct iovec iov[2];
+ void *buf;
+ ssize_t buflen;
+ int ret = -1;
+
+ switch (pdu->be_type) {
+ case SNMP_C_TRAP:
+ vblist = traphandler_v1translate(pdu, community, 0);
+ if (vblist == NULL)
+ goto done;
+ break;
+ case SNMP_C_TRAPV2:
+ if (ober_scanf_elements(pdu, "{SSe}", &elm) == -1)
+ goto done;
+ if (elm->be_type != BER_TYPE_INTEGER)
+ goto done;
+ vblist = ober_unlink_elements(elm);
+ break;
+ default:
+ log_warnx("unsupported SNMP trap version '%d'", pdu->be_type);
+ goto done;
+ }
+
+ (void)ober_string2oid("1.3.6.1.2.1.1.3.0", &sysUpTimeOID);
+ (void)ober_string2oid("1.3.6.1.6.3.1.1.4.1.0", &snmpTrapOIDOID);
+ if (ober_scanf_elements(vblist, "{{od}{oo}", &o1, &sysUpTime, &o2,
+ &snmpTrapOID) == -1 ||
+ ober_oid_cmp(&o1, &sysUpTimeOID) != 0 ||
+ ober_oid_cmp(&o2, &snmpTrapOIDOID) != 0) {
+ goto done;
+ }
+ for (elm = vblist->be_sub->be_next->be_next; elm != NULL;
+ elm = elm->be_next) {
+ if (ober_scanf_elements(elm, "{oe}", &o1, &elm2) == -1 ||
+ elm2->be_next != NULL)
+ goto done;
+ }
- bzero(&ber, sizeof(ber));
ober_set_application(&ber, smi_application);
- ober_set_readbuf(&ber, buf, n);
- if ((*req = ober_read_elements(&ber, NULL)) == NULL)
+ if ((buflen = ober_write_elements(&ber, vblist)) == -1 ||
+ ober_get_writebuf(&ber, &buf) == -1)
goto done;
- if (ober_scanf_elements(*req, "{dSe", &vers, &elm) == -1)
+ iov[0].iov_base = sa;
+ iov[0].iov_len = sa->sa_len;
+ 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)
goto done;
- 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;
+ ret = 0;
+done:
+ ober_free(&ber);
+ if (vblist != NULL)
+ ober_free_elements(vblist);
+ return ret;
+}
+
+struct ber_element *
+traphandler_v1translate(struct ber_element *pdu, char *community, int proxy)
+{
+ struct ber_oid trapoid, enterprise, oid, snmpTrapAddressOid;
+ struct ber_oid snmpTrapCommunityOid, snmpTrapEnterpriseOid;
+ struct ber_element *elm, *last, *vblist, *vb0;
+ void *agent_addr;
+ size_t agent_addrlen;
+ 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 ||
+ agent_addrlen != 4 ||
+ vblist->be_type != BER_TYPE_SEQUENCE) {
+ errno = EINVAL;
+ return NULL;
+ }
+ switch (generic_trap) {
+ case 0:
+ (void)ober_string2oid("1.3.6.1.6.3.1.1.5.1", &trapoid);
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 1:
+ (void)ober_string2oid("1.3.6.1.6.3.1.1.5.2", &trapoid);
+ break;
+ case 2:
+ (void)ober_string2oid("1.3.6.1.6.3.1.1.5.3", &trapoid);
+ break;
+ case 3:
+ (void)ober_string2oid("1.3.6.1.6.3.1.1.5.4", &trapoid);
+ break;
+ case 4:
+ (void)ober_string2oid("1.3.6.1.6.3.1.1.5.5", &trapoid);
+ break;
+ case 5:
+ (void)ober_string2oid("1.3.6.1.6.3.1.1.5.6", &trapoid);
+ break;
+ case 6:
+ trapoid = enterprise;
+ trapoid.bo_id[trapoid.bo_n++] = 0;
+ trapoid.bo_id[trapoid.bo_n++] = specific_trap;
break;
-
default:
- log_warnx("unsupported SNMP trap version '%d'", vers);
- goto done;
+ errno = EINVAL;
+ return NULL;
}
- ober_free(&ber);
- return (0);
-
- done:
- ober_free(&ber);
- if (*req)
- ober_free_elements(*req);
- *req = NULL;
- return (-1);
-}
-
-void
-traphandler_v1translate(struct ber_oid *oid, u_int gtype, u_int etype)
-{
- /* append 'specific trap' number to 'enterprise specific' traps */
- if (gtype >= 6) {
- oid->bo_id[oid->bo_n] = 0;
- oid->bo_id[oid->bo_n + 1] = etype;
- oid->bo_n += 2;
+ vb0 = ober_unlink_elements(vblist);
+ if ((vblist = ober_add_sequence(NULL)) == 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) {
+ ober_free_elements(vb0);
+ ober_free_elements(vblist);
+ return NULL;
+ }
+
+ if (proxy) {
+ (void)ober_string2oid("1.3.6.1.6.3.18.1.3.0",
+ &snmpTrapAddressOid);
+ (void)ober_string2oid("1.3.6.1.6.3.18.1.4.0",
+ &snmpTrapCommunityOid);
+ (void)ober_string2oid("1.3.6.1.6.3.1.1.4.3.0",
+ &snmpTrapEnterpriseOid);
+ for (elm = vblist->be_sub; elm != NULL; elm = elm->be_next) {
+ if (ober_get_oid(elm->be_sub, &oid) == -1) {
+ ober_free_elements(vblist);
+ return NULL;
+ }
+ if (ober_oid_cmp(&oid, &snmpTrapAddressOid) == 0)
+ hasaddress = 1;
+ else if (ober_oid_cmp(&oid, &snmpTrapCommunityOid) == 0)
+ hascommunity = 1;
+ else if (ober_oid_cmp(&oid,
+ &snmpTrapEnterpriseOid) == 0)
+ hasenterprise = 1;
+ last = elm;
+ }
+ if (!hasaddress || !hascommunity || !hasenterprise) {
+ if (ober_printf_elements(last, "{Oxt}{Os}{OO}",
+ &snmpTrapAddressOid, agent_addr, 4,
+ BER_CLASS_APPLICATION, SNMP_T_IPADDR,
+ &snmpTrapCommunityOid, community,
+ &snmpTrapEnterpriseOid, &enterprise) == NULL) {
+ ober_free_elements(vblist);
+ return NULL;
+ }
+ }
}
+ return vblist;
}
int
@@ -308,14 +420,13 @@ traphandler_fork_handler(struct privsep_
{
struct privsep *ps = p->p_ps;
struct snmpd *env = ps->ps_env;
- char oidbuf[SNMP_MAX_OID_STRLEN];
+ struct ber ber = {0};
struct sockaddr *sa;
char *buf;
ssize_t n;
- struct ber_element *req, *iter;
- struct trapcmd *cmd;
+ struct ber_element *vblist;
struct ber_oid trapoid;
- u_int uptime;
+ struct trapcmd *cmd;
struct passwd *pw;
int verbose;
@@ -339,22 +450,28 @@ 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)
+ ober_set_application(&ber, smi_application);
+ ober_set_readbuf(&ber, buf, n);
+
+ if ((vblist = ober_read_elements(&ber, NULL)) == NULL)
fatalx("couldn't parse SNMP trap message");
+ ober_free(&ber);
- smi_oid2string(&trapoid, oidbuf, sizeof(oidbuf), 0);
+ if (ober_get_oid(vblist->be_sub->be_next->be_sub->be_next,
+ &trapoid) == -1)
+ fatalx("couldn't parse SNMP trap message");
+
if ((cmd = trapcmd_lookup(&trapoid)) != NULL)
- trapcmd_exec(cmd, sa, iter, oidbuf, uptime);
+ trapcmd_exec(cmd, sa, vblist->be_sub);
- if (req != NULL)
- ober_free_elements(req);
+ ober_free_elements(vblist);
exit(0);
}
void
trapcmd_exec(struct trapcmd *cmd, struct sockaddr *sa,
- struct ber_element *iter, char *trapoid, u_int uptime)
+ struct ber_element *vb)
{
char oidbuf[SNMP_MAX_OID_STRLEN];
struct ber_oid oid;
@@ -405,18 +522,8 @@ trapcmd_exec(struct trapcmd *cmd, struct
if (dprintf(s[0], "%s\n", host) == -1)
goto out;
- if (dprintf(s[0],
- "iso.org.dod.internet.mgmt.mib-2.system.sysUpTime.0 %u\n",
- uptime) == -1)
- goto out;
-
- if (dprintf(s[0],
- "iso.org.dod.internet.snmpV2.snmpModules.snmpMIB.snmpMIBObjects."
- "snmpTrap.snmpTrapOID.0 %s\n", trapoid) == -1)
- goto out;
-
- for (; iter != NULL; iter = iter->be_next) {
- if (ober_scanf_elements(iter, "{oe}", &oid, &elm) == -1)
+ for (; vb != NULL; vb = vb->be_next) {
+ if (ober_scanf_elements(vb, "{oe}", &oid, &elm) == -1)
goto out;
if ((value = smi_print_element(elm)) == NULL)
goto out;
@@ -439,7 +546,7 @@ trapcmd_exec(struct trapcmd *cmd, struct
} else {
log_debug("child %i finished", child);
}
- close(s[1]);
+ close(s[1]);
return;
}
