Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug where DDM/Windows profiles with secrets were not being marked Verified. #25065

Merged
merged 4 commits into from
Jan 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 30 additions & 34 deletions server/datastore/mysql/apple_mdm.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,6 @@ SELECT
name,
identifier,
raw_json,
checksum,
token,
created_at,
uploaded_at,
Expand Down Expand Up @@ -2348,6 +2347,7 @@ var mdmEntityTypeToDynamicNames = map[string]map[string]string{
"mdmEntityLabelsTable": "mdm_declaration_labels",
"appleEntityUUIDColumn": "apple_declaration_uuid",
"hostMDMAppleEntityTable": "host_mdm_apple_declarations",
"checksumColumn": "token",
},
"profile": {
"entityUUIDColumn": "profile_uuid",
Expand All @@ -2358,6 +2358,7 @@ var mdmEntityTypeToDynamicNames = map[string]map[string]string{
"mdmEntityLabelsTable": "mdm_configuration_profile_labels",
"appleEntityUUIDColumn": "apple_profile_uuid",
"hostMDMAppleEntityTable": "host_mdm_apple_profiles",
"checksumColumn": "checksum",
},
}

Expand All @@ -2377,7 +2378,7 @@ func generateDesiredStateQuery(entityType string) string {
h.platform as host_platform,
mae.identifier as ${entityIdentifierColumn},
mae.name as ${entityNameColumn},
mae.checksum as checksum,
mae.${checksumColumn} as ${checksumColumn},
mae.secrets_updated_at as secrets_updated_at,
0 as ${countEntityLabelsColumn},
0 as count_non_broken_labels,
Expand Down Expand Up @@ -2411,7 +2412,7 @@ func generateDesiredStateQuery(entityType string) string {
h.platform as host_platform,
mae.identifier as ${entityIdentifierColumn},
mae.name as ${entityNameColumn},
mae.checksum as checksum,
mae.${checksumColumn} as ${checksumColumn},
mae.secrets_updated_at as secrets_updated_at,
COUNT(*) as ${countEntityLabelsColumn},
COUNT(mel.label_id) as count_non_broken_labels,
Expand All @@ -2433,7 +2434,7 @@ func generateDesiredStateQuery(entityType string) string {
ne.type = 'Device' AND
( %s )
GROUP BY
mae.${entityUUIDColumn}, h.uuid, h.platform, mae.identifier, mae.name, mae.checksum, mae.secrets_updated_at
mae.${entityUUIDColumn}, h.uuid, h.platform, mae.identifier, mae.name, mae.${checksumColumn}, mae.secrets_updated_at
HAVING
${countEntityLabelsColumn} > 0 AND count_host_labels = ${countEntityLabelsColumn}

Expand All @@ -2450,7 +2451,7 @@ func generateDesiredStateQuery(entityType string) string {
h.platform as host_platform,
mae.identifier as ${entityIdentifierColumn},
mae.name as ${entityNameColumn},
mae.checksum as checksum,
mae.${checksumColumn} as ${checksumColumn},
mae.secrets_updated_at as secrets_updated_at,
COUNT(*) as ${countEntityLabelsColumn},
COUNT(mel.label_id) as count_non_broken_labels,
Expand All @@ -2476,7 +2477,7 @@ func generateDesiredStateQuery(entityType string) string {
ne.type = 'Device' AND
( %s )
GROUP BY
mae.${entityUUIDColumn}, h.uuid, h.platform, mae.identifier, mae.name, mae.checksum, mae.secrets_updated_at
mae.${entityUUIDColumn}, h.uuid, h.platform, mae.identifier, mae.name, mae.${checksumColumn}, mae.secrets_updated_at
HAVING
-- considers only the profiles with labels, without any broken label, with results reported after all labels were created and with the host not in any label
${countEntityLabelsColumn} > 0 AND ${countEntityLabelsColumn} = count_non_broken_labels AND ${countEntityLabelsColumn} = count_host_updated_after_labels AND count_host_labels = 0
Expand All @@ -2492,7 +2493,7 @@ func generateDesiredStateQuery(entityType string) string {
h.platform as host_platform,
mae.identifier as ${entityIdentifierColumn},
mae.name as ${entityNameColumn},
mae.checksum as checksum,
mae.${checksumColumn} as ${checksumColumn},
mae.secrets_updated_at as secrets_updated_at,
COUNT(*) as ${countEntityLabelsColumn},
COUNT(mel.label_id) as count_non_broken_labels,
Expand All @@ -2514,7 +2515,7 @@ func generateDesiredStateQuery(entityType string) string {
ne.type = 'Device' AND
( %s )
GROUP BY
mae.${entityUUIDColumn}, h.uuid, h.platform, mae.identifier, mae.name, mae.checksum, mae.secrets_updated_at
mae.${entityUUIDColumn}, h.uuid, h.platform, mae.identifier, mae.name, mae.${checksumColumn}, mae.secrets_updated_at
HAVING
${countEntityLabelsColumn} > 0 AND count_host_labels >= 1
`, func(s string) string { return dynamicNames[s] })
Expand Down Expand Up @@ -2568,7 +2569,7 @@ func generateEntitiesToInstallQuery(entityType string) string {
ON hmae.${entityUUIDColumn} = ds.${entityUUIDColumn} AND hmae.host_uuid = ds.host_uuid
WHERE
-- entity has been updated
( hmae.checksum != ds.checksum ) OR IFNULL(hmae.secrets_updated_at < ds.secrets_updated_at, FALSE) OR
( hmae.${checksumColumn} != ds.${checksumColumn} ) OR IFNULL(hmae.secrets_updated_at < ds.secrets_updated_at, FALSE) OR
-- entity in A but not in B
( hmae.${entityUUIDColumn} IS NULL AND hmae.host_uuid IS NULL ) OR
-- entities in A and B but with operation type "remove"
Expand Down Expand Up @@ -4216,17 +4217,15 @@ INSERT INTO mdm_apple_declarations (
identifier,
name,
raw_json,
checksum,
secrets_updated_at,
uploaded_at,
team_id
)
VALUES (
?,?,?,?,UNHEX(MD5(raw_json)),?,CURRENT_TIMESTAMP(),?
?,?,?,?,?,CURRENT_TIMESTAMP(),?
)
ON DUPLICATE KEY UPDATE
uploaded_at = IF(checksum = VALUES(checksum) AND name = VALUES(name), uploaded_at, CURRENT_TIMESTAMP()),
checksum = VALUES(checksum),
uploaded_at = IF(raw_json = VALUES(raw_json) AND name = VALUES(name) AND NULLIF(secrets_updated_at = VALUES(secrets_updated_at), TRUE), uploaded_at, CURRENT_TIMESTAMP()),
secrets_updated_at = VALUES(secrets_updated_at),
name = VALUES(name),
identifier = VALUES(identifier),
Expand Down Expand Up @@ -4408,10 +4407,9 @@ INSERT INTO mdm_apple_declarations (
identifier,
name,
raw_json,
checksum,
secrets_updated_at,
uploaded_at)
(SELECT ?,?,?,?,?,UNHEX(MD5(?)),?,CURRENT_TIMESTAMP() FROM DUAL WHERE
(SELECT ?,?,?,?,?,?,CURRENT_TIMESTAMP() FROM DUAL WHERE
NOT EXISTS (
SELECT 1 FROM mdm_windows_configuration_profiles WHERE name = ? AND team_id = ?
) AND NOT EXISTS (
Expand All @@ -4430,10 +4428,9 @@ INSERT INTO mdm_apple_declarations (
identifier,
name,
raw_json,
checksum,
secrets_updated_at,
uploaded_at)
(SELECT ?,?,?,?,?,UNHEX(MD5(?)),?,CURRENT_TIMESTAMP() FROM DUAL WHERE
(SELECT ?,?,?,?,?,?,CURRENT_TIMESTAMP() FROM DUAL WHERE
NOT EXISTS (
SELECT 1 FROM mdm_windows_configuration_profiles WHERE name = ? AND team_id = ?
) AND NOT EXISTS (
Expand All @@ -4442,9 +4439,8 @@ INSERT INTO mdm_apple_declarations (
)
ON DUPLICATE KEY UPDATE
identifier = VALUES(identifier),
uploaded_at = IF(checksum = VALUES(checksum) AND name = VALUES(name), uploaded_at, CURRENT_TIMESTAMP()),
raw_json = VALUES(raw_json),
checksum = VALUES(checksum)`
uploaded_at = IF(raw_json = VALUES(raw_json) AND name = VALUES(name) AND NULLIF(secrets_updated_at = VALUES(secrets_updated_at), TRUE), uploaded_at, CURRENT_TIMESTAMP()),
raw_json = VALUES(raw_json)`

return ds.insertOrUpsertMDMAppleDeclaration(ctx, stmt, declaration)
}
Expand All @@ -4461,7 +4457,7 @@ func (ds *Datastore) insertOrUpsertMDMAppleDeclaration(ctx context.Context, insO

err := ds.withTx(ctx, func(tx sqlx.ExtContext) error {
res, err := tx.ExecContext(ctx, insOrUpsertStmt,
declUUID, tmID, declaration.Identifier, declaration.Name, declaration.RawJSON, declaration.RawJSON,
declUUID, tmID, declaration.Identifier, declaration.Name, declaration.RawJSON,
declaration.SecretsUpdatedAt,
declaration.Name, tmID, declaration.Name, tmID)
if err != nil {
Expand Down Expand Up @@ -4783,13 +4779,13 @@ func mdmAppleBatchSetPendingHostDeclarationsDB(
) (updatedDB bool, err error) {
baseStmt := `
INSERT INTO host_mdm_apple_declarations
(host_uuid, status, operation_type, checksum, secrets_updated_at, declaration_uuid, declaration_identifier, declaration_name)
(host_uuid, status, operation_type, token, secrets_updated_at, declaration_uuid, declaration_identifier, declaration_name)
VALUES
%s
ON DUPLICATE KEY UPDATE
status = VALUES(status),
operation_type = VALUES(operation_type),
checksum = VALUES(checksum),
token = VALUES(token),
secrets_updated_at = VALUES(secrets_updated_at)
`

Expand All @@ -4804,7 +4800,7 @@ func mdmAppleBatchSetPendingHostDeclarationsDB(
status,
COALESCE(operation_type, '') AS operation_type,
COALESCE(detail, '') AS detail,
checksum,
token,
secrets_updated_at,
declaration_uuid,
declaration_identifier,
Expand Down Expand Up @@ -4855,11 +4851,11 @@ func mdmAppleBatchSetPendingHostDeclarationsDB(
Status: status,
OperationType: d.OperationType,
Detail: d.Detail,
Checksum: d.Checksum,
Token: d.Token,
SecretsUpdatedAt: d.SecretsUpdatedAt,
}
valuePart := "(?, ?, ?, ?, ?, ?, ?, ?),"
args := []any{d.HostUUID, status, d.OperationType, d.Checksum, d.SecretsUpdatedAt, d.DeclarationUUID, d.Identifier, d.Name}
args := []any{d.HostUUID, status, d.OperationType, d.Token, d.SecretsUpdatedAt, d.DeclarationUUID, d.Identifier, d.Name}
return valuePart, args
}

Expand All @@ -4883,7 +4879,7 @@ func mdmAppleGetHostsWithChangedDeclarationsDB(ctx context.Context, tx sqlx.ExtC
SELECT
ds.host_uuid,
'install' as operation_type,
ds.checksum,
ds.token,
ds.secrets_updated_at,
ds.declaration_uuid,
ds.declaration_identifier,
Expand All @@ -4896,7 +4892,7 @@ func mdmAppleGetHostsWithChangedDeclarationsDB(ctx context.Context, tx sqlx.ExtC
SELECT
hmae.host_uuid,
'remove' as operation_type,
hmae.checksum,
hmae.token,
hmae.secrets_updated_at,
hmae.declaration_uuid,
hmae.declaration_identifier,
Expand All @@ -4919,14 +4915,14 @@ func mdmAppleGetHostsWithChangedDeclarationsDB(ctx context.Context, tx sqlx.ExtC
// MDMAppleStoreDDMStatusReport updates the status of the host's declarations.
func (ds *Datastore) MDMAppleStoreDDMStatusReport(ctx context.Context, hostUUID string, updates []*fleet.MDMAppleHostDeclaration) error {
getHostDeclarationsStmt := `
SELECT host_uuid, status, operation_type, HEX(checksum) as checksum, secrets_updated_at, declaration_uuid, declaration_identifier, declaration_name
SELECT host_uuid, status, operation_type, HEX(token) as token, secrets_updated_at, declaration_uuid, declaration_identifier, declaration_name
FROM host_mdm_apple_declarations
WHERE host_uuid = ?
`

updateHostDeclarationsStmt := `
INSERT INTO host_mdm_apple_declarations
(host_uuid, declaration_uuid, status, operation_type, detail, declaration_name, declaration_identifier, checksum, secrets_updated_at)
(host_uuid, declaration_uuid, status, operation_type, detail, declaration_name, declaration_identifier, token, secrets_updated_at)
VALUES
%s
ON DUPLICATE KEY UPDATE
Expand All @@ -4945,17 +4941,17 @@ ON DUPLICATE KEY UPDATE
return ctxerr.Wrap(ctx, err, "getting current host declarations")
}

updatesByChecksum := make(map[string]*fleet.MDMAppleHostDeclaration, len(updates))
updatesByToken := make(map[string]*fleet.MDMAppleHostDeclaration, len(updates))
for _, u := range updates {
updatesByChecksum[u.Checksum] = u
updatesByToken[u.Token] = u
}

var args []any
var insertVals strings.Builder
for _, c := range current {
if u, ok := updatesByChecksum[c.Checksum]; ok {
if u, ok := updatesByToken[c.Token]; ok {
insertVals.WriteString("(?, ?, ?, ?, ?, ?, ?, UNHEX(?), ?),")
args = append(args, hostUUID, c.DeclarationUUID, u.Status, u.OperationType, u.Detail, c.Identifier, c.Name, c.Checksum,
args = append(args, hostUUID, c.DeclarationUUID, u.Status, u.OperationType, u.Detail, c.Identifier, c.Name, c.Token,
c.SecretsUpdatedAt)
}
}
Expand Down
2 changes: 1 addition & 1 deletion server/datastore/mysql/apple_mdm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1084,7 +1084,7 @@ func expectAppleDeclarations(
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
ctx := context.Background()
return sqlx.SelectContext(ctx, q, &got,
`SELECT declaration_uuid, team_id, identifier, name, raw_json, checksum, created_at, uploaded_at FROM mdm_apple_declarations WHERE team_id = ?`,
`SELECT declaration_uuid, team_id, identifier, name, raw_json, token, created_at, uploaded_at FROM mdm_apple_declarations WHERE team_id = ?`,
tmID)
})

Expand Down
2 changes: 1 addition & 1 deletion server/datastore/mysql/mdm.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ FROM (
name,
'darwin' AS platform,
identifier,
checksum AS checksum,
token AS checksum,
created_at,
uploaded_at
FROM mdm_apple_declarations
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package tables

import (
"database/sql"
"fmt"
)

func init() {
MigrationClient.AddMigration(Up_20241231112624, Down_20241231112624)
}

func Up_20241231112624(tx *sql.Tx) error {
_, err := tx.Exec(`ALTER TABLE host_mdm_apple_declarations
RENAME COLUMN checksum TO token`)
if err != nil {
return fmt.Errorf("failed to alter mdm_apple_configuration_profiles table: %w", err)
}

_, err = tx.Exec(`ALTER TABLE mdm_apple_declarations
DROP COLUMN checksum`)
if err != nil {
return fmt.Errorf("failed to alter mdm_apple_configuration_profiles table: %w", err)
}

return nil
}

func Down_20241231112624(_ *sql.Tx) error {
return nil
}
7 changes: 3 additions & 4 deletions server/datastore/mysql/schema.sql

Large diffs are not rendered by default.

11 changes: 4 additions & 7 deletions server/fleet/apple_mdm.go
Original file line number Diff line number Diff line change
Expand Up @@ -588,9 +588,6 @@ type MDMAppleDeclaration struct {
// RawJSON is the raw JSON content of the declaration
RawJSON json.RawMessage `db:"raw_json" json:"-"`

// Checksum is a checksum of the JSON contents
Checksum string `db:"checksum" json:"-"`

// Token is used to identify if declaration needs to be re-applied.
// It contains the checksum of the JSON contents and secrets updated timestamp (if secret variables are present).
Token string `db:"token" json:"-"`
Expand Down Expand Up @@ -687,9 +684,9 @@ type MDMAppleHostDeclaration struct {
// either by the MDM protocol or the Fleet server.
Detail string `db:"detail" json:"detail"`

// Checksum contains the MD5 checksum of the declaration JSON uploaded
// by the IT admin. Fleet uses this value as the ServerToken.
Checksum string `db:"checksum" json:"-"`
// Token is used to identify if declaration needs to be re-applied.
// It contains the checksum of the JSON contents and secrets updated timestamp (if secret variables are present).
Token string `db:"token" json:"-"`

// SecretsUpdatedAt is the timestamp when the secrets were last updated or when this declaration was uploaded.
SecretsUpdatedAt *time.Time `db:"secrets_updated_at" json:"-"`
Expand All @@ -705,7 +702,7 @@ func (p MDMAppleHostDeclaration) Equal(other MDMAppleHostDeclaration) bool {
p.Identifier == other.Identifier &&
p.OperationType == other.OperationType &&
p.Detail == other.Detail &&
p.Checksum == other.Checksum &&
p.Token == other.Token &&
secretsEqual
}

Expand Down
2 changes: 1 addition & 1 deletion server/fleet/apple_mdm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ func TestMDMAppleHostDeclarationEqual(t *testing.T) {
fieldsInEqualMethod++
items[1].Detail = items[0].Detail
fieldsInEqualMethod++
items[1].Checksum = items[0].Checksum
items[1].Token = items[0].Token
fieldsInEqualMethod++
items[1].Status = &status1
fieldsInEqualMethod++
Expand Down
3 changes: 3 additions & 0 deletions server/fleet/datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -1954,6 +1954,9 @@ type ProfileVerificationStore interface {
// profile status. It deletes the row if the profile operation is "remove"
// and the status is "verifying" (i.e. successfully removed).
UpdateOrDeleteHostMDMAppleProfile(ctx context.Context, profile *HostMDMAppleProfile) error
// ExpandEmbeddedSecrets expands the fleet secrets in a
// document using the secrets stored in the datastore.
ExpandEmbeddedSecrets(ctx context.Context, document string) (string, error)
}

var _ ProfileVerificationStore = (Datastore)(nil)
Expand Down
18 changes: 11 additions & 7 deletions server/fleet/mdm.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,12 +416,16 @@ func (m MDMConfigProfileAuthz) AuthzType() string {
// MDMConfigProfilePayload is the platform-agnostic struct returned by
// endpoints that return MDM configuration profiles (get/list profiles).
type MDMConfigProfilePayload struct {
ProfileUUID string `json:"profile_uuid" db:"profile_uuid"`
TeamID *uint `json:"team_id" db:"team_id"` // null for no-team
Name string `json:"name" db:"name"`
Platform string `json:"platform" db:"platform"` // "windows" or "darwin"
Identifier string `json:"identifier,omitempty" db:"identifier"` // only set for macOS
Checksum []byte `json:"checksum,omitempty" db:"checksum"` // only set for macOS
ProfileUUID string `json:"profile_uuid" db:"profile_uuid"`
TeamID *uint `json:"team_id" db:"team_id"` // null for no-team
Name string `json:"name" db:"name"`
Platform string `json:"platform" db:"platform"` // "windows" or "darwin"
Identifier string `json:"identifier,omitempty" db:"identifier"` // only set for macOS
// Checksum is the following
// - for Apple configuration profiles: the MD5 checksum of the profile contents
// - for Apple device declarations: the MD5 checksum of the profile contents and secrets updated timestamp (if profile contains secret variables)
// - for Windows: always empty
Checksum []byte `json:"checksum,omitempty" db:"checksum"`
CreatedAt time.Time `json:"created_at" db:"created_at"`
UploadedAt time.Time `json:"updated_at" db:"uploaded_at"` // NOTE: JSON field is still `updated_at` for historical reasons, would be an API breaking change
LabelsIncludeAll []ConfigurationProfileLabel `json:"labels_include_all,omitempty" db:"-"`
Expand Down Expand Up @@ -493,7 +497,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,
Expand Down
Loading
Loading