1) In FIPS mode, Classic RDP security is not allowed at all.
2) In FIPS mode xrdp-keygen creates an empty file
3) Documentation wording improved around the security_level setting
4) Logging improved around the security negotiation
5) Warnings now generated if Classic RDP security is negotiated
These Coverity warnings all relate to the user of g_setenv() where the
return result isn't checked.
An additional void function g_setenv_log() is provided which logs
failures to set environment variables, and returns no status. This is
used in all the places where g_setenv_is currently called.
xrdp contains two functions which do similar things:-
- g_htoi() converts a hex string to an integer, ignoring unrecognised
characters
- xrdp_wm_htoi() converts a hex string to an integer, ignoring leading
whitespace, but terminating on unrecognised characters
An analysis of the uses of g_htoi() shows that the only place where
unrecognised characters might be encountered is parsing lines from
xrdp_keyboard.ini, where all values have an '0x' prefix (i.e. the 'x'
is unrecognised)
An analysis of xrdp_wm_htoi() shows that the functionality to ignore
leading whitespace is not used.
Both functions are replaced with a re-written g_htoi() which is const-
correct and provided with test cases. This function behaves in
the same way as the atoi() library function, in that it terminates on
an unexpected character.
The use of g_htoi() in parsing lines from xrdp_keyboard.ini is replaced
with a call to g_atoix() which handles the '0x' prefix correctly.
Too many places in xrdp use strncpy() to copy strings to fixed-length
buffers, when this is not the correct function to use.
This PR makes sure strlcpy() from the BSDs is available as a saner
alternative. This function is available by default on Linux and FreeBSD.
cppcheck 2.17.0 adds checks that a NULL pointer returned from malloc() and
calloc() is not used.
We do this quite a lot.
I've addressed this by adding functions g_malloc_nofail() and
g_calloc_nofail() which either allocate memory or abort.
functions are now called in places where we are not making these
checks.
Many of these checks are in test programs or example programs.
I've modified the list16 module to handle out-of-memory conditions.
Coverity has generated a number of 'Data race condition' and 'Double
lock' false positives. A lot of these seem to be caused by the NULL
guard in tc_mutex_unlock() not being paired with a NULL guard in
tc_mutex_lock(). This PR adds a NULL guard to tc_mutex_lock().
It should be noted, that on Linux at least, passing NULL to
tc_mutex_lock() causes a segfault. We clearly aren't doing this at the
moment, or we'd know about it. A log message is generated if a NULL
call is made, rather than failing silently.
This allows the `xrdp` part of the path `/etc/xrdp` where config files
are placed to be customizable. This change is useful when trying the
stable version and the devel version alternately.
The function as specified used gettimeofday() which is susceptible
to manual time changes, and is obsoleted in POSIX.1-2008. The
replacement uses clock_gettime(CLOCK_MONOTONIC, ) which is not
susceptible to manual time changes (at least on Linux) and cannot run
backwards.
Also, on systems with 32-bit integers, the value returned by this
function wraps around every 49.7 days. To cope with a wraparound in
a way compliant with the C standard, this value needs to return an
unsigned integer type rather than a signed integer type.
This is not year 2038 compliant on systems with 32-bit integers.
The call can be replaced with the standard C time() call. On
POSIX systems, time_t is guaranteed to be an integer type.
Commit 80fab03198 introduced a way to
prevent waitforx going to the network when trying to open a display,
and hence potentially blocking.
This method turned out to be invalidated by libxcb version 1.16 and
1.17
This change adds an explicit check that the Unix socket for the display
in /tmp/.X11-unix/Xn is open before trying to connect to display ':n'.
This has the same effect.
Some desktop environments are now checking for free space before
copying files to a destination.
To support this, the FUSE filesystem needs to convert the statvfs()
system call to the relevent PDUs from [MS-RDPEFS]
SSL_CTX_set_ecdh_auto() was introduced for OpenSSL 1.0.2. It
has no effect for OpenSSL 1.1.0 and later. For versions before
1.0.2 and after (and including 1.1.0) it should not be called.
The macro was erroneously being called twice for OpenSSL 3.0.0 and
later - this has also been remedied
We always now indicate we support skipping channel joins. If the client
indicates this too, expect no channel join requests from the client.
If we do get some, process them anyway.
This commit allows a keycode_set to be specified as a module parameter
in xrdp.ini. This has the following effects:-
1) xrdp loads the specified keycode set for mapping RDP scancodes to
X11 keycodes. These are then passed to xorgxrdp as part of key press/
key release events.
2) The name of the XKB rules which use the specified keycode set are
passed to xorgxrdp so that XKB can be configured with rules which
match the chosen keycodes.
The effect is to remove all keycode set dependencies from xorgxrdp.
Normally evdev rules and evdev keycodes will be used but base rules and
base keycodes can be used instead for applications that require them.
Also, any systems which do not ship the evdev rules can be made to
work with base rules.
The Brazilian ABNT2 Keyboard layout contains a keypad
decimal key which doesn't exist on other keypads:-
https://www.kbdlayout.info/kbdbr/virtualkeys
This key is curently mapped in xorgxrdp to keycode 134 (basic mapping),
but isn't present in the scancode map. It needs to be added so that it
is available to VNC sessions and will be mapped for xorgxrdp when we
move to evdev keycode mappings.
Replace definitions in ms-rdpbcgr.h marked as TODO with the
names defined in [MS-RDPBCGR]
Some other simplifications around the fake Unicode event processing
have also been made.
The mapping from scancodes to the indexes used in xrdp_keymap
is not well designed and contains an implicit dependency on
keycode values.
This mapping is alse slightly different from the index used for
the 'keys' map in the xrdp_wm structure.
This commit introduces support for mapping scancodes directly
to 'scancode indexes' suitable for indexing into both structures.
Some renaming is also done; [MS-RDPBCGR] uses the terms scancode
and keyCode interchangeably. An effort is made to use key_code for a
raw value from a TS_KEYBOARD_EVENT, and scancode for a value which is
produced by the scancode module.
This commit changes the license response PDU to be constructed rather
than simply being contained as a binary blob.
Some constants in common/ms-rdpbcgr.h are renamed with the values
from the specification.
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.
- 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.
If ./configure is used with devel logging, but without --enable-pixman,
the stub pixman development files are used.
However, in this configuration, the pixman_region_selfcheck() function
is declared, but not defined.
This is a regression introduced in 7e58209b19
matt335672