snmpd(8) and p5-Net-SNMP

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

snmpd(8) and p5-Net-SNMP

Alf Schlichting
Hello,

(Initially sent to reyk@ 2 weeks ago, but he is most likely busy with
other things)

I am struggling with the following phenomen:

While snmpget and friends work just fine, p5-Net-SNMP fails with OpenBSD's snmpd(8).
It looks to me like net-snmpd gives back a different OID (1.3.6.1.6.3.15.1.1.4.0) in the
second packet then snmpd(8) (1.3.6.1.6.3.15.1.1.1.0) which gets interpreted as an
error by the p5-Net-SNMP library while snmpget eats it without complaining.

this works for both net-snmpd and OpenBSD's snmpd(8):
$ snmpget  -v3 -u snmp -A xxxxxxxx -l authNoPriv localhost sysContact.0
SNMPv2-MIB::sysContact.0 = STRING: [hidden email]
$

This is from a -current amd64 system from 2-3 weeks ago.

I have no idea if p5-Net-SNMP or snmpd(8) is wrong or if I am doing
something stupid, so here it goes:)

Alf


How to repeat:

$ doas cat /etc/snmpd.conf
ext_addr="127.0.0.1"
listen on $ext_addr

system contact "[hidden email]"
system location "right here"

seclevel auth
user "snmp" authkey "xxxxxxxx" auth "hmac-md5"

$ doas sed -e '/#/d' -e '/^$/d' < /etc/snmp/snmpd.conf
agentAddress  udp:161
view   systemonly  included   .1.3.6.1.2.1.1
view   systemonly  included   .1.3.6.1.2.1.25.1
 rocommunity public  default    -V systemonly
 rouser   authOnlyUser
sysLocation    Basement
sysContact     Me <[hidden email]>
sysServices    72
proc  mountd
proc  ntalkd    4
proc  sendmail 10 1
disk       /     10000
disk       /var  5%
includeAllDisks  10%
load   12 10 5
 trapsink     localhost public
iquerySecName   internalUser      
rouser          internalUser
defaultMonitors          yes
linkUpDownNotifications  yes
 extend    test1   /bin/echo  Hello, world!
 extend-sh test2   echo Hello, world! ; echo Hi there ; exit 35
 master          agentx

$ cat spass.pl
#! /usr/bin/perl

use strict;
use warnings;

use Net::SNMP;

my $OID_sysContact = '1.3.6.1.2.1.1.4.0';

my ($session, $error) = Net::SNMP->session(
                                           -hostname     => 'localhost',
                                           -version      => 'snmpv3',
                                           -username     => 'snmp',
                                           -authprotocol => 'md5',
                                           -authpassword => 'xxxxxxxx',
                                           -maxmsgsize   => 65507, # cosmetic, match snmpget
                                          );

if (!defined $session) {
    printf "ERROR: %s.\n", $error;
    exit 1;
}

my $result = $session->get_request(
                                   -varbindlist => [ $OID_sysContact ],
                                  );

if (!defined $result) {
    printf "ERROR: %s.\n", $session->error();
    $session->close();
    exit 1;
}

printf "The sysContact for host '%s': '%s'.\n",
  $session->hostname(), $result->{$OID_sysContact};

$session->close();

exit 0;



$ doas tcpdump -s 32768 -w /tmp/dump -i lo0 udp and port 161 &
[1] 29068
tcpdump: listening on lo0, link-type LOOP
$ doas rcctl start netsnmpd
netsnmpd(ok)
$ ./spass.pl
The sysContact for host 'localhost': 'Me <[hidden email]>'.
$ doas rcctl stop netsnmpd
netsnmpd(ok)
$ doas rcctl start snmpd
snmpd(ok)
$ ./spass.pl
ERROR: Received usmStatsUnsupportedSecLevels.0 Report-PDU with value 1 during discovery.
$ kill 29068
/bin/ksh: kill: 29068: Operation not permitted
$ fg
doas tcpdump -s 32768 -w /tmp/dump -i lo0 udp a
  C-c C-c
