From c991278e45f5d9da32160bf6b54a602618f39faa Mon Sep 17 00:00:00 2001 From: unknown Date: Sun, 24 Sep 2023 20:48:44 -0700 Subject: [PATCH 1/7] Added Warning Log when either Well_known_client or well known server dont have http || https prefix --- clientapi/routing/routing.go | 4 ++++ federationapi/routing/routing.go | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index d4aa1d08d1..89de79f432 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -129,6 +129,10 @@ func Setup( })).Methods(http.MethodGet, http.MethodOptions) } + if !strings.HasPrefix(string(cfg.Matrix.ServerName), "http://") && !strings.HasPrefix(string(cfg.Matrix.ServerName), "https://") { + logrus.Warn("The well_known_server_name does not start with http:// or https:// does not start with 'http://' or 'https://'. Some clients may fail to connect.") + } + publicAPIMux.Handle("/versions", httputil.MakeExternalAPI("versions", func(req *http.Request) util.JSONResponse { return util.JSONResponse{ diff --git a/federationapi/routing/routing.go b/federationapi/routing/routing.go index dc7a363e72..ac651a0204 100644 --- a/federationapi/routing/routing.go +++ b/federationapi/routing/routing.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "net/http" + "strings" "sync" "time" @@ -123,6 +124,10 @@ func Setup( ).Methods(http.MethodGet, http.MethodOptions) } + if !strings.HasPrefix(string(cfg.Matrix.ServerName), "http://") && !strings.HasPrefix(string(cfg.Matrix.ServerName), "https://") { + logrus.Warn("The well_known_server_name does not start with http:// or https:// does not start with 'http://' or 'https://'. Some clients may fail to connect.") + } + // Ignore the {keyID} argument as we only have a single server key so we always // return that key. // Even if we had more than one server key, we would probably still ignore the From 56fbe9d62b276f3c4370a5483ba9372946282496 Mon Sep 17 00:00:00 2001 From: unknown Date: Sun, 24 Sep 2023 20:54:51 -0700 Subject: [PATCH 2/7] Do something about invalid well-known names --- clientapi/routing/routing.go | 2 +- federationapi/routing/routing.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index 89de79f432..93021fc410 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -129,7 +129,7 @@ func Setup( })).Methods(http.MethodGet, http.MethodOptions) } - if !strings.HasPrefix(string(cfg.Matrix.ServerName), "http://") && !strings.HasPrefix(string(cfg.Matrix.ServerName), "https://") { + if !strings.HasPrefix(string(cfg.Matrix.ServerName), "http://") || !strings.HasPrefix(string(cfg.Matrix.ServerName), "https://") { logrus.Warn("The well_known_server_name does not start with http:// or https:// does not start with 'http://' or 'https://'. Some clients may fail to connect.") } diff --git a/federationapi/routing/routing.go b/federationapi/routing/routing.go index ac651a0204..59b80307b6 100644 --- a/federationapi/routing/routing.go +++ b/federationapi/routing/routing.go @@ -124,7 +124,7 @@ func Setup( ).Methods(http.MethodGet, http.MethodOptions) } - if !strings.HasPrefix(string(cfg.Matrix.ServerName), "http://") && !strings.HasPrefix(string(cfg.Matrix.ServerName), "https://") { + if !strings.HasPrefix(string(cfg.Matrix.ServerName), "http://") || !strings.HasPrefix(string(cfg.Matrix.ServerName), "https://") { logrus.Warn("The well_known_server_name does not start with http:// or https:// does not start with 'http://' or 'https://'. Some clients may fail to connect.") } From b35a44c19dbfdfab72f96ec2b2e677c859ce9430 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Sat, 25 Nov 2023 20:52:36 +0100 Subject: [PATCH 3/7] Update prefix checking --- clientapi/routing/routing.go | 4 ---- federationapi/routing/routing.go | 5 ----- setup/config/config_global.go | 6 ++++++ 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index 93021fc410..d4aa1d08d1 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -129,10 +129,6 @@ func Setup( })).Methods(http.MethodGet, http.MethodOptions) } - if !strings.HasPrefix(string(cfg.Matrix.ServerName), "http://") || !strings.HasPrefix(string(cfg.Matrix.ServerName), "https://") { - logrus.Warn("The well_known_server_name does not start with http:// or https:// does not start with 'http://' or 'https://'. Some clients may fail to connect.") - } - publicAPIMux.Handle("/versions", httputil.MakeExternalAPI("versions", func(req *http.Request) util.JSONResponse { return util.JSONResponse{ diff --git a/federationapi/routing/routing.go b/federationapi/routing/routing.go index 59b80307b6..dc7a363e72 100644 --- a/federationapi/routing/routing.go +++ b/federationapi/routing/routing.go @@ -18,7 +18,6 @@ import ( "context" "fmt" "net/http" - "strings" "sync" "time" @@ -124,10 +123,6 @@ func Setup( ).Methods(http.MethodGet, http.MethodOptions) } - if !strings.HasPrefix(string(cfg.Matrix.ServerName), "http://") || !strings.HasPrefix(string(cfg.Matrix.ServerName), "https://") { - logrus.Warn("The well_known_server_name does not start with http:// or https:// does not start with 'http://' or 'https://'. Some clients may fail to connect.") - } - // Ignore the {keyID} argument as we only have a single server key so we always // return that key. // Even if we had more than one server key, we would probably still ignore the diff --git a/setup/config/config_global.go b/setup/config/config_global.go index 5b4ccf4002..a5444d5706 100644 --- a/setup/config/config_global.go +++ b/setup/config/config_global.go @@ -10,6 +10,7 @@ import ( "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/gomatrixserverlib/fclient" "github.com/matrix-org/gomatrixserverlib/spec" + "github.com/sirupsen/logrus" "golang.org/x/crypto/ed25519" ) @@ -114,6 +115,11 @@ func (c *Global) Verify(configErrs *ConfigErrors) { checkNotEmpty(configErrs, "global.server_name", string(c.ServerName)) checkNotEmpty(configErrs, "global.private_key", string(c.PrivateKeyPath)) + // Check that client well-known has a proper format + if c.WellKnownClientName != "" && !(strings.HasPrefix(c.WellKnownClientName, "http://") || strings.HasPrefix(c.WellKnownClientName, "https://")) { + logrus.Warn("The configuration for well_known_client_name does not have a proper format, consider adding http:// or https://. Some clients may fail to connect.") + } + for _, v := range c.VirtualHosts { v.Verify(configErrs) } From e45b125d3abc2f197b64cab5446e1293db7c37a6 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Sat, 25 Nov 2023 20:58:08 +0100 Subject: [PATCH 4/7] Use url.Parse instead of verifying just the prefix --- setup/config/config_global.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/setup/config/config_global.go b/setup/config/config_global.go index a5444d5706..0d9b79202e 100644 --- a/setup/config/config_global.go +++ b/setup/config/config_global.go @@ -3,6 +3,7 @@ package config import ( "fmt" "math/rand" + "net/url" "strconv" "strings" "time" @@ -116,8 +117,14 @@ func (c *Global) Verify(configErrs *ConfigErrors) { checkNotEmpty(configErrs, "global.private_key", string(c.PrivateKeyPath)) // Check that client well-known has a proper format - if c.WellKnownClientName != "" && !(strings.HasPrefix(c.WellKnownClientName, "http://") || strings.HasPrefix(c.WellKnownClientName, "https://")) { - logrus.Warn("The configuration for well_known_client_name does not have a proper format, consider adding http:// or https://. Some clients may fail to connect.") + + if c.WellKnownClientName != "" { + parsedURL, err := url.Parse(c.WellKnownClientName) + if err != nil { + logrus.WithError(err).Warn("The configuration for well_known_client_name does not have a proper format, consider adding http:// or https://. Some clients may fail to connect.") + } else { + c.WellKnownClientName = parsedURL.String() + } } for _, v := range c.VirtualHosts { From 6fe9d0b531236a1ecdb17b2d6cfca1e0afaf86f0 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Sat, 25 Nov 2023 21:19:47 +0100 Subject: [PATCH 5/7] Revert "Use url.Parse instead of verifying just the prefix" This reverts commit e45b125d3abc2f197b64cab5446e1293db7c37a6. --- setup/config/config_global.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/setup/config/config_global.go b/setup/config/config_global.go index 0d9b79202e..a5444d5706 100644 --- a/setup/config/config_global.go +++ b/setup/config/config_global.go @@ -3,7 +3,6 @@ package config import ( "fmt" "math/rand" - "net/url" "strconv" "strings" "time" @@ -117,14 +116,8 @@ func (c *Global) Verify(configErrs *ConfigErrors) { checkNotEmpty(configErrs, "global.private_key", string(c.PrivateKeyPath)) // Check that client well-known has a proper format - - if c.WellKnownClientName != "" { - parsedURL, err := url.Parse(c.WellKnownClientName) - if err != nil { - logrus.WithError(err).Warn("The configuration for well_known_client_name does not have a proper format, consider adding http:// or https://. Some clients may fail to connect.") - } else { - c.WellKnownClientName = parsedURL.String() - } + if c.WellKnownClientName != "" && !(strings.HasPrefix(c.WellKnownClientName, "http://") || strings.HasPrefix(c.WellKnownClientName, "https://")) { + logrus.Warn("The configuration for well_known_client_name does not have a proper format, consider adding http:// or https://. Some clients may fail to connect.") } for _, v := range c.VirtualHosts { From 55c10b52cf6ffb6a1bbba498aaed62a0c3636d08 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Sat, 25 Nov 2023 21:37:07 +0100 Subject: [PATCH 6/7] Fail startup if well_known_client_name is configured incorrectly --- setup/config/config_global.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/setup/config/config_global.go b/setup/config/config_global.go index a5444d5706..3234dadb4d 100644 --- a/setup/config/config_global.go +++ b/setup/config/config_global.go @@ -10,7 +10,6 @@ import ( "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/gomatrixserverlib/fclient" "github.com/matrix-org/gomatrixserverlib/spec" - "github.com/sirupsen/logrus" "golang.org/x/crypto/ed25519" ) @@ -116,8 +115,8 @@ func (c *Global) Verify(configErrs *ConfigErrors) { checkNotEmpty(configErrs, "global.private_key", string(c.PrivateKeyPath)) // Check that client well-known has a proper format - if c.WellKnownClientName != "" && !(strings.HasPrefix(c.WellKnownClientName, "http://") || strings.HasPrefix(c.WellKnownClientName, "https://")) { - logrus.Warn("The configuration for well_known_client_name does not have a proper format, consider adding http:// or https://. Some clients may fail to connect.") + if c.WellKnownClientName != "" && !strings.HasPrefix(c.WellKnownClientName, "http://") && !strings.HasPrefix(c.WellKnownClientName, "https://") { + configErrs.Add("The configuration for well_known_client_name does not have a proper format, consider adding http:// or https://. Some clients may fail to connect.") } for _, v := range c.VirtualHosts { From 972c51dda910bdb3d4f4986d314b207d4a500a32 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Sat, 25 Nov 2023 21:58:03 +0100 Subject: [PATCH 7/7] Fix test --- setup/config/config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup/config/config_test.go b/setup/config/config_test.go index 8a65c990f8..eeefb425f5 100644 --- a/setup/config/config_test.go +++ b/setup/config/config_test.go @@ -54,7 +54,7 @@ global: key_id: ed25519:auto key_validity_period: 168h0m0s well_known_server_name: "localhost:443" - well_known_client_name: "localhost:443" + well_known_client_name: "https://localhost" trusted_third_party_id_servers: - matrix.org - vector.im