glpi-project / glpi-agent

GLPI Agent
GNU General Public License v2.0
240 stars 60 forks source link

Cisco C9300 equipment mac address not retrieved (SNMP) #372

Closed shmsh9 closed 1 year ago

shmsh9 commented 1 year ago

Bug reporting acknowledgment

Yes, I read it

Professional support

None

Describe the bug

Hello all,

I have some cisco C9300 switches that get correctly inventoried by the agent (1.4) but the equipment mac addresses connected to it are not retrieved. I found the oid to query with snmpwalk : SNMPv2-SMI::enterprises.9.9.315.1.2.1.1.10.PORT but I don't know what to patch to get it working. Should I add a new "module" like lib/GLPI/Agent/SNMP/C9300.pm and if so what "method" should implement the return of the mac addresses ? Thank you for your help and sorry if it's the wrong place

To reproduce

Launch net-inventory on a cisco C9300 switch

Expected behavior

The switch should return the mac address of the connected equipments

Operating system

Linux

GLPI Agent version

1.4

GLPI version

10.0.6

GLPIInventory plugin or FusionInventory for GLPI plugin version

GLPI Inventory v1.0.6

Additional context

No response

g-bougard commented 1 year ago

Hi @shmsh9

can you provide a snmpwalk output using the command given in documentation for such a device ?

shmsh9 commented 1 year ago

Hi @g-bougard Of course. How can I send this privately (there is some "sensitive" information in the file) ?

g-bougard commented 1 year ago

You can send it to gbougard <at> teclib <dot> com.

g-bougard commented 1 year ago

Hi @shmsh9 thank you for the walk. As I understand, your problem is related to the seen connections where you think MAC address is missing. But do you mean GLPI doesn't manage to link devices ? From your walk, I can see 6 CONNECTIONS nodes where the CDP protocol gives the SYSNAME, the IP and the IFDESCR of some other Cisco remote devices. Are you talking about that devices ?

shmsh9 commented 1 year ago

Hi @g-bougard , No those devices are indeed detected fine I am talking about the devices you can find at SNMPv2-SMI::enterprises.9.9.315.1.2.1.1.10 Those are not in the XML produced by net-inventory

g-bougard commented 1 year ago

I can generate an XML from the walk you gave to me. That XML can be imported and this generates 3 devices in GLPI as they should 3 components from the scanned device. Is that okay for you ? And do these devices well imported from your point of view ?

If I understand well your sentence, you're speaking about other devices which are not part of the device for which you gave me a walk. But they are connected to that devices. Did you configured an ip range from which these devices could be inventoried ?

shmsh9 commented 1 year ago

Yes the 3 devices are well imported. Exactly, the missing evices are connected to the switch and are already inventoried by their ips. But they are not associated with the switch since the switch inventory does not get their mac adresses (but querying the oid I sent you does get you their mac addresses)

g-bougard commented 1 year ago

I'm really not sure to understand. Maybe I'm missing something. Do you want the agent get the MAC address for one device from another device ?

shmsh9 commented 1 year ago

I mean that only cisco (probably due do cisco discovery protocol) devices connected to the switches get their mac address populated on the net-inventory : image

While other ports with non cisco equipments have no tag (but are connected and can be retrieved by the OID).

g-bougard commented 1 year ago

For each port found on a the device, the agent is looking into CDP, LLDP and eventually EDP tables to find any connected device. It tries to store in the CONNECTIONS XML node every information that will permit GLPI to link the remote device to the related port on that device. Anyway you still need to inventory the remote device too to have the linked fully established in GLPI. By now, I still don't understand what you think is wrong. Can you try to clarify your real problem ?

shmsh9 commented 1 year ago

Ok sorry I think I am missing something and created a XY problem...

I have switches with office devices connected to it and I want GLPI to have an regularly updated view of what's attached on each switch port. Right now only network gears do generate a CONNECTIONS entry for the ports and I was assuming that since I can query each port mac address individually I can just patch the inventory software to add CONNECTIONS entry with the results of this query. Since the connected computers/printers are already imported in GLPI with their mac addresses I also assumed that adding a port on a switch with a CONNECTIONS containing this mac address will make the link. Thank you for your patience

g-bougard commented 1 year ago

I think I'm starting to understand. You want to add the CONNECTIONS node everywhere it seems to miss. You noticed there are MAC Addresses on the OIDs like SNMPv2-SMI::enterprises.9.9.315.1.2.1.1.10.PORT and you think we should add them to the CONNECTIONS nodes. First did you verify the MAC you're seeing are exactly the MAC of the connected devices ? Then I found the description of the OID you're speaking about in the CISCO-PORT-SECURITY-MIB:

