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

Restructure session start #2592

Merged
merged 9 commits into from
Mar 27, 2023

Conversation

matt335672
Copy link
Member

@matt335672 matt335672 commented Mar 13, 2023

This is now ready for review, following my own testing.

This is a major change to sesman.

There are quite a few file changes, bu the main implementation points are:-

  1. sesman/session.c is split into two files
    a) session_list.c contains all the list-handling code.
    b) session.c now contains just the code to manage a single session.
  2. session starting logic is reworked to make it possible to detect X server failures as a separate class of error

I've split session.c into two with #1961 in mind - I'm getting close to the point where I can split sesman entirely. session_list.c contains all the code for sesman and session.c contains all the code for sesexec.

The reworking of session starting logic allows me to use @derekschrock's waitforx utility to check for a working X server before starting the session. If the X server isn't started, this is reported back to xrdp.

Not only is the log file more readable with this change, but error reporting is improved. If the X server can't be started, the logging window now looks like this, rather than blue-screening for a minute or so:-
image

Also sesman logging is vastly improved on starting a session. Here's a development logging example:-

2023-03-13T21:36:13.871+0000] [INFO ] [g_sck_accept(os_calls.c:1297)] Socket 13: connection accepted from AF_UNIX
[2023-03-13T21:36:13.980+0000] [INFO ] [process_sys_login_request(scp_process.c:296)] Received system login request from xrdp for user: testuser IP: ::ffff:1.1.1.1
[2023-03-13T21:36:14.400+0000] [INFO ] [verify_pam_conv(verify_user_pam.c:138)] Handling struct pam_message { style = PAM_PROMPT_ECHO_OFF, msg = "Password: " }
[2023-03-13T21:36:14.198+0000] [INFO ] [access_login_allowed(access.c:56)] Terminal Server Users group is disabled, allowing authentication
[2023-03-13T21:36:14.246+0000] [INFO ] [authenticate_and_authorize_connection(scp_process.c:160)] Access permitted for user=testuser uid=1001
[2023-03-13T21:36:14.330+0000] [INFO ] [process_create_session_request(scp_process.c:483)] Received request from xrdp to create a session for user testuser type=Xorg geometry=1920x1080, bpp=24, shell="", dir=""
[2023-03-13T21:36:14.374+0000] [INFO ] [session_start(session.c:836)] calling auth_start_session for uid=1001 from pid 1621183
[2023-03-13T21:36:14.722+0000] [INFO ] [start_x_server(session.c:500)] Starting X server on display 10: /usr/lib/xorg/Xorg :10 -auth .Xauthority -config xrdp/xorg.conf -noreset -nolisten tcp -logfile .xorgxrdp.%s.log
[2023-03-13T21:36:15.819+0000] [INFO ] [session_start_subprocess(session.c:774)] X server :10 is working
[2023-03-13T21:36:15.867+0000] [INFO ] [session_start_subprocess(session.c:775)] Starting window manager for display :10
[2023-03-13T21:36:15.910+0000] [INFO ] [session_start_subprocess(session.c:786)] Starting the xrdp channel server for display :10
[2023-03-13T21:36:15.912+0000] [INFO ] [start_window_manager(session.c:292)] Using the default window manager on display 10: /etc/xrdp/startwm.sh
[2023-03-13T21:36:15.969+0000] [INFO ] [allocate_and_start_session(scp_process.c:247)] ++ created session: username testuser, ip ::ffff:172.19.73.10
[2023-03-13T21:36:15.970+0000] [INFO ] [run_xrdp_session(session.c:556)] Session in progress on display :10. Waiting until the window manager (pid 1621440) exits to end the session

and at session end:-

2023-03-13T21:36:37.351+0000] [INFO ] [run_xrdp_session(session.c:564)] Window manager (pid 1621440, display 10) finished normally in 21 secs
[2023-03-13T21:36:38.250+0000] [INFO ] [run_xrdp_session(session.c:596)] Terminating X server (pid 1621263) on display :10
[2023-03-13T21:36:38.322+0000] [INFO ] [run_xrdp_session(session.c:602)] Terminating the xrdp channel server (pid 1621441) on display :10
[2023-03-13T21:36:38.461+0000] [INFO ] [run_xrdp_session(session.c:609)] X server pid 1621263 on display :10 finished
[2023-03-13T21:36:38.700+0000] [INFO ] [run_xrdp_session(session.c:615)] xrdp channel server pid 1621441 on display :10 finished
[2023-03-13T21:36:38.778+0000] [INFO ] [cleanup_sockets(session.c:136)] cleanup_sockets:
[2023-03-13T21:36:38.841+0000] [INFO ] [session_start(session.c:844)] Calling auth_stop_session from pid 1621183
[2023-03-13T21:36:38.984+0000] [INFO ] [sig_sesman_session_end(sig.c:112)] Process 1621183 has exited
[2023-03-13T21:36:39.494+0000] [INFO ] [session_kill(session_list.c:382)] Calling auth_end for pid 1621183 from pid 1502282
[2023-03-13T21:36:39.133+0000] [INFO ] [session_kill(session_list.c:388)] ++ terminated session: UID 1001 (testuser), display :10.0, session_pid 1621183, ip ::ffff:172.19.73.10

@matt335672 matt335672 force-pushed the restructure_session_start branch 2 times, most recently from f7a87a3 to 7ff266e Compare March 16, 2023 14:18
@matt335672 matt335672 force-pushed the restructure_session_start branch 4 times, most recently from 61878f1 to 8e802f7 Compare March 18, 2023 11:23
@matt335672 matt335672 marked this pull request as ready for review March 18, 2023 11:43
@matt335672 matt335672 mentioned this pull request Mar 20, 2023
- Add g_pipe()
- Add g_file_duplicate_on()
- Rework struct exit_status to make it easier to parse
- Add optional status return to g_waitchild()
This is a hangover from SCP V1 and is no longer required with
the move to libipm
This makes an exact copy of session.[ch] in session_list.[ch].
The intention is to be able to follow changes in git with
the --follow switch
Rename functions in the session_list module so it's clearer where
they are defined.
The wait_for_xserver() call is refactored so that it can
be called from root context to wait for an X server run by
a specific user.
@matt335672 matt335672 merged commit 3ee8eb9 into neutrinolabs:devel Mar 27, 2023
@matt335672 matt335672 deleted the restructure_session_start branch October 5, 2023 14:07
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.

1 participant