xpra icon
Bug tracker and wiki

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#614 closed enhancement (fixed)

make it possible to enable and disable all input parsers individually (lz4, codecs, etc)

Reported by: Antoine Martin Owned by: J. Max Mena
Priority: blocker Milestone: 0.14
Component: core Version: 0.12.x
Keywords: Cc:

Description

Following the recent problems with lz4 (in particular Hacking CERN - Exploiting python-lz4 for Particles and Profit, and Raising Lazarus - The 20 Year Old Bug that Went to Mars: Linux Kernel, ffmpeg..).
There have been numerous vulnerabilities in ffmpeg over the years, and even the venerable zlib is not immune to bugs. And zlib is also used in PNG...

It makes sense to assume that every non-trivial input parser is going to have issues.

This is more problematic for platforms like win32 and osx, for which we are forced to ship a large number of libraries ourselves because their respective OS vendor provides very little: this means we also become responsible for updating the installers every time a new flaw is discovered. It also means that the more security conscious users cannot pre-emptively disable this code.

The solution is to provide options to allow as many of those parsers to be switched on or off via the command line (or configuration file) - which is much easier and faster than installing newer versions of the software.

The priority should be for the parsers that can most easily be abused: the network layer must parse data before the connection is authenticated.

List of new switches required:

  • lz4 and zlib (at least one should be enabled)
  • bencode and rencode (at least one must be enabled)
  • crypto layer
  • video decoders (we already have switches for encoders and csc, see: --video-encoders= and --csc-modules=)
  • picture encoders and decoders: to deal with non-video codecs (webp, PIL, ..)

Change History (12)

comment:1 Changed 3 years ago by Antoine Martin

  • preparatory work (refactoring and cleanups) in: r6963 + r6953, r6952, r6951, r6950, r6949
  • r6934 adds yaml packet encoding support
  • r6964 adds bz2 compression support
  • r6965 adds the ability to select which "compressors" and "packet-encoders" are enabled
Last edited 3 years ago by Antoine Martin (previous) (diff)

comment:2 Changed 3 years ago by Antoine Martin

  • r6966 does the same for encodings

Remaining tasks:

  • crypto: (not that big a deal) should get done as part of #198 / #584 (ticket updated)
  • dealing with errors when the server does not support our compression or encoding (xpra info fails silently atm)

comment:3 Changed 3 years ago by Antoine Martin

Dealing with errors much improved in r6969.

comment:4 Changed 3 years ago by Antoine Martin

Owner: changed from Antoine Martin to alas

Notes:

  • we now enable lz4 with all compression levels, not just with 1 as per previous versions)
  • trunk versions from r6964 to r6986 used bz2, which is now replaced by lzo (so don't mix, client and servers using those versions)


This is as much for your own information as it is for testing:

  • testing that the server will not use a compressor not listed (defaults to 'all'):
    xpra start :10 --compressors=lz4,lzo
    xpra attach :10 --compressors=zlib
    

the client should fail with (the same message should appear in the server log file and include the client's connecting TCP address or socket):

Failed to connect: 'disconnect: invalid compression: zlib is not enabled'
  • testing that the special value 'all' works from the command line and allows the client to use any compressor option, and that the compressor chosen by the client is the one used. Check using xpra info or session-info's "Connection" tab
  • if more than one compressor is specified with lz4, lz4 should be used
  • testing that '--compressors=none' forces the client to not use compression (either -z=0 or also using '--compressors=none')
  • your must specify at least one value with --packet-encoders= (one of bencode, rencode, yaml - your build must have them all to test)
  • same as above: "all", verify that we cannot use one that is not enabled, etc..
  • same for encodings, ie: --encodings=png,rgb,webp, etc..
  • for encodings, we should not be able to select an encoding --encoding= which is not in the list of available encodings --encodings=x,y,z
  • important: checking that all of these options can be set using the xpra.conf file too
Last edited 3 years ago by Antoine Martin (previous) (diff)

comment:5 Changed 3 years ago by Antoine Martin

Priority: majorblocker

(raising: blocker for release)

comment:6 Changed 3 years ago by J. Max Mena

Testing Win7 r7041 against Fedora 20 r7041 server:

  • Connecting to a server with --compressors=lz4,lzo and a client with --compressors=zlib throws:

packet no 0 data: 'disconnect: invalid compression: zlib is not enabled\n' - small typo

  • Confirmed in Session Info that lz4 is used by the client with ---compressors=all on both the client and the server.
  • --compressors=none causes the client to fail to connect unless the client selects --compressors=none as an argument. I get the same

packet no 0 data: 'disconnect: invalid compression: zlib is not enabled\n' if I don't force the client to use --compressors=none.

  • Starting the server with --encodings=png,webp and the client with --encodings=h264 will allow the client to connect anyway, but with h264,jpeg,png,rgb,rgb32 encodings listed for the client in Session Info, and the server has png,webp. I'm pretty sure that's not expected behavior.
  • Changing the xpra.conf on the client to only rgb24 connects anyway as well, with the same encodings listed, with rgb24 as well so the list becomes jpeg,png,rgb,rgb24,rgb32.
    • Changing the server xpra.conf to only png and the client xpra.conf to only rgb24 allows the client to connect anyway with jpeg,png,rgb,rgb24 showing up in Xpra Info

It currently looks like the Win-Client is partially ignoring what encodings it is allowed to use.

comment:7 Changed 3 years ago by Antoine Martin

Owner: changed from alas to J. Max Mena

"small typo"


I assume you mean the "\n"? The plain-text response detection should be fixed in r7078 so the output now shows up as:
Failed to connect: 'disconnect: invalid packet encoding: yaml is disabled'


(..) I'm pretty sure that's not expected behavior.


Right you are, this is fixed in r7081.


Changing the xpra.conf..


Probably the same bugs fixed above.


I've also improved the error messages that come out.

Can you try again please?

Last edited 3 years ago by Antoine Martin (previous) (diff)

comment:8 Changed 3 years ago by J. Max Mena

Retested with newer Windows r7088 against Fedora 20 r7041(our current builds don't have x264 for some odd reason):

  • Windows now listens to what encodings you tell it to use.
  • However connecting a Windows client with an encoding not listed by the server (for example, the server has --encodings=png,webp and the client tries to connect with --encodings=rgb24), the Windows client disconnects with the following message:
    2014-08-01 16:00:45,634 server failure: disconnected before the session could be
     established
    2014-08-01 16:00:45,635 server requested disconnect: server error accepting new
    connection
    2014-08-01 16:00:45,888 Connection lost
    


with the same print on the server side. Is this expected behavior?

Other than that it seems to be behaving nicely.

comment:9 Changed 3 years ago by Antoine Martin

Resolution: fixed
Status: newclosed

server failure: disconnected before the session could be established


That's what older servers say (you were on r7041), newer ones provide more informative messages.

Closing.

Last edited 3 years ago by Antoine Martin (previous) (diff)

comment:10 Changed 3 years ago by Antoine Martin

Note: this was not tested fully for packet encoders, see ticket:473#comment:22

And doubly so:

Last edited 3 years ago by Antoine Martin (previous) (diff)

comment:11 Changed 3 years ago by Antoine Martin

And one more: r7689.

comment:12 Changed 3 years ago by Antoine Martin

As per https://winswitch.org/trac/ticket/266, we have a problem where older versions of xpra (before v0.11.x it seems) do not send zlib=1 and so we end up not using compression at all (or worse before r7689: crashing the connection!).
That is fixed in r7691. (this change is for v0.14.x only as trunk is not compatible with versions older than 0.12 anyway)

Note: See TracTickets for help on using tickets.