From d9210b8b1fb08aaefde6f105f173a130fcf09df3 Mon Sep 17 00:00:00 2001
From: Joachim Bauch <bauch@struktur.de>
Date: Mon, 30 Nov 2020 11:42:18 +0100
Subject: [PATCH] Canonicalize all URLs before comparisons / lookups.

Otherwise urls like "https://server:443/foo/" will fail to find entries for
"https://server/foo/" and vice versa.
---
 src/signaling/api_signaling.go              | 20 ++++++++++++++++++++
 src/signaling/backend_configuration.go      |  9 +++++++++
 src/signaling/backend_configuration_test.go |  6 +++++-
 src/signaling/clientsession.go              |  4 ++++
 4 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/src/signaling/api_signaling.go b/src/signaling/api_signaling.go
index c69de5c..8fdebe5 100644
--- a/src/signaling/api_signaling.go
+++ b/src/signaling/api_signaling.go
@@ -25,6 +25,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/url"
+	"strings"
 )
 
 const (
@@ -203,6 +204,17 @@ const (
 	HelloClientTypeVirtual = "virtual"
 )
 
+func hasStandardPort(u *url.URL) bool {
+	switch u.Scheme {
+	case "http":
+		return u.Port() == "80"
+	case "https":
+		return u.Port() == "443"
+	default:
+		return false
+	}
+}
+
 type ClientTypeInternalAuthParams struct {
 	Random string `json:"random"`
 	Token  string `json:"token"`
@@ -217,6 +229,10 @@ func (p *ClientTypeInternalAuthParams) CheckValid() error {
 	} else if u, err := url.Parse(p.Backend); err != nil {
 		return err
 	} else {
+		if strings.Contains(u.Host, ":") && hasStandardPort(u) {
+			u.Host = u.Hostname()
+		}
+
 		p.parsedBackend = u
 	}
 	return nil
@@ -266,6 +282,10 @@ func (m *HelloClientMessage) CheckValid() error {
 			} else if u, err := url.ParseRequestURI(m.Auth.Url); err != nil {
 				return err
 			} else {
+				if strings.Contains(u.Host, ":") && hasStandardPort(u) {
+					u.Host = u.Hostname()
+				}
+
 				m.Auth.parsedUrl = u
 			}
 		case HelloClientTypeInternal:
diff --git a/src/signaling/backend_configuration.go b/src/signaling/backend_configuration.go
index 2d28d1e..a1ecc35 100644
--- a/src/signaling/backend_configuration.go
+++ b/src/signaling/backend_configuration.go
@@ -197,6 +197,11 @@ func getConfiguredHosts(backendIds string, config *goconf.ConfigFile) (hosts map
 			continue
 		}
 
+		if strings.Contains(parsed.Host, ":") && hasStandardPort(parsed) {
+			parsed.Host = parsed.Hostname()
+			u = parsed.String()
+		}
+
 		secret, _ := config.GetString(id, "secret")
 		if u == "" || secret == "" {
 			log.Printf("Backend %s is missing or incomplete, skipping", id)
@@ -241,6 +246,10 @@ func (b *BackendConfiguration) GetCompatBackend() *Backend {
 }
 
 func (b *BackendConfiguration) GetBackend(u *url.URL) *Backend {
+	if strings.Contains(u.Host, ":") && hasStandardPort(u) {
+		u.Host = u.Hostname()
+	}
+
 	entries, found := b.backends[u.Host]
 	if !found {
 		if b.allowAll {
diff --git a/src/signaling/backend_configuration_test.go b/src/signaling/backend_configuration_test.go
index 208376e..1a468e1 100644
--- a/src/signaling/backend_configuration_test.go
+++ b/src/signaling/backend_configuration_test.go
@@ -101,9 +101,11 @@ func TestIsUrlAllowed(t *testing.T) {
 	valid_urls := [][]string{
 		[]string{"https://domain.invalid/foo", string(testBackendSecret) + "-foo"},
 		[]string{"https://domain.invalid/foo/", string(testBackendSecret) + "-foo"},
+		[]string{"https://domain.invalid:443/foo/", string(testBackendSecret) + "-foo"},
 		[]string{"https://domain.invalid/foo/folder", string(testBackendSecret) + "-foo"},
 		[]string{"https://domain.invalid/bar", string(testBackendSecret) + "-bar"},
 		[]string{"https://domain.invalid/bar/", string(testBackendSecret) + "-bar"},
+		[]string{"https://domain.invalid:443/bar/", string(testBackendSecret) + "-bar"},
 		[]string{"https://domain.invalid/bar/folder/", string(testBackendSecret) + "-bar"},
 		[]string{"https://otherdomain.invalid/", string(testBackendSecret) + "-lala"},
 		[]string{"https://otherdomain.invalid/folder/", string(testBackendSecret) + "-lala"},
@@ -111,6 +113,7 @@ func TestIsUrlAllowed(t *testing.T) {
 	invalid_urls := []string{
 		"https://domain.invalid",
 		"https://domain.invalid/",
+		"https://domain.invalid:8443/foo/",
 		"https://www.domain.invalid/foo/",
 		"https://domain.invalid/baz/",
 	}
@@ -118,7 +121,7 @@ func TestIsUrlAllowed(t *testing.T) {
 	config.AddOption("backend", "backends", "foo, bar, lala, missing")
 	config.AddOption("foo", "url", "https://domain.invalid/foo")
 	config.AddOption("foo", "secret", string(testBackendSecret)+"-foo")
-	config.AddOption("bar", "url", "https://domain.invalid/bar/")
+	config.AddOption("bar", "url", "https://domain.invalid:443/bar/")
 	config.AddOption("bar", "secret", string(testBackendSecret)+"-bar")
 	config.AddOption("lala", "url", "https://otherdomain.invalid/")
 	config.AddOption("lala", "secret", string(testBackendSecret)+"-lala")
@@ -150,6 +153,7 @@ func TestIsUrlAllowed_AllowAll(t *testing.T) {
 	valid_urls := []string{
 		"http://domain.invalid",
 		"https://domain.invalid",
+		"https://domain.invalid:443",
 	}
 	invalid_urls := []string{
 		"domain.invalid",
diff --git a/src/signaling/clientsession.go b/src/signaling/clientsession.go
index cf60a59..33bb671 100644
--- a/src/signaling/clientsession.go
+++ b/src/signaling/clientsession.go
@@ -123,6 +123,10 @@ func NewClientSession(hub *Hub, privateId string, publicId string, data *Session
 		if u, err := url.Parse(backendUrl); err != nil {
 			return nil, err
 		} else {
+			if strings.Contains(u.Host, ":") && hasStandardPort(u) {
+				u.Host = u.Hostname()
+			}
+
 			s.backendUrl = backendUrl
 			s.parsedBackendUrl = u
 		}