any takers?
On Fri, 2021-06-04 at 22:11 +0200, Martijn van Duren wrote:
> ping
>
> On Fri, 2021-05-28 at 08:19 +0200, Martijn van Duren wrote:
> > As the original comment said:
> > /*
> > * This should probably go into parsevarbinds, but that's for a
> > * next refactor
> > */
> >
> > This moves the pdu parsing of traps into parsevarbinds.
> > Added bonus, we can now see the packets in debugging mode:
> > startup
> > snmpe: listening on udp 127.0.0.1:162
> > ktable_new: new ktable for rtableid 0
> > snmpe_parse: 127.0.0.1:162: SNMPv2 'public' pdutype SNMPv2-Trap request
> > 1329591927
> > snmpe_parse: 127.0.0.1:162: SNMPv1 'public' pdutype Trap request 0
> >
> > OK?
> >
> > martijn@
> >
> > Index: snmpd.h
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v
> > retrieving revision 1.95
> > diff -u -p -r1.95 snmpd.h
> > --- snmpd.h 20 May 2021 08:53:12 -0000 1.95
> > +++ snmpd.h 28 May 2021 06:17:16 -0000
> > @@ -776,6 +776,7 @@ int proc_flush_imsg(struct privsep *, e
> >
> > /* traphandler.c */
> > int traphandler_parse(struct snmp_message *);
> > +int traphandler_v1translate(struct snmp_message *, int);
> > int traphandler_priv_recvmsg(struct privsep_proc *, struct imsg *);
> > void trapcmd_free(struct trapcmd *);
> > int trapcmd_add(struct trapcmd *);
> > Index: snmpe.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> > retrieving revision 1.71
> > diff -u -p -r1.71 snmpe.c
> > --- snmpe.c 20 May 2021 08:53:12 -0000 1.71
> > +++ snmpe.c 28 May 2021 06:17:16 -0000
> > @@ -368,6 +368,10 @@ snmpe_parse(struct snmp_message *msg)
> > msg->sm_errstr = "trapv1 request on !SNMPv1
> > message";
> > goto parsefail;
> > }
> > + /* Internally we work with SNMPv2-Traps */
> > + if (traphandler_v1translate(msg, 0) == -1)
> > + goto fail;
> > + a = msg->sm_pdu->be_sub;
> > case SNMP_C_TRAPV2:
> > if (msg->sm_pdutype == SNMP_C_TRAPV2 &&
> > !(msg->sm_version == SNMP_V2 ||
> > @@ -390,16 +394,9 @@ snmpe_parse(struct snmp_message *msg)
> > goto fail;
> > }
> > stats->snmp_intraps++;
> > - /*
> > - * This should probably go into parsevarbinds, but that's
> > for a
> > - * next refactor
> > - */
> > - if (traphandler_parse(msg) == -1)
> > - goto fail;
> > - /* Shortcircuit */
> > - return 0;
> > + break;
> > default:
> > - msg->sm_errstr = "invalid context";
> > + msg->sm_errstr = "invalid pdutype";
> > goto parsefail;
> > }
> >
> > @@ -450,7 +447,14 @@ snmpe_parsevarbinds(struct snmp_message
> > struct ber_element *pvarbind = NULL, *end;
> > char buf[BUFSIZ];
> > struct ber_oid o;
> > - int i;
> > + int i = 1;
> > +
> > + if (msg->sm_pdutype == SNMP_C_TRAP ||
> > + msg->sm_pdutype == SNMP_C_TRAPV2) {
> > + if (traphandler_parse(msg) == -1)
> > + goto varfail;
> > + return 0;
> > + }
> >
> > msg->sm_errstr = "invalid varbind element";
> >
> > @@ -458,7 +462,7 @@ snmpe_parsevarbinds(struct snmp_message
> > msg->sm_varbindresp = NULL;
> > end = NULL;
> >
> > - for (i = 1; varbind != NULL && i < SNMPD_MAXVARBIND;
> > + for (; varbind != NULL && i < SNMPD_MAXVARBIND;
> > varbind = varbind->be_next, i++) {
> > if (ober_scanf_elements(varbind, "{oeS$}", &o, &value) ==
> > -1) {
> > stats->snmp_inasnparseerrs++;
> > @@ -543,7 +547,7 @@ snmpe_parsevarbinds(struct snmp_message
> > msg->sm_errorindex = 0;
> >
> > return 0;
> > - varfail:
> > +varfail:
> > 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)
> > @@ -790,18 +794,18 @@ snmpe_recvmsg(int fd, short sig, void *a
> > void
> > snmpe_dispatchmsg(struct snmp_message *msg)
> > {
> > - if (msg->sm_pdutype == SNMP_C_TRAP ||
> > - msg->sm_pdutype == SNMP_C_TRAPV2) {
> > - snmp_msgfree(msg);
> > - return;
> > - }
> > /* dispatched to subagent */
> > /* XXX Do proper error handling */
> > (void) snmpe_parsevarbinds(msg);
> >
> > - /* respond directly */
> > - msg->sm_pdutype = SNMP_C_GETRESP;
> > - snmpe_response(msg);
> > + if (msg->sm_pdutype == SNMP_C_TRAP ||
> > + msg->sm_pdutype == SNMP_C_TRAPV2)
> > + snmp_msgfree(msg);
> > + else {
> > + /* respond directly */
> > + msg->sm_pdutype = SNMP_C_GETRESP;
> > + snmpe_response(msg);
> > + }
> > }
> >
> > void
> > Index: traphandler.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/snmpd/traphandler.c,v
> > retrieving revision 1.21
> > diff -u -p -r1.21 traphandler.c
> > --- traphandler.c 22 Feb 2021 11:31:09 -0000 1.21
> > +++ traphandler.c 28 May 2021 06:17:16 -0000
> > @@ -45,8 +45,6 @@
> >
> > int traphandler_priv_recvmsg(struct privsep_proc *, struct imsg *);
> > int traphandler_fork_handler(struct privsep_proc *, struct imsg *);
> > -struct ber_element *
> > - traphandler_v1translate(struct snmp_message *, int);
> > int trapcmd_cmp(struct trapcmd *, struct trapcmd *);
> > void trapcmd_exec(struct trapcmd *, struct sockaddr *,
> > struct ber_element *);
> > @@ -76,25 +74,17 @@ traphandler_parse(struct snmp_message *m
> > ssize_t buflen;
> > int ret = -1;
> >
> > - switch (msg->sm_pdu->be_type) {
> > - case SNMP_C_TRAP:
> > - if ((vblist = traphandler_v1translate(msg, 0)) == NULL)
> > - goto done;
> > - break;
> > - case SNMP_C_TRAPV2:
> > - if (ober_scanf_elements(msg->sm_pdu, "{SSe}$", &elm) == -1)
> > {
> > - stats->snmp_inasnparseerrs++;
> > - goto done;
> > - }
> > - if (elm->be_type != BER_TYPE_INTEGER) {
> > - stats->snmp_inasnparseerrs++;
> > - goto done;
> > - }
> > - vblist = ober_unlink_elements(elm);
> > - break;
> > - default:
> > - fatalx("%s called without proper context", __func__);
> > + if (ober_scanf_elements(msg->sm_pdu, "{SSe}$", &elm) == -1) {
> > + stats->snmp_inasnparseerrs++;
> > + msg->sm_errstr = "invalid pdu";
> > + goto done;
> > + }
> > + if (elm->be_type != BER_TYPE_INTEGER) {
> > + stats->snmp_inasnparseerrs++;
> > + msg->sm_errstr = "invalid pdu";
> > + goto done;
> > }
> > + vblist = ober_unlink_elements(elm);
> >
> > (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);
> > @@ -103,11 +93,13 @@ traphandler_parse(struct snmp_message *m
> > ober_oid_cmp(&o1, &sysUpTimeOID) != 0 ||
> > ober_oid_cmp(&o2, &snmpTrapOIDOID) != 0) {
> > stats->snmp_inasnparseerrs++;
> > + msg->sm_errstr = "invalid pdu";
> > 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, "{oS$}", &o1) == -1) {
> > + msg->sm_errstr = "invalid pdu";
> > stats->snmp_inasnparseerrs++;
> > goto done;
> > }
> > @@ -140,7 +132,7 @@ done:
> > return ret;
> > }
> >
> > -struct ber_element *
> > +int
> > traphandler_v1translate(struct snmp_message *msg, int proxy)
> > {
> > struct snmp_stats *stats = &snmpd_env->sc_stats;
> > @@ -158,7 +150,8 @@ traphandler_v1translate(struct snmp_mess
> > agent_addrlen != 4 ||
> > vblist->be_type != BER_TYPE_SEQUENCE) {
> > stats->snmp_inasnparseerrs++;
> > - return NULL;
> > + msg->sm_errstr = "invalid pdu";
> > + return -1;
> > }
> > switch (generic_trap) {
> > case 0:
> > @@ -184,33 +177,33 @@ traphandler_v1translate(struct snmp_mess
> > /* Officially this should be 128, but BER_MAX_OID_LEN is 64
> > */
> > if (trapoid.bo_n + 2 > BER_MAX_OID_LEN) {
> > stats->snmp_inasnparseerrs++;
> > - return NULL;
> > + msg->sm_errstr = "enterprise too large";
> > + return -1;
> > }
> > trapoid.bo_id[trapoid.bo_n++] = 0;
> > trapoid.bo_id[trapoid.bo_n++] = specific_trap;
> > break;
> > default:
> > stats->snmp_inasnparseerrs++;
> > - return NULL;
> > + msg->sm_errstr = "invalid generic-trap";
> > + return -1;
> > }
> >
> > /* work aronud net-snmp's snmptrap: It adds an EOC element in
> > vblist */
> > if (vblist->be_len != 0)
> > vb0 = ober_unlink_elements(vblist);
> > + vblist = ober_unlink_elements(msg->sm_pdu);
> > + ober_free_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) {
> > + if (ober_printf_elements(msg->sm_pdu, "ddd{{od}{oO}e}", 0, 0, 0,
> > + "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);
> > - return NULL;
> > + msg->sm_errstr = strerror(errno);
> > + return -1;
> > }
> >
> > if (proxy) {
> > @@ -220,11 +213,12 @@ traphandler_v1translate(struct snmp_mess
> > &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)
> > {
> > + ober_scanf_elements(msg->sm_pdu, "{SSS{e", &elm);
> > + for (; 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;
> > + return -1;
> > }
> > if (ober_oid_cmp(&oid, &snmpTrapAddressOid) == 0)
> > hasaddress = 1;
> > @@ -243,11 +237,11 @@ traphandler_v1translate(struct snmp_mess
> > &snmpTrapEnterpriseOid, &enterprise) == NULL) {
> > msg->sm_errstr = strerror(errno);
> > ober_free_elements(vblist);
> > - return NULL;
> > + return -1;
> > }
> > }
> > }
> > - return vblist;
> > + return 0;
> > }
> >
> > int
> >
>
