Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some characteristics of internal TLS 1.2 connections in XTLS #16

Closed
AkinoKaede opened this issue Sep 15, 2021 · 10 comments
Closed

Some characteristics of internal TLS 1.2 connections in XTLS #16

AkinoKaede opened this issue Sep 15, 2021 · 10 comments

Comments

@AkinoKaede
Copy link

AkinoKaede commented Sep 15, 2021

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.

Go/conn.go

Lines 1323 to 1334 in 3632bf3

if c.DirectOut {
if s := len(b) - 31; s >= 0 && b[s] == 21 {
if b[s+1] == 3 && b[s+2] == 3 && b[s+3] == 0 && b[s+4] == 26 {
if c.SHOW {
fmt.Println(c.MARK, "discarded 21 3 3 0 26 at s =", s)
}
c.Connection.Write(b[:s])
return s + 31, nil
}
}
return c.Connection.Write(b)
}

Thanks for your work.

Your,
AkinoKaede

@AkinoKaede AkinoKaede changed the title A characteristic of TLS 1.2 in XTLS direct mode A characteristic of internal connections is TLS 1.2 in XTLS direct mode Sep 15, 2021
@AkinoKaede
Copy link
Author

AkinoKaede commented Sep 15, 2021

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 AkinoKaede changed the title A characteristic of internal connections is TLS 1.2 in XTLS direct mode A characteristic of internal TLS 1.2 connections in XTLS direct mode Sep 16, 2021
@AkinoKaede
Copy link
Author

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 AkinoKaede changed the title A characteristic of internal TLS 1.2 connections in XTLS direct mode Some characteristics of internal TLS 1.2 connections in XTLS direct mode Sep 17, 2021
@badO1a5A90
Copy link
Member

badO1a5A90 commented Sep 18, 2021

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
Copy link
Author

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
Copy link
Member

badO1a5A90 commented Sep 18, 2021

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
Copy link
Author

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
Copy link
Author

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

@AkinoKaede
Copy link
Author

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

@yuhan6665
Copy link
Member

Thanks for your post. Should be fixed in XTLS/Xray-core#1235

@RPRX
Copy link
Member

RPRX commented Feb 27, 2023

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

@RPRX RPRX closed this as completed Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants