Skip to content
This repository has been archived by the owner on Mar 8, 2022. It is now read-only.

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

Open
Jaloko opened this issue Dec 7, 2017 · 7 comments
Open

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

Jaloko opened this issue Dec 7, 2017 · 7 comments

Comments

@Jaloko
Copy link

Jaloko commented Dec 7, 2017

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.

@bangert
Copy link
Collaborator

bangert commented Dec 12, 2017

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.

@Jaloko
Copy link
Author

Jaloko commented Dec 12, 2017

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
Copy link
Owner

calmh commented Dec 13, 2017

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.

@bangert
Copy link
Collaborator

bangert commented Dec 13, 2017

@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.

@Jaloko
Copy link
Author

Jaloko commented Dec 13, 2017

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
Copy link
Owner

calmh commented Dec 13, 2017

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.

@Jaloko
Copy link
Author

Jaloko commented Dec 13, 2017

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants