etingof / pysnmp

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

Counter32 is being parsed as a signed int, causing an error #190

Open scotloach opened 6 years ago

scotloach commented 6 years ago

I noticed the library was timing out a response when the response was actually received on the wire.

Tracking it down got me to this error: 2018-08-24 20:16:16,006 pysnmp: receiveMessage: <ConstraintsIntersection object at 0x7f77477d5e48 consts <ValueRangeConstraint object at 0x7f77477d5cc0 consts 0, 4294967295>> failed at: ValueConstraintError('<ValueRangeConstraint object at 0x7f77477d5cc0 consts 0, 4 294967295> failed at: ValueConstraintError(-266262832,)',) at Counter32

I am digging in the code and the Counter32 is parsed as a signed integer which is causing it to fail the range check in Counter32. While I continue to investigate, I'm posting this in hopes that someone with knowledge of this code can help.

It gets into the asn1 library in IntegerDecoder, which decodes the bytes here: value = from_bytes(head, signed=True)

But, a Counter32 should be parsed as unsigned.

etingof commented 6 years ago

This may have something to do with malformed Counter32 encoding. Could you please share the blob (or a piece of it) of the response message causing this?

I do not think there is an unsigned integer type in ASN.1, the sign is determined from encoding. The sign kwarg indicates whether two's complement is used in integer's serialized representation.

scotloach commented 6 years ago

Here are some detailed logs from parsing the last part of the packet that triggers the error. I'm happy to provide whatever I can to help you. pysnmp.log

scotloach commented 6 years ago

FWIW, neither net-snmp nor wireshark seem to have trouble parsing it. image

etingof commented 6 years ago

TL;DR; the encoding is wrong.

Long story: here is what we have on input:

2018-08-25 12:47:12,193 pyasn1: decoder called at scope Message.PDUs.ResponsePDU.VarBindList with state 0, working with up to 164 octets of substrate: 
00000: 30 12 06 0A 2B 06 01 02 01 02 02 01 10 02 41 04 
00016: FE FC E8 88 30 11 06 0A 2B 06 01 02 01 02 02 01 
00032: 11 01 41 03 30 C7 92 30 11 06 0A 2B 06 01 02 01 
00048: 02 02 01 11 02 41 03 34 83 83 30 10 06 0A 2B 06 
00064: 01 02 01 02 02 01 12 01 41 02 32 05 30 11 06 0A 
00080: 2B 06 01 02 01 02 02 01 12 02 41 03 2D 78 7A 30 
00096: 0F 06 0A 2B 06 01 02 01 02 02 01 13 01 41 01 00 
00112: 30 10 06 0A 2B 06 01 02 01 02 02 01 13 02 41 02 
00128: 17 B0 30 0F 06 0A 2B 06 01 02 01 02 02 01 14 01 
00144: 41 01 00 30 0F 06 0A 2B 06 01 02 01 02 02 01 14 
00160: 02 41 01 00

We are interested in this piece:

00000:                                     41 04 
00016: FE FC E8 88

Which looks like this as bytes:

>>> bytes([int(x, 16) for x in ['41', '04', 'FE', 'FC', 'E8', '88']])
b'A\x04\xfe\xfc\xe8\x88'

If we decode it as Counter32 it fails:

>>> from pysnmp.hlapi import *
>>> from pyasn1.codec.ber import encoder, decoder
>>> decoder.decode(b'A\x04\xfe\xfc\xe8\x88', Counter32())
...
pyasn1.type.error.ValueConstraintError: -16979832

But we can decode it as signed Integer (if we fix the tag):

>>> decoder.decode(b'\x02\x04\xfe\xfc\xe8\x88', Integer())
(<Integer value object at 0x10cb21be0 tagSet <TagSet object at 0x1047560f0 tags 0:0:2> subtypeSpec <ConstraintsIntersection object at 0x10471ff98 consts <ValueRangeConstraint object at 0x10471ff60 consts -2147483648, 2147483647>> payload [-16979832]>, b'')

The problem is in the first payload octet e.g. \xfe - its most significant bit is 1 which in BER indicates negative sign. We can just add some more zeros (one octet) in front to get rid of the sign:

>>> decoder.decode(b'A\x05\x00\xfe\xfc\xe8\x88', Counter32())
(<Counter32 value object at 0x10cb242e8 tagSet <TagSet object at 0x1048df400 tags 64:0:1> subtypeSpec <ConstraintsIntersection object at 0x1048df438 consts <ValueRangeConstraint object at 0x1048df2b0 consts 0, 4294967295>> payload [4277987464]>, b'')

Although this analysis does not really solve your problem, does it? May be we should have a way to tell pysnmp to tolerate wrong encoding to some degree...? But it's not clear how to interpret it in this particular case, for instance. Should we reset the leading bit in \xfe or add some more zeros in front?

ihartwig commented 4 years ago

I'm running into this exact situation where a Counter32 value of 3,600,462,004 (0x D6 9A B0 B4) is interpreted as a negative number and causes packet parsing to fail. This value is handled as an unsigned 32-bit value by snmpwalk:

snmpwalk device-hostname -c public -v2c "1.3.6.1.2.1.2.2.1.10"
iso.3.6.1.2.1.2.2.1.10.1 = Counter32: 3661783614

Having read the discussion above about the most significant bit in payload, which also applies to my usage, it does seem that the net-snmp (snmpget) does diverge from the most specific ASN.1 definition. They can probably do this because RFC1902 / RFC2578 defines the Counter32 type as an implicit variant of Integer with its own type value (0x41).

Counter32 ::=
    [APPLICATION 1]
        IMPLICIT INTEGER (0..4294967295)

By the letter of the ASN.1 definition it would be up to the device creating the response to add the 0x00 pad byte and expand length. However, it also may be a reasonable implementation for SNMP types described specifically as positive integers to be interpreted as unsigned at the bits level, like they are in net-snmp / wireshark. Hope this info is helpful.

The case where I ran across this actually returns a RequestTimedOut('No SNMP response received before timeout',) error when pysnmp did get bytes back in the response before running into this decoding issue. Bubbling up the relevant error would be very helpful here, but that is probably a topic for another issue.

rednach commented 2 years ago

have you seen docs/source/faq/ignored-snmp-packets.rst? The hack proposed worked for me...