diff --git a/server/datastore/mysql/apple_mdm.go b/server/datastore/mysql/apple_mdm.go index 0655e677e065..90d02da740aa 100644 --- a/server/datastore/mysql/apple_mdm.go +++ b/server/datastore/mysql/apple_mdm.go @@ -1737,7 +1737,7 @@ ON DUPLICATE KEY UPDATE // nolint:gosec // ignore G101: Potential hardcoded credentials const secretsUpdatedInProfile = ` UPDATE mdm_apple_configuration_profiles -SET secrets_updated_at = CURRENT_TIMESTAMP(6) +SET secrets_updated_at = NOW(6) WHERE team_id = ? AND identifier = ?` // use a profile team id of 0 if no-team @@ -4659,7 +4659,7 @@ func batchSetDeclarationLabelAssociationsDB(ctx context.Context, tx sqlx.ExtCont func (ds *Datastore) MDMAppleDDMDeclarationsToken(ctx context.Context, hostUUID string) (*fleet.MDMAppleDDMDeclarationsToken, error) { const stmt = ` SELECT - COALESCE(MD5((count(0) + GROUP_CONCAT(CONCAT(HEX(mad.checksum), COALESCE(hmad.secrets_updated_at, '')) + COALESCE(MD5((count(0) + GROUP_CONCAT(hmad.token ORDER BY mad.uploaded_at DESC separator ''))), '') AS checksum, COALESCE(MAX(mad.created_at), NOW()) AS latest_created_timestamp @@ -4688,7 +4688,7 @@ WHERE func (ds *Datastore) MDMAppleDDMDeclarationItems(ctx context.Context, hostUUID string) ([]fleet.MDMAppleDDMDeclarationItem, error) { const stmt = ` SELECT - CONCAT(HEX(mad.checksum), COALESCE(hmad.secrets_updated_at, '')) as checksum, + token, mad.identifier FROM host_mdm_apple_declarations hmad @@ -4711,7 +4711,7 @@ func (ds *Datastore) MDMAppleDDMDeclarationsResponse(ctx context.Context, identi // declarations are removed, but the join would provide an extra layer of safety. const stmt = ` SELECT - mad.raw_json, CONCAT(HEX(mad.checksum), COALESCE(hmad.secrets_updated_at, '')) as checksum + mad.raw_json, token FROM host_mdm_apple_declarations hmad JOIN mdm_apple_declarations mad ON hmad.declaration_uuid = mad.declaration_uuid diff --git a/server/datastore/mysql/apple_mdm_test.go b/server/datastore/mysql/apple_mdm_test.go index 1bc438362423..7f5a741dabf9 100644 --- a/server/datastore/mysql/apple_mdm_test.go +++ b/server/datastore/mysql/apple_mdm_test.go @@ -1120,7 +1120,7 @@ func expectAppleDeclarations( require.NotEmpty(t, gotD.DeclarationUUID) require.True(t, strings.HasPrefix(gotD.DeclarationUUID, fleet.MDMAppleDeclarationUUIDPrefix)) gotD.DeclarationUUID = "" - gotD.Checksum = "" // don't care about md5checksum here + gotD.Token = "" // don't care about md5checksum here gotD.CreatedAt = time.Time{} @@ -1400,6 +1400,7 @@ func testMDMAppleProfileManagement(t *testing.T, ds *Datastore) { for _, p := range got { require.NotEmpty(t, p.Checksum) p.Checksum = nil + p.SecretsUpdatedAt = nil } require.ElementsMatch(t, want, got) } @@ -7337,7 +7338,10 @@ func testMDMAppleProfileLabels(t *testing.T, ds *Datastore) { matchProfiles := func(want, got []*fleet.MDMAppleProfilePayload) { // match only the fields we care about for _, p := range got { + assert.NotEmpty(t, p.Checksum) p.Checksum = nil + assert.NotZero(t, p.SecretsUpdatedAt) + p.SecretsUpdatedAt = nil } require.ElementsMatch(t, want, got) } diff --git a/server/datastore/mysql/mdm.go b/server/datastore/mysql/mdm.go index c5d656abc382..687de8d5978a 100644 --- a/server/datastore/mysql/mdm.go +++ b/server/datastore/mysql/mdm.go @@ -546,6 +546,7 @@ OR // // Note(victor): Why is the status being set to nil? Shouldn't it be set to pending? // Or at least pending for install and nil for remove profiles. Please update this comment if you know. + // This method is called bulkSetPendingMDMHostProfilesDB, so it is confusing that the status is NOT explicitly set to pending. _, updates.AppleDeclaration, err = mdmAppleBatchSetHostDeclarationStateDB(ctx, tx, batchSize, nil) if err != nil { return updates, ctxerr.Wrap(ctx, err, "bulk set pending apple declarations") diff --git a/server/datastore/mysql/migrations/tables/20241223115925_AddSecretsUpdatedAt.go b/server/datastore/mysql/migrations/tables/20241223115925_AddSecretsUpdatedAt.go index b03742b5e404..46331c1bddf7 100644 --- a/server/datastore/mysql/migrations/tables/20241223115925_AddSecretsUpdatedAt.go +++ b/server/datastore/mysql/migrations/tables/20241223115925_AddSecretsUpdatedAt.go @@ -10,9 +10,12 @@ func init() { } func Up_20241223115925(tx *sql.Tx) error { + // Using DATETIME instead of TIMESTAMP for secrets_updated_at to avoid future Y2K38 issues, + // since this date is used to detect if profile needs to be updated. + // secrets_updated_at are updated when profile contents have not changed but secret variables in the profile have changed _, err := tx.Exec(`ALTER TABLE mdm_apple_configuration_profiles - ADD COLUMN secrets_updated_at TIMESTAMP(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6), + ADD COLUMN secrets_updated_at DATETIME(6) NULL, MODIFY COLUMN created_at TIMESTAMP(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6), MODIFY COLUMN uploaded_at TIMESTAMP(6) NULL DEFAULT NULL`) if err != nil { @@ -21,16 +24,21 @@ func Up_20241223115925(tx *sql.Tx) error { // Add secrets_updated_at column to host_mdm_apple_profiles _, err = tx.Exec(`ALTER TABLE host_mdm_apple_profiles - ADD COLUMN secrets_updated_at TIMESTAMP(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6)`) + ADD COLUMN secrets_updated_at DATETIME(6) NULL`) if err != nil { return fmt.Errorf("failed to add secrets_updated_at to host_mdm_apple_profiles table: %w", err) } // secrets_updated_at are updated when profile contents have not changed but secret variables in the profile have changed _, err = tx.Exec(`ALTER TABLE mdm_apple_declarations - ADD COLUMN secrets_updated_at TIMESTAMP(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6), + ADD COLUMN secrets_updated_at DATETIME(6) NULL, MODIFY COLUMN created_at TIMESTAMP(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6), - MODIFY COLUMN uploaded_at TIMESTAMP(6) NULL DEFAULT NULL`) + MODIFY COLUMN uploaded_at TIMESTAMP(6) NULL DEFAULT NULL, + -- token is used to identify if declaration needs to be re-applied + -- using HEX for consistency and slight compression + -- mdm_apple_declarations.token formula must match host_mdm_apple_declarations.token formula + ADD COLUMN token VARCHAR(63) COLLATE utf8mb4_unicode_ci GENERATED ALWAYS AS + (CONCAT(HEX(checksum), COALESCE(CONCAT('-', HEX(TO_SECONDS(secrets_updated_at)), '-', HEX(MICROSECOND(secrets_updated_at)))), '')) VIRTUAL NULL`) if err != nil { return fmt.Errorf("failed to alter mdm_apple_declarations table: %w", err) } @@ -38,7 +46,11 @@ func Up_20241223115925(tx *sql.Tx) error { // Add secrets_updated_at column to host_mdm_apple_declarations _, err = tx.Exec(`ALTER TABLE host_mdm_apple_declarations -- defaulting to NULL for backward compatibility with existing declarations - ADD COLUMN secrets_updated_at TIMESTAMP(6) NULL`) + ADD COLUMN secrets_updated_at DATETIME(6) NULL, + -- token is used to identify if declaration needs to be re-applied + -- using HEX for consistency and slight compression + ADD COLUMN token VARCHAR(63) COLLATE utf8mb4_unicode_ci GENERATED ALWAYS AS + (CONCAT(HEX(checksum), COALESCE(CONCAT('-', HEX(TO_SECONDS(secrets_updated_at)), '-', HEX(MICROSECOND(secrets_updated_at)))), '')) VIRTUAL NULL`) if err != nil { return fmt.Errorf("failed to add secrets_updated_at to host_mdm_apple_declarations table: %w", err) } diff --git a/server/datastore/mysql/schema.sql b/server/datastore/mysql/schema.sql index fac727f3ec12..1fe85600369d 100644 --- a/server/datastore/mysql/schema.sql +++ b/server/datastore/mysql/schema.sql @@ -423,7 +423,8 @@ CREATE TABLE `host_mdm_apple_declarations` ( `declaration_uuid` varchar(37) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '', `declaration_identifier` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci NOT NULL, `declaration_name` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '', - `secrets_updated_at` timestamp(6) NULL DEFAULT NULL, + `secrets_updated_at` datetime(6) DEFAULT NULL, + `token` varchar(63) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci GENERATED ALWAYS AS (concat(hex(`checksum`),coalesce(concat(_utf8mb4'-',hex(to_seconds(`secrets_updated_at`)),_utf8mb4'-',hex(microsecond(`secrets_updated_at`)))),_utf8mb4'')) VIRTUAL, PRIMARY KEY (`host_uuid`,`declaration_uuid`), KEY `status` (`status`), KEY `operation_type` (`operation_type`), @@ -446,7 +447,7 @@ CREATE TABLE `host_mdm_apple_profiles` ( `profile_uuid` varchar(37) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '', `created_at` timestamp(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6), `updated_at` timestamp(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), - `secrets_updated_at` timestamp(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6), + `secrets_updated_at` datetime(6) DEFAULT NULL, PRIMARY KEY (`host_uuid`,`profile_uuid`), KEY `status` (`status`), KEY `operation_type` (`operation_type`), @@ -854,7 +855,7 @@ CREATE TABLE `mdm_apple_configuration_profiles` ( `uploaded_at` timestamp(6) NULL DEFAULT NULL, `checksum` binary(16) NOT NULL, `profile_uuid` varchar(37) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '', - `secrets_updated_at` timestamp(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6), + `secrets_updated_at` datetime(6) DEFAULT NULL, PRIMARY KEY (`profile_uuid`), UNIQUE KEY `idx_mdm_apple_config_prof_team_identifier` (`team_id`,`identifier`), UNIQUE KEY `idx_mdm_apple_config_prof_team_name` (`team_id`,`name`), @@ -884,7 +885,8 @@ CREATE TABLE `mdm_apple_declarations` ( `created_at` timestamp(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6), `uploaded_at` timestamp(6) NULL DEFAULT NULL, `auto_increment` bigint NOT NULL AUTO_INCREMENT, - `secrets_updated_at` timestamp(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6), + `secrets_updated_at` datetime(6) DEFAULT NULL, + `token` varchar(63) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci GENERATED ALWAYS AS (concat(hex(`checksum`),coalesce(concat(_utf8mb4'-',hex(to_seconds(`secrets_updated_at`)),_utf8mb4'-',hex(microsecond(`secrets_updated_at`)))),_utf8mb4'')) VIRTUAL, PRIMARY KEY (`declaration_uuid`), UNIQUE KEY `idx_mdm_apple_declaration_team_identifier` (`team_id`,`identifier`), UNIQUE KEY `idx_mdm_apple_declaration_team_name` (`team_id`,`name`), diff --git a/server/fleet/apple_mdm.go b/server/fleet/apple_mdm.go index b53ad99dfaa9..882041fa7492 100644 --- a/server/fleet/apple_mdm.go +++ b/server/fleet/apple_mdm.go @@ -321,7 +321,7 @@ type MDMAppleProfilePayload struct { HostUUID string `db:"host_uuid"` HostPlatform string `db:"host_platform"` Checksum []byte `db:"checksum"` - SecretsUpdatedAt time.Time `db:"secrets_updated_at"` + SecretsUpdatedAt *time.Time `db:"secrets_updated_at"` Status *MDMDeliveryStatus `db:"status" json:"status"` OperationType MDMOperationType `db:"operation_type"` Detail string `db:"detail"` @@ -587,8 +587,13 @@ type MDMAppleDeclaration struct { // RawJSON is the raw JSON content of the declaration RawJSON json.RawMessage `db:"raw_json" json:"-"` + // Token is used to identify if declaration needs to be re-applied. + // It contains the checksum of the JSON contents and seconds-microseconds of secrets updated timestamp (if secret variables are present). + Token string `db:"token" json:"-"` + // Checksum is a checksum of the JSON contents - Checksum string `db:"checksum" json:"-"` + // BOZO: Needed? + // Checksum string `db:"checksum" json:"-"` // labels associated with this Declaration LabelsIncludeAll []ConfigurationProfileLabel `db:"-" json:"labels_include_all,omitempty"` @@ -762,7 +767,7 @@ type MDMAppleDDMManifest struct { // https://developer.apple.com/documentation/devicemanagement/declarationitemsresponse type MDMAppleDDMDeclarationItem struct { Identifier string `db:"identifier"` - ServerToken string `db:"checksum"` + ServerToken string `db:"token"` } // MDMAppleDDMDeclarationResponse represents a declaration in the datastore. It is used for the DDM diff --git a/server/fleet/apple_mdm_test.go b/server/fleet/apple_mdm_test.go index cf18288e5e82..80e914eb41ca 100644 --- a/server/fleet/apple_mdm_test.go +++ b/server/fleet/apple_mdm_test.go @@ -419,6 +419,70 @@ func TestMDMProfileIsWithinGracePeriod(t *testing.T) { } } +func TestMDMAppleHostDeclarationEqual(t *testing.T) { + t.Parallel() + + // This test is intended to ensure that the Equal method on MDMAppleHostDeclaration is updated when new fields are added. + // The Equal method is used to identify whether database update is needed. + + items := [...]MDMAppleHostDeclaration{{}, {}} + + numberOfFields := 0 + for i := 0; i < len(items); i++ { + rValue := reflect.ValueOf(&items[i]).Elem() + numberOfFields = rValue.NumField() + for j := 0; j < numberOfFields; j++ { + field := rValue.Field(j) + switch field.Kind() { + case reflect.String: + valueToSet := fmt.Sprintf("test %d", i) + field.SetString(valueToSet) + case reflect.Int: + field.SetInt(int64(i)) + case reflect.Bool: + field.SetBool(i%2 == 0) + case reflect.Pointer: + field.Set(reflect.New(field.Type().Elem())) + default: + t.Fatalf("unhandled field type %s", field.Kind()) + } + } + } + + status0 := MDMDeliveryStatus("status") + status1 := MDMDeliveryStatus("status") + items[0].Status = &status0 + assert.False(t, items[0].Equal(items[1])) + + // Set known fields to be equal + fieldsInEqualMethod := 0 + items[1].HostUUID = items[0].HostUUID + fieldsInEqualMethod++ + items[1].DeclarationUUID = items[0].DeclarationUUID + fieldsInEqualMethod++ + items[1].Name = items[0].Name + fieldsInEqualMethod++ + items[1].Identifier = items[0].Identifier + fieldsInEqualMethod++ + items[1].OperationType = items[0].OperationType + fieldsInEqualMethod++ + items[1].Detail = items[0].Detail + fieldsInEqualMethod++ + items[1].Checksum = items[0].Checksum + fieldsInEqualMethod++ + items[1].Status = &status1 + fieldsInEqualMethod++ + items[1].SecretsUpdatedAt = items[0].SecretsUpdatedAt + fieldsInEqualMethod++ + assert.Equal(t, fieldsInEqualMethod, numberOfFields, "MDMAppleHostDeclaration.Equal needs to be updated for new/updated field(s)") + assert.True(t, items[0].Equal(items[1])) + + // Set pointers to nil + items[0].Status = nil + items[1].Status = nil + assert.True(t, items[0].Equal(items[1])) +} + func TestConfigurationProfileLabelEqual(t *testing.T) { t.Parallel() diff --git a/server/fleet/mdm.go b/server/fleet/mdm.go index b441067398b8..1f5609fc6c23 100644 --- a/server/fleet/mdm.go +++ b/server/fleet/mdm.go @@ -492,7 +492,7 @@ func NewMDMConfigProfilePayloadFromAppleDDM(decl *MDMAppleDeclaration) *MDMConfi Name: decl.Name, Identifier: decl.Identifier, Platform: "darwin", - Checksum: []byte(decl.Checksum), + Checksum: []byte(decl.Token), CreatedAt: decl.CreatedAt, UploadedAt: decl.UploadedAt, LabelsIncludeAll: decl.LabelsIncludeAll, diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index ddb9e94d97bc..a3cf2da1c4dc 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -4260,7 +4260,7 @@ func (svc *MDMAppleDDMService) handleActivationDeclaration(ctx context.Context, }, "ServerToken": "%s", "Type": "com.apple.activation.simple" -}`, parts[2], references, d.Checksum) +}`, parts[2], references, d.Token) return []byte(response), nil } @@ -4283,7 +4283,7 @@ func (svc *MDMAppleDDMService) handleConfigurationDeclaration(ctx context.Contex if err := json.Unmarshal([]byte(expanded), &tempd); err != nil { return nil, ctxerr.Wrap(ctx, err, "unmarshaling stored declaration") } - tempd["ServerToken"] = d.Checksum + tempd["ServerToken"] = d.Token b, err := json.Marshal(tempd) if err != nil { diff --git a/server/service/integration_mdm_ddm_test.go b/server/service/integration_mdm_ddm_test.go index 91b23f2a466a..5dc805d3ac04 100644 --- a/server/service/integration_mdm_ddm_test.go +++ b/server/service/integration_mdm_ddm_test.go @@ -484,10 +484,6 @@ func (s *integrationMDMTestSuite) TestAppleDDMSecretVariables() { require.Equal(t, d.Identifier, m.Identifier) } } - calcChecksum := func(source []byte) string { - csum := fmt.Sprintf("%x", md5.Sum(source)) //nolint:gosec - return strings.ToUpper(csum) - } tmpl := ` { @@ -516,17 +512,6 @@ func (s *integrationMDMTestSuite) TestAppleDDMSecretVariables() { decls[1] = []byte(strings.ReplaceAll(string(decls[1]), myBash, "$"+fleet.ServerSecretPrefix+"BASH")) secretProfile := decls[2] decls[2] = []byte("${" + fleet.ServerSecretPrefix + "PROFILE}") - declsByChecksum := map[string]fleet.MDMAppleDeclaration{ - calcChecksum(decls[0]): { - Identifier: "com.fleet.config0", - }, - calcChecksum(decls[1]): { - Identifier: "com.fleet.config1", - }, - calcChecksum(decls[2]): { - Identifier: "com.fleet.config2", - }, - } // Create declarations // First dry run @@ -585,7 +570,7 @@ SELECT identifier, name, raw_json, - checksum, + token, created_at, uploaded_at FROM mdm_apple_declarations @@ -599,19 +584,28 @@ WHERE name = ?` } nameToIdentifier := make(map[string]string, 3) nameToUUID := make(map[string]string, 3) + declsByChecksum := map[string]fleet.MDMAppleDeclaration{} decl := getDeclaration(t, "N0") nameToIdentifier["N0"] = decl.Identifier nameToUUID["N0"] = decl.DeclarationUUID + declsByChecksum[decl.Token] = fleet.MDMAppleDeclaration{ + Identifier: "com.fleet.config0", + } decl = getDeclaration(t, "N1") assert.NotContains(t, string(decl.RawJSON), myBash) assert.Contains(t, string(decl.RawJSON), "$"+fleet.ServerSecretPrefix+"BASH") nameToIdentifier["N1"] = decl.Identifier nameToUUID["N1"] = decl.DeclarationUUID + declsByChecksum[decl.Token] = fleet.MDMAppleDeclaration{ + Identifier: "com.fleet.config1", + } decl = getDeclaration(t, "N2") assert.Equal(t, string(decl.RawJSON), "${"+fleet.ServerSecretPrefix+"PROFILE}") nameToIdentifier["N2"] = decl.Identifier nameToUUID["N2"] = decl.DeclarationUUID - + declsByChecksum[decl.Token] = fleet.MDMAppleDeclaration{ + Identifier: "com.fleet.config2", + } // trigger a profile sync s.awaitTriggerProfileSchedule(t)