14 packets received by filter
0 packets dropped by kernel
$ doas tcpdump -Xnr /tmp/dump
tcpdump: WARNING: snaplen raised from 116 to 32768
10:18:04.483709 127.0.0.1.13982 > 127.0.0.1.161: [version(3)>1]
  0000: 4500 005c 0e31 0000 4011 6e5e 7f00 0001  E..\.1..@.n^....
  0010: 7f00 0001 369e 00a1 0048 06b1 303e 0201  ....6....H..0>..
  0020: 0330 1102 0426 133d 2c02 0300 ffe3 0401  .0...&.=,.......
  0030: 0402 0103 0410 300e 0400 0201 0002 0100  ......0.........
  0040: 0400 0400 0400 3014 0400 0400 a00e 0204  ......0.........
  0050: 2408 b557 0201 0002 0100 3000            $..W......0.

10:18:04.484364 127.0.0.1.161 > 127.0.0.1.13982: [version(3)>1]
  0000: 4500 008f 9329 0000 4011 e932 7f00 0001  E....)..@..2....
  0010: 7f00 0001 00a1 369e 007b 8496 3071 0201  ......6..{..0q..
  0020: 0330 1102 0426 133d 2c02 0300 ffe3 0401  .0...&.=,.......
  0030: 0002 0103 0421 301f 0411 8000 1f88 8008  .....!0.........
  0040: 7e31 4e1c f5ab 5800 0000 0002 0105 0201  ~1N...X.........
  0050: 0504 0004 0004 0030 3604 1180 001f 8880  .......06.......
  0060: 087e 314e 1cf5 ab58 0000 0000 0400 a81f  .~1N...X........
  0070: 0204 2408 b557 0201 0002 0100 3011 300f  ..$..W......0.0.
  0080: 060a 2b06 0106 030f 0101 0400 4101 01    ..+.........A..

10:18:04.491350 127.0.0.1.13982 > 127.0.0.1.161: [version(3)>1]
  0000: 4500 008e ef82 0000 4011 8cda 7f00 0001  E.......@.......
  0010: 7f00 0001 369e 00a1 007a 0efd 3070 0201  ....6....z..0p..
  0020: 0330 1102 046d 72f1 a802 0300 ffe3 0401  .0...mr.........
  0030: 0502 0103 0431 302f 0411 8000 1f88 8008  .....10/........
  0040: 7e31 4e1c f5ab 5800 0000 0002 0100 0201  ~1N...X.........
  0050: 0004 0473 6e6d 7004 0c41 f9fe 5dbb b36c  ...snmp..A..]..l
  0060: 3850 5e22 d604 0030 2504 1180 001f 8880  8P^"...0%.......
  0070: 087e 314e 1cf5 ab58 0000 0000 0400 a00e  .~1N...X........
  0080: 0204 2fb1 a1d0 0201 0002 0100 3000       ../.........0.

