etingof / pysnmp

Python SNMP library
http://snmplabs.com/pysnmp/
BSD 2-Clause "Simplified" License
576 stars 193 forks source link

Exception raised when processing an SNMPv3 report PDU response w/an empty varBindList #330

Open gurnec opened 4 years ago

gurnec commented 4 years ago

(PySNMP v4.4.12)

As I understand it, errors such as report PDUs (received by PySNMP) in response to requests (sent by PySNMP) should cause the request to be retried up to the specified retry count, and also when performing SNMPv3 discovery.

This code here is responsible for checking if an SNMPv3 retry is appropriate, based in part on the value of statusInformation:

        errorIndication = statusInformation['errorIndication']

        if errorIndication in (errind.notInTimeWindow, errind.unknownEngineID):
            origDiscoveryRetries += 1
            origRetries = 0
        else:
            origDiscoveryRetries = 0
            origRetries += 1

        if origRetries > origRetryCount or origDiscoveryRetries > self.__options.get('discoveryRetries', 4):

When an SNMPv3 report PDU is received, the code below runs first to create the statusInformation which is then later used above:

    # 7.2.11
    if pduType in rfc3411.internalClassPDUs:
        # 7.2.11a
        varBinds = pMod.apiPDU.getVarBinds(pdu)
        if varBinds:
            statusInformation = error.StatusInformation(
                errorIndication=_snmpErrors.get(varBinds[0][0], errind.ReportPduReceived(varBinds[0][0].prettyPrint())),
                oid=varBinds[0][0], val=varBinds[0][1],
                sendPduHandle=sendPduHandle
            )
        else:
            statusInformation = error.StatusInformation(
                sendPduHandle=sendPduHandle
            )

If the report PDU has a varBindList, statusInformation is created with an errorIndication reflecting the first varBind, e.g. if it's a usmStatsUnknownEngineIDs, the code in the second block assigns errind.unknownEngineID to errorIndication, and then the code in the first block continues with normal discovery.

However if the report PDU has an empty varBindList (which may be uncommon), errorIndication isn't set to anything in that else clause just above, and accessing statusInformation['errorIndication'] in the first code block raises a KeyError instead of starting a retry.

I think the correct response is to attempt a retry (a "normal" retry, not a "discovery" retry). If so, then perhaps errorIndication should be assigned to something in the else clause in the second code block, probably errind.reportPduReceived, -or- 'errorIndication' should be accessed via .get() in the first code block.

I'm posting this because I have a (buggy I believe) piece of gear (MikroTik RouterOS) that's responding to initial SNMPv3 GetBulkRequests with just what I described above, instead of the usual usmStatsUnknownEngineIDs.

I'd be happy post tracebacks (although they're pretty long...) or to write out some specifics to reproduce this if it's helpful, just let me know!

lextm commented 1 year ago

A retry is not likely to get a different response from the buggy agent, so I don't think PySNMP should retry blindly here. As the caller of PySNMP, however, you can add your own retry mechanism if you like.