-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[v0.10] Merge unprivileged stuff to v0.10 #3341
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
runtime_user and runtime_group are added to the xrdp.ini file so that the service knows how to reduce privilege (cherry picked from commit 17a5656)
- xrdp_listen.c is refactored so we can create the listening socket(s) before dropping privileges. - The code which reads startup params from xrdp.ini is moved from xrdp_listen.c to xrdp.c, so it is only called once if we test the listen before starting the daemon. (cherry picked from commit ddff9eb)
Now we have g_file_open_rw() we don't need to try to write to the PID file to see if we can. Just leave the file open and write to it after forking. (cherry picked from commit 2446c20)
(cherry picked from commit b1d8428)
If xrdp is running with dropped privileges it won't be able to delete the PID file it's created. Places where xrdp is stopped need to cater for this. It's prefereable to do this than make the PID file writeable by xrdp with dropped privileges, as this can still lead to DoS attacks if an attacker manages to modify the PID file from a compromised xrdp process. (cherry picked from commit ce355fc)
(cherry picked from commit 48255da)
The unprivileged user needs to be able to read the certificate and key files to offer TLS, but should not be able to write to then. This commit checks the TLS files are read-only, rather than simply readable (cherry picked from commit 0ebf4cf)
While here, ignore build artifacts of chkpriv tools. Follow-up to: neutrinolabs#2974 (cherry picked from commit c2b8cbf)
- Do not include substitutedd xrdp-chkpriv into tarball - Dot not install xrdp-chkpriv.in While here, drop exec permission from *.c source file. (cherry picked from commit f61a591)
While here, drop exec permission from xrdp-chkpriv.in. The exec permission will be granted to substituted xrdp-chkpriv script during `make install` process. (cherry picked from commit a857f0b)
matt335672
approved these changes
Dec 13, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems OK to me. As far as I can tell, it's unchanged from the changes we made for devel.
I ran it up, and it seems to work fine.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.