Add VeNCrypt plain and vncauth methods to LibVNCClient

[mdevaev]

Is your feature request related to a problem? Please describe. bVNC based on libvncclient cannot connect to the Pi-KVM server without encryption if vencrypt is enabled:

• The vencrypt/plain method (without x509/tls) is not implemented in the library. I assume for security reasons, however this may be acceptable in trusted local networks.
• vencrypt/vncauth (i.e. method 2) is also not working. This is not part of the vencrypt specification, however vencrypt is quite vague about it, so now there is a situation where some clients handle this, and some don't. For example TigerVNC works, Vinagre does not work. I believe that the client should handle this situation to ensure maximum compatibility with any servers. Vencrypt contains tlsvnc, logically the same should be supported without encryption.

Describe the solution you'd like

• Add support for Vencrypt/Plain and Vencrypt/Vnc.

Additional context The problem was detected when the bVNC client and the Pi-KVM VNC server interact without encryption.

[bk138]

I assume you're referring to LibVNCClient? This actually has VeNCrypt support, but maybe it's incomplete in some aspects? Are you able to spec out what exactly is missing?

[mdevaev]

@bk138 Two problems:

• rfbVeNCryptPlain/256 (user+pass without encryption) is defined but not supported.
• There is rfbVeNCryptTLSVNC/258 (vnc passkey with encryption) but there is no something like rfbVeNCryptVNC/2 (vnc passkey without encryption).

The last authorization method is not described in the specification. But I suggest implementing subtype 2 as a fallback in case the client and server can't agree on a connection. The fact is that most clients give priority to vencrypt when connecting, including libvncclient. However, if the server does not offer any supported authorization methods inside vencrypt, the connection will be denied.

An example: the server only supports vncauth and vencrypt+plain. The client only supports vncauth and vencrypt+tls. The client requests the use of vencrypt, but cannot process vencrypt+plain. The protocol does not imply a downgrade and the only chance to pass authorization is to offer vencrypt+vncauth. My server does this, the tigervnc client and (it seems) tightvnc behave exactly the same. This is a non-standard extension that improves compatibility.

[bk138]

I've updated the issue title to reflect the actual meaning of the request. Please check @mdevaev. Regarding the implementation work, I'm currently busy with other duties so this very probably won't be done by me. I am, however, able to spend time on reviewing and eventually merging contributions.

[mdevaev]

Thanks. @iiordanov what do you think?

[iiordanov]

Looks good except I am not sure whether (vencrypt/vncauth) is worth implementing given it's not in the VeNCrypt spec. What does it give us over (vencrypt/plain)?

[mdevaev]

This is useful for the client to improve compatibility. If the server is claimed by vencrypt, then the client does not know in advance whether they will agree on authorization or not. If it turns out that plain encryption is not supported, then there is no way to downgrade from vencrypt to the usual vncauth mechanism. If vncauth is supported inside vencrypt, it will allow you to pass authorization and establish a connection. Some clients and servers support this, which is a more universal approach.

[iiordanov]

Makes sense since there is no way to downgrade.

On Sun, Jan 17, 2021 at 8:33 PM Maxim Devaev notifications@github.com wrote:

This is useful for the client to improve compatibility. If the server is claimed by vencrypt, then the client does not know in advance whether they will agree on authorization or not. If it turns out that plain encryption is not supported, then there is no way to downgrade from vencrypt to the usual vncauth mechanism. If vncauth is supported inside vencrypt, it will allow you to pass authorization and establish a connection. Some clients and servers support this, which is a more universal approach.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LibVNC/libvncserver/issues/458#issuecomment-761925148, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAK3EV3TI3YCSNZPDTVWIFLS2OFV3ANCNFSM4WFUYMIQ .

-- The conscious mind has only one thread of execution.

[mdevaev]

It seems support for both of these methods should be simple. I looked in the code, there is a place where just need to add new sub-type numbers. And somewhere further along the code, needs to disable tls/ssl wrapping for the socket.

https://github.com/LibVNC/libvncserver/blob/master/libvncclient/rfbproto.c#L1053

@iiordanov could you try? I could probably try to make a patch, but I don't have a client where it can be embedded.

[mdevaev]

@bk138 Implemented it in #529, could you take a look?

[bk138]

@bk138 Implemented it in #529, could you take a look?

Yes, thanks! Am on holiday right now, so expect ~ 2 weeks.

[mdevaev]

@bk138 I wish you a good rest :)