10:18:04.491717 127.0.0.1.161 > 127.0.0.1.13982: [version(3)>1]
  0000: 4500 00a0 e04b 0000 4011 9bff 7f00 0001  E....K..@.......
  0010: 7f00 0001 00a1 369e 008c dc17 3081 8102  ......6.....0...
  0020: 0103 3011 0204 6d72 f1a8 0203 00ff e304  ..0...mr........
  0030: 0101 0201 0304 3130 2f04 1180 001f 8880  ......10/.......
  0040: 087e 314e 1cf5 ab58 0000 0000 0201 0502  .~1N...X........
  0050: 0105 0404 736e 6d70 040c c799 5060 7d40  ....snmp....P`}@
  0060: 14b7 9d51 26e1 0400 3036 0411 8000 1f88  ...Q&...06......
  0070: 8008 7e31 4e1c f5ab 5800 0000 0004 00a8  ..~1N...X.......
  0080: 1f02 042f b1a1 d002 0100 0201 0030 1130  .../.........0.0
  0090: 0f06 0a2b 0601 0603 0f01 0102 0041 0101  ...+.........A..

10:18:04.492945 127.0.0.1.13982 > 127.0.0.1.161: [version(3)>1]
  0000: 4500 009c 2385 0000 4011 58ca 7f00 0001  E...#...@.X.....
  0010: 7f00 0001 369e 00a1 0088 dc8c 307e 0201  ....6.......0~..
  0020: 0330 1102 042f e1a0 9602 0300 ffe3 0401  .0.../..........
  0030: 0502 0103 0431 302f 0411 8000 1f88 8008  .....10/........
  0040: 7e31 4e1c f5ab 5800 0000 0002 0105 0201  ~1N...X.........
  0050: 0504 0473 6e6d 7004 0c50 8c7a d478 95c1  ...snmp..P.z.x..
  0060: 130a 283c 0804 0030 3304 1180 001f 8880  ..(<...03.......
  0070: 087e 314e 1cf5 ab58 0000 0000 0400 a01c  .~1N...X........
  0080: 0204 67d5 fdd2 0201 0002 0100 300e 300c  ..g.........0.0.
  0090: 0608 2b06 0102 0101 0400 0500            ..+.........

10:18:04.493217 127.0.0.1.161 > 127.0.0.1.13982: [version(3)>1]
  0000: 4500 00b2 27b9 0000 4011 5480 7f00 0001  E...'...@.T.....
  0010: 7f00 0001 00a1 369e 009e 4550 3081 9302  ......6...EP0...
  0020: 0103 3011 0204 2fe1 a096 0203 00ff e304  ..0.../.........
  0030: 0101 0201 0304 3130 2f04 1180 001f 8880  ......10/.......
  0040: 087e 314e 1cf5 ab58 0000 0000 0201 0502  .~1N...X........
  0050: 0105 0404 736e 6d70 040c 3f3d dda4 73e0  ....snmp..?=..s.
  0060: f425 1ded 9577 0400 3048 0411 8000 1f88  .%...w..0H......
  0070: 8008 7e31 4e1c f5ab 5800 0000 0004 00a2  ..~1N...X.......
  0080: 3102 0467 d5fd d202 0100 0201 0030 2330  1..g.........0#0
  0090: 2106 082b 0601 0201 0104 0004 154d 6520  !..+.........Me
  00a0: 3c73 6e6d 7040 6578 616d 706c 652e 636f  <[hidden email]
  00b0: 6d3e                                     m>

10:18:23.878696 127.0.0.1.42305 > 127.0.0.1.161: [version(3)>1]
  0000: 4500 005c 51b3 0000 4011 2adc 7f00 0001  E..\Q...@.*.....
  0010: 7f00 0001 a541 00a1 0048 d151 303e 0201  .....A...H.Q0>..
  0020: 0330 1102 0402 41d4 4502 0300 ffe3 0401  .0....A.E.......
  0030: 0402 0103 0410 300e 0400 0201 0002 0100  ......0.........
  0040: 0400 0400 0400 3014 0400 0400 a00e 0204  ......0.........
  0050: 2b74 2d34 0201 0002 0100 3000            +t-4......0.

10:18:23.878892 127.0.0.1.161 > 127.0.0.1.42305: [version(3)>1]
  0000: 4500 0087 81e5 0000 4011 fa7e 7f00 0001  E.......@..~....
  0010: 7f00 0001 00a1 a541 0073 b0ba 3069 0201  .......A.s..0i..
  0020: 0330 1102 0402 41d4 4502 0300 ffe3 0401  .0....A.E.......
  0030: 0002 0103 0420 301e 040d 8000 75cb 80d1  ..... 0.....u...
  0040: 297f d558 ac05 dd02 0100 0204 58ac 05df  )..X........X...
  0050: 0400 0400 0400 302f 040d 8000 75cb 80d1  ......0/....u...
  0060: 297f d558 ac05 dd04 00a8 1c02 0100 0201  )..X............
  0070: 0002 0100 3011 300f 060a 2b06 0106 030f  ....0.0...+.....
  0080: 0101 0100 4101 01                        ....A..

$

Reply | Threaded
Open this post in threaded view
|

Re: snmpd(8) and p5-Net-SNMP

Alf Schlichting
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)

Reply | Threaded
Open this post in threaded view
|

Re: snmpd(8) and p5-Net-SNMP

Alf Schlichting
Shameless ping!

On Tue, Mar 28, 2017 at 12:45:20PM +0200, alf wrote:

> 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)
>