Fix bad snmp context option (#13497)

* Fix bad snmp context option
Just port queries to SnmpQuery and remove bad code to prevent new usages

* Allow unordered

* add stub on Mock

LibreNMS/Data/Source/NetSnmpQuery.php

@@ -130,10 +130,14 @@ class NetSnmpQuery implements SnmpQueryInterface
     /**
      * Set a context for the snmp query
      * This is most commonly used to fetch alternate sets of data, such as different VRFs
+     *
+     * @param  string  $v2  Version 2/3 context name
+     * @param  string|null  $v3  Version 3 context name if different from v2 context name
+     * @return \LibreNMS\Data\Source\SnmpQueryInterface
      */
-    public function context(string $context): SnmpQueryInterface
+    public function context(string $v2, string $v3 = null): SnmpQueryInterface
     {
-        $this->context = $context;
+        $this->context = $this->device->snmpver === 'v3' && $v3 !== null ? $v3 : $v2;
 
         return $this;
     }
@@ -149,6 +153,17 @@ class NetSnmpQuery implements SnmpQueryInterface
         return $this;
     }
 
+    /**
+     * Do not error on out of order indexes.
+     * Use with caution as we could get stuck in an infinite loop.
+     */
+    public function allowUnordered(): SnmpQueryInterface
+    {
+        $this->options = array_merge($this->options, ['-Cc']);
+
+        return $this;
+    }
+
     /**
      * Output all OIDs numerically
      */

LibreNMS/Data/Source/SnmpQueryInterface.php

@@ -58,6 +58,12 @@ interface SnmpQueryInterface
      */
     public function mibDir(?string $dir): SnmpQueryInterface;
 
+    /**
+     * Do not error on out of order indexes.
+     * Use with caution as we could get stuck in an infinite loop.
+     */
+    public function allowUnordered(): SnmpQueryInterface;
+
     /**
      * Output all OIDs numerically
      */

includes/discovery/arp-table.inc.php

