From 45e6e90d11fa518cf95a2a1b2574dcf5678e1f09 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 16 Jun 2026 06:58:30 +0000 Subject: [PATCH] types: consolidate DNS, identifier, and proto helpers --- hscontrol/types/api_key.go | 9 +--- hscontrol/types/common.go | 6 +-- hscontrol/types/config.go | 87 +++++++++++----------------------- hscontrol/types/node.go | 49 ++++++------------- hscontrol/types/preauth_key.go | 4 +- hscontrol/types/types_view.go | 4 +- hscontrol/types/users.go | 52 ++++++++------------ 7 files changed, 69 insertions(+), 142 deletions(-) diff --git a/hscontrol/types/api_key.go b/hscontrol/types/api_key.go index 552acd8b..3ad3450e 100644 --- a/hscontrol/types/api_key.go +++ b/hscontrol/types/api_key.go @@ -29,14 +29,7 @@ func (key *APIKey) Proto() *v1.ApiKey { Id: key.ID, } - // Show prefix format: distinguish between new (12-char) and legacy (7-char) keys - if len(key.Prefix) == NewAPIKeyPrefixLength { - // New format key (12-char prefix) - protoKey.Prefix = "hskey-api-" + key.Prefix + "-***" - } else { - // Legacy format key (7-char prefix) or fallback - protoKey.Prefix = key.Prefix + "***" - } + protoKey.Prefix = key.maskedPrefix() if key.Expiration != nil { protoKey.Expiration = timestamppb.New(*key.Expiration) diff --git a/hscontrol/types/common.go b/hscontrol/types/common.go index 3f0802ad..5e112800 100644 --- a/hscontrol/types/common.go +++ b/hscontrol/types/common.go @@ -241,11 +241,7 @@ func (rn *AuthRequest) FinishAuth(verdict AuthVerdict) { return } - select { - case rn.finished <- verdict: - default: - } - + rn.finished <- verdict close(rn.finished) } diff --git a/hscontrol/types/config.go b/hscontrol/types/config.go index 0293918c..a09b1ebb 100644 --- a/hscontrol/types/config.go +++ b/hscontrol/types/config.go @@ -602,7 +602,7 @@ func validateServerConfig() error { // Minimum inactivity time out is keepalive timeout (60s) plus a few seconds // to avoid races - minInactivityTimeout, _ := time.ParseDuration("65s") + minInactivityTimeout := 65 * time.Second ephemeralTimeout := resolveEphemeralInactivityTimeout() if ephemeralTimeout <= minInactivityTimeout { @@ -891,15 +891,15 @@ func dns() (DNSConfig, error) { return dns, nil } -// globalResolvers returns the global DNS resolvers -// defined in the config file. +// parseResolvers converts nameserver strings into DNS resolvers. // If a nameserver is a valid IP, it will be used as a regular resolver. // If a nameserver is a valid URL, it will be used as a DoH resolver. // If a nameserver is neither a valid URL nor a valid IP, it will be ignored. -func (d *DNSConfig) globalResolvers() []*dnstype.Resolver { +// When domain is non-empty, it is included in the warning for invalid entries. +func parseResolvers(nameservers []string, domain string) []*dnstype.Resolver { var resolvers []*dnstype.Resolver - for _, nsStr := range d.Nameservers.Global { + for _, nsStr := range nameservers { if _, err := netip.ParseAddr(nsStr); err == nil { //nolint:noinlineerr resolvers = append(resolvers, &dnstype.Resolver{ Addr: nsStr, @@ -916,43 +916,29 @@ func (d *DNSConfig) globalResolvers() []*dnstype.Resolver { continue } - log.Warn().Str("nameserver", nsStr).Msg("invalid global nameserver, ignoring") + e := log.Warn().Str("nameserver", nsStr) + if domain != "" { + e = e.Str("domain", domain) + } + + e.Msg("invalid nameserver, ignoring") } return resolvers } +// globalResolvers returns the global DNS resolvers +// defined in the config file. +func (d *DNSConfig) globalResolvers() []*dnstype.Resolver { + return parseResolvers(d.Nameservers.Global, "") +} + // splitResolvers returns a map of domain to DNS resolvers. -// If a nameserver is a valid IP, it will be used as a regular resolver. -// If a nameserver is a valid URL, it will be used as a DoH resolver. -// If a nameserver is neither a valid URL nor a valid IP, it will be ignored. func (d *DNSConfig) splitResolvers() map[string][]*dnstype.Resolver { routes := make(map[string][]*dnstype.Resolver) for domain, nameservers := range d.Nameservers.Split { - var resolvers []*dnstype.Resolver - - for _, nsStr := range nameservers { - if _, err := netip.ParseAddr(nsStr); err == nil { //nolint:noinlineerr - resolvers = append(resolvers, &dnstype.Resolver{ - Addr: nsStr, - }) - - continue - } - - if _, err := url.Parse(nsStr); err == nil { //nolint:noinlineerr - resolvers = append(resolvers, &dnstype.Resolver{ - Addr: nsStr, - }) - - continue - } - - log.Warn().Str("nameserver", nsStr).Str("domain", domain).Msg("invalid split dns nameserver, ignoring") - } - - routes[domain] = resolvers + routes[domain] = parseResolvers(nameservers, domain) } return routes @@ -1015,43 +1001,24 @@ func warnBanner(lines []string) { log.Warn().Msg(b.String()) } -func prefixV4() (*netip.Prefix, bool, error) { - prefixV4Str := viper.GetString("prefixes.v4") +func parsePrefixConfig(key string, standardRange netip.Prefix, family string) (*netip.Prefix, bool, error) { + s := viper.GetString(key) - if prefixV4Str == "" { + if s == "" { return nil, false, nil } - prefixV4, err := netip.ParsePrefix(prefixV4Str) + prefix, err := netip.ParsePrefix(s) if err != nil { - return nil, false, fmt.Errorf("parsing IPv4 prefix from config: %w", err) + return nil, false, fmt.Errorf("parsing %s prefix from config: %w", family, err) } builder := netipx.IPSetBuilder{} - builder.AddPrefix(tsaddr.CGNATRange()) + builder.AddPrefix(standardRange) ipSet, _ := builder.IPSet() - return &prefixV4, !ipSet.ContainsPrefix(prefixV4), nil -} - -func prefixV6() (*netip.Prefix, bool, error) { - prefixV6Str := viper.GetString("prefixes.v6") - - if prefixV6Str == "" { - return nil, false, nil - } - - prefixV6, err := netip.ParsePrefix(prefixV6Str) - if err != nil { - return nil, false, fmt.Errorf("parsing IPv6 prefix from config: %w", err) - } - - builder := netipx.IPSetBuilder{} - builder.AddPrefix(tsaddr.TailscaleULARange()) - ipSet, _ := builder.IPSet() - - return &prefixV6, !ipSet.ContainsPrefix(prefixV6), nil + return &prefix, !ipSet.ContainsPrefix(prefix), nil } // trustedProxies rejects 0.0.0.0/0 and ::/0 because they defeat the @@ -1108,12 +1075,12 @@ func LoadServerConfig() (*Config, error) { logConfig := logConfig() zerolog.SetGlobalLevel(logConfig.Level) - prefix4, v4NonStandard, err := prefixV4() + prefix4, v4NonStandard, err := parsePrefixConfig("prefixes.v4", tsaddr.CGNATRange(), "IPv4") if err != nil { return nil, err } - prefix6, v6NonStandard, err := prefixV6() + prefix6, v6NonStandard, err := parsePrefixConfig("prefixes.v6", tsaddr.TailscaleULARange(), "IPv6") if err != nil { return nil, err } diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go index 260936b4..6212c33e 100644 --- a/hscontrol/types/node.go +++ b/hscontrol/types/node.go @@ -664,23 +664,11 @@ func (node *Node) PeerChangeFromMapRequest(req tailcfg.MapRequest) tailcfg.PeerC ret.DiscoKey = &req.DiscoKey } - if node.Hostinfo != nil && - node.Hostinfo.NetInfo != nil && - req.Hostinfo != nil && - req.Hostinfo.NetInfo != nil && - node.Hostinfo.NetInfo.PreferredDERP != req.Hostinfo.NetInfo.PreferredDERP { - ret.DERPRegion = req.Hostinfo.NetInfo.PreferredDERP - } - if req.Hostinfo != nil && req.Hostinfo.NetInfo != nil { - // If there is no stored Hostinfo or NetInfo, use - // the new PreferredDERP. - if node.Hostinfo == nil { - ret.DERPRegion = req.Hostinfo.NetInfo.PreferredDERP - } else if node.Hostinfo.NetInfo == nil { - ret.DERPRegion = req.Hostinfo.NetInfo.PreferredDERP - } else if node.Hostinfo.NetInfo.PreferredDERP != req.Hostinfo.NetInfo.PreferredDERP { - // If there is a PreferredDERP check if it has changed. + // Use the new PreferredDERP when there is no stored Hostinfo or + // NetInfo, or when the stored PreferredDERP has changed. + if node.Hostinfo == nil || node.Hostinfo.NetInfo == nil || + node.Hostinfo.NetInfo.PreferredDERP != req.Hostinfo.NetInfo.PreferredDERP { ret.DERPRegion = req.Hostinfo.NetInfo.PreferredDERP } } @@ -699,23 +687,7 @@ func (node *Node) PeerChangeFromMapRequest(req tailcfg.MapRequest) tailcfg.PeerC // EndpointsChanged compares two endpoint slices and returns true if they differ. // The comparison is order-independent - endpoints are sorted before comparison. func EndpointsChanged(oldEndpoints, newEndpoints []netip.AddrPort) bool { - if len(oldEndpoints) != len(newEndpoints) { - return true - } - - if len(oldEndpoints) == 0 { - return false - } - - // Make copies to avoid modifying the original slices - oldCopy := slices.Clone(oldEndpoints) - newCopy := slices.Clone(newEndpoints) - - // Sort both slices to enable order-independent comparison - slices.SortFunc(oldCopy, netip.AddrPort.Compare) - slices.SortFunc(newCopy, netip.AddrPort.Compare) - - return !slices.Equal(oldCopy, newCopy) + return !equalUnordered(oldEndpoints, newEndpoints, netip.AddrPort.Compare) } func (node *Node) RegisterMethodToV1Enum() v1.RegisterMethod { @@ -1137,6 +1109,13 @@ func (nv NodeView) HasNetworkChanges(other NodeView) bool { // prefixes, order-independent. Inputs are cloned before sorting so // callers' slices are not mutated. func equalPrefixesUnordered(a, b []netip.Prefix) bool { + return equalUnordered(a, b, netip.Prefix.Compare) +} + +// equalUnordered reports whether a and b contain the same elements, +// order-independent. Inputs are cloned before sorting so callers' +// slices are not mutated. +func equalUnordered[E comparable](a, b []E, cmp func(E, E) int) bool { if len(a) != len(b) { return false } @@ -1144,8 +1123,8 @@ func equalPrefixesUnordered(a, b []netip.Prefix) bool { ac := slices.Clone(a) bc := slices.Clone(b) - slices.SortFunc(ac, netip.Prefix.Compare) - slices.SortFunc(bc, netip.Prefix.Compare) + slices.SortFunc(ac, cmp) + slices.SortFunc(bc, cmp) return slices.Equal(ac, bc) } diff --git a/hscontrol/types/preauth_key.go b/hscontrol/types/preauth_key.go index a78ff3d7..3f789fdf 100644 --- a/hscontrol/types/preauth_key.go +++ b/hscontrol/types/preauth_key.go @@ -97,8 +97,8 @@ func (key *PreAuthKey) Proto() *v1.PreAuthKey { // For new keys (with prefix/hash), show the prefix so users can identify the key // For legacy keys (with plaintext key), show the full key for backwards compatibility - if key.Prefix != "" { - protoKey.Key = "hskey-auth-" + key.Prefix + "-***" + if masked := key.maskedPrefix(); masked != "" { + protoKey.Key = masked } else if key.Key != "" { // Legacy key - show full key for backwards compatibility // TODO: Consider hiding this in a future major version diff --git a/hscontrol/types/types_view.go b/hscontrol/types/types_view.go index 51db6253..4bdaa778 100644 --- a/hscontrol/types/types_view.go +++ b/hscontrol/types/types_view.go @@ -112,7 +112,9 @@ func (v UserView) ProviderIdentifier() sql.NullString { return v.ж.ProviderIden // Provider is the origin of the user account, // same as RegistrationMethod, without authkey. -func (v UserView) Provider() string { return v.ж.Provider } +func (v UserView) Provider() string { return v.ж.Provider } + +// TODO(kradalby): See if we can fill in Gravatar here. func (v UserView) ProfilePicURL() string { return v.ж.ProfilePicURL } // A compilation failure here means this code must be regenerated, with the command at the top of this file. diff --git a/hscontrol/types/users.go b/hscontrol/types/users.go index e5a9e7a5..546f5d06 100644 --- a/hscontrol/types/users.go +++ b/hscontrol/types/users.go @@ -94,6 +94,7 @@ type User struct { // same as RegistrationMethod, without authkey. Provider string + // TODO(kradalby): See if we can fill in Gravatar here. ProfilePicURL string } @@ -134,16 +135,11 @@ func (u *User) Display() string { return cmp.Or(u.DisplayName, u.Username()) } -// TODO(kradalby): See if we can fill in Gravatar here. -func (u *User) profilePicURL() string { - return u.ProfilePicURL -} - func (u *User) TailscaleUser() tailcfg.User { return tailcfg.User{ ID: tailcfg.UserID(u.ID), //nolint:gosec // UserID is bounded DisplayName: u.Display(), - ProfilePicURL: u.profilePicURL(), + ProfilePicURL: u.ProfilePicURL, Created: u.CreatedAt, } } @@ -165,7 +161,7 @@ func (u *User) TailscaleLogin() tailcfg.Login { Provider: u.Provider, LoginName: u.Username(), DisplayName: u.Display(), - ProfilePicURL: u.profilePicURL(), + ProfilePicURL: u.ProfilePicURL, } } @@ -178,7 +174,7 @@ func (u *User) TailscaleUserProfile() tailcfg.UserProfile { ID: tailcfg.UserID(u.ID), //nolint:gosec // UserID is bounded LoginName: u.Username(), DisplayName: u.Display(), - ProfilePicURL: u.profilePicURL(), + ProfilePicURL: u.ProfilePicURL, } } @@ -366,19 +362,7 @@ func CleanIdentifier(identifier string) string { u, err := url.Parse(identifier) if err == nil && u.Scheme != "" { // Clean path by removing empty segments and whitespace within segments - parts := strings.FieldsFunc(u.Path, func(c rune) bool { return c == '/' }) - for i, part := range parts { - parts[i] = strings.TrimSpace(part) - } - // Remove empty parts after trimming - cleanParts := make([]string, 0, len(parts)) - for _, part := range parts { - if part != "" { - cleanParts = append(cleanParts, part) - } - } - - if len(cleanParts) == 0 { + if cleanParts := cleanSlashSegments(u.Path); len(cleanParts) == 0 { u.Path = "" } else { u.Path = "/" + strings.Join(cleanParts, "/") @@ -390,16 +374,7 @@ func CleanIdentifier(identifier string) string { } // Handle non-URL identifiers - parts := strings.FieldsFunc(identifier, func(c rune) bool { return c == '/' }) - // Clean whitespace from each part - cleanParts := make([]string, 0, len(parts)) - for _, part := range parts { - trimmed := strings.TrimSpace(part) - if trimmed != "" { - cleanParts = append(cleanParts, trimmed) - } - } - + cleanParts := cleanSlashSegments(identifier) if len(cleanParts) == 0 { return "" } @@ -407,6 +382,21 @@ func CleanIdentifier(identifier string) string { return strings.Join(cleanParts, "/") } +// cleanSlashSegments splits s on '/', trims whitespace from each segment, and +// returns the non-empty segments. +func cleanSlashSegments(s string) []string { + parts := strings.FieldsFunc(s, func(c rune) bool { return c == '/' }) + + cleanParts := make([]string, 0, len(parts)) + for _, part := range parts { + if part = strings.TrimSpace(part); part != "" { + cleanParts = append(cleanParts, part) + } + } + + return cleanParts +} + type OIDCUserInfo struct { Sub string `json:"sub"` Name string `json:"name"`