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
