build-trust / ockam

Orchestrate end-to-end encryption, cryptographic identities, mutual authentication, and authorization policies between distributed applications – at massive scale.
https://ockam.io
Apache License 2.0
4.45k stars 559 forks source link

Close the responder side of a secure channel in case of inactivity #5164

Open etorreborre opened 1 year ago

etorreborre commented 1 year ago

Current behavior

When a SecureChannelListener receives a connection request to establish a secure channel, two workers are created:

However when the initiator shuts down its connection (and maybe opens a new one) there is no removal of those 2 workers on the responder side. This can eventually lead to issues like a node slowing down or lacking memory.

Desired behavior

One way to fix this issue is to add a maximum_idle_time: Duration field to the SecureChannelListenerOptions and start a timeout which is reset every time the HandshakeWorker receives a message (the HandshakeWorker is the worker which starts the secure channel handshake and eventually receives messages to decrypt).

If a message is not received in time then shutdown the HandshakeWorker and the corresponding EncryptorWorker.

Let's set a default value for the maximum_idle_time to 60 seconds.

SanjoDeundiak commented 1 year ago

Agree with the approach. That also plays well with the fact that we monitor our connections by sending pings, so channels that we may still need in the future don't idle even if we don't send any useful payload.

SanjoDeundiak commented 1 year ago

I also think that there are more things that we need to use a similar approach to, like portals and tcp connections

Alef-gabriel commented 1 year ago

Hello friends, i can take this too?

Alef-gabriel commented 1 year ago

Hi @SanjoDeundiak, Would be something looks like Medic but for channels?

davide-baldo commented 1 year ago

Hi @Alef-gabriel ! the idea is that the ping/pong of the Medic will keep it alive and the logic to shut down the unused secure channel will be within (or at least controlled by) the HandshakeWorker, currently we never automatically stop remote secure channels. The implementation could be similar to the current Medic, by running a check every minute: if a packet has not passed in the last minute, close the secure channel.

Alef-gabriel commented 1 year ago

Hi @davide-baldo! I understand I've been working on this task, implementing a timeout, but I think adding a monitor is a good idea. What do you prefer?

davide-baldo commented 1 year ago

A timeout is should be even better! continue with your implementation, if you want some early feedback feel free to open a draft PR