Channel security #234

jadahl · 2018-06-12T08:11:59Z

Hi,

In GNOME Remote Desktop I'm using LibVNCServer to add VNC support. Currently I see no way to add my own encryption layer (I currently want to add TLS-ANON to begin with), so I have come with a couple of API additions allowing me to add it.

Add ways to affect how LibVNCServer reads/writes/peeks/... on the socket

This allows me to replace read with the TLS receive function etc, in a similar way as of how WebSockets TLS have been implemented. What I currently do is to first use the default socket I/O functions, then after a secure channel is set up, I override the I/O functions with ones that use the TLS library.

Add channel security handlers

A "channel" security handler here is simply a security handler that LibVNCServer already has, but that is advertised prior to any other security handler. This allows me to require using rfbTLS before continuing with VNC-AUTH etc. It works more or less by first advertising only channel security handlers (e.g. could be only rfbTLS but any other that sets up a secure channel). When that is done, the channel security handler will send the security handler list excluding the channel security handlers.

With these two additions, I can get a flow looking like this:

Server advertises channel security handlers: [TLS]
Client chooses TLS
Server and client performs TLS handshake
Upon success, the server replaces the I/O functions with ones that uses TLS library
Server advertises security handlers: eg [AUTH]
Client chooses AUTH
Session continues as normal using a TLS encrypted channel

bk138 · 2018-06-13T08:17:19Z

Thanks! I'll try to review on Friday. In the meantime, can you have a look at the CI build failure for Windows?

jadahl · 2018-06-13T08:28:27Z

@bk138 Thanks! The error is

%DEVENV_EXE% db_dll.dsp /upgrade
Command exited with code 1

which I don't really understand the meaning of. Does it have anything to do with API stability?

What is unclear to me is how API stability is dealt with. Both patches changes the rfb.h requiring recompilation of users (new vfuncs, new state enum value). It could be avoided by adding the new struct fields/state enums in the end I suppose, but as far as I could tell (and I might very well be wrong), things have been added without such a concern in the past, and compilation flags might change the struct field placement as well.

bk138 · 2018-06-16T16:54:24Z

That's a pretty big change that needs a while to digest ;-) Before getting into the details, I'd like to understand a bit more the bigger picture:

So this kinda splits up the usual RFB session setup into a twofold process where one first sets up "channel" security and then afterwards ordinary RFB security type?

Isn' that very similar to what https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#tight-security-type already does?

I've the feeling this would'nt work with standard VNC clients as there's a two stage sectype setup?

I might be wrong with above assumptions, TBH, I've to dive into the proto spec again to better understand what you're trying to achieve.

jadahl · 2018-06-18T08:45:13Z

So this kinda splits up the usual RFB session setup into a twofold process where one first sets up "channel" security and then afterwards ordinary RFB security type?

Right, first set up the channel, then if that is successful (client supports a security channel negotiation type), continue with the authorization. This is how Vino adds TLS support, and it is this what I'm trying to reimplement without using Vino's LibVNCServer fork.

Isn' that very similar to what https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#tight-security-type already does?

Seems somewhat similar indeed, and would be good to have. What I'm initially targeting, however, is to support what ever security mechanism that Vino supports. I have relied on reading source code for this though, as I've not find any documentation of how the TLS security type (18) should work.

I've the feeling this would'nt work with standard VNC clients as there's a two stage sectype setup?

My assumption is that any client that worked with Vino with TLS encryption enabled (which is the default) should support this. I've so far successfully tested with Vinagre and Remmina.

bk138 · 2018-06-26T19:05:33Z

Thanks for the explanation. This is just to let you know I haven't forgotten about this, I need some time for thinking and digging up info on sectype 18.

bk138 · 2018-06-27T08:02:35Z

@jadahl can you explain what you're aiming for on a user perspective level? I reckon it's

an encrypted VNC connection (auth and all other RFB payloads)
with a known set of clients, in this case vinagre and remmina
?

jadahl · 2018-06-27T09:14:38Z

Yes, or, all clients that worked when using encryption in Vino. I don't know what other clients supports security type 18.

bk138 · 2018-06-29T10:56:21Z

FYI I started a bit of work on unifying encryption and crypto between LibVNCServer and LibVNCClient. As Remmina is using LibVNCClient (dunno about vinagre?), I'll try to model basic server encryption based on the client counterpart that already exists. The code in your PR here will be quite helpful I guess.

jadahl · 2019-04-15T06:55:37Z

@bk138 any update on this? Any way I can implement anon-tls and other channel security using e.g. rfbTLS? (to keep backward compatibility with the clients currently supported).

jadahl · 2019-11-27T09:59:36Z

Fixed a couple of bugs in this PR. Any update on getting it merged, or anything that would make it possible to implement the same level of channel security out-of-tree?

jadahl · 2019-11-27T10:20:50Z

Replying to an earlier question regarding client support: Vinagre and GNOME Boxes both use gtk-vnc as a client side library.

bk138 · 2019-11-27T11:41:26Z

Thank you, @jadahl ! Currently, simply lacking time as day job consumes too much. Hopefully over the holidays...

rdieter · 2020-07-01T15:47:05Z

Can this be rebased to resolve conflicts please?

Add API to make it possible to channel RFB input and output through another layer, for example TLS. This is done by making it possible to override the default read/write/peek functions.

