-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Add Hysteria2 Protocol #2721
Add Hysteria2 Protocol #2721
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #2721 +/- ##
==========================================
- Coverage 37.79% 36.78% -1.01%
==========================================
Files 654 664 +10
Lines 38723 39771 +1048
==========================================
- Hits 14636 14631 -5
- Misses 22477 23524 +1047
- Partials 1610 1616 +6
☔ View full report in Codecov by Sentry. |
I had merged newest commit and fixed go-lint. |
Hi! Thanks for your contribution. Since this merge request includes switching the dependency, it will be a longer review process, however I will try my best to expatiate the process. |
Hi, after some interal discuss, it seems there is no active maintainer within V2Ray could take over the maintaince of hysteria transport. Is it possible for you to refactor this code, so that the original quic transport is left as is, and make the necessary change to make hysteria a new transport that can be disabled when something goes wrong by removing the import line in |
I am happy to merge it after the usage of forked quic from hysteria is isolated from main quic. |
Thanks. I will dig into it later. But I have been very busy lately, and I may reply very late. Anyway, I will try my best. |
Hi, I tried to add a QUIC-based HTTP3 tranpsort which required by Hysteria, and it works. I use the code from here. So it's possible to add Hysteria proxy protocol to V2ray. I can take responsibility for maintenance if you want. Assumed configuration:
{
"tag": "demo1",
"protocol": "hysteria2",
"settings": {},
"streamSettings": {
"transport": "hysteria2",
"transportSettings": {
"congestion": {
"type": "brutal",
"download_mbps": 200,
"upload_mbps": 200,
"password": "password"
}
},
"security": "tls",
"securitySettings": {}
}
}
{
"tag": "demo2",
"protocol": "vmess",
"settings": {},
"streamSettings": {
"transport": "hysteria2",
"transportSettings": {
"congestion": {
"type": "bbr",
"password": "" // empty means no password.
}
},
"security": "tls",
"securitySettings": {}
}
} What do you think? @xiaokangwang |
I think it should be fine so long it can be selectively compiled and don't interference with existing quic related features. This means it is expected that existing code rely on quic like dns or quic transports are not impacted by this change. So long as this requirement is satisfied, I am happy to merge it so long as there is someone ready to maintain Hysteria stack in the future. |
Thanks. I think make it separately and will not affect others existing features. |
Hi, packages like dns, common/protocol are still depended on newly introduced package. Could you have a look and make sure the hy2 fork of quic is not used outside its own transport and proxy implementaion, or justify it? |
Please let me know when you are ready for another review! |
Thanks. Currently, I had restored to original quic-go and added a http3 transport that based on hysteria quic, supports I will add a hysteria2 proxy protocol in later so that it can work with the offical one. But I am busy on my daily job, I plan to complete this PR in the Lunar New Year. I Will request another code review when I finished. |
Hi @xiaokangwang, Apologies for the delay. I'm ready for the code review. Here's what I've accomplished: Added Hysteria2 transport to V2ray, utilizing QUIC and HTTP3Now, any proxy protocol in V2ray can utilize Hy2 as a transport layer, such as Implemented Hysteria2 proxy compatible with the official versionIt functions well with the ordinary server I use. However, UDP requires additional testing. You can also examine some proxy tests for more insight. Provided some real configuration examplesI'm uncertain about the appropriate location for these examples. They're mainly for testing purposes, so they might be subject to change. Added documentationFor more detailed information, including special features, you can refer to the documentation. |
proxy/hysteria2/client.go
Outdated
} | ||
newError("tunneling request to ", destination, " via ", server.Destination().NetAddr()).WriteToLog(session.ExportIDToError(ctx)) | ||
|
||
hyConn, IsHy2Transport := conn.(*hy2_transport.HyConn) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If traffic stat is enabled, conn is *internet.StatCouterConnection
but not *hysteria2.HyConn
, this type assertion will fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't handle any traffic statistics. Is it necessary? Because I need to expose the WritePacket() method of the transport connection to the proxy layer. If so, I believe I'll need to implement it more aggressively.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a reminder and I don't know how widely it is used. Maybe usually some GUIs have traffic stats function?
By doing this
iConn := conn
if statConn, ok := conn.(*internet.StatCouterConnection); ok {
iConn = statConn.Connection
}
hyConn, IsHy2Transport := iConn.(*hy2_transport.HyConn)
I don't know if maintainers will consider it "overfit".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I will figure out a better solution, any other comments? Thanks.
proxy/hysteria2/server.go
Outdated
func (s *Server) Process(ctx context.Context, network net.Network, conn internet.Connection, dispatcher routing.Dispatcher) error { | ||
sid := session.ExportIDToError(ctx) | ||
|
||
hyConn, IsHy2Transport := conn.(*hy2_transport.HyConn) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If traffic stat is enabled, conn is *internet.StatCouterConnection
but not *hysteria2.HyConn
, this type assertion will fail.
proxy/hysteria2/protocol.go
Outdated
|
||
// ReadMultiBufferWithMetadata reads udp packet with destination | ||
func (r *PacketReader) ReadMultiBufferWithMetadata() (*PacketPayload, error) { | ||
_, data, dest, _ := r.HyConn.ReadPacket() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_, data, dest, _ := r.HyConn.ReadPacket() | |
_, data, dest, err := r.HyConn.ReadPacket() | |
if err != nil { | |
return nil, err | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Catch this error otherwise udp-disabled client will panic.
P.S. The whole code derived from trojan has many leftover that can be deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will fix it as you said. Thanks for the thorough review.
Can you let hy2 transport's client use v2ray system dialer? I think implementing a custom |
I don't really get you, please elaborate it. In my opinion, this PR will not use "net.ListenUDP()" in JimmyHuang454/hysteria, it will hijack all traffic into v2ray. |
Android clients may use I mean something like the below. (I don't know why --- a/transport/internet/hysteria2/dialer.go
+++ b/transport/internet/hysteria2/dialer.go
@@ -52,6 +52,14 @@ func InitAddress(dest net.Destination) (net.Addr, error) {
return destAddr, nil
}
+type connFactory struct {
+ NewFunc func(addr net.Addr) (net.PacketConn, error)
+}
+
+func (f *connFactory) New(addr net.Addr) (net.PacketConn, error) {
+ return f.NewFunc(addr)
+}
+
func NewHyClient(dest net.Destination, streamSettings *internet.MemoryStreamConfig) (hy_client.Client, error) {
tlsConfig, err := InitTLSConifg(streamSettings)
if err != nil {
@@ -68,6 +76,18 @@ func NewHyClient(dest net.Destination, streamSettings *internet.MemoryStreamConf
TLSConfig: *tlsConfig,
Auth: config.GetPassword(),
ServerAddr: serverAddr,
+ ConnFactory: &connFactory{
+ NewFunc: func(addr net.Addr) (net.PacketConn, error) {
+ rawConn, err := internet.ListenSystemPacket(context.Background(), &net.UDPAddr{
+ IP: []byte{0, 0, 0, 0},
+ Port: 0,
+ }, streamSettings.SocketSettings)
+ if err != nil {
+ return nil, err
+ }
+ return rawConn.(*net.UDPConn), nil
+ },
+ },
})
if err != nil {
return nil, err |
Thanks, you are right. I forgot to handle that on the client side. |
建议保留brutal作为quic的一种流控算法选择,不必考虑它与hysteria的兼容性,反正hysteria已经被作为新的协议和传输层实现了。多一种选择不是坏事。 |
proxy/hysteria2/protocol.go
Outdated
return int(quicvarint.Len(uint64(s))) | ||
} | ||
|
||
func (c *ConnWriter) writeTcpHeader() error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be replaced by hyProtocol.WriteTCPRequest
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid it won't work because WriteTCPRequest()
in hyProtocol executes i := VarintPut(buf, FrameTypeTCPRequest)
. However, this PR aims to separate the transport layer and the proxy layer, with FrameTypeTCPRequest
being sent by the transport layer.
proxy/hysteria2/server.go
Outdated
return newError("failed to send response").Base(err) | ||
} | ||
|
||
address := strings.Split(reqAddr, ":") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't work for IPv6. Maybe use net.SplitHostPort
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. My bad.
func InitTLSConifg(streamSettings *internet.MemoryStreamConfig) (*hyClient.TLSConfig, error) { | ||
tlsSetting := CheckTLSConfig(streamSettings, true) | ||
if tlsSetting == nil { | ||
tlsSetting = &tls.Config{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are ConfigFromStreamSettings
and GetTLSConfig
in github.com/v2fly/v2ray-core/v5/transport/internet/tls
.
A lock is needed for By the way, identifying clients only by |
Ofcourse.
I have switched to using IP addresses to differentiate between different outbound connections. At the moment, it is indeed difficult to reuse a single port for multiple inbound connections. |
Hi, I found that the values of congestion |
My bad. Use the Brutal congestion algorithm when both |
Is that Mbps or MBps? The multiplier should be 1000*1000/8 not 1000*1000. Hysteria's case-insensitive conversation is confusing... https://github.com/apernet/hysteria/blob/master/app/internal/utils/bpsconv.go#L10-L53 |
My bad again. It should be |
Hi! Is this merge request ready for another review? |
Hi @xiaokangwang ! I'm ready. Many thanks to @dyhkwong for the thorough check; he provided a lot of very useful suggestions. I believe that Hysteria2's UDP needs more testing, as I don't fully understand how V2ray's UDP works specifically. Moreover, Hysteria2's UDP transmission is revolutionary. |
The merge review is already underway, please avoid adding new commits as they could get lost in the review process. |
I have finished pull request review and this merge request is ready to be merged. Thank you for your work!!! Your code have undergone significant revisions during your development, for this reason it will be squashed upon merge. I am aware this might impact your commit count, and please understand this is kind of unavoidable to reduce merge conflict. Your original commit have been backuped at: https://github.com/xiaokangwang/v2ray-core-1/tree/dev-hy2-orig-backup . You are also invited to join V2Fly's internal chat group. Please send your public key here, and I will send you an invite link encrypted to your public key. |
The following configuration was tested: h2_h2_client.json
hy2_hy2_server.json
hy2_client.yaml
hy2_server.yaml
vmess_hy2_client.json
vmess_hy2_server.json
The self-signed certificate is generated with:
The pinned hash is generated with:
|
Use hysteria's quic-go to support BBR and brutal congestion algorithm
Same as hysteria,
BBR
andBrutal
congestion algorithm can improve performance.Usage:
send_mbps
is needed when congestion'stype
isbrutal
, meaning the max bandwidth of current network can use to send.brutal
, it's just ok to usebbr
in both client and server side.Fix
vprotogen
bug and update all Protobuf datavprotogen
can not work, because it failed to recognize new version ofprotoc
, so that we can not add new feature to v2ray.Update go version to 1.21
fix #2701 #2644