Connect: verify signature in privileged updater against hardcoded Windows signing cert

integration/autoupdate/tools/connect_privileged_updater_windows_test.go

@@ -261,6 +261,19 @@ func TestPrivilegedUpdateServiceAllowOnlyOneClientConnection(t *testing.T) {
 	}
 }
 
+func TestPrivilegedUpdateServiceRejectsUnsignedUpdate(t *testing.T) {
+	up := update{
+		version: "999.0.0",
+		binary:  []byte("payload"),
+	}
+
+	// Keep signature verification enabled for this test to ensure unsigned
+	// updates are rejected.
+	err := runPrivilegedUpdaterFlow(t, up, withServiceTestVerifySignature(nil))
+	require.Error(t, err)
+	require.ErrorContains(t, err, "verifying update signature")
+}
+
 type serviceConfig struct {
 	privilegedupdater.ServiceTestConfig
 	checksumServerResponseWriter func(http.ResponseWriter)
@@ -286,6 +299,12 @@ func withServiceTestPolicyToolsVersion(version string) privilegedServiceMainConf
 	}
 }
 
+func withServiceTestVerifySignature(fn func(updatePath string) error) privilegedServiceMainConfigOption {
+	return func(cfg *serviceConfig) {
+		cfg.VerifySignature = fn
+	}
+}
+
 type update struct {
 	version string
 	binary  []byte
@@ -332,6 +351,7 @@ func runPrivilegedUpdaterFlow(t *testing.T, update update, opts ...privilegedSer
 			PolicyCDNBaseURL:             server.URL,
 			HTTPClient:                   server.Client(),
 			PipeAuthenticatedUsersAccess: cfg.PipeAuthenticatedUsersAccess,
+			VerifySignature:              cfg.VerifySignature,
 		})
 		// We are attempting to run a non-exe file.
 		// It will fail, so we check if we ran the correct file.
@@ -409,6 +429,8 @@ func getDefaultConfig(t *testing.T) *privilegedupdater.ServiceTestConfig {
 		UpdateBaseDir:               t.TempDir(),
 		// Allow Authenticated Users to create the pipe in tests.
 		PipeAuthenticatedUsersAccess: windows.GENERIC_READ | windows.GENERIC_WRITE,
+		// Integration test updates are unsigned.
+		VerifySignature: func(string) error { return nil },
 	}
 }
 

lib/teleterm/autoupdate/privilegedupdater/authenticode_windows.go

