Skip to content

Commit 1a41d2a

Browse files
mdevaevmdevaev
authored
libvncclient: allow non-encrypted subauth methods inside VeNCrypt (#529)
Following #458 rfbNoAuth and rfbVncAuth are not actually part of VeNCrypt, however it is important to support them to ensure better compatibility. When establishing a connection, the client does not know whether the server supports encryption, and always prefers VeNCrypt if enabled Next, if encryption is not available on the server, the connection will fail. Since the RFB doesn't have any downgrade methods in case of failure, a client that does not support unencrypted VeNCrypt methods will never be able to connect. rfbVeNCryptPlain is also supported for better compatibility. The RFB specification also considers any ordinary subauths are valid, which legitimizes this solution: - https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#vencrypt > any of the normal VNC security types (except VeNCrypt) may be sent For security and backward compatibility reasons, encrypted connections take precedence over unencrypted ones
1 parent 088e876 commit 1a41d2a

3 files changed

Lines changed: 96 additions & 34 deletions

File tree

‎libvncclient/rfbproto.c‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,18 +1127,36 @@ InitialiseRFBConnection(rfbClient* client)
11271127
if (!HandleVeNCryptAuth(client)) return FALSE;
11281128

11291129
switch (client->subAuthScheme) {
1130+
/*
1131+
* rfbNoAuth and rfbVncAuth are not actually part of VeNCrypt, however
1132+
* it is important to support them to ensure better compatibility.
1133+
* When establishing a connection, the client does not know whether
1134+
* the server supports encryption, and always prefers VeNCrypt if enabled.
1135+
* Next, if encryption is not available on the server, the connection
1136+
* will fail. Since the RFB doesn't have any downgrade methods in case
1137+
* of failure, a client that does not support unencrypted VeNCrypt methods
1138+
* will never be able to connect.
1139+
*
1140+
* The RFB specification also considers any ordinary subauths are valid,
1141+
* which legitimizes this solution.
1142+
*
1143+
* rfbVeNCryptPlain is also supported for better compatibility.
1144+
*/
11301145

1146+
case rfbNoAuth:
11311147
case rfbVeNCryptTLSNone:
11321148
case rfbVeNCryptX509None:
11331149
rfbClientLog("No sub authentication needed\n");
11341150
if (!rfbHandleAuthResult(client)) return FALSE;
11351151
break;
11361152

1153+
case rfbVncAuth:
11371154
case rfbVeNCryptTLSVNC:
11381155
case rfbVeNCryptX509VNC:
11391156
if (!HandleVncAuth(client)) return FALSE;
11401157
break;
11411158

1159+
case rfbVeNCryptPlain:
11421160
case rfbVeNCryptTLSPlain:
11431161
case rfbVeNCryptX509Plain:
11441162
if (!HandlePlainAuth(client)) return FALSE;

‎libvncclient/tls_gnutls.c‎

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -314,10 +314,9 @@ ReadVeNCryptSecurityType(rfbClient* client, uint32_t *result)
314314
{
315315
uint8_t count=0;
316316
uint8_t loop=0;
317-
uint8_t flag=0;
318317
uint32_t tAuth[256], t;
319318
char buf1[500],buf2[10];
320-
uint32_t authScheme;
319+
uint32_t origAuthScheme, authScheme;
321320

322321
if (!ReadFromRFBServer(client, (char *)&count, 1)) return FALSE;
323322

@@ -335,8 +334,10 @@ ReadVeNCryptSecurityType(rfbClient* client, uint32_t *result)
335334
if (!ReadFromRFBServer(client, (char *)&tAuth[loop], 4)) return FALSE;
336335
t=rfbClientSwap32IfLE(tAuth[loop]);
337336
rfbClientLog("%d) Received security type %d\n", loop, t);
338-
if (flag) continue;
339-
if (t==rfbVeNCryptTLSNone ||
337+
if (t==rfbNoAuth ||
338+
t==rfbVncAuth ||
339+
t==rfbVeNCryptPlain ||
340+
t==rfbVeNCryptTLSNone ||
340341
t==rfbVeNCryptTLSVNC ||
341342
t==rfbVeNCryptTLSPlain ||
342343
#ifdef LIBVNCSERVER_HAVE_SASL
@@ -347,11 +348,16 @@ ReadVeNCryptSecurityType(rfbClient* client, uint32_t *result)
347348
t==rfbVeNCryptX509VNC ||
348349
t==rfbVeNCryptX509Plain)
349350
{
350-
flag++;
351-
authScheme=t;
352-
rfbClientLog("Selecting security type %d (%d/%d in the list)\n", authScheme, loop, count);
353-
/* send back 4 bytes (in original byte order!) indicating which security type to use */
354-
if (!WriteToRFBServer(client, (char *)&tAuth[loop], 4)) return FALSE;
351+
if (
352+
authScheme==0 ||
353+
authScheme==rfbNoAuth ||
354+
authScheme==rfbVncAuth ||
355+
authScheme==rfbVeNCryptPlain)
356+
{
357+
/* for security reasons, the encrypted type has a higher priority */
358+
origAuthScheme=tAuth[loop];
359+
authScheme=t;
360+
}
355361
}
356362
tAuth[loop]=t;
357363
}
@@ -368,6 +374,12 @@ ReadVeNCryptSecurityType(rfbClient* client, uint32_t *result)
368374
buf1);
369375
return FALSE;
370376
}
377+
else
378+
{
379+
rfbClientLog("Selecting security type %d\n", authScheme);
380+
/* send back 4 bytes (in original byte order!) indicating which security type to use */
381+
if (!WriteToRFBServer(client, (char *)&origAuthScheme, 4)) return FALSE;
382+
}
371383
*result = authScheme;
372384
return TRUE;
373385
}
@@ -459,8 +471,6 @@ HandleVeNCryptAuth(rfbClient* client)
459471
gnutls_certificate_credentials_t x509_cred = NULL;
460472
int ret;
461473

