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.

Sign up for GitHub

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

Jump to bottom

Restructure session start #2592

Merged
merged 9 commits into from
Mar 27, 2023
Merged

Restructure session start #2592

merged 9 commits into from
Mar 27, 2023

Conversation

matt335672
Copy link
Member

@matt335672 matt335672 commented Mar 13, 2023 •
edited
Loading

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 mentioned this pull request Mar 17, 2023
Merged
@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 mentioned this pull request Mar 18, 2023
Merged
@matt335672 matt335672 force-pushed the restructure_session_start branch from 8e802f7 to 72f64b0 Compare March 18, 2023 11:38
@matt335672 matt335672 marked this pull request as ready for review March 18, 2023 11:43
@matt335672 matt335672 force-pushed the restructure_session_start branch from 72f64b0 to 2922744 Compare March 18, 2023 11:44
@matt335672 matt335672 mentioned this pull request Mar 20, 2023
Merged
@matt335672
os_calls changes
8b9f9b4 
- 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()
@matt335672
Remove clone_session_params()
c0b9bf9 
This is a hangover from SCP V1 and is no longer required with
the move to libipm
@matt335672
Duplicate session module to session_list module
84c19e0 
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
@matt335672
Split sesman/session.c into session.c and session_list.c
fb25de0
@matt335672 matt335672 force-pushed the restructure_session_start branch from 2922744 to 9196526 Compare March 23, 2023 18:12
@matt335672
Rename session_list module functions
5cb5542 
Rename functions in the session_list module so it's clearer where
they are defined.
@matt335672
Make wait objects private to sesman.c
6bf0aed
@matt335672
Add session create status code for failed X server
efd871b
@matt335672
xwait changes (breaks compile)
34ad55f 
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 force-pushed the restructure_session_start branch from 9196526 to 8d74aef Compare March 24, 2023 10:57
@matt335672 matt335672 force-pushed the restructure_session_start branch from 8d74aef to 991770c Compare March 24, 2023 10:59
@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
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@matt335672