calmh / node-snmp-native

Native Javascript SNMP library for Node.js
MIT License
252 stars 65 forks source link

Incorrect oid validation #26

Closed justinsvegliato closed 9 years ago

justinsvegliato commented 9 years ago

The module incorrectly marks the OID .1.0.8802.1.1.2.1.4.1.1.9 as invalid since it doesn't begin with .1.3. While most OIDs begin with .1.3, there are quite a few that don't. The link below includes some information on this particular OID if you're interested:

http://www.mibdepot.com/cgi-bin/getmib3.cgi?win=mib_a&i=1&n=LLDP-MIB&r=rittal&f=LLDP-MIB&v=v2&t=tab&o=lldpRemSysName

I've also narrowed down the issue to lines 132 to 134 of asn1ber.js:

} else if (!skipValidation && (oid[0] !== 1 || oid[1] !== 3)) { throw new Error("SNMP OIDs always start with .1.3."); }

I might be misunderstanding the code, but it looks like we only need to replace (oid[0] !== 1 || oid[1] !== 3) with oid[0] !== 1 to appropriately mark the OID as valid.

What steps can I take to resolve this issue? Thanks for your help. I really appreciate it.

calmh commented 9 years ago

I don't think those lines are useful any more, especially if they are incorrect. File a PR to remove that else clause and any tests that expect it?

justinsvegliato commented 9 years ago

Thanks for the quick response. It's appreciated. I'll tackle this as soon as I get home tonight.

calmh commented 9 years ago

I'll publish a new version later on today, thanks.

gregmac commented 9 years ago

asn1 actually has several valid OID ranges:

Is there something in SNMP that restricts these further, that warrant this check? I actually had to add the skipValidation flag to get around this, but perhaps a better fix is just to allow the technically valid ranges for both MIB OIDs and values of type Object-Identifier.

Most devices only have MIB OIDs starting with .1.3 but if you request something else (eg .0.23.1.2) nothing catastrophic happens: it just returns NoSuchName.

calmh commented 9 years ago

No, no reason. I think that ^ resolves it.

gregmac commented 9 years ago

Perfect, that makes more sense. Thanks!