462-
if (!InitializeTLS()) return FALSE;
463-
464474
/* Read VeNCrypt version */
465475
if (!ReadFromRFBServer(client, (char *)&major, 1) ||
466476
!ReadFromRFBServer(client, (char *)&minor, 1))
@@ -489,16 +499,18 @@ HandleVeNCryptAuth(rfbClient* client)
489499
}
490500

491501
if (!ReadVeNCryptSecurityType(client, &authScheme)) return FALSE;
492-
if (!ReadFromRFBServer(client, (char *)&status, 1) || status != 1)
493-
{
494-
rfbClientLog("Server refused VeNCrypt authentication %d (%d).\n", authScheme, (int)status);
495-
return FALSE;
496-
}
497502
client->subAuthScheme = authScheme;
498503

499-
/* Some VeNCrypt security types are anonymous TLS, others are X509 */
500504
switch (authScheme)
501505
{
506+
/* Unencrypted types do not require additional actions */
507+
case rfbNoAuth:
508+
case rfbVncAuth:
509+
case rfbVeNCryptPlain:
510+
return TRUE;
511+
break;
512+
513+
/* Some VeNCrypt security types are anonymous TLS, others are X509 */
502514
case rfbVeNCryptTLSNone:
503515
case rfbVeNCryptTLSVNC:
504516
case rfbVeNCryptTLSPlain:
@@ -507,11 +519,21 @@ HandleVeNCryptAuth(rfbClient* client)
507519
#endif /* LIBVNCSERVER_HAVE_SASL */
508520
anonTLS = TRUE;
509521
break;
522+
510523
default:
511524
anonTLS = FALSE;
512525
break;
513526
}
514527

