From a52a00473732450ac1a1e002336fc812233ddf20 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 17 May 2026 11:25:53 +0000 Subject: [PATCH] management: validate and normalize relay endpoint transports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds Relay.HasURLs/AllURLs helpers so all relay-enabled checks use the same definition ("any URL is configured, regardless of whether it came from Addresses or Endpoints"), and a Normalize step run at startup that: - Drops unknown transport identifiers (anything outside ws/quic/wt), reporting them via a warning log so misconfigurations surface loudly instead of being silently advertised to clients. - De-duplicates endpoint URLs and per-endpoint transports. - Preserves empty Transports lists — that's the valid "unknown, let the client try whatever it supports" signal, distinct from a typo'd list. The two relay-token guard sites in server.go switch to HasURLs so a deployment with only Endpoints (no Addresses) actually issues tokens; before this change those branches would skip token generation. The startup log now prints the merged URL list plus per-endpoint transport hints, matching what clients will receive. --- management/cmd/management.go | 10 +- management/internals/server/config/config.go | 114 ++++++++++++++++++ .../internals/server/config/relay_test.go | 99 +++++++++++++++ management/internals/shared/grpc/server.go | 4 +- 4 files changed, 224 insertions(+), 3 deletions(-) create mode 100644 management/internals/server/config/relay_test.go diff --git a/management/cmd/management.go b/management/cmd/management.go index 27d8055e7..6cde19713 100644 --- a/management/cmd/management.go +++ b/management/cmd/management.go @@ -320,7 +320,15 @@ func LogConfigInfo(cfg *nbconfig.Config) { log.Infof("running with the embedded IdP: %v", cfg.EmbeddedIdP.Issuer) } if cfg.Relay != nil { - log.Infof("Relay addresses: %v", cfg.Relay.Addresses) + if unknown := cfg.Relay.Normalize(); len(unknown) > 0 { + log.Warnf("Relay config contains unknown transport identifiers (dropped): %v", unknown) + } + log.Infof("Relay URLs: %v", cfg.Relay.AllURLs()) + for _, ep := range cfg.Relay.Endpoints { + if len(ep.Transports) > 0 { + log.Infof("Relay endpoint %s advertises transports: %v", ep.URL, ep.Transports) + } + } } } diff --git a/management/internals/server/config/config.go b/management/internals/server/config/config.go index f29d7f682..6e965fe42 100644 --- a/management/internals/server/config/config.go +++ b/management/internals/server/config/config.go @@ -114,6 +114,120 @@ type RelayEndpoint struct { Transports []string } +// KnownRelayTransports is the set of transport identifiers the management +// server accepts in RelayEndpoint.Transports. Anything outside this set is +// silently dropped at config load — we don't want a typo in Transports to +// turn into clients trying a dialer that doesn't exist. +var KnownRelayTransports = map[string]struct{}{ + "ws": {}, + "quic": {}, + "wt": {}, +} + +// HasURLs reports whether any relay address is configured (either via the +// legacy Addresses slice or via Endpoints). Callers that only care +// "is the relay feature on for this server" should use this rather than +// checking either field directly. +func (r *Relay) HasURLs() bool { + if r == nil { + return false + } + if len(r.Addresses) > 0 { + return true + } + for _, ep := range r.Endpoints { + if ep.URL != "" { + return true + } + } + return false +} + +// AllURLs returns every relay URL the management server will advertise, +// preserving order with Endpoints listed first and any Addresses not also +// covered by an Endpoint appended after. Used for logging and for callers +// that want a flat URL list without caring about transport hints. +func (r *Relay) AllURLs() []string { + if r == nil { + return nil + } + seen := make(map[string]struct{}, len(r.Endpoints)+len(r.Addresses)) + out := make([]string, 0, len(r.Endpoints)+len(r.Addresses)) + for _, ep := range r.Endpoints { + if ep.URL == "" { + continue + } + if _, ok := seen[ep.URL]; ok { + continue + } + seen[ep.URL] = struct{}{} + out = append(out, ep.URL) + } + for _, addr := range r.Addresses { + if addr == "" { + continue + } + if _, ok := seen[addr]; ok { + continue + } + seen[addr] = struct{}{} + out = append(out, addr) + } + return out +} + +// Normalize trims unknown transport identifiers from each Endpoint, dropping +// dupes and empty URLs. Returns the unknown transports it discarded so the +// caller can surface them as a warning at startup. +// +// Does not error on empty Transports — an empty list is a valid "unknown, +// try everything" signal, distinct from "I tried to declare it but typoed". +func (r *Relay) Normalize() (unknownTransports []string) { + if r == nil { + return nil + } + if len(r.Endpoints) == 0 { + return nil + } + dropped := map[string]struct{}{} + urlSeen := make(map[string]struct{}, len(r.Endpoints)) + cleaned := make([]RelayEndpoint, 0, len(r.Endpoints)) + for _, ep := range r.Endpoints { + if ep.URL == "" { + continue + } + if _, dup := urlSeen[ep.URL]; dup { + continue + } + urlSeen[ep.URL] = struct{}{} + + filtered := make([]string, 0, len(ep.Transports)) + tSeen := make(map[string]struct{}, len(ep.Transports)) + for _, t := range ep.Transports { + if _, ok := KnownRelayTransports[t]; !ok { + dropped[t] = struct{}{} + continue + } + if _, ok := tSeen[t]; ok { + continue + } + tSeen[t] = struct{}{} + filtered = append(filtered, t) + } + cleaned = append(cleaned, RelayEndpoint{URL: ep.URL, Transports: filtered}) + } + r.Endpoints = cleaned + + if len(dropped) == 0 { + return nil + } + out := make([]string, 0, len(dropped)) + for t := range dropped { + out = append(out, t) + } + return out +} + // HttpServerConfig is a config of the HTTP Management service server type HttpServerConfig struct { LetsEncryptDomain string diff --git a/management/internals/server/config/relay_test.go b/management/internals/server/config/relay_test.go new file mode 100644 index 000000000..cea74f1cc --- /dev/null +++ b/management/internals/server/config/relay_test.go @@ -0,0 +1,99 @@ +package config + +import ( + "sort" + "testing" +) + +func TestRelay_HasURLs(t *testing.T) { + t.Parallel() + cases := []struct { + name string + r *Relay + want bool + }{ + {name: "nil", r: nil, want: false}, + {name: "empty", r: &Relay{}, want: false}, + {name: "addresses only", r: &Relay{Addresses: []string{"rels://a"}}, want: true}, + {name: "endpoints only", r: &Relay{Endpoints: []RelayEndpoint{{URL: "rels://a"}}}, want: true}, + {name: "endpoint with empty url", r: &Relay{Endpoints: []RelayEndpoint{{URL: ""}}}, want: false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := tc.r.HasURLs(); got != tc.want { + t.Fatalf("HasURLs = %v, want %v", got, tc.want) + } + }) + } +} + +func TestRelay_AllURLs_dedupes_and_preserves_order(t *testing.T) { + t.Parallel() + r := &Relay{ + Addresses: []string{"rels://shared", "rels://addr-only"}, + Endpoints: []RelayEndpoint{ + {URL: "rels://ep-only"}, + {URL: "rels://shared"}, // also in Addresses; should not double up + {URL: ""}, // skipped + }, + } + got := r.AllURLs() + want := []string{"rels://ep-only", "rels://shared", "rels://addr-only"} + if len(got) != len(want) { + t.Fatalf("AllURLs = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("AllURLs[%d] = %q, want %q", i, got[i], want[i]) + } + } +} + +func TestRelay_Normalize_drops_unknown_transports_and_dupes(t *testing.T) { + t.Parallel() + r := &Relay{ + Endpoints: []RelayEndpoint{ + {URL: "rels://a", Transports: []string{"ws", "ws", "wt", "bogus", "h2"}}, + {URL: "rels://b", Transports: []string{"quic", "wt"}}, + {URL: "rels://a", Transports: []string{"ws"}}, // duplicate URL — dropped + {URL: ""}, // empty URL — dropped + }, + } + unknown := r.Normalize() + + // Unknown transports should be reported (order not specified). + sort.Strings(unknown) + wantUnknown := []string{"bogus", "h2"} + if len(unknown) != len(wantUnknown) { + t.Fatalf("unknown = %v, want %v", unknown, wantUnknown) + } + for i := range wantUnknown { + if unknown[i] != wantUnknown[i] { + t.Fatalf("unknown[%d] = %q, want %q", i, unknown[i], wantUnknown[i]) + } + } + + if len(r.Endpoints) != 2 { + t.Fatalf("endpoints after Normalize = %d, want 2: %#v", len(r.Endpoints), r.Endpoints) + } + if r.Endpoints[0].URL != "rels://a" || len(r.Endpoints[0].Transports) != 2 || + r.Endpoints[0].Transports[0] != "ws" || r.Endpoints[0].Transports[1] != "wt" { + t.Fatalf("endpoint a after Normalize = %#v", r.Endpoints[0]) + } + if r.Endpoints[1].URL != "rels://b" || len(r.Endpoints[1].Transports) != 2 { + t.Fatalf("endpoint b after Normalize = %#v", r.Endpoints[1]) + } +} + +func TestRelay_Normalize_keeps_empty_transports(t *testing.T) { + t.Parallel() + // An empty Transports list is "unknown — try every dialer", which is a + // valid signal we must preserve (distinct from "I typoed all entries"). + r := &Relay{Endpoints: []RelayEndpoint{{URL: "rels://a"}}} + if u := r.Normalize(); len(u) != 0 { + t.Fatalf("Normalize reported unknown transports on empty list: %v", u) + } + if len(r.Endpoints) != 1 || len(r.Endpoints[0].Transports) != 0 { + t.Fatalf("endpoint mutated: %#v", r.Endpoints) + } +} diff --git a/management/internals/shared/grpc/server.go b/management/internals/shared/grpc/server.go index 1d8234304..970b04b4e 100644 --- a/management/internals/shared/grpc/server.go +++ b/management/internals/shared/grpc/server.go @@ -824,7 +824,7 @@ func (s *Server) Login(ctx context.Context, req *proto.EncryptedMessage) (*proto func (s *Server) prepareLoginResponse(ctx context.Context, peer *nbpeer.Peer, netMap *types.NetworkMap, postureChecks []*posture.Checks) (*proto.LoginResponse, error) { var relayToken *Token var err error - if s.config.Relay != nil && len(s.config.Relay.Addresses) > 0 { + if s.config.Relay.HasURLs() { relayToken, err = s.secretsManager.GenerateRelayToken() if err != nil { log.Errorf("failed generating Relay token: %v", err) @@ -915,7 +915,7 @@ func (s *Server) sendInitialSync(ctx context.Context, peerKey wgtypes.Key, peer } var relayToken *Token - if s.config.Relay != nil && len(s.config.Relay.Addresses) > 0 { + if s.config.Relay.HasURLs() { relayToken, err = s.secretsManager.GenerateRelayToken() if err != nil { log.Errorf("failed generating Relay token: %v", err)