@@ -23,11 +23,9 @@ package privilegedupdater
 
 import (
 	"crypto/x509"
-	"errors"
+	"crypto/x509/pkix"
 	"fmt"
-	"os"
 	"slices"
-	"syscall"
 	"unsafe"
 
 	"github.com/gravitational/trace"
@@ -40,30 +38,8 @@ const (
 	cmsgSignerCertInfoParam = 7
 )
 
-// verifySignature checks if the update is signed by the same entity as the running service.
+// verifySignature checks if the update is signed by Teleport.
 func verifySignature(updatePath string) error {
-	servicePath, err := os.Executable()
-	if err != nil {
-		return trace.Wrap(err)
-	}
-	servicePathPtr, err := windows.UTF16PtrFromString(servicePath)
-	if err != nil {
-		return trace.Wrap(err)
-	}
-
-	if err = verifyTrust(servicePathPtr); err != nil {
-		if errors.Is(err, syscall.Errno(windows.TRUST_E_NOSIGNATURE)) {
-			log.Warn("service is not signed, skipping signature verification")
-			return nil
-		}
-		return trace.Wrap(err)
-	}
-
-	serviceCert, err := getCert(servicePathPtr)
-	if err != nil {
-		return trace.Wrap(err, "getting service certificate")
-	}
-
 	updatePathPtr, err := windows.UTF16PtrFromString(updatePath)
 	if err != nil {
 		return trace.Wrap(err)
@@ -76,8 +52,8 @@ func verifySignature(updatePath string) error {
 		return trace.Wrap(err, "getting update certificate")
 	}
 
-	if !compareSubjectProperties(serviceCert, updateCert) {
-		return trace.BadParameter("signature verification failed: update and service subjects do not match (service: %s, update: %s)", logCert(serviceCert), logCert(updateCert))
+	if !hasTeleportSubject(updateCert) {
+		return trace.BadParameter("signature verification failed: update subject does not match Teleport subject (teleport: %s, update: %s)", logCert(teleportCert), logCert(updateCert))
 	}
 
 	return nil
@@ -158,13 +134,9 @@ func getCert(path *uint16) (*x509.Certificate, error) {
 	return cert, trace.Wrap(err)
 }
 
-// compareSubjectProperties checks whether two certificates appear to belong to the same publisher by comparing a subset
-// of their X.509 Subject fields.
+// hasTeleportSubject checks whether updateCert has the expected Teleport publisher subject by comparing a subset of X.509 Subject fields.
 // The cert validity must be checked first with verifyTrust.
 //
-// For Teleport's Windows signing certificate the subject typically looks like:
-// CN="Gravitational, Inc.", O="Gravitational, Inc.", L="Oakland", ST="California", C="US"
-//
 // Security notes:
 //   - This does NOT provide cryptographic authenticity guarantees. An attacker could theoretically obtain a certificate
 //     with identical subject fields. However, obtaining such a certificate from a CA trusted by Windows is non-trivial
@@ -181,19 +153,29 @@ func getCert(path *uint16) (*x509.Certificate, error) {
 //     https://github.com/electron-userland/electron-builder/blob/02e59ba8a3b02e1b3ab20035ff43f48ea20880b7/packages/electron-updater/src/windowsExecutableCodeSignatureVerifier.ts#L69-L85
 //   - Tailscale verifies only the CN:
 //     https://github.com/tailscale/tailscale/blob/3ec5be3f510f74738179c1023468343a62a7e00f/clientupdate/clientupdate_windows.go#L70-L74
-func compareSubjectProperties(cert1, cert2 *x509.Certificate) bool {
-	if cert1 == nil || cert2 == nil {
+func hasTeleportSubject(updateCert *x509.Certificate) bool {
+	if updateCert == nil {
 		return false
 	}
 
-	s1, s2 := cert1.Subject, cert2.Subject
+	s1, s2 := teleportCert.Subject, updateCert.Subject
 	return s1.CommonName == s2.CommonName &&
 		slices.Equal(s1.Organization, s2.Organization) &&
 		slices.Equal(s1.Locality, s2.Locality) &&
 		slices.Equal(s1.Province, s2.Province) &&
 		slices.Equal(s1.Country, s2.Country)
 }
 
+var teleportCert = &x509.Certificate{
+	Subject: pkix.Name{
+		CommonName:   "Gravitational, Inc.",
+		Organization: []string{"Gravitational, Inc."},
+		Locality:     []string{"Oakland"},
+		Province:     []string{"California"},
+		Country:      []string{"US"},
+	},
+}
+
 func logCert(cert *x509.Certificate) string {
 	if cert == nil {
 		return "<nil>"

lib/teleterm/autoupdate/privilegedupdater/service_windows.go

@@ -99,6 +99,9 @@ type ServiceTestConfig struct {
 	// PipeAuthenticatedUsersAccess overrides Authenticated Users access mask in
 	// the named pipe DACL. If zero, SafePipeReadWriteAccess is used.
 	PipeAuthenticatedUsersAccess uint32
+	// VerifySignature overrides signature verification.
+	// If nil, verifySignature is used.
+	VerifySignature func(updatePath string) error
 }
 
 // InstallService installs the Teleport Connect privileged update service.
@@ -177,7 +180,11 @@ func (h *handler) Execute(ctx context.Context, _ []string) (err error) {
 		return trace.Wrap(err, "checking if update is upgrade")
 	}
 
-	if err = verifySignature(updatePath); err != nil {
+	verifySignatureFn := verifySignature
+	if h.testCfg.VerifySignature != nil {
+		verifySignatureFn = h.testCfg.VerifySignature
+	}
+	if err = verifySignatureFn(updatePath); err != nil {
 		return trace.Wrap(err, "verifying update signature")
 	}
 

web/packages/teleterm/README.md

@@ -145,6 +145,18 @@ GOOS=windows CGO_ENABLED=1 go build -o build/tsh.exe  -ldflags '-w -s' -buildvcs
 It's important for the executable to end with `.exe`. If that command doesn't work, you can always
 inspect what we currently do in [our Windows build pipeline scripts](https://github.com/gravitational/teleport/blob/983017b23f65e49350615bfbbe52b7f1080ea7b9/build.assets/windows/build.ps1#L377).
 
+#### Certificate requirements for updater
+
+The privileged updater on Windows (for per-machine updates) verifies update signatures before running the installer:
+
+- `WinVerifyTrust` must succeed for the update executable.
+- The signer subject must match the Teleport publisher subject:
+  `CN=Gravitational, Inc., O=Gravitational, Inc., L=Oakland, ST=California, C=US`.
+
+The hardcoded values are kept in `authenticode_windows.go`.
+As a result, the service binary itself does not need to be signed (e.g., in OSS builds), but all updates must be
+properly signed.
+
 #### Native dependencies on Windows
 
 On Windows, you need to pay special attention to [the dev tools needed by node-pty](https://github.com/microsoft/node-pty?tab=readme-ov-file#windows),