calmh/node-snmp-native

getSubtree won't retrieve all oids when not in numeric order

Jaloko opened this issue · 7 comments

As the title suggests the getSubtree function doesn't return all oids if they are not in order. Example below:

When requesting 1.3.6.1.2.1.17.4.3.1.1 with snmpwalk I get the below output and a lot more. You will see after they are not ordered which is fine as they are returned anyway.

OID=.1.3.6.1.2.1.17.4.3.1.1.0.34.100.181.251.62, Type=OctetString, Value=                             
OID=.1.3.6.1.2.1.17.4.3.1.1.120.165.4.237.241.250, Type=OctetString, Value=                         
OID=.1.3.6.1.2.1.17.4.3.1.1.244.206.70.191.124.155, Type=OctetString, Value=                         
OID=.1.3.6.1.2.1.17.4.3.1.1.244.206.70.191.124.154, Type=OctetString, Value=                        
OID=.1.3.6.1.2.1.17.4.3.1.1.120.227.181.207.79.69, Type=OctetString, Value=                         
OID=.1.3.6.1.2.1.17.4.3.1.1.208.23.194.138.146.72, Type=OctetString, Value=                           
OID=.1.3.6.1.2.1.17.4.3.1.1.2.156.2.161.21.112, Type=OctetString, Value=  
.... (there are alot more after this)

Now if I use snmp-native I get nothing as the result. But looking deeper if I add some logging in the getSubtree method I find it stops at 244.206.70.191.124.155 because the next entry is 244.206.70.191.124.154. Which is less than the original.

[ 1, 3, 6, 1, 2, 1, 17, 4, 3, 1, 1, 0, 34, 100, 181, 251, 62 ]
[ 1, 3, 6, 1, 2, 1, 17, 4, 3, 1, 1, 120, 165, 4, 237, 241, 250 ]
[ 1, 3, 6, 1, 2, 1, 17, 4, 3, 1, 1, 244, 206, 70, 191, 124, 155 ]

So this led me to believe the problem lies in the compareOids function. Specifically this piece of code which checks if each value is greater than the last.

    for (i = 0; i < mlen; i++) {
        if (oidA[i] > oidB[i]) {
            return -1;
        } else if (oidB[i] > oidA[i]) {
            return 1;
        }
    }

Now I changed it to this which is working from what I can tell but the original oid needs to be parsed in.

    if(originalOid.length < mlen) {
        for(var i = 0; i < originalOid.length; i++) {
            // Check the oids contain the root oid if they don't stop
            if(originalOid[i] != oidA[i] || originalOid[i] != oidB[i]) {
                return -1;
            }
        }
        return 1;
    } else {
        return -1;
    }

Is there another way to retrieve a list of oids like this that I am missing? If not would be great if getSubtree would support these.

i think it would be nice if node-snmp-native would support broken hardware like the one you describe. however, i think changing the compareOid function would be the wrong place to implement this. instead a new method should implement the algorithm you describe.

I guess I don't understand what getSubtree's purpose is. I thought it was to get all values after a root node you specify like 1.3.6.1.2.1.17.4.3.1.1. In what cases would you only want this method to get ordered oids.

If this were to be implemented in a new method we could do it like snmpwalk and have it use a start and end point to find the values. I imagine its usage like this:

getBetween([1,3,6,1,2,1,17,4,3,1,1], [1,3,6,1,2,1,17,4,3,1,2]).then((result) => {
    // Do something
}).catch(err => console.log(err));
calmh commented

Many devices loop back to the first OID when the last one is reached. We detect this as the end of the iteration, or we would loop forever. Your device returns OIDs in random order, which is unusual and arguably broken.

net-snmp has an option to turn off this check (-Cc to your snmpwalk above), relying then on the device returning an error when walking past the last OID. We could implement something like that as well.

@Jaloko i meant your algorithm should not be implemented in compareOid(), but in another function, perhaps called compareOidRoot(). it would still be called from getSubtree().

one could then have a config option which uses this alternative method of comparing oids to satisfy weird equipment.

Interesting I tried that same OID on a different brand of switch and this time they are ordered. Is there a standard that says they should be ordered?

@bangert Right, I understand now. What should this config option be called?

calmh commented

I wasn't sure what standard said what so I went looking, and RFC 1905, "Protocol Operations for Version 2 of the Simple Network Management Protocol (SNMPv2)" specifies that variables for GetNext and similar operations are "lexicographically ordered" when it figures out which is the next one to return,

4.2.2. The GetNextRequest-PDU
...
(1) The variable is located which is in the lexicographically ordered
list of the names of all variables which are accessible by this
request and whose name is the first lexicographic successor of the

and so on and so forth. I think this covers it. But it is a well known breakage that not all things follow, so I'm all for supporting it somehow.

I was going to use a root OID check but I just realised there is already a check that does this. That first check always stops it when it hits the end of the tree with my devices anyway. So we would simply have to not check if its increasing based on the option. Thoughts?

// Don't check if the oid is increasing when cc option is specified
var oidIncreasing = false;
if(!options.cc) {
    oidIncreasing = (compareOids(vbs.slice(-1)[0].oid, varbinds[0].oid) !== 1);
}

if (varbinds[0].value === 'endOfMibView' || varbinds[0].value === 'noSuchObject' || varbinds[0].value === 'noSuchInstance') {
} else if (vbs.length && oidIncreasing) {
} else {
}

@calmh Thanks for that. Good to know.