Skip to content

Experimental proxy mode#96

Open
jonbarrow wants to merge 5 commits intomasterfrom
feat/proxy-mode
Open

Experimental proxy mode#96
jonbarrow wants to merge 5 commits intomasterfrom
feat/proxy-mode

Conversation

@jonbarrow
Copy link
Member

Resolves #XXX

Changes:

Adds in an experimental "ProxiedMode" setting. This setting allows a PRUDP server to sit behind a proxy server, so long as the proxy server can properly encode/decode/forward the server/client packets. This is mostly useful for features such as balancing multiple server instances, in-flight packet inspection, etc.

When in this mode, all packets hitting the PRUDP server will be prepended with the original clients IP/port, so that NEX can still track this for features like matchmaking, and all packets sent from the server will have this data also attached so that the proxy server can route the response back to the destination. The proxy server is expected to strip this data before forwarding to the client.

When in this mode, the clients SocketConnection.ProxyAddress is populated to the proxy servers address, and its SocketConnection.Address value is overwritten with the data from the new packet header. This is done in an attempt to let the NEX protocol handlers continue to rely on SocketConnection.Address without worrying about whether or not the client is behind a proxy.

Also includes an example proxy server, though all it does is map between proxy ports and upstream servers. It does not do any inspection, balancing, etc.

Untested, but should work. At least for non-matchmaking connections. I need some help making sure this won't break matchmaking.

adds in an experimental "ProxiedMode" setting. this setting allows a PRUDP server to sit behind a proxy server, so long as the proxy server can properly encode/decode/forward the server/client packets. this is mostly useful for features such as balancing multiple server instances, in-flight packet inspection, etc.
@ashquarky
Copy link
Member

It's worth noting there's a standardised protocol for this sort of thing (also includes IPv6 and TCP for wss): https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt
Using the standard one would make packet captures easier

It's probably fine to do our own thing to start with though.

@DaniElectra
Copy link
Member

I think it would be a good idea to add a special header (with magic and version number), that way we can expand the proxy in the future without too much issues of making changes (since those could be identified with the version)

@DaniElectra
Copy link
Member

It's worth noting there's a standardised protocol for this sort of thing (also includes IPv6 and TCP for wss): haproxy.org/download/1.8/doc/proxy-protocol.txt

This doesn't include UDP though, making something custom in this case should be good

@jonbarrow
Copy link
Member Author

Using the standard one would make packet captures easier

Normal packet captures, such as those taken with Hokaku, wouldn't be affected by this. The client is still sending and receiving normal, unmodified, PRUDP packets. A packet capture would see the client talking to the proxy, thinking it's the real origin server like any other. The modification happens purely on the servers side of things, where the source/destination sections are added/removed by the PRUDP server/proxy prior to the packet being processed by the NEX server/sent back to the client

@jonbarrow
Copy link
Member Author

@ashquarky @DaniElectra I've made it so that the proxy handling is now modular like we have for the encryption/compression implementations, as well as added 2 more protocol headers

I don't super love the fact that it modifies the addresses the way it does, but it was the cleanest way I could get it working for now

That said, I'm not confident that the PROXY protocol v2 implementation is 100% correct, and maybe SocketConnection.ProxyAddress could be renamed since now it's being used even when not proxying?

@jonbarrow
Copy link
Member Author

@DaniElectra poke

func NewSocketConnection(server *PRUDPServer, address net.Addr, webSocketConnection *gws.Conn) *SocketConnection {
return &SocketConnection{
Server: server,
ProxyAddress: cloneAddr(address), // * Need to make a copy of the net.Addr so it can be worked with independently
Copy link
Member

Choose a reason for hiding this comment

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

I think we could just pass a reference to the SocketConnectionto the Parse and Encode functions, and assign the client address from there. That way we can avoid the hack from below

return 7
}

func (pspp *PRUDPSimpleProxyProtocol) Parse(clientAddress net.Addr, proxyAddress net.Addr, packet []byte) ([]byte, error) {
Copy link
Member

Choose a reason for hiding this comment

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

This should reject the packet if the version isn't lower or equal to the current version for good measure

return 38
}

func (cspp *CloudflareSimpleProxyProtocol) Parse(clientAddress net.Addr, proxyAddress net.Addr, packet []byte) ([]byte, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Same here, we want to reject the packet if it doesn't have the magic

return -1
}

func (hpp *HAProxyProxyProtocolV2) Parse(clientAddress net.Addr, proxyAddress net.Addr, packet []byte) ([]byte, error) {
Copy link
Member

Choose a reason for hiding this comment

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

ditto

return
}

// * Always version 0 for now, skip the version byte
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Comment on lines +69 to +71
if clientIP.To4() != nil {
clientIP = clientIP.To16()
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if clientIP.To4() != nil {
clientIP = clientIP.To16()
}
if clientIP.To4() == nil {
clientIP = clientIP.To16()
}

To4 returns nil when it isn't IPv4. I assume this is what was meant? same below

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.

3 participants