Opened 17 months ago

Closed 4 months ago

#1118 closed Feature Wish (fixed)

OpenVPN 2.4.6 does not respect -tls-cipher priority when using TLS 1.3

Reported by: jimdoe Owned by: Steffan Karger
Priority: major Milestone: release 2.4.6
Component: Crypto Version: OpenVPN 2.4.6 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords: tls-cipher, tls 1.3, chacha20-poly1305
Cc:

Description

When using OpenVPN 2.4.6 compiled against the openssl 1.1.1 crypto library, which includes support for TLS 1.3 Final, OpenVPN no longer respects the --tls-cipher option in config files. When using TLS 1.2, TLS-ECDHE-ECDSA-WITH-CHACHA20-POLY1305-SHA256 is used if listed in --tls-cipher (taking priority over TLS-ECDHE-ECDSA-AES256-GCM-SHA384). If TLS 1.3 is used, TLS_AES_256_GCM_SHA384 is used, even if TLS_CHACHA20_POLY1305_SHA256 is given priority using the --tls-cipher option.

Thus it seems impossible to use the ChaCha20-Poly1305 Cipher on the TLS Control Channel when using tls 1.3, as TLS_AES_256_GCM_SHA384 is always given priority. ChaCha20-Poly1305 may be desired on lower powered devices without hardware AES acceleration. Currently this can be circumvented by using the --tls-version-max 1.2 option.

Attachments (1)

0001-Add-support-for-tls-ciphersuites-for-TLS-1.3.patch (14.2 KB) - added by plaisthos 17 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 17 months ago by tincantech

subscribed

comment:2 Changed 17 months ago by plaisthos

Resolution: notabug
Status: newclosed

While I don't doubt that what you seeing is correct, this is not an OpenVPN bug. tls-cipher hase been and always is just passing this information to the OpenSSL library. So OpenSSL not respecting the order here anymore is an OpenSSL bug or intended behaviour change.

comment:3 Changed 17 months ago by plaisthos

Btw. did you try adding TLS_CHACHA20_POLY1305_SHA256 to the tls-cipher list? That should be a proper TLS 1.3 and might actually get picked for TLS 1.3 connections

comment:4 Changed 17 months ago by plaisthos

So fun with OpenSSL. tls-cipher is only for 1.2 and below (using SSL_CTX_set_cipher_list).

For TLS 1.3 OpenSSL invented a new API that is 1.3 only: SSL_CTX_set_ciphersuites.

So we just do not support setting 1.3 ciphersuites yet.

Having both tls-cipher and tls-ciphersuites is probably too confusing. But I am not sure what a good name for the tls-ciphersuites setting would be.

comment:5 Changed 17 months ago by plaisthos

Resolution: notabug
Status: closedreopened
Type: Bug / DefectFeature Wish

comment:6 in reply to:  3 Changed 17 months ago by jimdoe

Replying to plaisthos:

Btw. did you try adding TLS_CHACHA20_POLY1305_SHA256 to the tls-cipher list? That should be a proper TLS 1.3 and might actually get picked for TLS 1.3 connections

Yep, already tried that. Currently my --tls-cipher looks like

tls-cipher TLS_CHACHA20_POLY1305_SHA256:TLS-ECDHE-ECDSA-WITH-CHACHA20-POLY1305-SHA256

But going by your later comments I'm guessing you've discovered that didn't make a difference

comment:7 Changed 17 months ago by plaisthos

Since OpenSSL 1.1.1 with OpenVPN sounds like compiling yourself are you willing to test my patch?

In my test with --tls-ciphersuites "TLS_CHACHA20_POLY1305_SHA256 it works fine:

Control Channel: TLSv1.3, cipher TLSv1.3 TLS_CHACHA20_POLY1305_SHA256

comment:8 in reply to:  7 Changed 17 months ago by jimdoe

Replying to plaisthos:

Since OpenSSL 1.1.1 with OpenVPN sounds like compiling yourself are you willing to test my patch?

In my test with --tls-ciphersuites "TLS_CHACHA20_POLY1305_SHA256 it works fine:

Control Channel: TLSv1.3, cipher TLSv1.3 TLS_CHACHA20_POLY1305_SHA256

Great that you wrote a patch - thanks! Whilst my server is self-compiled, my clients are not, and it is only in some of client configs that I specify the tls-cipher option (depending on the device). I suppose I could apply your patch to the server and use the new --tls-ciphersuites option to prioritise ChaCha20 on tls 1.3 connections, but I won't have the time until the weekend, if not later. Would be great if someone could test it out sooner. Thanks again for your work plaisthos

comment:9 Changed 17 months ago by jimdoe

I managed to find some time to apply your patch and compile, and I can confirm it seems to be working great :)

I added the option

--tls-ciphersuites TLS_CHACHA20_POLY1305_SHA256:TLS_AES_256_GCM-SHA384

to my server config, and I can confirm that the server initializes just fine, and upon connection using my client with OpenSSL 1.1.1, I am now getting

Control Channel: TLSv1.3, cipher TLSv1.3 TLS_CHACHA20_POLY1305_SHA256

Thanks for this. I hope it gets merged. I will report back here if I run into any issues in the meantime.

Whilst perhaps confusing having both --tls-cipher and --tls-ciphersuites, I think that, with enough info in the documentation regarding their differences with regards tls 1.2 and earlier, and tls 1.3, it is the only feasible option. And, like you say in the patch, tls-cipher has always been advertised as for advanced use, and shouldn't be used without understanding what it does and the potential dangers of using it.

comment:10 Changed 4 months ago by Steffan Karger

Resolution: fixed
Status: reopenedclosed

The patch was applied to release/2.4 and master a year ago, but we never reported back to this ticket.

commit ea4ee31333a0cddb5c8dd4185f9426df13c76947 (master)
commit 9de7fe0a7bf1111ebea9ca9e28b2def9ae3e53ff (release/2.4)

Closing this now.

Note: See TracTickets for help on using tickets.