This patch adds checks for the sound channel and plugins:
* checks for protocol bytes;
* checks for malloc/calloc;
* be more strict when interacting with sound plugin
1. Export fps related API so that subsystem implementation no longer need to know about details in encoder structure.
2. Discard frameList dictionary.
The 'value' in this dictionary is never used and not properly free'ed when client is disconnected.
The dictionary was used to calculate 'inflight' frame count. Once an ACK is received from client, an item in the dictionary is removed.
We then calculate 'inflight' frame by the count of the items in the dictionary.
However, some rdp clients (win7 mstsc) skips frame ACK if it is inactive, ACK of some frame would actually never arrive.
We actually don't need the dictionary. We only need to record the latest acknowledged frame id, and the difference between last sent frame id is the inflight frame count.
3. Minor fix in default fps calculation. encoder->frameAck is wrongly used as integer while it's actually bool flag.
This patch contains:
* checks for malloc return value + treat callers;
* modified malloc() + ZeroMemory() to calloc();
* misc fixes of micro errors seen during the code audit:
** some invalid checks in gcc.c, also there were some possible
integer overflow. This is interesting because at the end the data are parsed
and freed directly, so it's a vulnerability in some kind of dead code (at least
useless);
** fixed usage of GetComputerNameExA with just one call, when 2 were used
in misc places. According to MSDN GetComputerNameA() is supposed to return
an error when called with NULL;
** there were a bug in the command line parsing of shadow;
** in freerdp_dynamic_channel_collection_add() the size of array was multiplied
by 4 instead of 2 on resize
1. Introduce message queue in shadow client. No longer use subsytem->MsgPipe->out to deliver message to clients.
We used to use subsytem->MsgPipe->out for messages which need to be sent to client. But it's not correct. Only one client would get the message if multiple client exists
This problem make the fix in PR #2643 incomplete.
Introduced reference count based solution to release resource taken by the message.
Also added APIs for client message delivery.
Also fixed msg pipe in subsystem to clean resource when destroyed.
2. Discard unused StopEvent in client. We actually use quit message instead.
3. Enhance disposal of channels.
Free context for remdesk and encomsp channels. The original fix only stop the threads, but doesn't release resource.
Dispose channels earlier. The channels are built on client->vcm. Disposing channels after client->vcm is closed cause unknown behavior.
Original fix is #2644
4. Start to add audio support.
If no entry for the <host> <port> combination
was found in the v2 file and there is a legacy file
check if a matching <host> entry can be found.
In case there is a matching entry and the <fingerprint>
also matches, create a new entry in the v2 file using the
current port.
1)
Added missing checks for CreateEvent which also required the
following related changes:
- changed freerdp_context_new API to BOOL
- changed freerdp_peer_context_new API to BOOL
- changed pRdpClientNew callback to BOOL
- changed pContextNew callback to BOOL
- changed psPeerAccepted callback to BOOL
- changed psPeerContextNew callback to BOOL
2)
Fixed lots of missing alloc and error checks in the
changed code's neighbourhood.
3)
Check freerdp_client_codecs_prepare result to avoid segfaults
caused by using non-initialized codecs.
4)
Fixed deadlocks in x11 caused by missing xf_unlock_x11() calls
in some error handlers
5)
Some fixes in thread pool:
- DEFAULT_POOL assignment did not match TP_POOL definition
- don't free the pool pointer if it points to the static DEFAULT_POOL
- added error handling and cleanup in InitializeThreadpool
When a hdc is created no initial or default objects are created
therefore can the first call of gdi_SelectObject return NULL.
Because of this checking the return value of gdi_SelectObject failed
for newly create hdc causing errors (disconnects).
Since all types of HGDIOBJECT are handled and the return value of
gdi_SelectObject isn't used the recently added checks were removed
again.
rfx_process_message_sync:
- simplified the check if the header messages got processed
rfx_process_message_tileset:
- ObjectPool_Take result was not checked
- fail if TS_RFX_TILE block type is not CBT_TILE
- CreateThreadpoolWork result was not checked
- post decoding loop code segfaulted in error case
rfx_decoder_tile_new:
- missing malloc check
rfx_message_free:
- segfault protection
rfx_write_message_tileset:
- segfault protection
- removed some unneeded null checks for free()
- fixed a memory leak in shadow_client
- removed rfx_compose_message_header from API
Changed the following functions to BOOL, check the result
where they are called and handle failures:
- rfx_compose_message
- rfx_compose_message_header
- rfx_write_tile
- rfx_write_message_tileset
- rfx_write_message_frame_begin
- rfx_write_message_region
- rfx_write_message_frame_end
- rfx_write_message
rfx_process_message:
- check memory allocation failures
- verify protocol-conform order of data messages to prevents memory
leaks caused by repeated allocations
- verify that header messages were parsed/received before the
data messages
- treat unknown rlgr mode as error
- fixed/added error handling
- fixed all callers to check/handle result
rfx_encode_message:
- fixed incorrect usage of realloc
- missing malloc check
- missing check of CreateThreadpoolWork
- correct cleanup on failure (threadpool, memory)
- check rfx_encode_message result
rfx_encode_messages:
- check rfx_split_message result
- correct cleanup on failure
- prevent memory leak on failure
rfx_write_message_context:
- fixed invalid channelId value (must be 0xFF for WBT_CONTEXT)
rfx_process_message_codec_versions:
- fixed invalid read size of codec_version (it is 16bit)
rfx_process_message_channels:
- verify protocol conform channelId value
rfx_process_message_region:
- replaced invalid reallocs with malloc
- read and verify regionType and numTileSets from stream
rfx_process_message_tileset:
- check allocation results
- fixed incorrect usages of realloc
setupWorkers:
- fixed incorrect usages of realloc
rfx_split_message:
- removed dead code
- missing malloc check
rfx_compose_message:
- fixed a memory leak
- check/handle rfx_encode_message result
* top level GDI functions return 0 on error and != 0 otherwise but the
low level functions (16bpp.c, 8bpp.c 32bpp.c) which are called did it
exactly the other way around. Those were adapted.
* change gdi_InvalidateRegion to BOOL and check calls where appropriate
* integrate comments from pull request
tls_disconnect shut down the ssl stream but didn't inform
the BIO(s) about this therefore could happen that a second shut down
was initiated (e.g. in bio_rdp_tls_free) causing rather long delays.
After removing the shut down from tls_disconnect the only thing the
function does is to prepare/send an alert therefore it was renamed to
tls_send_alert.
Small cleanup of passing around decorations flag.
Limit PercentScreen to single monitor vs. entire desktop. IMO - this is better behavior in a multimonitor environment.
Handle fullscreen windows better:
1. Ensure that size hints are set to allow resizing before setting a window to fullscreen as some window managers do not behave properly.
2. Handle fullscreen toggles without destroying and recreating window.
3. Use NET_WM_STATE_FULLSCREEN Extended Window Manager Hint for fullscreen functionality
4. Use the NET_WM_FULLSCREEN_MONITORS Extended Window Manager Hint when appropriate
5. When a single monitor fullscreen is requested - use the current monitor(as determined from mouse location)
6. Handle cases where there is no local monitor at coordinate 0,0. The Windows server expect there to be a monitor at this location, so we maintain offset if necessary between our local primary monitor and the server side primary monitor located at 0,0.
Extend rdp_pointer with function SetPosition. Can then be used by
clients support setting pointer by server which might be used in
shadowing scenarios.
The Programmer Dvorak keyboard layout is supported by Xkb but support
in Windows is only available through an open-source add-on driver. It
is plausible that those that use this layout in X11 also installs this
driver on Windows instead of using the standard Dvorak variant there.
This changeset recognizes Programmer Dvorak as its own variant, and
assigns this a layout ID which matches the one used in the Windows
driver so that it will be selected when you logon. If this layout is
not available, it will now revert to the regular United States layout.
Tested with Ubuntu Precise 12.04 connecting to Windows 7 SP1.
The input->FocusInEvent callback implementations (normal and fast-path) have
always sent the mouse position even if the pointer was outside of the freerdp
client area. In addition xfreerdp used the wrong pointer coordinates which
were relative to the root window instead of its own.
On focus-in the pointer position must only be sent if the pointer is
currently within the program's client area. However, the clients had no way
to pass that information to input->FocusInEvent which required an API change.
- removed mouse pointer x, y parameters from input interface's FocusInEvent
- clients are responsible to call input->MouseEvent on focus-in if necessary
- fixed xfreerdp and wfreerdp accordingly
[MS-RDPBCGR] Section 5.3 describes the encryption level and method values for
standard RDP security.
Looking at the current usage of these values in the FreeRDP code gives me
reason to believe that there is a certain lack of understanding of how these
values should be handled.
The encryption level is only configured on the server side in the "Encryption
Level" setting found in the Remote Desktop Session Host Configuration RDP-Tcp
properties dialog and this value is never transferred from the client to the
server over the wire.
The possible options are "None", "Low", "Client Compatible", "High" and
"FIPS Compliant". The client receices this value in the Server Security Data
block (TS_UD_SC_SEC1), probably only for informational purposes and maybe to
give the client the possibility to verify if the server's decision for the
encryption method confirms to the server's encryption level.
The possible encryption methods are "NONE", "40BIT", "56BIT", "128BIT" and
"FIPS" and the RDP client advertises the ones it supports to the server in the
Client Security Data block (TS_UD_CS_SEC).
The server's configured encryption level value restricts the possible final
encryption method.
Something that I was not able to find in the documentation is the priority
level of the individual encryption methods based on which the server makes its
final method decision if there are several options.
My analysis with Windows Servers reveiled that the order is 128, 56, 40, FIPS.
The server only chooses FIPS if the level is "FIPS Comliant" or if it is the
only method advertised by the client.
Bottom line:
* FreeRDP's client side does not need to set settings->EncryptionLevel
(which was done quite frequently).
* FreeRDP's server side does not have to set the supported encryption methods
list in settings->EncryptionMethods
Changes in this commit:
Removed unnecessary/confusing changes of EncryptionLevel/Methods settings
Refactor settings->DisableEncryption
* This value actually means "Advanced RDP Encryption (NLA/TLS) is NOT used"
* The old name caused lots of confusion among developers
* Renamed it to "UseRdpSecurityLayer" (the compare logic stays untouched)
Any client's setting of settings->EncryptionMethods were annihilated
* All clients "want" to set all supported methods
* Some clients forgot 56bit because 56bit was not supported at the time the
code was written
* settings->EncryptionMethods was overwritten anyways in nego_connect()
* Removed all client side settings of settings->EncryptionMethods
The default is "None" (0)
* Changed nego_connect() to advertise all supported methods if
settings->EncryptionMethods is 0 (None)
* Added a commandline option /encryption-methods:comma separated list of the
values "40", "56", "128", "FIPS". E.g. /encryption-methods:56,128
* Print warning if server chooses non-advertised method
Verify received level and method in client's gcc_read_server_security_data
* Only accept valid/known encryption methods
* Verify encryption level/method combinations according to MS-RDPBCGR 5.3.2
Server implementations can now set settings->EncryptionLevel
* The default for settings->EncryptionLevel is 0 (None)
* nego_send_negotiation_response() changes it to ClientCompatible in that case
* default to ClientCompatible if the server implementation set an invalid level
Fix server's gcc_write_server_security_data
* Verify server encryption level value set by server implementations
* Choose rdp encryption method based on level and supported client methods
* Moved FIPS to the lowest priority (only used if other methods are possible)
Updated sample server
* Support RDP Security (RdpKeyFile was not set)
* Added commented sample code for setting the security level
The fOpRedundant field of the GlyphIndex primary drawing order
(MS-RDPEGDI, chapter 2.2.2.2.1.1.2.13) was neglected which resulted in some
severe text rendering errors.
With BOOL as return type it is not possible to differentiate between
success and "no data can be read" (when a channel read would block).
rdpsnd_server_handle_messages returns now int with the following
possible values:
-1 if no data could be read
0 error (like connection close) (formerly FALSE)
1 succsess (also if further bytes need to be read) (formerly TRUE)
Not using -1 for error cases was chosen to be compatible with the BOOL
return values used before.
MS-RDPEA 2.2.2.3 states (Appendix A <6>) that quality mode PDU
should be ignored if the client version is < 6. For these clients the sound
channel got never activated since activated was only called after
reception of the quality mode PDU.
For clients < version 6 activated is now called after
CLIENT_AUDIO_VERSION_AND_FORMATS was received.
Conflicts:
channels/drdynvc/client/dvcman.c
include/freerdp/codec/h264.h
libfreerdp/codec/h264.c after this merge h264 doesn't work anymore!!
libfreerdp/utils/svc_plugin.c
Fixes clang compiler warning:
"warning: the value of the size argument in 'strncat' is too large,
might lead to a buffer overflow [-Wstrncat-size]"
strncat requires an extra byte for '\0' so dest needs to have a size of
n+1
CLRCONV_INVERT and/or CLRCONV_ALPHA is set.
* include/freerdp/codec/color.h (RGB32_to_BGR32): New inline function.
* libfreerdp/codec/color.c:
(freerdp_image_convert_24bpp): Fix CLRCONV_ALPHA and CLRCONV_INVERT
processing for 32bpp destination.
Unroll the conversion loop to process four pixels in one go using
32-bit load and store operations.
(freerdp_color_convert_drawing_order_color_to_gdi_color): Declare new
function.
* libfreerdp/codec/color.c:
(freerdp_color_convert_drawing_order_color_to_gdi_color): Implement.
(freerdp_image_convert_8bpp): Properly use the ARGB32/ABGR32/RGB32/BGR32
macros when converting 8bpp data to 32bpp.
(freerdp_image_convert_32bpp): Fix CLRCONV_ALPHA and CLRCONV_INVERT
processing for 32bpp destination.
(freerdp_mono_image_convert): Use ARGB32/ABGR32 when converting to 32bpp
and CLRCONV_ALPHA is set.
* libfreerdp/core/orders.c: Color data from drawing orders is
interpreted in big endian mode.
* libfreerdp/core/update.c (update_read_palette): Likewise.
* libfreerdp/gdi/16bpp.c (gdi_get_color_16bpp): GDI colors are stored as
RGB now.
* libfreerdp/gdi/32bpp.c (gdi_get_color_32bpp): Likewise.
* libfreerdp/gdi/gdi.c:
Use freerdp_color_convert_drawing_order_color_to_gdi_color() to convert
from drawing order color representation to GDI color representation
troughout.
* libfreerdp/gdi/graphics.c (gdi_Glyph_BeginDraw): Likewise.
(gdi_Glyph_EndDraw): Likewise.
This adds 2 arguments:
/tls-ciphers List of permitted openssl ciphers - see ciphers(1)
/tls-ciphers-netmon Use tls ciphers that netmon can parse
With KB2919355, client/server negotiate the use of
TLS cipher TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
which works fine except that netmon can't parse it.
By adding commandline /tls-ciphers-netmon, we restrict
the available ciphers to a list that netmon can
deal with. Also adds /tls-ciphers, which
accepts a string arg, for further customization.
# By Marc-André Moreau (20) and others
# Via Mike McDonald (6) and others
* 'master' of git://github.com/awakecoding/FreeRDP: (26 commits)
libfreerdp-codec: fix C++ headers
libfreerdp-codec: fix ClearCodec short vbar cache hit
libfreerdp-codec: improve ClearCodec error checking
libfreerdp-codec: fix ClearCodec RLEX decoding
libfreerdp-codec: ClearCodec fix error codes and wrapping around of cursors
libfreerdp-codec: fix some ClearCodec flag checking
Fixed issue with last merge.
Added #ifdef WITH_OPENH264 ... #endif to appropriate places in the code.
libfreerdp-codec: handle long vbar length mismatch
channels/rdpgfx: add egfx command line options and settings
libfreerdp-codec: reduce number of variables
libfreerdp-codec: improve ClearCodec robustness
libfreerdp-codec: simplify ClearCodec code
Initial implementation of H.264 decoder for MS-RDPEGFX
libfreerdp-codec: improve ClearCodec subcodec xStart, yStart handling
libfreerdp-codec: improve ClearCodec subcodec support
libfreerdp-codec: improve ClearCodec error checking
libfreerdp-codec: more ClearCodec vBar caching
channels/rdpgfx: harden parsing code
libfreerdp-codec: add ClearCodec glyph cache
...
This big patch allows to have non-blocking writes. To achieve
this, it slightly changes the way transport is handled. The misc transport
layers are handled with OpenSSL BIOs. In the chain we insert a
bufferedBIO that will bufferize write calls that couldn't be honored.
For an access with Tls security the BIO chain would look like this:
FreeRdp Code ===> SSL bio ===> buffered BIO ===> socket BIO
The buffered BIO will store bytes that couldn't be send because of
blocking write calls.
This patch also rework TSG so that it would look like this in the
case of SSL security with TSG:
(TSG in)
> SSL BIO => buffered BIO ==> socket BIO
/
FreeRdp => SSL BIO => TSG BIO
\
> SSL BIO => buffered BIO ==> socket BIO
(TSG out)
So from the FreeRDP point of view sending something is only BIO_writing
on the frontBio (last BIO on the left).
This adds a ringbuffer implementation that targets bytes sending.
The ringbuffer can grow when there's not enough room, that's why it's
not thread-safe (locking must be done externally). It will be shrinked
to its initial size as soon as the used bytes are the half of the
initial size.
This patch changes the prototype for decode_base64 so that the encode / decode
method are consistant (encode(BYTE *) => char* and decode(char*) => BYTE*).
It also does some improvements with unrolling loops so that end conditions are
tested only at the end.
The patch also adds some unitary tests.
Before the patch base64_decode() made valgrind complain about uninitialized
bits, after valgrind is happy and very quiet.