528+
/* Ack is only requred for the encrypted connection */
529+
if (!ReadFromRFBServer(client, (char *)&status, 1) || status != 1)
530+
{
531+
rfbClientLog("Server refused VeNCrypt authentication %d (%d).\n", authScheme, (int)status);
532+
return FALSE;
533+
}
534+
535+
if (!InitializeTLS()) return FALSE;
536+
515537
/* Get X509 Credentials if it's not anonymous */
516538
if (!anonTLS)
517539
{

‎libvncclient/tls_openssl.c‎

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -443,10 +443,9 @@ ReadVeNCryptSecurityType(rfbClient* client, uint32_t *result)
443443
{
444444
uint8_t count=0;
445445
uint8_t loop=0;
446-
uint8_t flag=0;
447446
uint32_t tAuth[256], t;
448447
char buf1[500],buf2[10];
449-
uint32_t authScheme;
448+
uint32_t origAuthScheme, authScheme;
450449

451450
if (!ReadFromRFBServer(client, (char *)&count, 1)) return FALSE;
452451

@@ -464,8 +463,10 @@ ReadVeNCryptSecurityType(rfbClient* client, uint32_t *result)
464463
if (!ReadFromRFBServer(client, (char *)&tAuth[loop], 4)) return FALSE;
465464
t=rfbClientSwap32IfLE(tAuth[loop]);
466465
rfbClientLog("%d) Received security type %d\n", loop, t);
467-
if (flag) continue;
468-
if (t==rfbVeNCryptTLSNone ||
466+
if (t==rfbNoAuth ||
467+
t==rfbVncAuth ||
468+
t==rfbVeNCryptPlain ||
469+
t==rfbVeNCryptTLSNone ||
469470
t==rfbVeNCryptTLSVNC ||
470471
t==rfbVeNCryptTLSPlain ||
471472
#ifdef LIBVNCSERVER_HAVE_SASL
@@ -476,11 +477,16 @@ ReadVeNCryptSecurityType(rfbClient* client, uint32_t *result)
476477
t==rfbVeNCryptX509VNC ||
477478
t==rfbVeNCryptX509Plain)
478479
{
479-
flag++;
480-
authScheme=t;
481-
rfbClientLog("Selecting security type %d (%d/%d in the list)\n", authScheme, loop, count);
482-
/* send back 4 bytes (in original byte order!) indicating which security type to use */
483-
if (!WriteToRFBServer(client, (char *)&tAuth[loop], 4)) return FALSE;
480+
if (
481+
authScheme==0 ||
482+
authScheme==rfbNoAuth ||
483+
authScheme==rfbVncAuth ||
484+
authScheme==rfbVeNCryptPlain)
485+
{
486+
/* for security reasons, the encrypted type has a higher priority */
487+
origAuthScheme=tAuth[loop];
488+
authScheme=t;
489+
}
484490
}
485491
tAuth[loop]=t;
486492
}
@@ -497,6 +503,12 @@ ReadVeNCryptSecurityType(rfbClient* client, uint32_t *result)
497503
buf1);
498504
return FALSE;
499505
}
506+
else
507+
{
508+
rfbClientLog("Selecting security type %d\n", authScheme);
509+
/* send back 4 bytes (in original byte order!) indicating which security type to use */
510+
if (!WriteToRFBServer(client, (char *)&origAuthScheme, 4)) return FALSE;
511+
}
500512
*result = authScheme;
501513
return TRUE;
502514
}
@@ -530,8 +542,6 @@ HandleVeNCryptAuth(rfbClient* client)
530542
rfbCredential *cred = NULL;
531543
rfbBool result = TRUE;
532544

533-
if (!InitializeTLS()) return FALSE;
534-
535545
/* Read VeNCrypt version */
536546
if (!ReadFromRFBServer(client, (char *)&major, 1) ||
537547
!ReadFromRFBServer(client, (char *)&minor, 1))
@@ -560,16 +570,18 @@ HandleVeNCryptAuth(rfbClient* client)
560570
}
561571

562572
if (!ReadVeNCryptSecurityType(client, &authScheme)) return FALSE;
563-
if (!ReadFromRFBServer(client, (char *)&status, 1) || status != 1)
564-
{
565-
rfbClientLog("Server refused VeNCrypt authentication %d (%d).\n", authScheme, (int)status);
566-
return FALSE;
567-
}
568573
client->subAuthScheme = authScheme;
569574

570-
/* Some VeNCrypt security types are anonymous TLS, others are X509 */
571575
switch (authScheme)
572576
{
577+
/* Unencrypted types do not require additional actions */
578+
case rfbNoAuth:
579+
case rfbVncAuth:
580+
case rfbVeNCryptPlain:
581+
return TRUE;
582+
break;
583+
584+
/* Some VeNCrypt security types are anonymous TLS, others are X509 */
573585
case rfbVeNCryptTLSNone:
574586
case rfbVeNCryptTLSVNC:
575587
case rfbVeNCryptTLSPlain:
@@ -578,11 +590,21 @@ HandleVeNCryptAuth(rfbClient* client)
578590
#endif /* LIBVNCSERVER_HAVE_SASL */
579591
anonTLS = TRUE;
580592
break;
593+
581594
default:
582595
anonTLS = FALSE;
583596
break;
584597
}
585598

599+
/* Ack is only requred for the encrypted connection */
600+
if (!ReadFromRFBServer(client, (char *)&status, 1) || status != 1)
601+
{
602+
rfbClientLog("Server refused VeNCrypt authentication %d (%d).\n", authScheme, (int)status);
603+
return FALSE;
604+
}
605+
606+
if (!InitializeTLS()) return FALSE;
607+
586608
/* Get X509 Credentials if it's not anonymous */
587609
if (!anonTLS)
588610
{

0 commit comments

Comments
 (0)