Go icon indicating copy to clipboard operation
Go copied to clipboard

Some characteristics of internal TLS 1.2 connections in XTLS

Open AkinoKaede opened this issue 3 years ago • 8 comments

Dear developers,

As of the design of XTLS direct mode, the close_notify alert should not be sent to the server, this is done by checking if the record type is alert and the record length equals 31, this includes a 5-bytes record header and an inner content whose length is assumed to be 26 bytes.

However, the length of this inner content may vary, causing the record length to unequal to 31. Consequently, this record is sent to the server accidentally.

This makes the detection of XTLS unambiguously accurate since it's rare that some correctly implemented client sends an alert record before closing a TLS 1.3 stream.

https://github.com/XTLS/Go/blob/3632bf3b74991685fba4906a68e4c49fb73d7ad5/conn.go#L1323-L1334

Thanks for your work.

Your, AkinoKaede

AkinoKaede avatar Sep 15 '21 05:09 AkinoKaede

I tested it on my computer. The Xray client sent a TLS record with alert before close the connection. And the cipher suite that been used by this TLS 1.2 connection is TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 (0xcca9).

curl --tlsv1.2 --tls-max 1.2--ciphers ECDHE-ECDSA-CHACHA20-POLY1305 https://www.google.com

image

AkinoKaede avatar Sep 15 '21 14:09 AkinoKaede

I found another characteristic of direct mode. The max cipher text size of TLS 1.2 is bigger than the max cipher text size of TLS 1.3. However, XTLS will send the application data whose length is bigger than the max size of TLS 1.3 to server. This is a problem that may happen if the internal connections are TLS 1.2 connections.

AkinoKaede avatar Sep 17 '21 16:09 AkinoKaede

we’ve already developed a working solution at https://github.com/badO1a5A90/Go/commits/1.17.1, however more testing is needed before publishing.

20210919002923 As you could tell from the screenshot, the behavior you described in this issue has been resolved.

In the mean time, we’ll be monitoring the impact, which should be minimal. We’ll release an update as soon as necessary testing are completed.

badO1a5A90 avatar Sep 18 '21 16:09 badO1a5A90

we’ve already developed a working solution at https://github.com/badO1a5A90/Go/commits/1.17.1, however more testing is needed before publishing.

In the mean time, we’ll be monitoring the impact, which should be minimal. We’ll release an update as soon as necessary testing are completed.

This problem has not been completely fixed, the internal connections may write the data includes both application data and alert.

AkinoKaede avatar Sep 18 '21 16:09 AkinoKaede

we’ve already developed a working solution at https://github.com/badO1a5A90/Go/commits/1.17.1, however more testing is needed before publishing. In the mean time, we’ll be monitoring the impact, which should be minimal. We’ll release an update as soon as necessary testing are completed.

This problem has not been completely fixed, the internal connections may write the data includes both application data and alert.

Could you elaborate on your point? Also, it would be much appreciated if you could suggest a solution as well.

badO1a5A90 avatar Sep 18 '21 16:09 badO1a5A90

we’ve already developed a working solution at https://github.com/badO1a5A90/Go/commits/1.17.1, however more testing is needed before publishing. In the mean time, we’ll be monitoring the impact, which should be minimal. We’ll release an update as soon as necessary testing are completed.

This problem has not been completely fixed, the internal connections may write the data includes both application data and alert.

Could you elaborate on your point? Also, it would be much appreciated if you could suggest a solution as well.

Traverse all TLS records and remove all TLS records with alerts.

AkinoKaede avatar Sep 21 '21 02:09 AkinoKaede

I read a report by gfwrev. For the second problem, XTLS will send the sequence number of TLS 1.2 AES-GCM.

AkinoKaede avatar Sep 21 '21 02:09 AkinoKaede

All in all, I think it’s not a good idea to send TLS 1.2 records without encrypted.

AkinoKaede avatar Sep 21 '21 02:09 AkinoKaede

Thanks for your post. Should be fixed in https://github.com/XTLS/Xray-core/pull/1235

yuhan6665 avatar Oct 29 '22 05:10 yuhan6665

XTLS Vision 不支持内层 TLSv1.2 裸奔,该 issue 所提及问题均已消失(已修复)

RPRX avatar Feb 27 '23 12:02 RPRX