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

Socks inbound: Support HTTP inbound by default #3682

Merged
merged 9 commits into from
Aug 15, 2024
Merged

Conversation

Fangliding
Copy link
Member

@Fangliding Fangliding commented Aug 14, 2024

RT 允许socks入站使用http,不是从HTTP inbound copy来的,而是调用http入站本身
兼容鉴权(如果socks5设置了密码 http也需要一样的密码才能访问)
默认开启
eg:

{
    "listen": "127.0.0.1",
    "port": "1080",
    "protocol": "socks",
    "settings": {
        "auth": "password",
        "udp": true,
        "ip": "127.0.0.1",
//      "mixed": true,   #Deprecated
        "accounts": [
            {
              "user": "114",
              "pass": "514"
            }
          ]
    }
}

@RPRX
Copy link
Member

RPRX commented Aug 14, 2024

  1. 改成只支持 CONNECT,我觉得群友写得挺好的,原逻辑内检查下是否 C 开头就行,不是的话继续原逻辑,注意性能
  2. 默认开启的话别加选项了,反正连 4 和 4a 都兼容,加个直接兼容 HTTP 入站吧,@mmmray what do u think?

@RPRX
Copy link
Member

RPRX commented Aug 14, 2024

就是说别套一层了,看着头大,直接在 Socks 内部处理逻辑那里分流给 HTTP 就行了

@Fangliding
Copy link
Member Author

Fangliding commented Aug 14, 2024

  1. 改成只支持 CONNECT,我觉得群友写得挺好的,原逻辑内检查下是否 C 开头就行,不是的话继续原逻辑,注意性能

em 主要是curl会用GET 改一下也行 改成G或者E吧 或者直接改成非5开头就进http 这样更简单 string都免了 我也这么想过不过想着这牌子还是SOCKS 能进SOCKS尽量SOCKS5

就是说别套一层了,看着头大,直接在 Socks 内部处理逻辑那里分流给 HTTP 就行了

直接复用http入站的Process函数是最方便的了 还里面就要处理更多函数了 感觉只会更乱

@RPRX
Copy link
Member

RPRX commented Aug 14, 2024

套一层的话性能会打折,splice 还会炸,你给俩 process 都加个参数传首字节吧,u know what i mean

proxy/socks/server.go Outdated Show resolved Hide resolved
proxy/socks/server.go Outdated Show resolved Hide resolved
@mmmray
Copy link
Collaborator

mmmray commented Aug 14, 2024

@mmmray what do u think?

I don't have any strong opinions on the interface, it seems that if socks is compatible with http by default, the entire http inbound can be deleted as well? (and it just becomes an alias for socks)

@RPRX
Copy link
Member

RPRX commented Aug 14, 2024

@mmmray what do u think?

I don't have any strong opinions on the interface, it seems that if socks is compatible with http by default, the entire http inbound can be deleted as well? (and it just becomes an alias for socks)

我觉得 Socks 可以默认兼容 HTTP 但反过来不妥,因为 HTTP 代理一定是域名,b 事更少用得更爽,所以有仅支持 HTTP 代理的需求

@Fangliding
Copy link
Member Author

OK 现在是直接从连接里读出一个字节来 我修改了两个process函数让它们的reader接上firstbyte

@Fangliding
Copy link
Member Author

em 不能随意修改process签名 套了一层娃

@mmmray
Copy link
Collaborator

mmmray commented Aug 14, 2024

this construction is not helpful right?

// we read some garbage byte that may not have been "ok" at
// all. return a reader that replays what we have read so far
reader := io.MultiReader(
bytes.NewReader(trashHeader),
lazyRawDownload,
)
readCloser := struct {
io.Reader
io.Closer
}{
Reader: reader,
Closer: lazyRawDownload,
}

because it will not work with splice...

@xqzr
Copy link
Contributor

xqzr commented Aug 14, 2024

主要是curl会用GET

-p

@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

this construction is not helpful right?

能不多套一层就不要多套一层

@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

Socks 那里看得我。。。我改吧

@Fangliding
Copy link
Member Author

这样么 昨晚上没看清从http复制来的 socks 这里确实用一个 io.Reader 就行了

@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

那个 reader 不是本来就是 buf.BufferedReader 吗

@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

还有我看了下,HTTP 的两个 buf.Copy 基于 conn,但 Socks 的第一个 buf.Copy 基于旧的 reader,顺便改成基于 conn 吧

@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

@Fangliding 别水群了,快改代码

@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

  1. 你看下 buf.BufferedReader 是什么就懂了
  2. 你对比下 Socks 入站和 HTTP 入站的 buf.Copy 就懂了

@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

算了我直接改吧

@RPRX RPRX changed the title 为socks入站新增兼容模式 Socks inbound: Support HTTP inbound Aug 15, 2024
@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

主要是现在 main 分支有一个新的 commit,不能直接 reset 过来,还得 cherry-pick

@Fangliding
Copy link
Member Author

重开了

RPRX added a commit that referenced this pull request Aug 15, 2024
@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

好了,下个版本的 Xray-core,Socks 入站默认兼容 HTTP 入站,反过来不是,理由详见 #3682 (comment)

@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

@Fangliding 改 Socks 入站文档,下个版本是 v1.8.24

@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

还有我看了下,HTTP 的两个 buf.Copy 基于 conn,但 Socks 的第一个 buf.Copy 基于旧的 reader,顺便改成基于 conn 吧

等下,好像不能这么改,@Fangliding 你把这部分 revert 后重新 commit 到 main 吧

@Fangliding
Copy link
Member Author

revert还是reset

@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

我的意思是把那部分逻辑还原之后 force-push

Fangliding added a commit that referenced this pull request Aug 15, 2024
@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

虽然正常的 Socks5 有 1-RTT 的握手,但像 Xray 的 Socks5 出站直接搞成 0-RTT 了,会导致旧的 reader 里面有被代理数据

我看了下 BufferedReader 读完 Buffer 后的 ReadMultiBuffer() 也挺干净的,虽然还是多了些逻辑

@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

@Fangliding 只还原 s.transport 那里,上面的别还原,看来你还是没看懂

@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

@Fangliding 就是只还原这个 commit 02a691a 从158 行开始的修改,交给你了

@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

@Fangliding 不是,你看上面 BufferedReader 那里,所以说要先看懂,感觉这样说来说去的效率好低,下次我自己来,这次就算了

Fangliding added a commit that referenced this pull request Aug 15, 2024
@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

而且我觉得在 firstbyte 加入之前没必要用 BufferedReader,即使用了,copy 时也该先把缓存 write 完然后直接读 conn,唉,v2

@RPRX
Copy link
Member

RPRX commented Aug 15, 2024

还有虽然 v2 到处传 multiBuffer 但绝大多时候都不 multi,浪费性能,还有 Buffer 和 pool 真的有必要吗,我咋感觉直接 make 更好

RPRX added a commit that referenced this pull request Aug 16, 2024
@RPRX RPRX changed the title Socks inbound: Support HTTP inbound Socks inbound: Support HTTP inbound by default Aug 16, 2024
@RPRX
Copy link
Member

RPRX commented Aug 16, 2024

标题补了 by default

@Fangliding
Copy link
Member Author

@RPRX 也关不掉吧()

@RPRX
Copy link
Member

RPRX commented Aug 16, 2024

@RPRX 也关不掉吧()

虽然关不掉但写上去更清晰

我试了下,看日志可以区分出来是 Socks 还是 HTTP,前者有标明 tcp/udp,后者没有,虽然按层级不该把 tcp/udp 标到 ip 前面

然后我早就觉得 *ray 的日志有点怪,就是说少了个 from 字样,补上了,类似于 Trojan-killer:XTLS/Trojan-killer#10 (comment)

RPRX added a commit that referenced this pull request Aug 16, 2024
RPRX added a commit that referenced this pull request Aug 16, 2024
zxspirit pushed a commit to zxspirit/Xray-core that referenced this pull request Aug 30, 2024
zxspirit pushed a commit to zxspirit/Xray-core that referenced this pull request Aug 30, 2024
@wutongskype

This comment was marked as off-topic.

@Fangliding

This comment was marked as off-topic.

@xqzr

This comment was marked as off-topic.

@wutongskype

This comment was marked as off-topic.

@Fangliding

This comment was marked as off-topic.

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

Successfully merging this pull request may close these issues.

5 participants