XTLS / Go

Go implementation of XTLS protocol.
https://t.me/projectXray
Mozilla Public License 2.0
278 stars 44 forks source link

Some characteristics of internal TLS 1.2 connections in XTLS #16

Closed AkinoKaede closed 1 year ago

AkinoKaede commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.

badO1a5A90 commented 2 years ago

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.

AkinoKaede commented 2 years ago

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.

badO1a5A90 commented 2 years ago

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.

AkinoKaede commented 2 years ago

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 commented 2 years ago

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

AkinoKaede commented 2 years ago

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

yuhan6665 commented 1 year ago

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

RPRX commented 1 year ago

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