Fix systemd sessions

[geiseri]

Move the session creation inside of the child fork. This will allow sesman to only have the child as a part of the user's session scope and the parent remain part of the services scope.

This was tested with:

1. Signon new session - xrdp-sesman parent in xrdp-sesman.service scope and xrdp-sesman child in the user's session scope.
2. Log off session - xrdp-sesman parent remains in xrdp-sesman.service scope and child xrdp-sesman is terminated along with the user's session.
3. Disconnect session - xrdp-sesman parent remains in xrdp-sesman.service scope and child xrdp-sesman remains inside of the user's scope.
4. Reconnect session - xrdp-sesman parent remains in xrdp-sesman.service scope and child xrdp-sesman remains inside of the user's scope. Old session is re-attached.

This should fix issue #778.

Signed-off-by: Ian Geiser geiseri@geekcentral.pub

[tfischer77]

Isn't that reverting the change done in #694? That one was done because pam_mkhomedir was not able to create a home directory...

[geiseri]

Yes, in essence it is. From what I can see in the code the actual setup of the X server happens in the parent process. This is not correct because it might (or might not) happen before the child creates the session. Hence why the delay thing was needed. I need to look into the fork order, but the VNC setup may need to be moved so it is run in the same process as the session is opened.

[geiseri]

I did confirm that at least on Arch linux VNC worked as well as pam_mkhomedir with this patch. Can someone else verify this? My test desktop is using pam_sssd with active directory, pam_mkhomedir and Xorg driver on a Linux KVM guest running Arch linux.

[metalefty]

With very very quick test, it looks good to me. If Jay agreed, I'm OK with merging this.

[moobyfr]

works for me too.

[tfischer77]

I am also using sssd with pam_mkhomedir. Unfortunately, as I'm on vacation, I'm not able to test it. Will do it in about 14 days. But if you confirmed that it works for you, I guess the it will also work with my setup.

[geiseri]

@tfischer77 what distro do you use? also are you using X11rdp, Xorg, or VNC?

[jsorg71]

If we are going to move auth_start_session to the child process, I think we should move auth_stop_session and auth_end too.

[tfischer77]

@geiseri I'm using debian stretch with xorgxrdp and the latest (or almost latest) devel branch.

[metalefty]

Please be careful not to cause regression of CVE-2017-6967.
I haven't checked it yet.

[geiseri]

@metalefty i need to confirm this, i think though that the change in the code may not have actually addressed the CVE, unless there is PAM init outside that I am missing.

[geiseri]

@jsorg71 i will look at moving those, but in theory those are okay where they are since they need to run no matter the exit status of the child. Right?

[jsorg71]

I made a possible replacement, PR #806. It calls auth_stop_session and auth_end in the same process and avoids the merge.

[metalefty]

Merged #806.