Further investigation shows that p5-Net-SNMP sends a first packet
unauthentified and with an empty request. Here snmpd(8) violates
RFC 3414. It should send OIDVAL_usmErrEngineId but instead sends
OIDVAL_usmErrSecLevel.
After that is fixed p5-Net-SNMP sends another empty request to
sync its time however authentificated this time. This is unexpected
for snmpd(8) so it doesn't handle this correctly.
Below is a patch to fix this of the works-for-me fashion, needs some
checking by someone with better understanding of snmp though.
Explanations inline, complete diff at end:
We asume that having the REPORT flag set indicates a discovery so
set OIDVAL_usmErrEngineid and answer back.
diff -ur snmpd/snmpe.c snmpd.modified/snmpe.c
--- snmpd/snmpe.c Tue Mar 28 12:19:03 2017
+++ snmpd.modified/snmpe.c Tue Mar 28 09:33:37 2017
@@ -220,11 +220,18 @@
msg->sm_flags = *flagstr;
if (MSG_SECLEVEL(msg) < env->sc_min_seclevel ||
msg->sm_secmodel != SNMP_SEC_USM) {
- /* XXX currently only USM supported */
- msg->sm_errstr = "unsupported security model";
- stats->snmp_usmbadseclevel++;
- msg->sm_usmerr = OIDVAL_usmErrSecLevel;
- goto parsefail;
+ if (MSG_REPORT(msg)) {
+ msg->sm_errstr = "no such engineid";
+ stats->snmp_usmnosuchengine++;
+ msg->sm_usmerr = OIDVAL_usmErrEngineId;
+ goto parsefail;
+ } else {
+ /* XXX currently only USM supported */
+ msg->sm_errstr = "unsupported security model";
+ stats->snmp_usmbadseclevel++;
+ msg->sm_usmerr = OIDVAL_usmErrSecLevel;
+ goto parsefail;
+ }
}
if ((a = usm_decode(msg, a, &msg->sm_errstr)) == NULL)
@@ -324,11 +331,12 @@
msg->sm_errstr = "invalid PDU";
goto fail;
}
This check gets in the way because p5-Net-SNMP sends after the first
unauthentificated packet a second one with an empty request to sync its time.
There should be a better way however, as is it is XXX.
- if (class != BER_CLASS_UNIVERSAL || type != BER_TYPE_SEQUENCE) {
- stats->snmp_silentdrops++;
- msg->sm_errstr = "invalid varbind";
- goto fail;
- }
+ /* XXX disabled for now */
+ /* if (class != BER_CLASS_UNIVERSAL || type != BER_TYPE_SEQUENCE) { */
+ /* stats->snmp_silentdrops++; */
+ /* msg->sm_errstr = "invalid varbind"; */
+ /* goto fail; */
+ /* } */
msg->sm_request = req;
msg->sm_error = errval;
Because of the second empty request in an otherwise valid packet first check
for MSG_REPORT and check if snmpe_parse has failed later.
@@ -485,7 +493,8 @@
struct snmp_stats *stats = &env->sc_stats;
ssize_t len;
struct snmp_message *msg;
-
+ int ret;
+
if ((msg = calloc(1, sizeof(*msg))) == NULL)
return;
@@ -517,17 +526,19 @@
smi_debug_elements(msg->sm_req);
#endif
- if (snmpe_parse(msg) == -1) {
- if (msg->sm_usmerr != 0 && MSG_REPORT(msg)) {
- usm_make_report(msg);
- snmpe_response(fd, msg);
- return;
- } else {
- snmp_msgfree(msg);
- return;
- }
+ ret = snmpe_parse(msg);
+
+ if (msg->sm_usmerr != 0 && MSG_REPORT(msg)) {
+ usm_make_report(msg);
+ snmpe_response(fd, msg);
+ return;
}
+ if (ret == -1) {
+ snmp_msgfree(msg);
+ return;
+ }
+
snmpe_dispatchmsg(msg, fd);
}
This check must be done differently so OIDVAL_usmErrTimeWindow gets set and
reported back to the client.
diff -ur snmpd/usm.c snmpd.modified/usm.c
--- snmpd/usm.c Tue Mar 28 12:19:03 2017
+++ snmpd.modified/usm.c Tue Mar 28 09:33:37 2017
@@ -269,21 +269,20 @@
goto done;
}
- if (engine_boots != 0LL && engine_time != 0LL) {
- now = snmpd_engine_time();
- if (engine_boots != snmpd_env->sc_engine_boots ||
- engine_time < (long long)(now - SNMP_MAX_TIMEWINDOW) ||
- engine_time > (long long)(now + SNMP_MAX_TIMEWINDOW)) {
- *errp = "out of time window";
- msg->sm_usmerr = OIDVAL_usmErrTimeWindow;
- stats->snmp_usmtimewindow++;
+ now = snmpd_engine_time();
+ if (engine_boots != snmpd_env->sc_engine_boots ||
+ engine_time < (long long)(now - SNMP_MAX_TIMEWINDOW) ||
+ engine_time > (long long)(now + SNMP_MAX_TIMEWINDOW)) {
+ *errp = "out of time window";
+ msg->sm_usmerr = OIDVAL_usmErrTimeWindow;
+ stats->snmp_usmtimewindow++;
+ if (engine_boots != 0LL && engine_time != 0LL)
goto done;
- }
+ } else {
+ msg->sm_engine_boots = (u_int32_t)engine_boots;
+ msg->sm_engine_time = (u_int32_t)engine_time;
}
-
- msg->sm_engine_boots = (u_int32_t)engine_boots;
- msg->sm_engine_time = (u_int32_t)engine_time;
-
+
memcpy(msg->sm_username, user, userlen);
msg->sm_username[userlen] = '\0';
msg->sm_user = usm_finduser(msg->sm_username);
Because the second request is already authentificated we must avoid clearing
sm_flags and sm_username in the answer otherwise p5-Net-SNMP will not accept
this packet.
@@ -481,9 +480,9 @@
struct ber_oid usmstat = OID(MIB_usmStats, 0, 0);
/* Always send report in clear-text */
- msg->sm_flags = 0;
+ /* msg->sm_flags = 0; */
msg->sm_context = SNMP_C_REPORT;
- msg->sm_username[0] = '\0';
+ /* msg->sm_username[0] = '\0'; */
usmstat.bo_id[OIDIDX_usmStats] = msg->sm_usmerr;
usmstat.bo_n = OIDIDX_usmStats + 2;
if (msg->sm_varbindresp != NULL)
------------------------------------------------------------------------------
Complete diff:
diff -ur snmpd/snmpe.c snmpd.modified/snmpe.c
--- snmpd/snmpe.c Tue Mar 28 12:19:03 2017
+++ snmpd.modified/snmpe.c Tue Mar 28 09:33:37 2017
@@ -220,11 +220,18 @@
msg->sm_flags = *flagstr;
if (MSG_SECLEVEL(msg) < env->sc_min_seclevel ||
msg->sm_secmodel != SNMP_SEC_USM) {
- /* XXX currently only USM supported */
- msg->sm_errstr = "unsupported security model";
- stats->snmp_usmbadseclevel++;
- msg->sm_usmerr = OIDVAL_usmErrSecLevel;
- goto parsefail;
+ if (MSG_REPORT(msg)) {
+ msg->sm_errstr = "no such engineid";
+ stats->snmp_usmnosuchengine++;
+ msg->sm_usmerr = OIDVAL_usmErrEngineId;
+ goto parsefail;
+ } else {
+ /* XXX currently only USM supported */
+ msg->sm_errstr = "unsupported security model";
+ stats->snmp_usmbadseclevel++;
+ msg->sm_usmerr = OIDVAL_usmErrSecLevel;
+ goto parsefail;
+ }
}
if ((a = usm_decode(msg, a, &msg->sm_errstr)) == NULL)
@@ -324,11 +331,12 @@
msg->sm_errstr = "invalid PDU";
goto fail;
}
- if (class != BER_CLASS_UNIVERSAL || type != BER_TYPE_SEQUENCE) {
- stats->snmp_silentdrops++;
- msg->sm_errstr = "invalid varbind";
- goto fail;
- }
+ /* XXX disabled for now */
+ /* if (class != BER_CLASS_UNIVERSAL || type != BER_TYPE_SEQUENCE) { */
+ /* stats->snmp_silentdrops++; */
+ /* msg->sm_errstr = "invalid varbind"; */
+ /* goto fail; */
+ /* } */
msg->sm_request = req;
msg->sm_error = errval;
@@ -485,7 +493,8 @@
struct snmp_stats *stats = &env->sc_stats;
ssize_t len;
struct snmp_message *msg;
-
+ int ret;
+
if ((msg = calloc(1, sizeof(*msg))) == NULL)
return;
@@ -517,17 +526,19 @@
smi_debug_elements(msg->sm_req);
#endif
- if (snmpe_parse(msg) == -1) {
- if (msg->sm_usmerr != 0 && MSG_REPORT(msg)) {
- usm_make_report(msg);
- snmpe_response(fd, msg);
- return;
- } else {
- snmp_msgfree(msg);
- return;
- }
+ ret = snmpe_parse(msg);
+
+ if (msg->sm_usmerr != 0 && MSG_REPORT(msg)) {
+ usm_make_report(msg);
+ snmpe_response(fd, msg);
+ return;
}
+ if (ret == -1) {
+ snmp_msgfree(msg);
+ return;
+ }
+
snmpe_dispatchmsg(msg, fd);
}
diff -ur snmpd/usm.c snmpd.modified/usm.c
--- snmpd/usm.c Tue Mar 28 12:19:03 2017
+++ snmpd.modified/usm.c Tue Mar 28 09:33:37 2017
@@ -269,21 +269,20 @@
goto done;
}
- if (engine_boots != 0LL && engine_time != 0LL) {
- now = snmpd_engine_time();
- if (engine_boots != snmpd_env->sc_engine_boots ||
- engine_time < (long long)(now - SNMP_MAX_TIMEWINDOW) ||
- engine_time > (long long)(now + SNMP_MAX_TIMEWINDOW)) {
- *errp = "out of time window";
- msg->sm_usmerr = OIDVAL_usmErrTimeWindow;
- stats->snmp_usmtimewindow++;
+ now = snmpd_engine_time();
+ if (engine_boots != snmpd_env->sc_engine_boots ||
+ engine_time < (long long)(now - SNMP_MAX_TIMEWINDOW) ||
+ engine_time > (long long)(now + SNMP_MAX_TIMEWINDOW)) {
+ *errp = "out of time window";
+ msg->sm_usmerr = OIDVAL_usmErrTimeWindow;
+ stats->snmp_usmtimewindow++;
+ if (engine_boots != 0LL && engine_time != 0LL)
goto done;
- }
+ } else {
+ msg->sm_engine_boots = (u_int32_t)engine_boots;
+ msg->sm_engine_time = (u_int32_t)engine_time;
}
-
- msg->sm_engine_boots = (u_int32_t)engine_boots;
- msg->sm_engine_time = (u_int32_t)engine_time;
-
+
memcpy(msg->sm_username, user, userlen);
msg->sm_username[userlen] = '\0';
msg->sm_user = usm_finduser(msg->sm_username);
@@ -481,9 +480,9 @@
struct ber_oid usmstat = OID(MIB_usmStats, 0, 0);
/* Always send report in clear-text */
- msg->sm_flags = 0;
+ /* msg->sm_flags = 0; */
msg->sm_context = SNMP_C_REPORT;
- msg->sm_username[0] = '\0';
+ /* msg->sm_username[0] = '\0'; */
usmstat.bo_id[OIDIDX_usmStats] = msg->sm_usmerr;
usmstat.bo_n = OIDIDX_usmStats + 2;
if (msg->sm_varbindresp != NULL)
