Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add restartability to sesman #3346

Open
wants to merge 19 commits into
base: devel
Choose a base branch
from

Conversation

matt335672
Copy link
Member

@matt335672 matt335672 commented Dec 16, 2024

This is now ready for review.

Fixes #800

Depends on #3328

See also #819

This PR allows for sesman to be restarted without losing sessions.

A restart of xrdp will still drop connections, but these can easily be restarted.

In brief, the design is this (I can add more details around this):-

  1. When sesexec starts a session, a listening socket is created in a directory (the 'restart directory') accessible by root only.
  2. When sesman restarts, before starting its listening socket it iterates over the sockets in the restart directory, and attempts to connect to each one.
  3. When sesexec receives the connection request, it validates the other end is running as root, and if so, re-announces the managed session.
  4. sesman adds the announced sessions to its session list.
  5. sesman restarts its listening socket.

This is not year 2038 compliant on systems with 32-bit integers.

The call can be replaced with the standard C time() call. On
POSIX systems, time_t is guaranteed to be an integer type.
This is currently unused in xrdp
The function as specified used gettimeofday() which is susceptible
to manual time changes, and is obsoleted in POSIX.1-2008. The
replacement uses clock_gettime(CLOCK_MONOTONIC, ) which is not
susceptible to manual time changes (at least on Linux) and cannot run
backwards.

Also, on systems with 32-bit integers, the value returned by this
function wraps around every 49.7 days. To cope with a wraparound in
a way compliant with the C standard, this value needs to return an
unsigned integer type rather than a signed integer type.
This function does not need to have global scope.
The function sesexce_scp_data_in in sesexec.c is a development
artefact and can safely be removed
This is used by sesman to connect to sockets in the restart
directory.
Adds a module which can be used when sesman is restarting. This is
initially used to rediscover sessions from a previous run.
Also add useful comment to session_send_term()
The module is a dummy to be filled in later

Other structural changes to sesexec:-
1) A failure of sesman needs to be detected and handled without
   causing sesexec to exit
2) If sesexec exits, the session can never be rediscovered. sesexec must
   be robust enough to stay up for the lifetime of the session so that
   the discovery function always works.
3) There is a mechanism for sesexec to terminate the session, but it
   doesn't work, as SIGCHLD is not processed while we are waiting for
   the session to finish. This needs fixing.
These sesexec functions are needed for the discovery module to
function.
Add functionality to sesexec discovery module to enable sesman
restarts.
The KillMode of process allows an xrdp connection to survive a
reatart of the xrdp process. This is of minimal benefit, as a
user can always simply reconnect - the session will survive the
restart. The downside is that xrdp will not benefit from any
security fixes, and may well end up out-of-sync with sesman.
@matt335672 matt335672 marked this pull request as ready for review December 19, 2024 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Running sessions "lost" if xrdp-sesman restarts
1 participant