eduvpn/vpn-server-api

Runaway sessions are not removed from the log database

jornane opened this issue · 4 comments

If OpenVPN crashes, the end date of a session is not written in the database. This prevents the cleanup script from removing session, because it refuses to remove sessions that are in use.

Proposed solution (can be multiple):

  • On boot, end all sessions
  • On start of an OpenVPN instance, end all running sessions that have an IP address that is served by the instance
  • On client connect, end earlier running sessions with the same IP address
    • For example, run an UPDATE query before the INSERT query.

Ending a session is done by setting the end time to the current time. The end time of the runaway session will thus be the start time of the OpenVPN server, or the connect time of another client. This should be OK because the IP will not have been used between the actual disconnect time and the logged disconnect time.

We can consider to mark runaway sessions with a boolean (runaway column in the table), so that the admin knows that the end time should be taken with a grain of salt.

On boot, end all sessions

This is not possible, there is no script to trigger this.

On start of an OpenVPN instance, end all running sessions that have an IP address that is served by the instance

There is no hook to do this... Also there are multiple OpenVPN processes involved so that quickly would become a mess.

On client connect, end earlier running sessions with the same IP address

  • For example, run an UPDATE query before the INSERT query.

This could totally work. I've been working on a database migration library as well to add the "client_lost" database column to indicate this happened to a connection as to make it possible to indicate the client connection was probably already lost some time ago before the "disconnected_at" column was set.

We are almost there! :) It requires some extensive testing as this will be a "hot" database migration taking place on existing deploys without any operator involvement. So better make sure we don't screw this up! :)

solved in master.

Fixed in e1bce24 I suppose? 👍

Yeah it was a whole bunch of commits... I worked on it in a branch first :)