Implement uds authentication #2472
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
matt335672
force-pushed
the
implement-uds-auth
branch
from
cbeeb6c
to
eb47e69
Compare
|
A couple of screenshots showing the improved logging:- Max session limit reached
Wrong password
Command sequence to start a session for a user from the command line, and list sessions:- $ sudo -u testuser xrdp-sesrun -t Xvnc ok display=:11 GUID=DD94AEC9-F12D-44CE-A22C-ACFDE96F4E43 $ sudo -u testuser xrdp-sesadmin -c=list Session ID: 144542 Display: :11 User: testuser Session type: Xvnc Screen size: 1280x1024, color depth 32 Started: Thu Dec 15 10:19:53 2022 I've not made major functional changes to the commands yet. I haven't implemented the admins group to be able to list all sessions, and I haven't started to tidy up the horrible interface to |
|
Just tested on FreeBSD, and found g_sck_get_peer_cred() was broken. Additional commit fixes #146 |
This was
linked to
issues
Dec 15, 2022
This was referenced Dec 15, 2022
Merged
Moving to a uid_t to store the user information makes a lot of sense. When doing this, we need a function to get information about a user from the uid_t As well as creating the function g_getuser_info_by_uid() we also rename g_getuser_info() to g_getuser_info_by_name() and make the parameter ordering more usual.
The intention is to improve decoupling of the modules making up sesman.
Messaging changes:- - Implement sys_login request message with username, password and IP address - Implement UDS login message for current user connected to sesman - Implement common login response message for login requests - Implement logout message so gateway authentications can be handled - with login/logout messages - Remove login info from the create session request - Existing gateway request/response messages removed - Add close connection message so that sesman can close terminated connections without displaying ERROR messages in the log. - Add a set_peername message so clients can send a name to sesman for improved logging. Other changes:- - Add status types for logging in and session creation, so that the front-end can supply the user with more informative errors in the event of an error occurring. - Users identities are now carried by UID rather than username, as xrdp and sesman are guaranteed to be on the same machine.
An extra method auth_uds() is added to the PAM module to allow a 'struct auth_info' to be created for a UDS login. The PAM stack is used to check the UDS user can be authorized. Also, an error code is returned from the auth module rather than a simple boolean. This allows a more complete status to be communicated to the user. See neutrinolabs#1921 and also neutrinolabs#909 and neutrinolabs#642
The previous commit introduced a new interface for the auth modules. This commit simply updates the other auth modules to use the new interface. The basic auth module is also updated so that if a user has a shadow password entry indicated, but the shadow entry cannot be found, an error is logged rather than silently succeeding. The BSD authentication module is also updated to allow it to be compiled on a Linux system for basic testing.
This change allows the authtest utility to exercise the updated auth module interface which includes UDS authentication and improved error logging.
Update sesman to cope with separate authentication/authorization (AA) and command processing. Also, internally users are now tracked by UID rather thn username. This addresses a problem found by some users using federated naming services (e.g. Active Directory) where the same user can be referred to in more than one way. See neutrinolabs#1823 The separation of AA in this way allows for multiple attempts to be made on one connection to get a password right. This addresses MaxLoginRetry not working (neutrinolabs#1739)
The sesman tools sesrun and sesadmin now use the separate authentication/authorization (AA) interface introduced to sesman by the previous comment. sesrun can use either password or UDS authentication. With some limitations, this can allow for automatic creation of sessions for local users without a password being needed. sesadmin now operates using UDS logins only and so a username and password are not required. To use sesadmin for another user, use su/sudo/doas to authenticate as the other user.
xrdp is updated to use the separate authenticate/authorization (AA) and command processing interface now provided by sesman. PAM processing has been removed entirely and moved into the seman PAM module. As a result, gateway processing for proxy use-cases can be made use of by non-PAM systems.
Socket level should be SOL_LOCAL rather than SOL_SOCKET - See 'man unix'.
matt335672
force-pushed
the
implement-uds-auth
branch
from
2fadedf
to
8a0a024
Compare
|
This also addresses #1303 I believe, at least in some environments. |
metalefty
reviewed
Jan 5, 2023
| case XRDP_SESSION_CODE: | ||
| type = SCP_SESSION_TYPE_XRDP; | ||
| break; | ||
| case XRDP_SESSION_CODE: |
There was a problem hiding this comment.
I think we can remove this support for x11rdp in xrdp 1.0. It is replaced with xorgxrdp and been kept for compatibility for a while. It's time to remove it because xrdp 1.0 is a big jump.
However, it can be done later.
metalefty
approved these changes
Jan 5, 2023
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.
Update : ready for review
Fixes #642
Fixes #909
Fixes #1739
Fixes #1823
See also #1921
This is moving some way towards the architectural changes in #1961.
This series of commits separates authentication+authorization (AA) from commands in the sesman interface. AA can be either via username/password, or via Unix Domain Socket (UDS) ownership.
Here are some notes for the commits:-
su/sudo/doasto authenticate as the other user.MaxLoginRetrynow has an effect.The most significant of these is that the sesman tools are starting to become more useful.