Re: snmpd getbulk replies

2017-07-27 Thread Stuart Henderson
On 2017/07/27 14:01, Gerhard Roth wrote:
> could you please try the updated patch below. I guess the non-repeater
> parameters was completely ignored before.

Nearly there :)

> - msg->sm_maxrepetitions);
> + (msg->sm_i < msg->sm_nonrepeaters) ?

This should be <= instead of <, then it's OK sthen@.

Thanks, this is an old bug!



Re: snmpd getbulk replies

2017-07-27 Thread Gerhard Roth
On Thu, 27 Jul 2017 12:46:23 +0100 Stuart Henderson  
wrote:
> On 2017/07/27 10:58, Gerhard Roth wrote:
> > Hi,
> > 
> > snmpd uses the same storage for sm_error and sm_nonrepeaters. Same applies
> > to sm_errorindex and sm_maxrepetitions. If we produce a response PDU to
> > a getbulk request, sm_error will carry the number of non-repeaters from
> > the request and sm_errorindex the max. number of repetitions. This is
> > wrong. We should clears those fields in case of success.
> 
> This is definitely an improvement. There still seems to be something not
> quite right though, non-repeaters are repeating - I wouldn't expect to see
> sysObjectID, sysUpTime, sysContact, ipDefaultTTL, ipInRecives, ipInHdrErrors
> here:
> 
> $ snmpbulkget -Cn2 -Cr4 -v2c -c public 127.0.0.1 SNMPv2-MIB::sysDescr \
>IP-MIB::ipForwarding IF-MIB::ifName IF-MIB::ifInOctets
> SNMPv2-MIB::sysDescr.0 = STRING: sym
> SNMPv2-MIB::sysObjectID.0 = OID: OPENBSD-BASE-MIB::openBSDDefaultObjectID
> SNMPv2-MIB::sysUpTime.0 = Timeticks: (18115) 0:03:01.15
> SNMPv2-MIB::sysContact.0 = STRING: root@some.email.address.example
> IP-MIB::ipForwarding.0 = INTEGER: forwarding(1)
> IP-MIB::ipDefaultTTL.0 = INTEGER: 64
> IP-MIB::ipInReceives.0 = Counter32: 26379088
> IP-MIB::ipInHdrErrors.0 = Counter32: 0
> IF-MIB::ifName.1 = STRING: em0
> IF-MIB::ifName.2 = STRING: em1
> IF-MIB::ifName.3 = STRING: enc0
> IF-MIB::ifName.4 = STRING: lo0
> IF-MIB::ifInOctets.1 = Counter32: 2586116638
> IF-MIB::ifInOctets.2 = Counter32: 0
> IF-MIB::ifInOctets.3 = Counter32: 0
> IF-MIB::ifInOctets.4 = Counter32: 508223137


Hi Steuart,

could you please try the updated patch below. I guess the non-repeater
parameters was completely ignored before.

Gerhard


Index: usr.sbin/snmpd/snmpe.c
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
retrieving revision 1.47
diff -u -p -u -p -r1.47 snmpe.c
--- usr.sbin/snmpd/snmpe.c  21 Apr 2017 13:50:23 -  1.47
+++ usr.sbin/snmpd/snmpe.c  27 Jul 2017 11:58:52 -
@@ -439,7 +439,8 @@ snmpe_parsevarbinds(struct snmp_message 
case SNMP_C_GETBULKREQ:
ret = mps_getbulkreq(msg, &msg->sm_c,
&msg->sm_end, &o,
-   msg->sm_maxrepetitions);
+   (msg->sm_i < msg->sm_nonrepeaters) ?
+   1 : msg->sm_maxrepetitions);
if (ret == 0 || ret == 1)
break;
msg->sm_error = SNMP_ERROR_NOSUCHNAME;
@@ -467,6 +468,8 @@ snmpe_parsevarbinds(struct snmp_message 
}
 
msg->sm_errstr = "none";
+   msg->sm_error = 0;
+   msg->sm_errorindex = 0;
 
return (ret);
  varfail:



Re: snmpd getbulk replies

2017-07-27 Thread Stuart Henderson
On 2017/07/27 10:58, Gerhard Roth wrote:
> Hi,
> 
> snmpd uses the same storage for sm_error and sm_nonrepeaters. Same applies
> to sm_errorindex and sm_maxrepetitions. If we produce a response PDU to
> a getbulk request, sm_error will carry the number of non-repeaters from
> the request and sm_errorindex the max. number of repetitions. This is
> wrong. We should clears those fields in case of success.

This is definitely an improvement. There still seems to be something not
quite right though, non-repeaters are repeating - I wouldn't expect to see
sysObjectID, sysUpTime, sysContact, ipDefaultTTL, ipInRecives, ipInHdrErrors
here:

$ snmpbulkget -Cn2 -Cr4 -v2c -c public 127.0.0.1 SNMPv2-MIB::sysDescr \
   IP-MIB::ipForwarding IF-MIB::ifName IF-MIB::ifInOctets
SNMPv2-MIB::sysDescr.0 = STRING: sym
SNMPv2-MIB::sysObjectID.0 = OID: OPENBSD-BASE-MIB::openBSDDefaultObjectID
SNMPv2-MIB::sysUpTime.0 = Timeticks: (18115) 0:03:01.15
SNMPv2-MIB::sysContact.0 = STRING: root@some.email.address.example
IP-MIB::ipForwarding.0 = INTEGER: forwarding(1)
IP-MIB::ipDefaultTTL.0 = INTEGER: 64
IP-MIB::ipInReceives.0 = Counter32: 26379088
IP-MIB::ipInHdrErrors.0 = Counter32: 0
IF-MIB::ifName.1 = STRING: em0
IF-MIB::ifName.2 = STRING: em1
IF-MIB::ifName.3 = STRING: enc0
IF-MIB::ifName.4 = STRING: lo0
IF-MIB::ifInOctets.1 = Counter32: 2586116638
IF-MIB::ifInOctets.2 = Counter32: 0
IF-MIB::ifInOctets.3 = Counter32: 0
IF-MIB::ifInOctets.4 = Counter32: 508223137



snmpd getbulk replies

2017-07-27 Thread Gerhard Roth
Hi,

snmpd uses the same storage for sm_error and sm_nonrepeaters. Same applies
to sm_errorindex and sm_maxrepetitions. If we produce a response PDU to
a getbulk request, sm_error will carry the number of non-repeaters from
the request and sm_errorindex the max. number of repetitions. This is
wrong. We should clears those fields in case of success.

Gerhard


Index: usr.sbin/snmpd/snmpe.c
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
retrieving revision 1.47
diff -u -p -u -p -r1.47 snmpe.c
--- usr.sbin/snmpd/snmpe.c  21 Apr 2017 13:50:23 -  1.47
+++ usr.sbin/snmpd/snmpe.c  27 Jul 2017 08:49:31 -
@@ -467,6 +467,8 @@ snmpe_parsevarbinds(struct snmp_message 
}
 
msg->sm_errstr = "none";
+   msg->sm_error = 0;
+   msg->sm_errorindex = 0;
 
return (ret);
  varfail: