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.
There are a number of ways the existing transport connect logic in
trans_connect could be improved for POSIX compatibility, and also
slightly tidied up:-
1) The same socket is re-used for multiple connect attempts following
failure which isn't behaviour defined by POSIX.1-2017 (although it
works on Linux).
2) An asynchronous connect is started, and then after a short
delay connect() is called again on the same socket. POSIX.1-2017
is clear that in this situation EALREADY is returned before the
connection is established, but is silent on the behaviour expected
when the connection is established. Returning success is an option,
but so is returning EISCONN. The current code assumes the connect()
call will succeed.
3) The code contains two virtually identical, quite complex loops for
TCP and UNIX sockets, differing only in the calls to create a socket
and connect it.
4) trans_connect() contains looping and retry logic, but this isn't
seen as sufficient by the chansrv connect code in xrdp/xrdp_mm.c and
the Xorg connect code in xup/xup.c. Both of these implement their own
looping and retry logic on top of the logic in trans_connect(),
resulting in slightly unpredictable behaviour with regard to
timeouts.
5) A socket number can technically be zero, but in a couple of places
this isn't allowed for.
This PR attempts to correct the implementation of trans_connect(),
and also to simplify the areas it is called from.
As part of the PR, the signature of the server_is_term member of the
xrdp module interface is changed to match the signature expected by the
is_term member of a struct trans. This allows for trans_connect()
in xrdp modules to directly access g_is_term() within the main xrdp
executable. At the moment this functionality is only used by the xup
module.
The leakage does not occur in the most usual use case of xrdp.
It occurs in NeutrinoRDP proxy mode with PAM authentication enabled.
Reported by @TOMATO-ONE
- Based on https://github.com/jsorg71/xrdp/tree/dynamic_monitor
- Tested with xorgxrdp
- Tested with vnc
- Only works with single monitor.
- Update documentation to clarify the difference between MSTSC and
Microsoft Remote Desktop.
- Does not include compatibility with /gfx at this time, which is still
in testing.
- Updates to include ms-rdpedisp.h header for the 2.2.2 specification of
the protocol.
- Adds new dynamic_monitor_layout struct that shares the number of
monitors with xrdp_client_info.h
- Does not allow for BPP changes because the RDP protocol doesn't
support it.
- Option to disable feature as NeutrinoRDP doesn't support it (It was
based on FreeRDP 1.0.1 which didn't yet have this feature.)
- Add CLIENT_MONITOR_DATA_MAXIMUM_MONITORS constant and reference
spec definition.
Depends on https://github.com/neutrinolabs/xorgxrdp/pull/183
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.
Versioning is for libraries. Modules are not libraries; no code is linked
against them.
Libtool makes sure the modules can be opened by dlopen(). That is already
true for ELF format, but other file formats may need special processing.
AM_CPPFLAGS is a documented Automake variable for C preprocessor flags
that should not be overridden when compiling the package.
There is no need to have two additional variables that are ultimately
merged into AM_CPPFLAGS.
Their names are also confusing. EXTDA_DIST is a documented Automake
variable. Everything else that starts with "EXTRA" is not.
This fixes loading modules compiled with a C++ compiler. Remote thandle
type, it's unused. Use tintptr for module data. Don't cast pointers to
long, they won't fit on Win64.
