When using PAM authentication, a copy is made of the username and password in the auth_info structure.
The password copy is not cleared from memory when the structure is deallocated. This could mean the password is revealed to an attacker from a coredump.
One solution is to clear the password when the struct is deallocated. However, the username and password in the auth_info struct are only required for the duration of the PAM conversation function. A better solution is to remove the username and password from the auth_info struct entirely, and just use pointers for the duration of the time the callback function is used.
The loadable sesman authentication modules use different types for the
authentication handle returned from auth_userpass(). The PAM module
uses a pointer, and the other modules use (effectively) a boolean. Within
sesman itself, a long or tbus (intptr_t) is used.
This PR replaces all of these types with a pointer to an incomplete type.
Consequently:-
- A single better-labelled type is used it all places within sesman so
it's more obvious what's being handled.
- There is no need to cast the authentication handle within the PAM
module to a long and back again.
- The compiler can check function signatures between auth.h and the
various verify modules.
This may throw a warning with clang-15+ when devel logs are disabled
Fixes
../../../xrdp-0.9.19/sesman/chansrv/chansrv.c:198:9: error: variable 'count' set but not used [-Werror,-Wunused-but-set-variable]
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Made session allocation policies more readable and maintainable.
The 'C' policy which was confusing before has been replaced with the
'Separate' keyword. This is a public interface change, but is unlikely
to affect many users.
The logging in session_get_bydata() is substantially improved, making
it far easier to spot why sessions are getting matched or not matched.
The connected client is currently described in two places in
the xrdp_client_info structure:-
1) In the connection_description field. This was introduced as
field client_ip by commit d797b2cf49
for xrdp v0.6.0
2) In the client_addr and client_port fields introduced by commit
25369460a1 for xrdp v0.8.0
This commit unifies these two sets of fields into a single
set of fields describing the connection IP and port (for
AF_INET/AF_INET6 connections only) and a connection description
for all connection types.
The code in os_calls to provide client logging has been simplified
somewhat which should make it easier to add new connection types (e.g.
AF_VSOCK).
The old connection_description field used to be passed to sesman to
inform sesman of the IP address of the client, and also to provide
a string for 'C' field session policy matching. 'C' field session policy
matching does not actually need this string (see #2239), and so now only
the IP field is passed to sesman.
The code in xrdp_mm.c to connect to chansrv over a TCP socket has
been removed, with the move to UDS. This PR simply removes the
chansrv TCP listening code. Without doing this, some configurations
result in a failure of xrdp to connect to chansrv.
The TCP socket implementation of sesman has a number of limitations,
namely that it is affected by firewalls, and also that determining the
user on the other end requires a full authentication process.
The advantage of the TCP socket is that sesman and xrdp can be run on
separate machines. This is however not supported by the xorgxrdp
backend (shared memory), and is insecure, in that passwords are sent
in-the-clear, and the connection is susceptible to MitM attacks. This
architecture has been deprecated in release notes since xrdp v0.9.17,
and although it will continue to be supported in any further releases
in the x0.9.x series, it will not be supported in the next major
version.
When sesman used a standard TCP socket, we were guaranteed only one copy
of sesman could run on on address, as standard TCP listening rules
enforced this. This isn't the case with Unix Domain sockets. This
module implements a locking mechanism for a UDS which emulates the
standard TCP socket behaviour.
This is required for PAM systems that depend on group membership being
available during PAM processing. This is used by pam_group on FreeBSD
and pam_group on Linux-PAM, although the functionality of both is
different.
Disable clipboard_event_selection_request call is overkill for
blocking text/image/file purpose.
For example, it breaks existing behavior (slow response from gedit,
gimp as a side effects)
Instead, in clipboard_event_selection_request, these media format will
be blocked respectively which depends on the following configurations
in sesman.ini [Security] section.
* RestrictInboundClipboard=text
* RestrictInboundClipboard=file
* RestrictInboundClipboard=image
You can also set comma separated list.
* RestrictInboundClipboard=text,file,image
RestrictOutboundClipboard kills all of test/file/image
transfer via clipboard.
For controlling each content type behavior,
clipboard_xevent is not appropriate place to block respectively.
Instead, in clipboard_event_selection_notify, these media type
will be blocked which depends on the following configurations in
sesman.ini [Security] section.
* RestrictOutboundClipboard=text
* RestrictOutboundClipboard=file
* RestrictOutboundClipboard=image
You can also set comma separated list
* RestrictOutboundClipboard=text, file, image
It supports the extended configurations for sesman.ini:
Before:
[Security]
RestrictOutboundClipboard=true or false
After:
[Security]
RestrictInboundClipboard=[true or false | text or file or image | comma separated list]
RestrictOutboundClipboard=[true or false | text or file or image | comma separated list]
Above configuration is disabled by default (false)
And it can be specified comma separated list like this:.
RestrictInboundClipboard=file, image
RestrictOutboundClipboard=text, file, image
Note that if RestrictOutboundClipboard=true,file is set,
file is ignored and it is treated as RestrictOutboundClipboard=true
It is same for RestrictInboundClipboard.
According to https://github.com/neutrinolabs/xrdp/wiki/Logging,
it may be better to emit this log message because this log is
useful for system administrator to know whether RestrictOutboundClipboard
configuration works or not
And raise log level to info because it is informative for system
administrator.
As g_file_atom2 is x-special/gnome-copied-files
(See g_file_atom2 definition in sesman/chansrv/clipboard.c),
it should be "x-special/gnome-copied-files" in this context.
Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
* Added s_rem(s) for getting the remaining bytes in a stream
* Added s_rem_out() macro
* Fixed 15bpp pointer error checking
* Combined the 512 and 2048 bit certificate sending code paths
* Other detailed comments and logging added following MS-RDPBCGR
There are two points.
Make sure cleanup files happen after chansrv and Xserver exit. If these
child processes lock socket files, the deletion might fail.
Usually, cleanup of xorgxrdp related socket files is handled by
xorgxrdp. Just in case it failed, perform cleanup also in sesman.
Fixes#1740. Thanks to @matt335672.
Sponsored by: Cybertrust Japan
Sponsored by: HAW International
This commit adds:
* replace multiple logging macros with LOG and LOG_DEVEL
* logging configuration for chanserv
* logging configuration for console output
* logging configuration for per file or method log level filtering for
debug builds
* file, line, and method name in log message for debug builds
The MS specs determine that the character buffer lenngths
for usernames, domains, passwords, alternate shells, etc
can be up to 512 characters including the mandatory null
terminator.
Constants from MS documents (MS-RDPBCGR etc) moved out of
common/xrdp_constants.h into includes named after the documents.
Similar includes moved from sesman/chansrv to the common area.
- Reimplemented inode store in separate module chansrv_xfs.[hc]
- Allowed atimes and mtimes to be written to Windows side
- Mapped file user write bit to (inverted) Windows FILE_ATTRIBUTE_READONLY bit
- Mapped file user execute bit to Windows FILE_ATTRIBUTE_SYSTEM bit
- Implemented improved security for remotely mounted drives
- Implemented USB device removal, allowing hot-plug/remove of memory sticks
- Fixed pagefile.sys breaking Ubuntu file browser
- Fixed write offset bug
- Allowed renaming of open files
- Improved reported error codes
- Fixed various memory leaks
- Addressed valgrind errors related to struct fuse_file_info pointers.
Fixes#1335.
In file included from ./irp.h:27:
./chansrv_fuse.h:39:5: error: unknown type name 'time_t'
time_t atime; /* Time of last access. */
^
./chansrv_fuse.h:40:5: error: unknown type name 'time_t'
time_t mtime; /* Time of last modification. */
^
./chansrv_fuse.h:41:5: error: unknown type name 'time_t'
time_t ctime; /* Time of last status change. */
^
3 errors generated.
*** Error code 1
- Replace xfuse_cb_enum_dir() directory callback for adding files with
more general xfuse_devredir_add_file_or_dir() to be called from a
directory or a lookup operation.
- Moved XRDP_INODE out of public interface for chansrv_fuse, and replaced
with simpler struct file_attr to pass to
xfuse_devredir_add_file_or_dir()
- Allow a struct file_attr to be placed in an IRP for assembly of file
attributes over multiple IRP_MJ_QUERY_INFORMATION requats.
- Add dev_redir_lookup_entry() to public interface for devredir.c
- Add xfuse_devredir_cb_lookup_entry() callback to public interface for
chansrv-fuse.c
- Remove unused 'is_synced' member from struct xrdp_inode
- Move prototype for xfuse_devredir_cb_write_file() to correct file
- Add const correctness to dev_redir_strings_ends_with() function
- Add const correctness to fuse_reverse_pathname() function
- Moved devredir_proc_cid_* functions out of devredir.h and made static
- Added XFUSE_DUMP_ADDED_ENTRIES maro for debugging
- Removed duplicate code path in xfuse_remove_dir_or_file()
- Removed duplicate code path in xfuse_cb_rename()
- Removed duplicate code path in xfuse_create_dir_or_file()
- Removed duplicate code path in xfuse_cb_open()
- Removed duplicate code path in xfuse_proc_opendir_req()
- Remove unused USE_SYNC_FLAG macro
- Remove unused members invoke_fuse, off, dirbuf1 from XFUSE_INFO
- Clear f_fifo_opendir entries on deinit
- Added some comments and fixed some others
as it is already deprecated. x11rdp is complicated to build and very few
people using it actually. However, some people still select "X11rdp"
session and get stuck despite not installing x11rdp.
https://github.com/neutrinolabs/xrdp/issues/962#issuecomment-430545526
People who really want to use x11rdp should revert this commit.
remove not used chansrv <-> xrdp messages
move static channel disable control into libxrdp
remove some blocking read, write chansrv calls
add drdynvc calls to libxrdp
add drdynvc calls to chansrv
channel cleanup
Unless X server failures are caught, these can cause a premature
exit of chansrv, giving it no chance to clean up. This is currently a
particular problem for fuser mounts.
As the Debian patch[1] expresses, spitting messages on the console when
a process starts in background is a bad idea. Everything should be
written to log file and daemon should start silently. This is a first
step to shut up daemons.
Got some idea from Debian Remote Maintainers and Thorsten Glaser,
thanks!
[1] 2751ad4d62/debian/patches/shutup-daemon.diff
Pull request #650 is not valid to avoid run session twice.
It certainly stops running session twice but causes #1016.
In FreeBSD, sesman process will run like this. The intermediate
sesman is needed to detect session termination correctly.
xrdp-sesman (daemon)
|
+- xrdp-sesman (FreeBSD specific intermediate sesman)
|
+- xrdp-sesman (bsd sesion leader & each session)
|
+- Xorg
+- startwm.sh
+- xrdp-chansrv
To stop runninng session twice correctly, just exit before the
intermediate sesman executes Xorg, WM and chansrv.
* Initialise inode table in `xfuse_create_share()` if necessary
* Add guard to `xfuse_init_xrdp_fs()` to prevent double initialisation of the inode table
