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
>
