Bug 7723

Summary: Diffie-Hellman parameters should come from RFC7919
Product: ThinLinc Reporter: Pierre Ossman <ossman>
Component: MiscAssignee: Linn <linma>
Status: CLOSED FIXED    
Severity: Normal CC: linma, samuel
Priority: P2 Keywords: prosaic, samuel_tester
Version: trunk   
Target Milestone: 4.14.0   
Hardware: PC   
OS: Unknown   
See Also: https://www.cendio.com/bugzilla/show_bug.cgi?id=3862
Acceptance Criteria:
* Make sure the Diffie-Hellman parameters used by tlstunnel are the same as the parameters defined in RFC 7919 * The code doesn't use any deprecated functions

Description Pierre Ossman cendio 2021-06-09 09:34:33 CEST 
The current best practices when it comes to Diffie-Hellman parameters is to use those from RFC 7919 rather than generating your own. The reason being that those have been carefully vetted against possible flaws that might make them easier to break.

We currently have our own parameters hard coded in to tlstunnel. Although there are no known issues with those we should switch to the current recommended method.

Also note that modern GnuTLS supposedly defaults to RFC 7919 so we likely just need to remove our existing code.
Comment 2 Linn cendio 2021-09-07 14:14:37 CEST 
To investigate how the DH-parameters are currently generated, I tested this by using gnutls-cli to webaccess and listening to network traffic on that port.

When using TLS 1.2, the DH-parameters are generated by ThinLinc and can be found in the data sent from the server. Note however that these DH-parameters are equal to the ones defined in RFC 7919 [1] (I tested with FFDHE2048). This means that even if the parameters aren't generated the recommended way, they still have the correct value.

When using TLS 1.3, the DH-parameters can't be seen in the data sent from the server, indicating that TLS 1.3 already uses the recommended way of handling these parameters. 


This means that for both TLS 1.2 and 1.3, the DH-parameters currently have the correct value. However, we still want to change how we currently handle the parameter generation.


[1]: https://www.rfc-editor.org/rfc/pdfrfc/rfc7919.txt.pdf
Comment 4 Linn cendio 2021-09-08 14:45:00 CEST 
I used one of the the fixes from TigerVNC (https://groups.google.com/g/tigervnc-devel/c/GHL650DS1II). I chose this one since it targets GnuTLS 3.6 and later, which is what ThinLinc supports.

Tested with TLS 1.2 and 1.3 for key exchanges with FFDHE2048 and FFDHE3072, and the correct DH-parameters are used. Also checked that our code did not contain any functions deprecated in GnuTLS 3.6 [1].


Tips for tester is to use gnutls-cli command to connect to Webaccess and to use the priority string to force use of one of the FFDHE-algorithms. The priority string can also be used to choose which TLS version to use.


[1]: https://gnutls.org/manual/gnutls.html#Parameter-generation
Comment 7 Linn cendio 2021-09-13 10:09:16 CEST 
The fix mentioned in comment 4 was for VNC, and not tlstunnel. The incorrect VNC fix has been reverted and a fix for tlstunnel has been made.

Tested against webaccess with TLS 1.2 and 1.3 for key exchanges with FFDHE2048 which works as expected, and checked that the code did not contain any deprecated functions.
Comment 8 Samuel Mannehed cendio 2021-09-14 16:43:59 CEST 
Looks fine. I tested server build 2274 on Ubuntu 20.10 and connected to both Web Access and Web Admin using the following browsers:

 ✔ Edge 93 on Windows 10
 ✔ IE on Windows 10
 ✔ Firefox on Windows 10
 ✔ Chrome on Fedora 34
 ✔ Firefox on Fedora 34

The web access and web admin logs did not log any relevant errors or warnings, nor did the browser consoles.

Also tested with TLS 1.2 from gnutls_priority in webaccess.hconf by setting it to:

> gnutls_priority=NORMAL:-VERS-TLS1.3

Connections still worked and the DEBUG log wrote:

> 2021-09-14 16:39:08 DEBUG tlwebaccess[12977]: [::ffff:10.47.4.80] TLS handshake was completed with: (TLS1.2)-(ECDHE-X25519)-(RSA-PSS-RSAE-SHA256)-(AES-128-GCM)