Incorrect session closing

[eiger-osm]

I am not sure this project is still maintained, but I'll leave this here in case it helps anyone understand session closing issues that might arise with EIPScanner.

Problem

Sessions are managed with the class SessionInfo, which contains a sockets::TCPSocket member _socket. The class SessionInfo has no explicit member function to close a session, it relies on its destructor to close sessions.

The destructor does 2 things:

1. it explicitly sends an UnregisterSession request at the encapsulation level
2. it implicitly destroys the _socket member, which in its own destructor closes the underlying TCP socket.

The resulting effect is: EIPScanner sends an UnregisterSession request immediately followed by closing the socket.

This behavior is not compliant with the CIP Vol. 2 specification. It can create issues with EtherNet/IP devices, sometimes leaving the remote sockets in a half-closed state and delaying or even preventing new connections to these devices.

The CIP Vol. 2 specification explicitly defines two options (see chapter 2-2.1.3.4 Terminating a Session) to terminate a session:

• either you just close the socket (no UnregisterSession shall be sent), the remote device will detect the connection loss and cleanup whatever it needs to clean up on its side
• or you send an UnregisterSession request and wait for the remote side to close the TCP connection. The receiver of an UnregisterSession request is responsible for closing the connection, not the sender.

Forcefully closing the connection while the receiver also initiates the close operation may lead to inconsistent socket states on the receiver side. When EIPScanner connects to embedded devices with limited resources supporting only a limited set of sockets, doing so can sometimes result in reconnection delays, until the remote device detects a timeout on its socket closing and eventually makes its sockets available again for reconnection.

Solution

Implementing a proper UnregisterSession handling requires waiting for the remote side, which is probably not what you'd like to do in a destructor. So going for "option 2" is probably something that should be done in a dedicated member function to be called in a controlled way.

Alternatively, commenting out/deleting the content of the destructor of SessionInfo will produce a valid behavior (option 1 of the CIP specification, no UnregisterSession is used).

SessionInfo::~SessionInfo() {
		//EncapsPacket packet = EncapsPacketFactory().createUnRegisterSessionPacket(_sessionHandle);
		//_socket.Send(packet.pack());
		//Logger(LogLevel::INFO) << "Unregistered session " << _sessionHandle;
}