From ea35a2d77d9bf6177bc17dfece1c9731e41a93e9 Mon Sep 17 00:00:00 2001 From: Sarah Gillespie <73313222+gillespi314@users.noreply.github.com> Date: Fri, 21 Feb 2025 15:57:09 -0600 Subject: [PATCH] Cleanup `host_certificates` table on host deletion (#26524) --- changes/23235-host-certificates | 1 + server/datastore/mysql/host_certificates.go | 9 ++++++--- server/datastore/mysql/hosts.go | 1 + server/datastore/mysql/hosts_test.go | 8 ++++++++ 4 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 changes/23235-host-certificates diff --git a/changes/23235-host-certificates b/changes/23235-host-certificates new file mode 100644 index 0000000000..e63315609f --- /dev/null +++ b/changes/23235-host-certificates @@ -0,0 +1 @@ +- Added new features to include certificates in host vitals for macOS, iOS, and iPadOS. diff --git a/server/datastore/mysql/host_certificates.go b/server/datastore/mysql/host_certificates.go index c4ab2d41b6..6bd7fe91bb 100644 --- a/server/datastore/mysql/host_certificates.go +++ b/server/datastore/mysql/host_certificates.go @@ -21,11 +21,11 @@ func (ds *Datastore) UpdateHostCertificates(ctx context.Context, hostID uint, ce for _, cert := range certs { if cert.HostID != hostID { // caller should ensure this does not happen - level.Debug(ds.logger).Log("msg", fmt.Sprintf("host ID does not match provided certificate: %d %d", hostID, cert.HostID)) + level.Debug(ds.logger).Log("msg", fmt.Sprintf("host certificates: host ID does not match provided certificate: %d %d", hostID, cert.HostID)) } if _, ok := incomingBySHA1[strings.ToUpper(hex.EncodeToString(cert.SHA1Sum))]; ok { // TODO: sha1 is broken so this could be a sign of a problem, how should we handle? - level.Info(ds.logger).Log("msg", "host has multiple certificates with the same SHA1, only the first will be recorded", "host_id", hostID, "sha1", string(cert.SHA1Sum)) + level.Info(ds.logger).Log("msg", "host certificates: host has multiple certificates with the same SHA1, only the first will be recorded", "host_id", hostID, "sha1", string(cert.SHA1Sum)) continue } incomingBySHA1[strings.ToUpper(hex.EncodeToString(cert.SHA1Sum))] = cert @@ -48,7 +48,7 @@ func (ds *Datastore) UpdateHostCertificates(ctx context.Context, hostID uint, ce if _, ok := existingBySHA1[sha1]; ok { // TODO: should we always update existing records? skipping updates reduces db load but // osquery is using sha1 so we consider subtleties - level.Debug(ds.logger).Log("msg", fmt.Sprintf("existing certificate: %s", sha1), "host_id", hostID) + level.Debug(ds.logger).Log("msg", fmt.Sprintf("host certificates: already exists: %s", sha1), "host_id", hostID) // TODO: silence this log after initial rollout period } else { toInsert = append(toInsert, incoming) } @@ -179,6 +179,9 @@ INSERT INTO host_certificates ( } func softDeleteHostCertsDB(ctx context.Context, tx sqlx.ExtContext, hostID uint, toDelete []uint) error { + // TODO: consider whether we should hard delete certs after a certain period of time if we are seeing + // the table grow too large with soft deleted records + if len(toDelete) == 0 { return nil } diff --git a/server/datastore/mysql/hosts.go b/server/datastore/mysql/hosts.go index 1dbd9d622a..c0051f0c0d 100644 --- a/server/datastore/mysql/hosts.go +++ b/server/datastore/mysql/hosts.go @@ -549,6 +549,7 @@ var hostRefs = []string{ "host_mdm_actions", "host_calendar_events", "upcoming_activities", + "host_certificates", } // NOTE: The following tables are explicity excluded from hostRefs list and accordingly are not diff --git a/server/datastore/mysql/hosts_test.go b/server/datastore/mysql/hosts_test.go index edb0fa529b..af10e4af83 100644 --- a/server/datastore/mysql/hosts_test.go +++ b/server/datastore/mysql/hosts_test.go @@ -2,6 +2,7 @@ package mysql import ( "context" + "crypto/sha1" "crypto/sha256" "database/sql" "encoding/json" @@ -7057,6 +7058,13 @@ func testHostsDeleteHosts(t *testing.T, ds *Datastore) { require.NoError(t, err) require.True(t, added) + // Add a host certificate + require.NoError(t, ds.UpdateHostCertificates(ctx, host.ID, []*fleet.HostCertificateRecord{{ + HostID: host.ID, + CommonName: "foo", + SHA1Sum: sha1.New().Sum([]byte("foo")), + }})) + // Check there's an entry for the host in all the associated tables. for _, hostRef := range hostRefs { var ok bool