Is github.com/xtls/xray-core safe? go package security report

GHSA-5wf9-h793-w73c

HIGH

// summary

Pinning a CA certificate via pinnedPeerCertSha256 can lead to the success of MITM attacks in some cases.

// details

https://github.com/XTLS/Xray-core/blob/45cf2898ab12e97a55dd8f1f3d78d903340bdc9e/transport/internet/tls/config.go#L333-L347

If r.Config.ServerName is empty, DNSName will be empty and certs[0].Verify(opts) will not verify against dNSName or iPAddress.

If a user pins a well-known and non-self-signed CA, an attacker can issue a leaf certificate through the Root CA (using the attacker's own domain name or IP address) and hijack the connections.

In Go crypto/tls, either ServerName or InsecureSkipVerify must be specified in the tls.Config. If the user specifies pinnedPeerCertSha256, InsecureSkipVerify will always be true so ServerName can be empty.

In most cases, if the user does not specify the serverName, Xray-core will use the server address as the r.Config.ServerName.

Thanks to GetTLSConfig(tls.WithDestination(dest)), ServerName will usually not be empty.

However, there are some leftovers:

https://github.com/XTLS/Xray-core/blob/45cf2898ab12e97a55dd8f1f3d78d903340bdc9e/transport/internet/hysteria/dialer.go#L348

GetTLSConfig() without tls.WithDestination(dest).

https://github.com/XTLS/Xray-core/blob/45cf2898ab12e97a55dd8f1f3d78d903340bdc9e/transport/internet/grpc/dial.go#L139-L142

GetTLSConfig() without tls.WithDestination(dest). Although ServerName will not be empty if address is a domain name, ServerName will be left empty if address is an IP address.

ServerName is not equivalent to SNI.

> ServerName is used to verify the hostname on the returned certificates unless InsecureSkipVerify is given. It is also included in the client's handshake to support virtual hosting unless it is an IP address.

In these cases, if the user does not specify the serverName, Xray will not use the server address as the r.Config.ServerName and r.Config.ServerName will be left empty.

// poc

client.json

{
	"log": {
		"loglevel": "debug"
	},
	"inbounds": [
		{
			"listen": "127.0.0.1",
			"port": 1080,
			"protocol": "socks"
		}
	],
	"outbounds": [
		{
			"protocol": "hysteria",
			"settings": {
				"address": "127.0.0.1",
				"port": 443,
				"version": 2
			},
			"streamSettings": {
				"security": "tls",
				"network": "hysteria",
				"tlsSettings": {
					"pinnedPeerCertSha256": "e4aa7ea894c9514ca42831cb0c7548a8eabd5e6f588702b9989790c4985d1b2f"
				},
				"hysteriaSettings": {
					"version": 2,
					"auth": "auth"
				}
			}
		}
	]
}

server.json

{
	"log": {
		"loglevel": "debug"
	},
	"inbounds": [
		{
			"listen": "127.0.0.1",
			"port": 443,
			"protocol": "hysteria",
			"settings": {
				"version": 2
			},
			"streamSettings": {
				"security": "tls",
				"network": "hysteria",
				"tlsSettings": {
					"certificates": [
						{
							"certificateFile": "server2.crt",
							"keyFile": "server2.key"
						}
					]
				},
				"hysteriaSettings": {
					"version": 2,
					"auth": "auth"
				}
			}
		}
	],
	"outbounds": [
		{
			"protocol": "freedom"
		}
	]
}

Both server1.crt and server2.crt are issued by the same CA. server1.crt is subjected to 127.0.0.1. server2.crt is subjected to 127.0.0.2.

xray tls hash --cert server1.crt
Leaf SHA256:       7f0f1a28f96b7d74e60bbbc263a108ac1243345110f77c45e5bc2fe02851551d
CA  SHA256:  e4aa7ea894c9514ca42831cb0c7548a8eabd5e6f588702b9989790c4985d1b2f

xray tls hash --cert server2.crt
Leaf SHA256:       32a842063280252bc746a913dce8e38b526021bfc7a2d784632d413dbee0c69c
CA  SHA256:  e4aa7ea894c9514ca42831cb0c7548a8eabd5e6f588702b9989790c4985d1b2f

server1.crt

server2.crt