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 problems with httpupgrade headers #3426

Closed
2 tasks done
mikeesierrah opened this issue Jun 5, 2024 · 5 comments · Fixed by #3427
Closed
2 tasks done

Some problems with httpupgrade headers #3426

mikeesierrah opened this issue Jun 5, 2024 · 5 comments · Fixed by #3427

Comments

@mikeesierrah
Copy link

Integrity requirements

  • I confirm that I have read the documentation, understand the meaning of all the configuration items I wrote, and did not pile up seemingly useful options or default values.
  • I searched issues and did not find any similar issues.

Version

1.8.13

Description

When I set custom headers in the HTTP upgrade client configuration:

"streamSettings": {
  "httpUpgradeSettings": {
    "host": "domain.com",
    "path": "/path",
    "headers": {
      "Sec-WebSocket-Version": "13",
      "Sec-WebSocket-Key": "X1xxxx=="
    }
  }
}

I get these packets inside pcap

GET /path HTTP/1.1
Host: domain.com
User-Agent: Go-http-client/1.1
Connection: upgrade
Sec-Websocket-Key: X1xxxx==
Sec-Websocket-Version: 13
Upgrade: websocket

At first glance, you might not notice the difference, but here’s what a standard WebSocket packet looks like in Wireshark:
image

You’ll see the first ‘S’ in WebSocket is capitalized in normal ‘WS’, but it’s in lowercase in ‘HTTPupgrade’, while in my configuration JSON, it’s in caps. Also, the ‘U’ in ‘Connection: Upgrade’ is capitalized in a normal WebSocket, but in lowercase in HTTPupgrade.

It seems the core is altering the letter capitalization.

While these might seem trivial, a firewall can easily detect these anomalies and block or flag them.

Reproduction Method

just try to add these settings to an httpupgrade connection and compare them with a normal ws

no tls ofcourse

Client config


{
  "inbounds":[
    {
      "listen":"127.0.0.1",
      "port":10808,
      "protocol":"socks",
      "settings":{
        "auth":"noauth",
        "udp":true,
        "userLevel":8
      },
      "sniffing":{
        "destOverride":[
          "http",
          "tls"
        ],
        "enabled":true
      },
      "tag":"socks"
    },
    {
      "listen":"127.0.0.1",
      "port":10809,
      "protocol":"http",
      "settings":{
        "userLevel":8
      },
      "tag":"http"
    }
  ],
  "outbounds":[
    {
      "protocol":"vless",
      "settings":{
        "vnext":[
          {
            "address":"somedomain.domain",
            "port":8080,
            "users":[
              {
                "encryption":"none",
                "flow":"",
                "id":"0157f5d9-7cfc-418c-c3b4-d9acab3a7af1",
                "level":8,
                "security":"auto"
              }
            ]
          }
        ]
      },
      "streamSettings":{
        "httpupgradeSettings":{
          "host":"domain.com",
          "path":"/path",
          "headers":{
            "Sec-WebSocket-Version":"13",
            "Sec-WebSocket-Key":"X1O+jYrandomstuff=="
          }
        },
        "network":"httpupgrade",
        "security":"none"
      },
      "tag":"proxy"
    },
    {
      "protocol":"freedom",
      "settings":{
  },
  "tag":"direct"
},
{
  "protocol":"blackhole",
  "settings":{
    "response":{
      "type":"http"
    }
  },
  "tag":"block"
}

]
}

Server config


N/A

Client log


N/A

Server log


N/A
@mmmray
Copy link
Collaborator

mmmray commented Jun 5, 2024

In iran there's a lot of configs without SSL, because IR GFW throttles all SSL more strictly than plain HTTP.

In the case of httpupgrade, it's almost a raw tcp connection. how about removing all use of net/http from the transport, and write bytes directly?

for key, value := range transportConfiguration.Header {
  io.WriteAll(conn, []byte(key + ": " + value + "\r\n")
}

it's easier to get rid of HTTP abstractions in this transport than in the websocket transport.

@Fangliding
Copy link
Member

Fangliding commented Jun 5, 2024

HTTP upgrade has never been designed to be used directly on the public network(TLS is recommend), the doc is very clear. Its possible fingerprints are far more than that.
In the RFC standard, the HTTP header is case insensitive, and a common practice is to capitalize the first letter of each word(MIME header), and that is what we done to the header(convert Sec-WebSocket-Version to Sec-Websocket-Version). I don't know why gorilla used Web"S"socket. In fact, it is also written as Websocket in Chrome

@Fangliding Fangliding closed this as not planned Won't fix, can't repro, duplicate, stale Jun 5, 2024
@mmmray
Copy link
Collaborator

mmmray commented Jun 5, 2024

That's fair. There is a need in Iran for a transport that can pass CDN and also camouflage as plaintext HTTP. Some fingerprint in the http body would be ok, for as long as the headers are right. It's probably not within scope of a bugreport.

@mmmray
Copy link
Collaborator

mmmray commented Jun 5, 2024

on second thought, i feel that if TLS-free configs are not only discouraged, but not supported, xray should not have the ability to set custom HTTP headers at all. what is the point?

In fact, it is also written as Websocket in Chrome

This is incorrect, tested Chrome 125. Devtools show Websocket, but the actual traffic dump shows WebSocket. Thank you devtools!

it's fair to close as not-planned. if I open a PR though, will it be accepted? It might be a performance improvement to get rid of net/http 😇

@yuhan6665
Copy link
Member

yuhan6665 commented Jun 5, 2024

I'm ok if there is a way to keep user defined capitalization

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 a pull request may close this issue.

4 participants