From 58020696fee08981f6da197998661e7951366f87 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 28 Jan 2026 13:37:22 +0000 Subject: [PATCH] zlog: add utility package for safe and consistent logging Add hscontrol/util/zlog package with: - zf subpackage: field name constants for compile-time safety - SafeHostinfo: wrapper that redacts device fingerprinting data - SafeMapRequest: wrapper that redacts client endpoints The zf (zerolog fields) subpackage provides short constant names (e.g., zf.NodeID instead of inline "node.id" strings) ensuring consistent field naming across all log statements. Security considerations: - SafeHostinfo never logs: OSVersion, DeviceModel, DistroName - SafeMapRequest only logs endpoint counts, not actual IPs --- hscontrol/util/zlog/fields.go | 31 ++++ hscontrol/util/zlog/hostinfo.go | 61 ++++++++ hscontrol/util/zlog/maprequest.go | 51 +++++++ hscontrol/util/zlog/zf/fields.go | 185 ++++++++++++++++++++++++ hscontrol/util/zlog/zlog_test.go | 227 ++++++++++++++++++++++++++++++ 5 files changed, 555 insertions(+) create mode 100644 hscontrol/util/zlog/fields.go create mode 100644 hscontrol/util/zlog/hostinfo.go create mode 100644 hscontrol/util/zlog/maprequest.go create mode 100644 hscontrol/util/zlog/zf/fields.go create mode 100644 hscontrol/util/zlog/zlog_test.go diff --git a/hscontrol/util/zlog/fields.go b/hscontrol/util/zlog/fields.go new file mode 100644 index 00000000..978b311f --- /dev/null +++ b/hscontrol/util/zlog/fields.go @@ -0,0 +1,31 @@ +// Package zlog provides zerolog utilities for safe and consistent logging. +// +// This package contains: +// - Safe wrapper types for external types (tailcfg.Hostinfo, tailcfg.MapRequest) +// that implement LogObjectMarshaler with security-conscious field redaction +// +// For field name constants, use the zf subpackage: +// +// import "github.com/juanfont/headscale/hscontrol/util/zlog/zf" +// +// # Usage Pattern: Sub-Loggers +// +// The recommended pattern is to create sub-loggers at function entry points: +// +// func (m *mapSession) serve() { +// log := log.With(). +// EmbedObject(m.node). +// EmbedObject(zlog.MapRequest(&m.req)). +// Logger() +// +// log.Info().Msg("Map session started") +// log.Debug().Caller().Msg("Processing request") +// } +// +// # Security Considerations +// +// The wrapper types in this package intentionally redact sensitive information: +// - Device fingerprinting data (OS version, device model, etc.) +// - Client endpoints and IP addresses +// - Full authentication keys (only prefixes are logged) +package zlog diff --git a/hscontrol/util/zlog/hostinfo.go b/hscontrol/util/zlog/hostinfo.go new file mode 100644 index 00000000..3152b8b2 --- /dev/null +++ b/hscontrol/util/zlog/hostinfo.go @@ -0,0 +1,61 @@ +package zlog + +import ( + "github.com/juanfont/headscale/hscontrol/util/zlog/zf" + "github.com/rs/zerolog" + "tailscale.com/tailcfg" +) + +// SafeHostinfo wraps tailcfg.Hostinfo for safe logging. +// +// SECURITY: This wrapper intentionally redacts device fingerprinting data +// that could be used to identify or track specific devices: +// - OSVersion, DeviceModel, DistroName, DistroVersion (device fingerprinting) +// - IPNVersion (client version fingerprinting) +// - Machine, FrontendLogID (device identifiers) +// +// Only safe fields are logged: +// - hostname: The device hostname +// - os: The OS family (e.g., "linux", "windows") without version +// - routable_ips_count: Number of advertised routes (not the actual routes) +// - request_tags: Tags requested by the client +// - derp: Preferred DERP region ID +type SafeHostinfo struct { + hi *tailcfg.Hostinfo +} + +// Hostinfo creates a SafeHostinfo wrapper for safe logging. +func Hostinfo(hi *tailcfg.Hostinfo) SafeHostinfo { + return SafeHostinfo{hi: hi} +} + +// MarshalZerologObject implements zerolog.LogObjectMarshaler. +func (s SafeHostinfo) MarshalZerologObject(e *zerolog.Event) { + if s.hi == nil { + return + } + + // Safe fields only - no device fingerprinting data. + e.Str(zf.Hostname, s.hi.Hostname) + e.Str(zf.OS, s.hi.OS) // OS family only, NOT version + + if len(s.hi.RoutableIPs) > 0 { + e.Int(zf.RoutableIPCount, len(s.hi.RoutableIPs)) + } + + if len(s.hi.RequestTags) > 0 { + e.Strs(zf.RequestTags, s.hi.RequestTags) + } + + if s.hi.NetInfo != nil && s.hi.NetInfo.PreferredDERP != 0 { + e.Int(zf.DERP, s.hi.NetInfo.PreferredDERP) + } + + // SECURITY: The following fields are intentionally NOT logged: + // - OSVersion, DistroName, DistroVersion, DistroCodeName: device fingerprinting + // - DeviceModel: device fingerprinting + // - IPNVersion: client version fingerprinting + // - Machine, FrontendLogID: device identifiers + // - GoArch, GoArchVar, GoVersion: build environment fingerprinting + // - Userspace, UserspaceRouter: network configuration details +} diff --git a/hscontrol/util/zlog/maprequest.go b/hscontrol/util/zlog/maprequest.go new file mode 100644 index 00000000..86cff8fa --- /dev/null +++ b/hscontrol/util/zlog/maprequest.go @@ -0,0 +1,51 @@ +package zlog + +import ( + "github.com/juanfont/headscale/hscontrol/util/zlog/zf" + "github.com/rs/zerolog" + "tailscale.com/tailcfg" +) + +// SafeMapRequest wraps tailcfg.MapRequest for safe logging. +// +// SECURITY: This wrapper does not log sensitive information: +// - Endpoints: Client IP addresses and ports +// - Hostinfo: Device fingerprinting data (handled by SafeHostinfo) +// - DERPForceWebsockets: Network configuration details +// +// Only safe fields are logged: +// - stream: Whether this is a streaming request +// - omit_peers: Whether peers should be omitted +// - version: Client capability version +// - node.key: Short form of the node key +// - endpoints_count: Number of endpoints (not the actual endpoints) +type SafeMapRequest struct { + req *tailcfg.MapRequest +} + +// MapRequest creates a SafeMapRequest wrapper for safe logging. +func MapRequest(req *tailcfg.MapRequest) SafeMapRequest { + return SafeMapRequest{req: req} +} + +// MarshalZerologObject implements zerolog.LogObjectMarshaler. +func (s SafeMapRequest) MarshalZerologObject(e *zerolog.Event) { + if s.req == nil { + return + } + + e.Bool(zf.Stream, s.req.Stream) + e.Bool(zf.OmitPeers, s.req.OmitPeers) + e.Int(zf.Version, int(s.req.Version)) + e.Str(zf.NodeKey, s.req.NodeKey.ShortString()) + + // Log counts only, NOT actual endpoints/IPs. + if len(s.req.Endpoints) > 0 { + e.Int(zf.EndpointsCount, len(s.req.Endpoints)) + } + + // SECURITY: The following fields are intentionally NOT logged: + // - Endpoints: Client IP addresses and ports + // - Hostinfo: Device fingerprinting data (use SafeHostinfo separately if needed) + // - DERPForceWebsockets: Network configuration details +} diff --git a/hscontrol/util/zlog/zf/fields.go b/hscontrol/util/zlog/zf/fields.go new file mode 100644 index 00000000..082f1db9 --- /dev/null +++ b/hscontrol/util/zlog/zf/fields.go @@ -0,0 +1,185 @@ +// Package zf provides zerolog field name constants for consistent logging. +// +// Using constants ensures typos are caught at compile time and enables +// easy refactoring. Import as: +// +// import "github.com/juanfont/headscale/hscontrol/util/zlog/zf" +// +// Usage: +// +// log.Info().Uint64(zf.NodeID, id).Str(zf.NodeName, name).Msg("...") +package zf + +// Node fields. +const ( + NodeID = "node.id" + NodeName = "node.name" + NodeKey = "node.key" + NodeKeyExisting = "node.key.existing" + NodeKeyRequest = "node.key.request" + NodeTags = "node.tags" + NodeIsTagged = "node.is_tagged" + NodeOnline = "node.online" + NodeExpired = "node.expired" + NodeHostname = "node.hostname" + ExistingNodeName = "existing.node.name" + CurrentHostname = "current_hostname" + RejectedHostname = "rejected_hostname" + OldHostname = "old_hostname" + NewHostnameField = "new_hostname" + OldGivenName = "old_given_name" + NewGivenName = "new_given_name" + NewName = "new_name" + GeneratedHostname = "generated.hostname" + RegistrationKey = "registration_key" //nolint:gosec // G101: not a credential + RegistrationMethod = "registrationMethod" + ExpiresAt = "expiresAt" +) + +// Machine fields. +const ( + MachineKey = "machine.key" +) + +// User fields. +const ( + UserID = "user.id" + UserName = "user.name" + UserDisplay = "user.display" + UserProvider = "user.provider" + UserCount = "user.count" + OldUser = "old.user" + NewUser = "new.user" +) + +// PreAuthKey fields. +const ( + PAKID = "pak.id" + PAKPrefix = "pak.prefix" + PAKTags = "pak.tags" + PAKReusable = "pak.reusable" + PAKEphemeral = "pak.ephemeral" + PAKUsed = "pak.used" + PAKIsTagged = "pak.is_tagged" + PAKExpiration = "pak.expiration" +) + +// APIKey fields. +const ( + APIKeyID = "api_key.id" + APIKeyPrefix = "api_key.prefix" //nolint:gosec // G101: not a credential + APIKeyExpiration = "api_key.expiration" //nolint:gosec // G101: not a credential + APIKeyLastSeen = "api_key.last_seen" //nolint:gosec // G101: not a credential +) + +// Route fields. +const ( + RoutesAnnounced = "routes.announced" + RoutesApproved = "routes.approved" + Prefix = "prefix" + FinalState = "finalState" + NewState = "newState" +) + +// Request/Response fields. +const ( + OmitPeers = "omit_peers" + Stream = "stream" + Version = "version" + StatusCode = "status_code" + RegistrationID = "registration_id" +) + +// Network fields. +const ( + EndpointsCount = "endpoints_count" + DERP = "derp" + Hostname = "hostname" + OS = "os" + RoutableIPCount = "routable_ips_count" + RequestTags = "request_tags" + InvalidHostname = "invalid_hostname" + NewHostname = "new_hostname" + URL = "url" + Path = "path" + ClientAddress = "client_address" + ClientVersion = "client_version" + MinimumVersion = "minimum_version" +) + +// Policy fields. +const ( + PolicyChanged = "policy.changed" + FilterHashOld = "filter.hash.old" + FilterHashNew = "filter.hash.new" + TagOwnerHashOld = "tagOwner.hash.old" + TagOwnerHashNew = "tagOwner.hash.new" + AutoApproveHashOld = "autoApprove.hash.old" + AutoApproveHashNew = "autoApprove.hash.new" + ExitSetHashOld = "exitSet.hash.old" + ExitSetHashNew = "exitSet.hash.new" +) + +// Connection/Channel fields. +const ( + Chan = "chan" + ConnID = "conn.id" + ConnectionIndex = "connection_index" + Address = "address" +) + +// gRPC fields. +const ( + Client = "client" + Request = "request" + Users = "users" +) + +// Worker/Processing fields. +const ( + WorkerID = "worker.id" + Reason = "reason" + Op = "op" + OK = "ok" + Changes = "changes" + Watching = "watching" + CleanedNodes = "cleaned_nodes" + Method = "method" + Signal = "signal" + Func = "func" +) + +// Duration fields. +const ( + TotalDuration = "total.duration" + TimeoutDuration = "timeout.duration" +) + +// Database fields. +const ( + Table = "table" + MigrationID = "migration_id" + Commit = "commit" + Records = "records" + Code = "code" + Got = "got" + Database = "database" + Index = "index" + Parent = "parent" + Type = "type" +) + +// Component field for sub-loggers. +const ( + Component = "component" +) + +// Debug environment variable fields. +const ( + DebugDeadlock = "HEADSCALE_DEBUG_DEADLOCK" + DebugDERPUseIP = "HEADSCALE_DEBUG_DERP_USE_IP" + DebugDumpConfig = "HEADSCALE_DEBUG_DUMP_CONFIG" + DebugHighCardinalityMetric = "HEADSCALE_DEBUG_HIGH_CARDINALITY_METRICS" + DebugProfilingEnabled = "HEADSCALE_DEBUG_PROFILING_ENABLED" + DebugTailSQLEnabled = "HEADSCALE_DEBUG_TAILSQL_ENABLED" +) diff --git a/hscontrol/util/zlog/zlog_test.go b/hscontrol/util/zlog/zlog_test.go new file mode 100644 index 00000000..975c8dda --- /dev/null +++ b/hscontrol/util/zlog/zlog_test.go @@ -0,0 +1,227 @@ +package zlog + +import ( + "bytes" + "encoding/json" + "net/netip" + "testing" + + "github.com/juanfont/headscale/hscontrol/util/zlog/zf" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "tailscale.com/tailcfg" + "tailscale.com/types/key" +) + +func TestSafeHostinfo_MarshalZerologObject(t *testing.T) { + tests := []struct { + name string + hostinfo *tailcfg.Hostinfo + wantFields map[string]any + wantAbsent []string // Fields that should NOT be present + }{ + { + name: "nil hostinfo", + hostinfo: nil, + wantFields: map[string]any{}, + }, + { + name: "basic hostinfo", + hostinfo: &tailcfg.Hostinfo{ + Hostname: "myhost", + OS: "linux", + }, + wantFields: map[string]any{ + zf.Hostname: "myhost", + zf.OS: "linux", + }, + }, + { + name: "hostinfo with routes and tags", + hostinfo: &tailcfg.Hostinfo{ + Hostname: "router", + OS: "linux", + RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/24")}, + RequestTags: []string{"tag:server"}, + }, + wantFields: map[string]any{ + zf.Hostname: "router", + zf.OS: "linux", + zf.RoutableIPCount: float64(1), + }, + }, + { + name: "hostinfo with netinfo", + hostinfo: &tailcfg.Hostinfo{ + Hostname: "myhost", + OS: "windows", + NetInfo: &tailcfg.NetInfo{ + PreferredDERP: 1, + }, + }, + wantFields: map[string]any{ + zf.Hostname: "myhost", + zf.OS: "windows", + zf.DERP: float64(1), + }, + }, + { + name: "sensitive fields are NOT logged", + hostinfo: &tailcfg.Hostinfo{ + Hostname: "myhost", + OS: "linux", + OSVersion: "5.15.0-generic", // Should NOT be logged + DeviceModel: "ThinkPad X1", // Should NOT be logged + IPNVersion: "1.50.0", // Should NOT be logged + }, + wantFields: map[string]any{ + zf.Hostname: "myhost", + zf.OS: "linux", + }, + wantAbsent: []string{"os_version", "device_model", "ipn_version", "OSVersion", "DeviceModel", "IPNVersion"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + + log := zerolog.New(&buf) + + log.Info().EmbedObject(Hostinfo(tt.hostinfo)).Msg("test") + + var result map[string]any + + err := json.Unmarshal(buf.Bytes(), &result) + require.NoError(t, err) + + // Check expected fields are present + for key, wantVal := range tt.wantFields { + assert.Equal(t, wantVal, result[key], "field %s", key) + } + + // Check sensitive fields are absent + for _, key := range tt.wantAbsent { + _, exists := result[key] + assert.False(t, exists, "sensitive field %s should not be logged", key) + } + }) + } +} + +func TestSafeMapRequest_MarshalZerologObject(t *testing.T) { + nodeKey := key.NewNode().Public() + + tests := []struct { + name string + req *tailcfg.MapRequest + wantFields map[string]any + wantAbsent []string + }{ + { + name: "nil request", + req: nil, + wantFields: map[string]any{}, + }, + { + name: "basic request", + req: &tailcfg.MapRequest{ + Stream: true, + OmitPeers: false, + Version: 100, + NodeKey: nodeKey, + }, + wantFields: map[string]any{ + zf.Stream: true, + zf.OmitPeers: false, + zf.Version: float64(100), + }, + }, + { + name: "request with endpoints - only count logged", + req: &tailcfg.MapRequest{ + Stream: false, + OmitPeers: true, + Version: 100, + NodeKey: nodeKey, + Endpoints: []netip.AddrPort{ + netip.MustParseAddrPort("192.168.1.100:41641"), + netip.MustParseAddrPort("10.0.0.50:41641"), + }, + }, + wantFields: map[string]any{ + zf.Stream: false, + zf.OmitPeers: true, + zf.EndpointsCount: float64(2), + }, + wantAbsent: []string{"endpoints", "Endpoints"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + + log := zerolog.New(&buf) + + log.Info().EmbedObject(MapRequest(tt.req)).Msg("test") + + var result map[string]any + + err := json.Unmarshal(buf.Bytes(), &result) + require.NoError(t, err) + + // Check expected fields are present + for key, wantVal := range tt.wantFields { + assert.Equal(t, wantVal, result[key], "field %s", key) + } + + // Check node.key is a short string (not full key) + if tt.req != nil { + nodeKeyStr, ok := result[zf.NodeKey].(string) + if ok { + // Short keys are truncated, full keys are 64+ chars + assert.Less(t, len(nodeKeyStr), 20, "node key should be short form") + } + } + + // Check sensitive fields are absent + for _, key := range tt.wantAbsent { + _, exists := result[key] + assert.False(t, exists, "sensitive field %s should not be logged", key) + } + }) + } +} + +func TestFieldConstants(t *testing.T) { + // Verify field constants follow the expected naming pattern + fieldTests := []struct { + constant string + expected string + }{ + {zf.NodeID, "node.id"}, + {zf.NodeName, "node.name"}, + {zf.NodeKey, "node.key"}, + {zf.MachineKey, "machine.key"}, + {zf.NodeTags, "node.tags"}, + {zf.NodeIsTagged, "node.is_tagged"}, + {zf.NodeOnline, "node.online"}, + {zf.NodeExpired, "node.expired"}, + {zf.UserID, "user.id"}, + {zf.UserName, "user.name"}, + {zf.PAKID, "pak.id"}, + {zf.PAKPrefix, "pak.prefix"}, + {zf.APIKeyID, "api_key.id"}, + {zf.APIKeyPrefix, "api_key.prefix"}, + {zf.OmitPeers, "omit_peers"}, + {zf.Stream, "stream"}, + } + + for _, tt := range fieldTests { + t.Run(tt.expected, func(t *testing.T) { + assert.Equal(t, tt.expected, tt.constant) + }) + } +}