cpsIfSecureLastMacAddress OBJECT-TYPE
        SYNTAX        MacAddress
        MAX-ACCESS    read-only
        STATUS        current
        DESCRIPTION   "This object indicates the last MAC 
                      address that is seen on this interface.

                      This object is also used as a variable in
                      the cpsSecureMacAddrViolation notification
                      to contain the value of the MAC address
                      which caused the violation."
        ::= { cpsIfConfigEntry 10 }

It explains this is the last seen mac, but I guess we can consider it as the current seen mac. Btw this means it will report only one mac even if there's 2 seens and maybe you'll see a device being disconnected and reconnected. Anyway this is not a connection support system, this is a security system dedicated to Cisco. The MIB is related to a "Port Security feature". Can you confirm my assumptions ?

shmsh9 commented 1 year ago

Yes that's exactly it ! I checked on a few devices but only on one switch I will check more next week but it seems accurate. Yes I understand this is a hackish way but I found no other trace of the mac addresses. all the oid supposed to return this seems missing on this model.

g-bougard commented 1 year ago

To experiment, you can add the following code in a lib/GLPI/Agent/SNMP/MibSupport/CiscoPortSecurity.pm file:

package GLPI::Agent::SNMP::MibSupport::CiscoPortSecurity;

use strict;
use warnings;

use parent 'GLPI::Agent::SNMP::MibSupportTemplate';

use GLPI::Agent::Tools;
use GLPI::Agent::Tools::SNMP;

use constant    priority => 5;

# See CISCO-SMI
use constant    cisco       => '.1.3.6.1.4.1.9'  ;
use constant    ciscoMgmt   => cisco . '.9'     ;

# See CISCO-PORT-SECURITY-MIB
use constant    ciscoPortSecurityMIB        => ciscoMgmt . '.315' ;
use constant    cpsGlobalPortSecurityEnable => ciscoPortSecurityMIB .'.1.1.3.0' ;
use constant    cpsIfConfigEntry            => ciscoPortSecurityMIB .'.1.2.1.1' ;
use constant    cpsIfSecureLastMacAddress   => cpsIfConfigEntry . '.10' ;

our $mibSupport = [
    {
        name        => "cisco-port-security",
        privateoid  => cpsGlobalPortSecurityEnable,
    }
];

sub run {
    my ($self) = @_;

    # Don't analyse anything if the feature is not enabled
    return unless $self->get(cpsGlobalPortSecurityEnable);

    my $cpsIfSecureLastMacAddress = $self->walk(cpsIfSecureLastMacAddress)
        or return;

    my $device = $self->device
        or return;

    foreach my $port (keys(%{$cpsIfSecureLastMacAddress})) {
        my $mac = getCanonicalMacAddress($cpsIfSecureLastMacAddress->{$port})
            or next;
        next unless $device->{PORTS} && $device->{PORTS}->{PORT} && $device->{PORTS}->{PORT}->{$port};
        $device->{PORTS}->{PORT}->{$port}->{CONNECTIONS}->{CONNECTION}->{MAC} = $mac;
    }
}

1;

__END__

=head1 NAME

GLPI::Agent::SNMP::MibSupport::CiscoPortSecurity - Inventory module to add connections
detected via Cisco Port Security feature.

=head1 DESCRIPTION

The module enhances Cisco support.

It should do what you expect.

g-bougard commented 1 year ago

Hi @shmsh9 you may not need to enable the rules I was speaking about. I may have been in trouble with my GLPI test instance ;-)

P.S.: comment outdated as previous comment has been updated

shmsh9 commented 1 year ago

Thank you , that's awesome !

it does not work I tried to add some logging like :

    my $cpsIfSecureLastMacAddress = $self->walk("1.3.6.1.4.1.9.9.315.1.2.1.1.10")
        or $self->logger->debug("snmpwalk failed") && return;

    my $device = $self->device
        or $self->logger->debug("device failed") && return;

but it failed :

[error] [thread 1] #1, [thread 1] Can't locate object method "logger" via package "GLPI::Agent::SNMP::MibSupport::CiscoPortSecurity" at /usr/share/glpi-agent/lib/GLPI/Agent/SNMP/MibSupport/CiscoPortSecurity.pm line 36.

How can I print something ?

g-bougard commented 1 year ago

'logger' should be a HASH key, not a method. Anyway, better try to use this code for these lines as mixing operators can be confusing when debugging:

    my $cpsIfSecureLastMacAddress = $self->walk("1.3.6.1.4.1.9.9.315.1.2.1.1.10");
    unless ($cpsIfSecureLastMacAddress) {
        $self->{logger}->debug("snmpwalk failed");
        return;
    }

    my $device = $self->device;
    unless ($device) {
        $self->{logger}->debug("device failed");
        return;
    }
shmsh9 commented 1 year ago