Add another type of security handler that is meant to be used initially to set up a secure channel. Regular security handlers would be advertised and processed after any channel security have succeeded. For example, this, together with the custom I/O functions allows a LibVNCServer user to implement TLS in combination with VNCAuth. This is done by adding a single channel security handler with the rfbTLS (18) with a handler that initiates a TLS session, and when a TLS session is initiated, the regular security handler list is sent.

jadahl · 2020-07-01T15:51:17Z

Can this be rebased to resolve conflicts please?

Rebased (but untested).

bk138 · 2020-07-01T16:09:03Z

I still have this on my TODO list, thanks @jadahl !

Neustradamus · 2020-10-06T02:15:19Z

Any news about this PR?

fcrozat · 2021-01-08T15:22:23Z

libvncserver/sockets.c

+int
+rfbPeekAtSocket(rfbClientPtr cl, char *buf, int len)
+{
+    cl->peekAtSocket(cl, buf, len);


this is causing compilation error when buiding with -Werror=return-type

I assume it should be return cl->peekAtSocket(cl, buf, len);

efimbushmanov · 2021-04-22T21:34:02Z

Seems ok?

tobydox · 2021-07-15T06:06:38Z

rfb/rfb.h

+
+    ClientReadFromSocket readFromSocket;         /* Read data from socket */
+    ClientPeekAtSocket peekAtSocket;             /* Peek at data from socket */
+    ClientHasPendingOnSocket hasPendingOnSocket; /* Peek at data from socket */


There's a minor copy&paste error in the comment.

tobydox · 2021-07-15T06:07:53Z

libvncserver/auth.c


    if (cl->protocolMajorVersion==3 && cl->protocolMinorVersion < 7)
    {
 	/* Make sure we use only RFB 3.3 compatible security types. */
+	if (channelSecurityHandlers) {
+	    rfbLog("VNC channel security enabled - RFB 3.3 client rejected\n");
+	    rfbClientConnFailed(cl, "Your viewer cannot hnadler required "


Typo: hnadler -> handle

bk138 force-pushed the master branch from 57a2948 to 2935d1f Compare May 9, 2019 20:43

bk138 mentioned this pull request Jun 30, 2019

Dynamic security handlers #313

Open

jadahl force-pushed the wip/channel-security branch from 79738f4 to 5c25f49 Compare November 27, 2019 09:58

bk138 mentioned this pull request Apr 11, 2020

Add TLS encryption to LibVNCServer #396

Open

3 tasks

jadahl added 2 commits July 1, 2020 17:50

libvncserver: Add API to add custom I/O entry points

450f4a5

Add API to make it possible to channel RFB input and output through another layer, for example TLS. This is done by making it possible to override the default read/write/peek functions.

jadahl force-pushed the wip/channel-security branch from 5c25f49 to 30b947d Compare July 1, 2020 15:50

fcrozat reviewed Jan 8, 2021

View reviewed changes

efimbushmanov approved these changes Apr 22, 2021

View reviewed changes

bk138 self-assigned this Jul 12, 2021

bk138 added this to the Release 1.0.0 milestone Jul 12, 2021

tobydox reviewed Jul 15, 2021

View reviewed changes

bk138 mentioned this pull request Jul 19, 2021

libvncclient: add hooks for custom socket I/O #477

Open

bk138 removed their assignment Aug 5, 2023

sergiomb2 mentioned this pull request Nov 15, 2023

caught XIO error: happens much more often when xdamage option is enabled LibVNC/x11vnc#154

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Channel security #234

Channel security #234

jadahl commented Jun 12, 2018

bk138 commented Jun 13, 2018

jadahl commented Jun 13, 2018

bk138 commented Jun 16, 2018

jadahl commented Jun 18, 2018

bk138 commented Jun 26, 2018

bk138 commented Jun 27, 2018

jadahl commented Jun 27, 2018

bk138 commented Jun 29, 2018

jadahl commented Apr 15, 2019

jadahl commented Nov 27, 2019

jadahl commented Nov 27, 2019

bk138 commented Nov 27, 2019

rdieter commented Jul 1, 2020

jadahl commented Jul 1, 2020

bk138 commented Jul 1, 2020

Neustradamus commented Oct 6, 2020

fcrozat Jan 8, 2021

efimbushmanov commented Apr 22, 2021

tobydox Jul 15, 2021

tobydox Jul 15, 2021

Channel security #234

Are you sure you want to change the base?

Channel security #234

Conversation

jadahl commented Jun 12, 2018

bk138 commented Jun 13, 2018

jadahl commented Jun 13, 2018

bk138 commented Jun 16, 2018

jadahl commented Jun 18, 2018

bk138 commented Jun 26, 2018

bk138 commented Jun 27, 2018

jadahl commented Jun 27, 2018

bk138 commented Jun 29, 2018

jadahl commented Apr 15, 2019

jadahl commented Nov 27, 2019

jadahl commented Nov 27, 2019

bk138 commented Nov 27, 2019

rdieter commented Jul 1, 2020

jadahl commented Jul 1, 2020

bk138 commented Jul 1, 2020

Neustradamus commented Oct 6, 2020

fcrozat Jan 8, 2021

Choose a reason for hiding this comment

efimbushmanov commented Apr 22, 2021

tobydox Jul 15, 2021

Choose a reason for hiding this comment

tobydox Jul 15, 2021

Choose a reason for hiding this comment