Repeated connect attempts with invalid certificate create many sessions once valid

[fritzz-98]

Describe the bug

When repeatedly attempting to connect to an OPC UA server with an invalid certificate, Milo appears to accumulate sessions. Once the certificate becomes valid again, the server reports many open sessions — as many as it can handle — as if all prior connection attempts were "queued up" until the certificate check passed.

This is a simplified snippet of the connection logic that is retriggered if it throws an error every 5 seconds:

try {
    EndpointDescription endpoint = validateAndGetEndpointDescription(opcUaUrl);
    OpcUaClient client = createClient(endpoint, opcUaUrl);
    client.connect().get();
}
catch (Exception ex) {
    logger.error("Could not connect to OPC-UA server <url={}>", opcUaUrl, ex);
}

When the server-side certificate flag is set to invalid, the call to client.connect() fails with:

org.eclipse.milo.opcua.stack.core.UaException: status=Bad_SecurityChecksFailed, description=An error occurred verifying security.

At some point during runtime we set the server-side certificate to a valid cert, the connection succeeds but the server now shows a large number of open sessions, as if every retry attempt resulted in a lingering session once the certificate became valid.

An easy fix for this is to keep a reference of the client and call the disconnect function in the catch block - I am not sure though if this is expected behavior, if our reconnect logic doesn't make sense or if this is an actual bug in the client implementation. Nevertheless I wanted to report it!

Additional context

• Eclipse Milo version: 1.0.5

[kevinherron]

What Server are you seeing this against?

The Server should be protecting against this, in particular because this check is done after creating a Session but before activating the Session.

A Server application should limit the number of Sessions. To protect against misbehaving Clients and denial of service attacks, the Server shall close the oldest Session that is not activated before reaching the maximum number of supported Sessions.

https://reference.opcfoundation.org/Core/Part4/v105/docs/5.7.2#:~:text=A%20Server%20application%20should%20limit%20the%20number%20of%20Sessions.%20To%20protect%20against%20misbehaving%20Clients%20and%20denial%20of%20service%20attacks%2C%20the%20Server%20shall%20close%20the%20oldest%20Session%20that%20is%20not%20activated%20before%20reaching%20the%20maximum%20number%20of%20supported%20Sessions.

These Sessions are never activated and should either time out or be cycled out to make room for new Sessions automatically.

[fritzz-98]

In this test I used an inhouse PLC-Sim with its own OpcUa Server implementation... tomorrow I will be able to test this with an actual Siemens PLC and can report back on this.

[kevinherron]

Looks like maybe the Milo Server implementation does not do this correctly either...

[kevinherron]

Also, just as general advice, you should not call connect() repeatedly because once you call it the first time the SDK will automatically keep trying to connect for you. If you want to manage this yourself, then you should call disconnect() after catching the Exception because that is how you would stop the Session FSM from continuing to try to create and activate a Session in the background.

You probably see many Client/Session created because you create a new Client each time and never shutdown or stop this Session FSM by calling disconnect().

[fritzz-98]

This connection logic is only triggered when client.connect() throws an exception. If the initial connection succeeds, all subsequent reconnection handling is managed internally by Milo.

We implemented this retry logic because our client application can start without a network connection or without the target PLC being available. At some point during runtime, a network or PLC may become available, and in that case the client needs to establish a connection.

If Milo already provides a reliable way to handle this scenario, then we may need to reconsider our custom retry logic.

[kevinherron]

Should already be handled. If you do your own thing be sure to disconnect to properly dispose of the OpcUaClient instance. You are creating many and leaving them connected in the background right now.

[kevinherron]

If you aren't persisting an EndpointDescription (or enough information to create one) then I could see the need to enter a looping state where you continually try to call the Discovery services to select an endpoint.

But once you have an EndpointDescription then you can create the OpcUaClient and call connect() just once and it will attempt connect and continue to try to connect if that fails.

[fritzz-98]

Aha! That's exactly what's happening in our implementation, we aren't presisting the EndpointDesciption but after connection properly once the reconnect behavior is handled by Milo. In that case we just need to call disconnect on the client in the catch block and it should be correct. Thank you!