Thanks ! I can make it work if i comment the block in lib/GLPI/Agent/Tools/Hardware.pm that changes vlan context (otherwise every walk or get fails) so it seems that reset_original_context() is not working correctly (I tried to call it directly from the run() ) of the submodule. Do you know wich one it is using ? lib/GLPI/Agent/Tools/Live.pm lib/GLPI/Agent/Tools/Mock.pm

=pod
        # get additional associated mac addresses from those vlans
        my @mac_addresses = ();
        foreach my $vlan (@vlans) {
            $logger->debug("switching SNMP context to vlan $vlan") if $logger;
            $snmp->switch_vlan_context($vlan);
            my $mac_addresses = _getKnownMacAddresses(
                snmp           => $snmp,
                address2port   => '.1.3.6.1.2.1.17.4.3.1.2', # dot1dTpFdbPort
                port2interface => '.1.3.6.1.2.1.17.1.4.1.2', # dot1dBasePortIfIndex
            );
            next unless $mac_addresses;

            push @mac_addresses, $mac_addresses;
        }
        $snmp->reset_original_context() if @vlans;
=cut
g-bougard commented 1 year ago

Live.pm one is used when requesting directly on devices. Mock.pm is used when loading a walk with the --file option of glpi-netinventory script.

The problem is probably the run() API is called after ports are inventoried.

shmsh9 commented 1 year ago

Thank you, Okay I narrowed it down to https://github.com/glpi-project/glpi-agent/blob/622aad6bf585987129db480e742d7a9d8673d72e/lib/GLPI/Agent/Tools/Hardware.pm#L907 If I comment this line your patch works fine (after deleting the equipments imported with the non modified agent).

