From 854499382e20e39a43b1643939b5fa9f167537f5 Mon Sep 17 00:00:00 2001 From: vapatel2 <149737089+vapatel2@users.noreply.github.com> Date: Wed, 8 Nov 2023 03:10:23 -0800 Subject: [PATCH] cmd/stdiscosrv: Prevent nil IPs from X-Forwarded-For (fixes #9189) (#9190) ### Purpose Treat X-Forwarded-For as a comma-separated string to prevent nil IP being returned by the Discovery Server ### Testing Unit Tests implemented Testing with a Discovery Client can be done as follows: ``` A simple example to replicate this entails running Discovery with HTTP, use Nginx as a reverse proxy and hardcode (as an example) a list of IPs in the X-Forwarded-For header. 1. Send an Announcement with tcp://0.0.0.0: 2. Query the DeviceID 3. Observe the returned IP Address is no longer nil; i.e. `tcp://:` ``` --- cmd/stdiscosrv/apisrv.go | 28 ++++++++++++++++++++++------ cmd/stdiscosrv/apisrv_test.go | 8 ++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/cmd/stdiscosrv/apisrv.go b/cmd/stdiscosrv/apisrv.go index c9c8344b3..d4bfd490b 100644 --- a/cmd/stdiscosrv/apisrv.go +++ b/cmd/stdiscosrv/apisrv.go @@ -136,7 +136,12 @@ func (s *apiSrv) handler(w http.ResponseWriter, req *http.Request) { } if s.useHTTP { - remoteAddr.IP = net.ParseIP(req.Header.Get("X-Forwarded-For")) + // X-Forwarded-For can have multiple client IPs; split using the comma separator + forwardIP, _, _ := strings.Cut(req.Header.Get("X-Forwarded-For"), ",") + + // net.ParseIP will return nil if leading/trailing whitespace exists; use strings.TrimSpace() + remoteAddr.IP = net.ParseIP(strings.TrimSpace(forwardIP)) + if parsedPort, err := strconv.ParseInt(req.Header.Get("X-Client-Port"), 10, 0); err == nil { remoteAddr.Port = int(parsedPort) } @@ -410,13 +415,13 @@ func fixupAddresses(remote *net.TCPAddr, addresses []string) []string { continue } - if remote != nil { - if host == "" || ip.IsUnspecified() { + if host == "" || ip.IsUnspecified() { + if remote != nil { // Replace the unspecified IP with the request source. // ... unless the request source is the loopback address or // multicast/unspecified (can't happen, really). - if remote.IP.IsLoopback() || remote.IP.IsMulticast() || remote.IP.IsUnspecified() { + if remote.IP == nil || remote.IP.IsLoopback() || remote.IP.IsMulticast() || remote.IP.IsUnspecified() { continue } @@ -432,11 +437,22 @@ func fixupAddresses(remote *net.TCPAddr, addresses []string) []string { } host = remote.IP.String() + + } else { + // remote is nil, unable to determine host IP + continue } - // If zero port was specified, use remote port. - if port == "0" && remote.Port > 0 { + } + + // If zero port was specified, use remote port. + if port == "0" { + if remote != nil && remote.Port > 0 { + // use remote port port = strconv.Itoa(remote.Port) + } else { + // unable to determine remote port + continue } } diff --git a/cmd/stdiscosrv/apisrv_test.go b/cmd/stdiscosrv/apisrv_test.go index 3514994a3..457ed404f 100644 --- a/cmd/stdiscosrv/apisrv_test.go +++ b/cmd/stdiscosrv/apisrv_test.go @@ -69,6 +69,14 @@ func TestFixupAddresses(t *testing.T) { remote: addr("123.123.123.123", 9000), in: []string{"tcp://44.44.44.44:0"}, out: []string{"tcp://44.44.44.44:9000"}, + }, { // remote ip nil + remote: addr("", 9000), + in: []string{"tcp://:22000", "tcp://44.44.44.44:9000"}, + out: []string{"tcp://44.44.44.44:9000"}, + }, { // remote port 0 + remote: addr("123.123.123.123", 0), + in: []string{"tcp://:22000", "tcp://44.44.44.44"}, + out: []string{"tcp://123.123.123.123:22000"}, }, }