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

Validate /websocket requests from browser dialer page #3295

Merged
merged 6 commits into from Apr 26, 2024

Conversation

mmmray
Copy link
Contributor

@mmmray mmmray commented Apr 21, 2024

Fix #3236

Validate the origin of inbound websocket requests by default.

If the user wants to change origin validation, they can set XRAY_BROWSER_DIALER_ORIGIN=http://custom.localhost.net:3000

The current implementation is very inconvenient, because the origin is validated very strictly. If XRAY_BROWSER_DIALER=http://127.0.0.1:3000, then the user has to visit 127.0.0.1, not localhost. Is there a function to normalize this difference? Should xray perform DNS requests to normalize host names (I think this could open more attack vectors, using DNS rebinding)

@RPRX
Copy link
Member

RPRX commented Apr 21, 2024

多加个环境变量不优雅,改为可以自定义 path 吧,比如这样 XRAY_BROWSER_DIALER=127.0.0.1:3000/path

顺便把现在的基于 /websocket 来决定是否 upgrade 改为基于 header,浏览器必须访问指定 path 才能加载 browser dialer page

这样可以防止本地程序通过扫端口来识别是否有 browser dialer,前提是环境变量没写系统里,比如 Xray 咕咕的 envs 配置项

@mmmray
Copy link
Contributor Author

mmmray commented Apr 21, 2024

Adding more environment variables is not elegant. Instead, you can customize the path, like this XRAY_BROWSER_DIALER=127.0.0.1:3000/path

this requires the user to choose a secret path. I was trying to make the default invocation (the value copypasted from docs) safe by default.

what do you think about sending a csrf token through the webpage, and sending it back from the browser when opening /websocket? it's less configuration as well.

@RPRX
Copy link
Member

RPRX commented Apr 21, 2024

what do you think about sending a csrf token through the webpage, and sending it back from the browser when opening /websocket? it's less configuration as well.

可以,Xray 启动时生成一个随机值就行

if conn, err := upgrader.Upgrade(w, r, nil); err == nil {
conns <- conn
} else {
newError("Browser dialer http upgrade unexpected error").AtError().WriteToLog()
}
} else {
w.Write(webpage)
w.Write([]byte("<script>\ncsrfToken = \""))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is a bit hard to read, but I did not like the idea of using cookies or a templating engine

@mmmray
Copy link
Contributor Author

mmmray commented Apr 23, 2024

这样可以防止本地程序通过扫端口来识别是否有 browser dialer,前提是环境变量没写系统里,比如 Xray 咕咕的 envs 配置项

not entirely sure I understand the translation, but my concern is not local applications, but rather webpages, who cannot read environment variables anyway

@mmmray mmmray changed the title Validate Origin of browser dialer page Validate /websocket requests from browser dialer page Apr 23, 2024
@mmmray
Copy link
Contributor Author

mmmray commented Apr 25, 2024

This PR is "ready"

@RPRX
Copy link
Member

RPRX commented Apr 26, 2024

This PR is "ready"

写复杂了 我改一下

@RPRX RPRX merged commit 8ce2a0e into XTLS:main Apr 26, 2024
34 checks passed
bygreencn added a commit to bygreencn/ray-core that referenced this pull request Apr 27, 2024
* xtls/main:
  README: Remove iamybj/docker-xray
  Revert "Makefile: export GOARCH, GOOS"
  v1.8.11
  Validate /websocket requests from browser dialer page (XTLS#3295)
  Revert "nosni"
  Bump github.com/cloudflare/circl from 1.3.7 to 1.3.8
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.

any website can grab the server connection from browser dialer on localhost
2 participants