That is weird because if I understand correctly it only set snmp->{context} which is then unset by reset_original_context() (and even forcing the delete $self->device->{snmp}->{context} inside lib/GLPI/Agent/SNMP/MibSupport/CiscoPortSecurity.pm does not fix it.

I guess this loop is leaving the SNMP object in some faulty state if the vlan context is set https://github.com/glpi-project/glpi-agent/blob/622aad6bf585987129db480e742d7a9d8673d72e/lib/GLPI/Agent/Tools/Hardware.pm#L905-L917

A view of $self->device->{snmp}->{session} at the begining of CiscoPortSecurity.pm run() with vlan context set :

_delay: 0
_transport: Net::SNMP::Transport::IPv4::UDP=HASH(0x7f68a85a45d8)
_version: 3
_nonblocking: 0
_transport_argv: ARRAY(0x7f68a83ea728)
Use of uninitialized value in concatenation (.) or string at /usr/share/glpi-agent/lib/GLPI/Agent/SNMP/MibSupport/CiscoPortSecurity.pm line 38.
_context_engine_id:
_discovery_queue: ARRAY(0x7f68a83ea6c8)
_pdu: Net::SNMP::PDU=HASH(0x7f68a8deaa20)
Use of uninitialized value in concatenation (.) or string at /usr/share/glpi-agent/lib/GLPI/Agent/SNMP/MibSupport/CiscoPortSecurity.pm line 38.
_callback:
_context_name: vlan-1865
_translate: 255
_hostname: 10.154.223.10
_error: Received authorizationError(16) error-status at error-index 0
_security: Net::SNMP::Security::USM=HASH(0x7f68a845d0a0)

the same without vlan context set :

_discovery_queue: ARRAY(0x7fb1143e9f08)
_error: The requested table is empty or does not exist
_nonblocking: 0
_pdu: Net::SNMP::PDU=HASH(0x7fb114f3a8e8)
_security: Net::SNMP::Security::USM=HASH(0x7fb11445d258)
Use of uninitialized value in concatenation (.) or string at /usr/share/glpi-agent/lib/GLPI/Agent/SNMP/MibSupport/CiscoPortSecurity.pm line 38.
_context_name:
_hostname: 10.154.223.10
Use of uninitialized value in concatenation (.) or string at /usr/share/glpi-agent/lib/GLPI/Agent/SNMP/MibSupport/CiscoPortSecurity.pm line 38.
_callback:
_delay: 0
Use of uninitialized value in concatenation (.) or string at /usr/share/glpi-agent/lib/GLPI/Agent/SNMP/MibSupport/CiscoPortSecurity.pm line 38.
_context_engine_id:
_transport_argv: ARRAY(0x7fb1143e9f68)
_version: 3
_translate: 255
_transport: Net::SNMP::Transport::IPv4::UDP=HASH(0x7fb1145a3f20)
g-bougard commented 1 year ago

Hi @shmsh9 the purpose of this loop is to make request to populate the @mac_addresses array which is then passed to _addKnownMacAddresses() function. It will try to set mac address on port connections. To know if the problem is really related to the vlan context switching, you also can comment the loop just after the # Finally add found mac addresse comment, so l. 932 to 937 instead of the loop which switches vlan context. I'm really not sure there can be an issue with the VLAN context switching.

shmsh9 commented 1 year ago

Hi @g-bougard, I confirm commenting the second loop does not fix any get() or walk() inside module's run(). But commenting _getKnownMacAddresses() to not perform any walk() while keeping the vlan switch does work.

Inside _getKnownMacAddresses() the line always fails (the oid is not implemented it seems on this model) so maybe it's the culprit

my $address2port   = $snmp->walk($params{address2port});
[debug] [thread 1] #1, address2port failed
[debug] [thread 1] #1, address2port failed
[debug] [thread 1] #1, switching SNMP context to vlan 1121
[debug] [thread 1] #1, address2port failed
[debug] [thread 1] #1, switching SNMP context to vlan 986
[debug] [thread 1] #1, address2port failed
[debug] [thread 1] #1, switching SNMP context to vlan 1910
[debug] [thread 1] #1, address2port failed
[debug] [thread 1] #1, switching SNMP context to vlan 1700
[debug] [thread 1] #1, address2port failed
[debug] [thread 1] #1, switching SNMP context to vlan 1863
[debug] [thread 1] #1, address2port failed
[debug] [thread 1] #1, switching SNMP context to vlan 1902
[debug] [thread 1] #1, address2port failed
[debug] [thread 1] #1, switching SNMP context to vlan 1865
[debug] [thread 1] #1, address2port failed
[debug] [thread 1] #1, switching SNMP context to vlan 1698
[debug] [thread 1] #1, address2port failed
[debug] [thread 1] #1, switching SNMP context to vlan 1864
[debug] [thread 1] #1, address2port failed
[debug] [thread 1] #1, cpsGlobalPortSecurityEnable failed
shmsh9 commented 1 year ago

Okay if I add my own walk method that reset the -contextname of the snmp session the submodule works without any agent modification !

sub walkTest {
    my ($self, $oid) = @_;
    return unless $oid;

    my $session = $self->device->{snmp}->{session};
    $options{'-maxrepetitions'} = 1                if $session->version() != 0;
    my $response = $session->get_table(
        '-baseoid' => $oid,
        '-contextname' =>
    );
    unless($response){
        my $err = $session->error_status();
        $self->device->{logger}->debug("walkTest() failed ! : $err");
        return;
    }
    return unless $response;

    my $values;
    my $offset = length($oid) + 1;

    foreach my $oid (keys %{$response}) {
        my $value = $response->{$oid};
        $self->device->{logger}->debug("$value");
        $values->{substr($oid, $offset)} = $value;
    }

    return $values;

}

it can be reduced to a simple

delete $self->device->{snmp}->{session}->{_context_name};

at the begining of the submodule run()

g-bougard commented 1 year ago

Hi @shmsh9

Okay, now I understand the problem. Thank you for your deep and very helpful investigation.

The problem is specific to SNMPv3 but must be fixed.

g-bougard commented 1 year ago

Hi @shmsh9

to avoid accessing to private member of public API, and after I took a deep look in the Net::SNMP code, can you try this patch ?

diff --git a/lib/GLPI/Agent/SNMP/Live.pm b/lib/GLPI/Agent/SNMP/Live.pm
index f9831d12b..70c3c93e8 100644
--- a/lib/GLPI/Agent/SNMP/Live.pm
+++ b/lib/GLPI/Agent/SNMP/Live.pm
@@ -121,7 +121,7 @@ sub reset_original_context {
                              undef   ;

     if ($version eq 'snmpv3') {
-        delete $self->{context};
+        $self->{context} = "";
     } else {
         $self->{session} = $self->{oldsession};
         delete $self->{oldsession};
@@ -135,7 +135,7 @@ sub get {

     my $session = $self->{session};
     my %options = (-varbindlist => [$oid]);
-    $options{'-contextname'} = $self->{context} if $self->{context};
+    $options{'-contextname'} = $self->{context} if defined($self->{context});

     my $response = $session->get_request(%options);

@@ -158,7 +158,7 @@ sub walk {

     my $session = $self->{session};
     my %options = (-baseoid => $oid);
-    $options{'-contextname'}    = $self->{context} if $self->{context};
+    $options{'-contextname'}    = $self->{context} if defined($self->{context});
     $options{'-maxrepetitions'} = 1                if $session->version() != 0;

     my $response = $session->get_table(%options);

This should reset the context as expected at the next get() or walk() calls. If this is not sufficient, we will have to handle the session in the same way than for SNMP v2c, but the patch should be a little larger as we have to store few more datas.

shmsh9 commented 1 year ago

Hi @g-bougard Thanks, Yes I confirm your patch works !