posture: add HealthTracker for serial number retrievallykathia/posture

Device posture checking can fail while enabled if tailscaled does not
have access to smbios. Previously, this was only observable by looking
in the tailscaled logs.

This changeset also cleans up some leftovers from prior refactors.

Fixes tailscale/corp#39314

Signed-off-by: Evan Lowry <evan@tailscale.com>

9 files changed, 26 insertions, 16 deletions

diff --git a/feature/posture/posture.go b/feature/posture/posture.go
index d8db1ac19..a32e7baf3 100644
--- a/feature/posture/posture.go
+++ b/feature/posture/posture.go
@@ -8,8 +8,10 @@ package posture
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/http"
 
+	"tailscale.com/health"
 	"tailscale.com/ipn/ipnext"
 	"tailscale.com/ipn/ipnlocal"
 	"tailscale.com/posture"
@@ -25,6 +27,15 @@ func init() {
 	ipnlocal.RegisterC2N("GET /posture/identity", handleC2NPostureIdentityGet)
 }
 
+var postureSerialWarnable = health.Register(&health.Warnable{
+	Code:     "posture-checking-serial-collection-failed",
+	Title:    "Device Posture: serial number collection failed",
+	Severity: health.SeverityMedium,
+	Text: func(args health.Args) string {
+		return fmt.Sprintf("Could not collect device serial numbers for posture checking. (%v)", args[health.ArgError])
+	},
+})
+
 func newExtension(logf logger.Logf, b ipnext.SafeBackend) (ipnext.Extension, error) {
 	e := &extension{
 		logf: logger.WithPrefix(logf, "posture: "),
@@ -70,9 +81,12 @@ func handleC2NPostureIdentityGet(b *ipnlocal.LocalBackend, w http.ResponseWriter
 	}
 
 	if choice.ShouldEnable(b.Prefs().PostureChecking()) {
-		res.SerialNumbers, err = posture.GetSerialNumbers(b.PolicyClient(), e.logf)
+		res.SerialNumbers, err = posture.GetSerialNumbers(b.PolicyClient())
 		if err != nil {
 			e.logf("c2n: GetSerialNumbers returned error: %v", err)
+			b.HealthTracker().SetUnhealthy(postureSerialWarnable, health.Args{health.ArgError: err.Error()})
+		} else {
+			b.HealthTracker().SetHealthy(postureSerialWarnable)
 		}
 
 		// TODO(tailscale/corp#21371, 2024-07-10): once this has landed in a stable release
diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index 5d8b210f0..cee5d7845 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -4212,6 +4212,8 @@ func (b *LocalBackend) CurrentUserForTest() (ipn.WindowsUserID, ipnauth.Actor) {
 	return b.pm.CurrentUserID(), b.currentUser
 }
 
+// CheckPrefs validates the provided user modifiable settings for correctness
+// and returns an error if they are invalid for the current backend.
 func (b *LocalBackend) CheckPrefs(p *ipn.Prefs) error {
 	b.mu.Lock()
 	defer b.mu.Unlock()
diff --git a/posture/serialnumber_macos.go b/posture/serialnumber_macos.go
index fed0d4111..06df43baa 100644
--- a/posture/serialnumber_macos.go
+++ b/posture/serialnumber_macos.go
@@ -58,12 +58,11 @@ import (
 	"fmt"
 	"strings"
 
-	"tailscale.com/types/logger"
 	"tailscale.com/util/syspolicy/policyclient"
 )
 
 // GetSerialNumber returns the platform serial sumber as reported by IOKit.
-func GetSerialNumbers(policyclient.Client, logger.Logf) ([]string, error) {
+func GetSerialNumbers(policyclient.Client) ([]string, error) {
 	csn := C.getSerialNumber()
 	serialNumber := C.GoString(csn)
 
diff --git a/posture/serialnumber_macos_test.go b/posture/serialnumber_macos_test.go
index 5f1aec5cd..9f4b8d79c 100644
--- a/posture/serialnumber_macos_test.go
+++ b/posture/serialnumber_macos_test.go
@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"testing"
 
-	"tailscale.com/types/logger"
 	"tailscale.com/util/cibuild"
 	"tailscale.com/util/syspolicy/policyclient"
 )
@@ -21,7 +20,7 @@ func TestGetSerialNumberMac(t *testing.T) {
 		t.Skip()
 	}
 
-	sns, err := GetSerialNumbers(policyclient.NoPolicyClient{}, logger.Discard)
+	sns, err := GetSerialNumbers(policyclient.NoPolicyClient{})
 	if err != nil {
 		t.Fatalf("failed to get serial number: %s", err)
 	}
diff --git a/posture/serialnumber_notmacos.go b/posture/serialnumber_notmacos.go
index e076b8f3d..7ee498127 100644
--- a/posture/serialnumber_notmacos.go
+++ b/posture/serialnumber_notmacos.go
@@ -12,7 +12,6 @@ import (
 	"strings"
 
 	"github.com/digitalocean/go-smbios/smbios"
-	"tailscale.com/types/logger"
 	"tailscale.com/util/syspolicy/policyclient"
 )
 
@@ -72,7 +71,7 @@ func init() {
 	numOfTables = len(validTables)
 }
 
-func GetSerialNumbers(polc policyclient.Client, logf logger.Logf) ([]string, error) {
+func GetSerialNumbers(polc policyclient.Client) ([]string, error) {
 	// Find SMBIOS data in operating system-specific location.
 	rc, _, err := smbios.Stream()
 	if err != nil {
diff --git a/posture/serialnumber_notmacos_test.go b/posture/serialnumber_notmacos_test.go
index 1009ea6b4..11223a8a3 100644
--- a/posture/serialnumber_notmacos_test.go
+++ b/posture/serialnumber_notmacos_test.go
@@ -11,7 +11,6 @@ import (
 	"fmt"
 	"testing"
 
-	"tailscale.com/types/logger"
 	"tailscale.com/util/syspolicy/policyclient"
 )
 
@@ -22,7 +21,7 @@ func TestGetSerialNumberNotMac(t *testing.T) {
 	// Comment out skip for local testing.
 	t.Skip()
 
-	sns, err := GetSerialNumbers(policyclient.NoPolicyClient{}, logger.Discard)
+	sns, err := GetSerialNumbers(policyclient.NoPolicyClient{})
 	if err != nil {
 		t.Fatalf("failed to get serial number: %s", err)
 	}
diff --git a/posture/serialnumber_stub.go b/posture/serialnumber_stub.go
index e040aacfb..2cea4648e 100644
--- a/posture/serialnumber_stub.go
+++ b/posture/serialnumber_stub.go
@@ -12,12 +12,12 @@ package posture
 
 import (
 	"errors"
+	"fmt"
 
-	"tailscale.com/types/logger"
 	"tailscale.com/util/syspolicy/policyclient"
 )
 
 // GetSerialNumber returns client machine serial number(s).
-func GetSerialNumbers(polc policyclient.Client, _ logger.Logf) ([]string, error) {
-	return nil, errors.New("not implemented")
+func GetSerialNumbers(polc policyclient.Client) ([]string, error) {
+	return nil, fmt.Errorf("not implemented: %w", errors.ErrUnsupported)
 }
diff --git a/posture/serialnumber_syspolicy.go b/posture/serialnumber_syspolicy.go
index 448fdb677..8e57037f4 100644
--- a/posture/serialnumber_syspolicy.go
+++ b/posture/serialnumber_syspolicy.go
@@ -8,7 +8,6 @@ package posture
 import (
 	"fmt"
 
-	"tailscale.com/types/logger"
 	"tailscale.com/util/syspolicy/pkey"
 	"tailscale.com/util/syspolicy/policyclient"
 )
@@ -16,7 +15,7 @@ import (
 // GetSerialNumbers returns the serial number of the device as reported by an
 // MDM solution. It requires configuration via the DeviceSerialNumber system policy.
 // This is the only way to gather serial numbers on iOS, tvOS and Android.
-func GetSerialNumbers(polc policyclient.Client, _ logger.Logf) ([]string, error) {
+func GetSerialNumbers(polc policyclient.Client) ([]string, error) {
 	s, err := polc.GetString(pkey.DeviceSerialNumber, "")
 	if err != nil {
 		return nil, fmt.Errorf("failed to get serial number from MDM: %v", err)
diff --git a/posture/serialnumber_test.go b/posture/serialnumber_test.go
index 20e726d9f..bebf58031 100644
--- a/posture/serialnumber_test.go
+++ b/posture/serialnumber_test.go
@@ -6,12 +6,11 @@ package posture
 import (
 	"testing"
 
-	"tailscale.com/types/logger"
 	"tailscale.com/util/syspolicy/policyclient"
 )
 
 func TestGetSerialNumber(t *testing.T) {
 	// ensure GetSerialNumbers is implemented
 	// or covered by a stub on a given platform.
-	_, _ = GetSerialNumbers(policyclient.NoPolicyClient{}, logger.Discard)
+	_, _ = GetSerialNumbers(policyclient.NoPolicyClient{})
 }