Properly filter host certificates by host on update when multiple hosts share the same certificate (#30578)

Fixes #30574.

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

<!-- Note that API documentation changes are now addressed by the
product design team. -->

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files)
for more information.
- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [x] Added/updated automated tests
- [x] Manual QA for all new/changed functionality

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Resolved issues with recording host certificate sources when multiple
hosts share the same certificate but have different usernames, improving
accuracy and performance.
* Addressed related performance and database load problems for these
scenarios.

* **Tests**
* Added new tests to ensure certificate source records remain properly
isolated per host, even when certificates are shared across hosts.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

changes/30574-confused-certs

@@ -0,0 +1 @@
+* Fixed host certificate source recording (including associated performance/database load issues) when multiple hosts share the same certificate on user keychains with differing usernames

server/datastore/mysql/host_certificates.go

@@ -77,6 +77,17 @@ func (ds *Datastore) UpdateHostCertificates(ctx context.Context, hostID uint, ho
 	for sha1, incoming := range incomingBySHA1 {
 		incomingSources := incomingSourcesBySHA1[sha1]
 		existingSources := existingSourcesBySHA1[sha1]
+
+		// sort by keychain (user/system) and username to ensure consistent ordering
+		sliceSortFunc := func(a, b certSourceToSet) int {
+			if a.Source != b.Source {
+				return strings.Compare(string(a.Source), string(b.Source))
+			}
+			return strings.Compare(a.Username, b.Username)
+		}
+		slices.SortFunc(incomingSources, sliceSortFunc)
+		slices.SortFunc(existingSources, sliceSortFunc)
+
 		if !slices.Equal(incomingSources, existingSources) {
 			toSetSourcesBySHA1[sha1] = incomingSources
 		}
@@ -141,7 +152,7 @@ func (ds *Datastore) UpdateHostCertificates(ctx context.Context, hostID uint, ho
 
 		if len(toSetSourcesBySHA1) > 0 {
 			// must reload the DB IDs to insert the host_certificates_sources rows
-			certIDsBySHA1, err := loadHostCertIDsForSHA1DB(ctx, tx, slices.Collect(maps.Keys(toSetSourcesBySHA1)))
+			certIDsBySHA1, err := loadHostCertIDsForSHA1DB(ctx, tx, hostID, slices.Collect(maps.Keys(toSetSourcesBySHA1)))
 			if err != nil {
 				return ctxerr.Wrap(ctx, err, "load host certs ids")
 			}
@@ -172,7 +183,7 @@ func (ds *Datastore) UpdateHostCertificates(ctx context.Context, hostID uint, ho
 	})
 }
 
-func loadHostCertIDsForSHA1DB(ctx context.Context, tx sqlx.QueryerContext, sha1s []string) (map[string]uint, error) {
+func loadHostCertIDsForSHA1DB(ctx context.Context, tx sqlx.QueryerContext, hostID uint, sha1s []string) (map[string]uint, error) {
 	if len(sha1s) == 0 {
 		return nil, nil
 	}
@@ -190,10 +201,11 @@ func loadHostCertIDsForSHA1DB(ctx context.Context, tx sqlx.QueryerContext, sha1s
 	FROM
 		host_certificates hc
 	WHERE
-		hc.sha1_sum IN (?)`
+		hc.sha1_sum IN (?) AND hc.host_id = ?`
 
 	var certs []*fleet.HostCertificateRecord
-	stmt, args, err := sqlx.In(stmt, binarySHA1s)
+	stmt, args, err := sqlx.In(stmt, binarySHA1s, hostID)
+
 	if err != nil {
 		return nil, ctxerr.Wrap(ctx, err, "building load host cert ids query")
 	}

server/datastore/mysql/host_certificates_test.go

@@ -26,6 +26,7 @@ func TestHostCertificates(t *testing.T) {
 	}{
 		{"UpdateAndList", testUpdateAndListHostCertificates},
 		{"Update with host_mdm_managed_certificates to update", testUpdatingHostMDMManagedCertificates},
+		{"Update certificate sources isolation", testUpdateHostCertificatesSourcesIsolation},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
@@ -318,3 +319,153 @@ func generateTestHostCertificateRecord(t *testing.T, hostID uint, template *x509
 
 	return fleet.NewHostCertificateRecord(hostID, parsed)
 }
+
+func testUpdateHostCertificatesSourcesIsolation(t *testing.T, ds *Datastore) {
+	// regression test for #30574
+	ctx := context.Background()
+
+	host1, err := ds.NewHost(ctx, &fleet.Host{
+		DetailUpdatedAt: time.Now(),
+		LabelUpdatedAt:  time.Now(),
+		PolicyUpdatedAt: time.Now(),
+		SeenTime:        time.Now(),
+		OsqueryHostID:   ptr.String("host1-osquery-id"),
+		NodeKey:         ptr.String("host1-node-key"),
+		UUID:            "host1-uuid",
+		Hostname:        "host1",
+	})
+	require.NoError(t, err)
+
+	host2, err := ds.NewHost(ctx, &fleet.Host{
+		DetailUpdatedAt: time.Now(),
+		LabelUpdatedAt:  time.Now(),
+		PolicyUpdatedAt: time.Now(),
+		SeenTime:        time.Now(),
+		OsqueryHostID:   ptr.String("host2-osquery-id"),
+		NodeKey:         ptr.String("host2-node-key"),
+		UUID:            "host2-uuid",
+		Hostname:        "host2",
+	})
+	require.NoError(t, err)
+
+	// Create identical certificates for both hosts (same SHA1 sum)
+	// This simulates the real-world scenario where multiple hosts have the same certificate
+	// installed (e.g., a company root CA certificate)
+	sharedCert := x509.Certificate{
+		Subject: pkix.Name{
+			Country:            []string{"US"},
+			CommonName:         "shared.example.com",
+			Organization:       []string{"Shared Org"},
+			OrganizationalUnit: []string{"Engineering"},
+		},
+		Issuer: pkix.Name{
+			Country:      []string{"US"},
+			CommonName:   "issuer.example.com",
+			Organization: []string{"Issuer"},
+		},
+		SerialNumber: big.NewInt(12345),
+		KeyUsage:     x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
+		ExtKeyUsage:  []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+
+		SignatureAlgorithm:    x509.SHA256WithRSA,
+		NotBefore:             time.Now().Add(-time.Hour).Truncate(time.Second).UTC(),
+		NotAfter:              time.Now().Add(24 * time.Hour).Truncate(time.Second).UTC(),
+		BasicConstraintsValid: true,
+	}
+
+	// Generate certificate records for both hosts using the same certificate data
+	// We need to create the certificate bytes once and reuse them to ensure same SHA1
+	certBytes, _, err := GenerateTestCertBytes(&sharedCert)
+	require.NoError(t, err)
+
+	block, _ := pem.Decode(certBytes)
+	parsed, err := x509.ParseCertificate(block.Bytes)
+	require.NoError(t, err)
+
+	host1Cert := fleet.NewHostCertificateRecord(host1.ID, parsed)
+	host1Cert.Source = fleet.UserHostCertificate
+	host1Cert.Username = "jdoe"
+
+	host2Cert := fleet.NewHostCertificateRecord(host2.ID, parsed)
+	host2Cert.Source = fleet.UserHostCertificate
+	host2Cert.Username = "jsmith"
+
+	// Add the same certificate to both hosts
+	require.NoError(t, ds.UpdateHostCertificates(ctx, host1.ID, host1.UUID, []*fleet.HostCertificateRecord{host1Cert}))
+	require.NoError(t, ds.UpdateHostCertificates(ctx, host2.ID, host2.UUID, []*fleet.HostCertificateRecord{host2Cert}))
+
+	// Verify both hosts have the correct certs, with the correct sources
+	host1Certs, _, err := ds.ListHostCertificates(ctx, host1.ID, fleet.ListOptions{})
+	require.NoError(t, err)
+	require.Len(t, host1Certs, 1)
+	require.Equal(t, fleet.UserHostCertificate, host1Certs[0].Source)
+	require.Equal(t, "jdoe", host1Certs[0].Username)
+
+	host2Certs, _, err := ds.ListHostCertificates(ctx, host2.ID, fleet.ListOptions{})
+	require.NoError(t, err)
+	require.Len(t, host2Certs, 1)
+	require.Equal(t, fleet.UserHostCertificate, host2Certs[0].Source)
+	require.Equal(t, "jsmith", host2Certs[0].Username)
+
+	// Trigger a change in host 2's cert sources to force delete/recreate of source records. Prior to the fix this
+	// wouldn't modify the correct certs because the DB query would return host 1's cert for the given hash.
+	host2CertUpdated := fleet.NewHostCertificateRecord(host2.ID, parsed)
+	host2CertUpdated.Source = fleet.UserHostCertificate
+	host2CertUpdated.Username = "janesmith"
+
+	require.NoError(t, ds.UpdateHostCertificates(ctx, host2.ID, host2.UUID, []*fleet.HostCertificateRecord{host2CertUpdated}))
+
+	// Verify host1's certificate source was *not* updated
+	host1CertsAfter, _, err := ds.ListHostCertificates(ctx, host1.ID, fleet.ListOptions{})
+	require.NoError(t, err)
+	require.Len(t, host1CertsAfter, 1)
+	require.Equal(t, "jdoe", host1CertsAfter[0].Username)
+
+	// Verify host2's certificate source *was* updated
+	host2CertsAfter, _, err := ds.ListHostCertificates(ctx, host2.ID, fleet.ListOptions{})
+	require.NoError(t, err)
+	require.Len(t, host2CertsAfter, 1)
+	require.Equal(t, "janesmith", host2CertsAfter[0].Username)
+
+	// Verify no-op case
+	err = ds.UpdateHostCertificates(ctx, host2.ID, host2.UUID, []*fleet.HostCertificateRecord{host2CertUpdated})
+	require.NoError(t, err)
+
+	// Verify host2's certificate source was updated
+	host2CertsNoop, _, err := ds.ListHostCertificates(ctx, host2.ID, fleet.ListOptions{})
+	require.NoError(t, err)
+	require.Len(t, host2CertsNoop, 1)
+	require.Equal(t, fleet.UserHostCertificate, host2CertsNoop[0].Source)
+	require.Equal(t, "janesmith", host2CertsNoop[0].Username)
+
+	// Confirm that adding the cert to the system store gets picked up properly
+	systemCertOnHost2 := fleet.NewHostCertificateRecord(host2.ID, parsed)
+	systemCertOnHost2.Source = fleet.SystemHostCertificate
+
+	require.NoError(t, ds.UpdateHostCertificates(ctx, host2.ID, host2.UUID, []*fleet.HostCertificateRecord{host2CertUpdated, systemCertOnHost2}))
+
+	// Verify host2 now has the certificate with both sources
+	host2CertsMultiSource, _, err := ds.ListHostCertificates(ctx, host2.ID, fleet.ListOptions{})
+	require.NoError(t, err)
+	require.Len(t, host2CertsMultiSource, 2)
+	var hasUserCert, hasSystemCert bool
+	for _, cert := range host2CertsMultiSource {
+		if cert.Source == fleet.UserHostCertificate {
+			require.Equal(t, "janesmith", cert.Username)
+			hasUserCert = true
+		} else {
+			require.Empty(t, cert.Username)
+			require.Equal(t, fleet.SystemHostCertificate, cert.Source)
+			hasSystemCert = true
+		}
+	}
+	require.True(t, hasUserCert)
+	require.True(t, hasSystemCert)
+
+	// Verify host1 still has only its original certificate source
+	host1CertsMultiSource, _, err := ds.ListHostCertificates(ctx, host1.ID, fleet.ListOptions{})
+	require.NoError(t, err)
+	require.Len(t, host1CertsMultiSource, 1)
+	require.Equal(t, fleet.UserHostCertificate, host1CertsMultiSource[0].Source)
+	require.Equal(t, "jdoe", host1CertsMultiSource[0].Username)
+}