@@ -26,18 +26,16 @@
 use LibreNMS\Config;
 
 foreach (DeviceCache::getPrimary()->getVrfContexts() as $context_name) {
-    $device['context_name'] = $context_name;
-
     if (file_exists(Config::get('install_dir') . "/includes/discovery/arp-table/{$device['os']}.inc.php")) {
         include Config::get('install_dir') . "/includes/discovery/arp-table/{$device['os']}.inc.php";
     } else {
-        $netToMediaPhysAddressSnmpFlags = '-OQUsetX';
-        if ($device['os'] == 'bintec-beip-plus') {
-            $netToMediaPhysAddressSnmpFlags = ['-OQUsetX', '-Cc'];
-        }
+        $arp_data = SnmpQuery::context($context_name)->walk('IP-MIB::ipNetToPhysicalPhysAddress')->table(1);
 
-        $arp_data = snmpwalk_group($device, 'ipNetToPhysicalPhysAddress', 'IP-MIB');
-        $arp_data = snmpwalk_group($device, 'ipNetToMediaPhysAddress', 'IP-MIB', 1, $arp_data, null, null, $netToMediaPhysAddressSnmpFlags);
+        $mediaQuery = SnmpQuery::context($context_name);
+        if ($device['os'] == 'bintec-beip-plus') {
+            $mediaQuery->allowUnordered();
+        }
+        $arp_data = $mediaQuery->walk('IP-MIB::ipNetToMediaPhysAddress')->table(1, $arp_data);
     }
 
     $sql = 'SELECT * from `ipv4_mac` WHERE `device_id`=? AND `context_name`=?';
@@ -53,8 +51,8 @@ foreach (DeviceCache::getPrimary()->getVrfContexts() as $context_name) {
         $port_id = $interface['port_id'];
 
         $port_arp = array_merge(
-            (array) $data['ipNetToMediaPhysAddress'],
-            is_array($data['ipNetToPhysicalPhysAddress']) ? (array) $data['ipNetToPhysicalPhysAddress']['ipv4'] : []
+            (array) $data['IP-MIB::ipNetToMediaPhysAddress'],
+            is_array($data['IP-MIB::ipNetToPhysicalPhysAddress']) ? (array) $data['IP-MIB::ipNetToPhysicalPhysAddress']['ipv4'] : []
         );
 
         echo "{$interface['ifName']}: \n";
@@ -122,7 +120,6 @@ foreach (DeviceCache::getPrimary()->getVrfContexts() as $context_name) {
         $insert_data,
         $sql,
         $params,
-        $entry,
-        $device['context_name']
+        $entry
     );
 }

includes/discovery/fdb-table/arubaos.inc.php

@@ -47,7 +47,9 @@ if (! empty($fdbPort_table)) {
 
     foreach ($fdbPort_table as $vlan => $data) {
         d_echo("VLAN: $vlan\n");
-        $dot1dBasePortIfIndex = snmpwalk_group($device, 'dot1dBasePortIfIndex', 'BRIDGE-MIB', 1, $dot1dBasePortIfIndex, null, $vlan);
+        $dot1dBasePortIfIndex = SnmpQuery::context($vlan, "vlan-$vlan")
+            ->walk('BRIDGE-MIB::dot1dBasePortIfIndex')
+            ->table(1, $dot1dBasePortIfIndex);
     }
 
     foreach ($dot1dBasePortIfIndex as $portLocal => $data) {

includes/discovery/fdb-table/ios.inc.php

@@ -19,12 +19,10 @@ foreach ($vtpdomains as $vtpdomain_id => $vtpdomain) {
         }
 
         if (($vlan['vtpVlanState'] === '1') && ($vlan_raw < 1002 || $vlan_raw > 1005)) {
-            $device_vlan = array_merge($device, ['community' => $device['community'] . '@' . $vlan_raw, 'context_name' => "vlan-$vlan_raw"]);
-
-            $fdbPort_table = snmpwalk_group($device_vlan, 'dot1dTpFdbPort', 'BRIDGE-MIB', 0);
+            $fdbPort_table = SnmpQuery::context($vlan_raw, "vlan-$vlan_raw")->walk('BRIDGE-MIB::dot1dTpFdbPort')->table();
 
             $portid_dict = [];
-            $dot1dBasePortIfIndex = snmpwalk_group($device_vlan, 'dot1dBasePortIfIndex', 'BRIDGE-MIB');
+            $dot1dBasePortIfIndex = SnmpQuery::context($vlan_raw, "vlan-$vlan_raw")->walk('BRIDGE-MIB::dot1dBasePortIfIndex')->table(1);
             foreach ($dot1dBasePortIfIndex as $portLocal => $data) {
                 $port = get_port_by_index_cache($device['device_id'], $data['dot1dBasePortIfIndex']);
                 $portid_dict[$portLocal] = $port['port_id'];
@@ -41,8 +39,6 @@ foreach ($vtpdomains as $vtpdomain_id => $vtpdomain) {
                 $insert[$vlan_id][$mac_address]['port_id'] = $port_id;
                 d_echo("vlan $vlan_id mac $mac_address port ($dot1dBasePort) $port_id\n");
             }
-
-            unset($device_vlan);
         } //end if operational
     } // end for each vlan
     echo PHP_EOL;

includes/snmp.inc.php

@@ -128,7 +128,7 @@ function gen_snmpget_cmd($device, $oids, $options = null, $mib = null, $mibdir =
  * @param  string  $mibdir  a mib directory to search for mibs, usually prepended with +
  * @return array the fully assembled command, ready to run
  */
-function gen_snmpwalk_cmd($device, $oids, $options = null, $mib = null, $mibdir = null, $strIndexing = null)
+function gen_snmpwalk_cmd($device, $oids, $options = null, $mib = null, $mibdir = null)
 {
     if ($device['snmpver'] == 'v1' || (isset($device['os']) && Config::getOsSetting($device['os'], 'snmp_bulk', true) == false)) {
         $snmpcmd = [Config::get('snmpwalk')];
@@ -140,7 +140,7 @@ function gen_snmpwalk_cmd($device, $oids, $options = null, $mib = null, $mibdir
         }
     }
 
-    return gen_snmp_cmd($snmpcmd, $device, $oids, $options, $mib, $mibdir, $strIndexing);
+    return gen_snmp_cmd($snmpcmd, $device, $oids, $options, $mib, $mibdir);
 } //end gen_snmpwalk_cmd()
 
 /**
@@ -154,13 +154,13 @@ function gen_snmpwalk_cmd($device, $oids, $options = null, $mib = null, $mibdir
  * @param  string  $mibdir  a mib directory to search for mibs, usually prepended with +
  * @return array the fully assembled command, ready to run
  */
-function gen_snmp_cmd($cmd, $device, $oids, $options = null, $mib = null, $mibdir = null, $strIndexing = null)
+function gen_snmp_cmd($cmd, $device, $oids, $options = null, $mib = null, $mibdir = null)
 {
     if (! isset($device['transport'])) {
         $device['transport'] = 'udp';
     }
 
-    $cmd = snmp_gen_auth($device, $cmd, $strIndexing);
+    $cmd = snmp_gen_auth($device, $cmd);
     $cmd = $options ? array_merge($cmd, (array) $options) : $cmd;
     if ($mib) {
         array_push($cmd, '-m', $mib);
@@ -653,10 +653,9 @@ function snmpwalk_cache_triple_oid($device, $oid, $array, $mib = null, $mibdir =
  * @param  mixed  $snmpFlags  flags to use for the snmp command
  * @return array grouped array of data
  */
-function snmpwalk_group($device, $oid, $mib = '', $depth = 1, $array = [], $mibdir = null, $strIndexing = null, $snmpFlags = '-OQUsetX')
+function snmpwalk_group($device, $oid, $mib = '', $depth = 1, $array = [], $mibdir = null, $snmpFlags = '-OQUsetX')
 {
-    d_echo("communityStringIndexing $strIndexing\n");
-    $cmd = gen_snmpwalk_cmd($device, $oid, $snmpFlags, $mib, $mibdir, $strIndexing);
+    $cmd = gen_snmpwalk_cmd($device, $oid, $snmpFlags, $mib, $mibdir);
     $data = rtrim(external_exec($cmd));
 
     $line = strtok($data, "\n");
@@ -740,7 +739,7 @@ function snmpwalk_cache_threepart_oid($device, $oid, $array, $mib = 0)
  * @param  array  $cmd
  * @return array
  */
-function snmp_gen_auth(&$device, $cmd = [], $strIndexing = null)
+function snmp_gen_auth(&$device, $cmd = [])
 {
     if ($device['snmpver'] === 'v3') {
         array_push($cmd, '-v3', '-l', $device['authlevel']);
@@ -764,7 +763,7 @@ function snmp_gen_auth(&$device, $cmd = [], $strIndexing = null)
             d_echo('DEBUG: ' . $device['snmpver'] . " : Unsupported SNMPv3 AuthLevel (wtf have you done ?)\n");
         }
     } elseif ($device['snmpver'] === 'v2c' || $device['snmpver'] === 'v1') {
-        array_push($cmd, '-' . $device['snmpver'], '-c', $device['community'] . ($strIndexing != null ? '@' . $strIndexing : null));
+        array_push($cmd, '-' . $device['snmpver'], '-c', $device['community']);
     } else {
         d_echo('DEBUG: ' . $device['snmpver'] . " : Unsupported SNMP Version (shouldn't be possible to get here)\n");
     }

tests/Mocks/SnmpQueryMock.php

@@ -106,6 +106,11 @@ class SnmpQueryMock implements SnmpQueryInterface
             ->translate($oid, $mib);
     }
 
+    public function allowUnordered(): SnmpQueryInterface
+    {
+        return $this;
+    }
+
     public function numeric(): SnmpQueryInterface
     {
         $this->numeric = true;