Added ability to upload profiles with secret variables using the /con…

…figuration_profiles endpoint. (#25012)

Added ability to upload profiles with secret variables using the
/configuration_profiles endpoint.
#25011

# Checklist for submitter

- [x] If database migrations are included, checked table schema to
confirm autoupdate
- [x] Manual QA for all new/changed functionality

---------

Co-authored-by: Gabriel Hernandez <ghernandez345@gmail.com>

server/datastore/mysql/migrations/tables/20241209164540_AddSecretVariablesTable.go

@@ -19,8 +19,9 @@ func Up_20241209164540(tx *sql.Tx) error {
 		id INT UNSIGNED NOT NULL AUTO_INCREMENT,
 		name VARCHAR(255) NOT NULL,
 		value BLOB NOT NULL, -- 64KB max value size
-		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),
+		-- Using DATETIME instead of TIMESTAMP to prevent future Y2K38 issues, since updated_at is used as a trigger to resend profiles
+		created_at DATETIME(6) NOT NULL DEFAULT NOW(6),
+		updated_at DATETIME(6) NOT NULL DEFAULT NOW(6) ON UPDATE NOW(6),
 		PRIMARY KEY (id),
 		CONSTRAINT idx_secret_variables_name UNIQUE (name)
 	) ENGINE = InnoDB DEFAULT CHARSET = utf8mb4 COLLATE = utf8mb4_unicode_ci`,

server/datastore/mysql/schema.sql

@@ -1661,8 +1661,8 @@ CREATE TABLE `secret_variables` (
   `id` int unsigned NOT NULL AUTO_INCREMENT,
   `name` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL,
   `value` blob NOT NULL,
-  `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),
+  `created_at` datetime(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6),
+  `updated_at` datetime(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6),
   PRIMARY KEY (`id`),
   UNIQUE KEY `idx_secret_variables_name` (`name`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;

server/service/apple_mdm.go

@@ -392,12 +392,17 @@ func (svc *Service) NewMDMAppleConfigProfile(ctx context.Context, teamID uint, r
 			Message: fmt.Sprintf("failed to parse config profile: %s", err.Error()),
 		})
 	}
-	// Save the original unexpanded profile
-	cp.Mobileconfig = b
 
 	if err := cp.ValidateUserProvided(); err != nil {
 		return nil, ctxerr.Wrap(ctx, &fleet.BadRequestError{Message: err.Error()})
 	}
+	err = validateConfigProfileFleetVariables(string(cp.Mobileconfig))
+	if err != nil {
+		return nil, ctxerr.Wrap(ctx, err, "validating fleet variables")
+	}
+
+	// Save the original unexpanded profile
+	cp.Mobileconfig = b
 
 	labelMap, err := svc.validateProfileLabels(ctx, labels)
 	if err != nil {
@@ -414,11 +419,6 @@ func (svc *Service) NewMDMAppleConfigProfile(ctx context.Context, teamID uint, r
 		// TODO what happens if mode is not set?s
 	}
 
-	err = validateConfigProfileFleetVariables(string(cp.Mobileconfig))
-	if err != nil {
-		return nil, ctxerr.Wrap(ctx, err, "validating fleet variables")
-	}
-
 	newCP, err := svc.ds.NewMDMAppleConfigProfile(ctx, *cp)
 	if err != nil {
 		var existsErr existsErrorInterface
@@ -512,19 +512,21 @@ func (svc *Service) NewMDMAppleDeclaration(ctx context.Context, teamID uint, r i
 		return nil, err
 	}
 
-	if err := svc.ds.ValidateEmbeddedSecrets(ctx, []string{string(data)}); err != nil {
+	dataWithSecrets, err := svc.ds.ExpandEmbeddedSecrets(ctx, string(data))
+	if err != nil {
 		return nil, fleet.NewInvalidArgumentError("profile", err.Error())
 	}
 
-	if err := validateDeclarationFleetVariables(string(data)); err != nil {
+	if err := validateDeclarationFleetVariables(dataWithSecrets); err != nil {
 		return nil, err
 	}
 
 	// TODO(roberto): Maybe GetRawDeclarationValues belongs inside NewMDMAppleDeclaration? We can refactor this in a follow up.
-	rawDecl, err := fleet.GetRawDeclarationValues(data)
+	rawDecl, err := fleet.GetRawDeclarationValues([]byte(dataWithSecrets))
 	if err != nil {
 		return nil, err
 	}
+	// After validation, we should no longer need to keep the expanded secrets.
 
 	if err := rawDecl.ValidateUserProvided(); err != nil {
 		return nil, err

server/service/integration_mdm_profiles_test.go

@@ -5271,3 +5271,192 @@ func (s *integrationMDMTestSuite) TestOTAProfile() {
 	require.Contains(t, string(b), fmt.Sprintf("%s/api/v1/fleet/ota_enrollment?enroll_secret=%s", cfg.ServerSettings.ServerURL, escSec))
 	require.Contains(t, string(b), cfg.OrgInfo.OrgName)
 }
+
+// TestAppleDDMSecretVariablesUpload tests uploading DDM profiles with secrets via the /configuration_profiles endpoint
+func (s *integrationMDMTestSuite) TestAppleDDMSecretVariablesUpload() {
+	tmpl := `
+{
+	"Type": "com.apple.configuration.decl%d",
+	"Identifier": "com.fleet.config%d",
+	"Payload": {
+		"ServiceType": "com.apple.bash%d",
+		"DataAssetReference": "com.fleet.asset.bash"
+	}
+}`
+
+	newProfileBytes := func(i int) []byte {
+		return []byte(fmt.Sprintf(tmpl, i, i, i))
+	}
+
+	getProfileContents := func(profileUUID string) string {
+		profile, err := s.ds.GetMDMAppleDeclaration(context.Background(), profileUUID)
+		require.NoError(s.T(), err)
+		return string(profile.RawJSON)
+	}
+
+	s.testSecretVariablesUpload(newProfileBytes, getProfileContents, "json", "darwin")
+}
+
+func (s *integrationMDMTestSuite) testSecretVariablesUpload(newProfileBytes func(i int) []byte,
+	getProfileContents func(profileUUID string) string, fileExtension string, platform string) {
+	t := s.T()
+	const numProfiles = 2
+	var profiles [][]byte
+	for i := 0; i < numProfiles; i++ {
+		profiles = append(profiles, newProfileBytes(i))
+	}
+	// Use secrets
+	myBash := "com.apple.bash0"
+	profiles[0] = []byte(strings.ReplaceAll(string(profiles[0]), myBash, "$"+fleet.ServerSecretPrefix+"BASH"))
+	secretProfile := profiles[1]
+	profiles[1] = []byte("${" + fleet.ServerSecretPrefix + "PROFILE}")
+
+	body, headers := generateNewProfileMultipartRequest(
+		t, "secret-config0."+fileExtension, profiles[0], s.token, nil,
+	)
+	res := s.DoRawWithHeaders("POST", "/api/latest/fleet/configuration_profiles", body.Bytes(), http.StatusUnprocessableEntity, headers)
+	assertBodyContains(t, res, `Secret variable \"$FLEET_SECRET_BASH\" missing`)
+
+	// Add secret(s) to server
+	req := secretVariablesRequest{
+		SecretVariables: []fleet.SecretVariable{
+			{
+				Name:  "FLEET_SECRET_BASH",
+				Value: myBash,
+			},
+			{
+				Name:  "FLEET_SECRET_PROFILE",
+				Value: string(secretProfile),
+			},
+		},
+	}
+	secretResp := secretVariablesResponse{}
+	s.DoJSON("PUT", "/api/latest/fleet/spec/secret_variables", req, http.StatusOK, &secretResp)
+	res = s.DoRawWithHeaders("POST", "/api/latest/fleet/configuration_profiles", body.Bytes(), http.StatusOK, headers)
+	var resp newMDMConfigProfileResponse
+	err := json.NewDecoder(res.Body).Decode(&resp)
+	require.NoError(t, err)
+	assert.NotEmpty(t, resp.ProfileUUID)
+
+	body, headers = generateNewProfileMultipartRequest(
+		t, "secret-config1."+fileExtension, profiles[1], s.token, nil,
+	)
+	s.DoJSON("PUT", "/api/latest/fleet/spec/secret_variables", req, http.StatusOK, &secretResp)
+	res = s.DoRawWithHeaders("POST", "/api/latest/fleet/configuration_profiles", body.Bytes(), http.StatusOK, headers)
+	err = json.NewDecoder(res.Body).Decode(&resp)
+	require.NoError(t, err)
+	assert.NotEmpty(t, resp.ProfileUUID)
+
+	var listResp listMDMConfigProfilesResponse
+	s.DoJSON("GET", "/api/latest/fleet/mdm/profiles", &listMDMConfigProfilesRequest{}, http.StatusOK, &listResp)
+	require.Len(t, listResp.Profiles, numProfiles)
+	profileUUIDs := make([]string, numProfiles)
+	for _, p := range listResp.Profiles {
+		switch p.Name {
+		case "secret-config0":
+			assert.Equal(t, platform, p.Platform)
+			profileUUIDs[0] = p.ProfileUUID
+		case "secret-config1":
+			assert.Equal(t, platform, p.Platform)
+			profileUUIDs[1] = p.ProfileUUID
+		default:
+			t.Errorf("unexpected profile %s", p.Name)
+		}
+	}
+
+	// Check that contents are masking secret values
+	for i := 0; i < numProfiles; i++ {
+		assert.Equal(t, string(profiles[i]), getProfileContents(profileUUIDs[i]))
+	}
+
+	// Delete profiles -- make sure there is no issue deleting profiles with secrets
+	for i := 0; i < numProfiles; i++ {
+		s.Do("DELETE", "/api/latest/fleet/configuration_profiles/"+profileUUIDs[i], nil, http.StatusOK)
+	}
+	s.DoJSON("GET", "/api/latest/fleet/mdm/profiles", &listMDMConfigProfilesRequest{}, http.StatusOK, &listResp)
+	require.Empty(t, listResp.Profiles)
+
+}
+
+// TestAppleConfigSecretVariablesUpload tests uploading Apple config profiles with secrets via the /configuration_profiles endpoint
+func (s *integrationMDMTestSuite) TestAppleConfigSecretVariablesUpload() {
+	tmpl := `
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+  <key>PayloadDescription</key>
+  <string>For secret variables</string>
+  <key>PayloadDisplayName</key>
+  <string>secret-config%d</string>
+  <key>PayloadIdentifier</key>
+  <string>PI%d</string>
+  <key>PayloadType</key>
+  <string>Configuration</string>
+  <key>PayloadUUID</key>
+  <string>%d</string>
+  <key>PayloadVersion</key>
+  <integer>1</integer>
+  <key>PayloadContent</key>
+  <array>
+    <dict>
+      <key>Bash</key>
+      <string>$FLEET_SECRET_BASH</string>
+      <key>PayloadDisplayName</key>
+      <string>secret payload</string>
+      <key>PayloadIdentifier</key>
+      <string>com.test.secret</string>
+      <key>PayloadType</key>
+      <string>com.test.secretd</string>
+      <key>PayloadUUID</key>
+      <string>476F5334-D501-4768-9A31-1A18A4E1E808</string>
+      <key>PayloadVersion</key>
+      <integer>1</integer>
+    </dict>
+  </array>
+</dict>
+</plist>`
+
+	newProfileBytes := func(i int) []byte {
+		return []byte(fmt.Sprintf(tmpl, i, i, i))
+	}
+
+	getProfileContents := func(profileUUID string) string {
+		profile, err := s.ds.GetMDMAppleConfigProfile(context.Background(), profileUUID)
+		require.NoError(s.T(), err)
+		return string(profile.Mobileconfig)
+	}
+
+	s.testSecretVariablesUpload(newProfileBytes, getProfileContents, "mobileconfig", "darwin")
+
+}
+
+// TestWindowsConfigSecretVariablesUpload tests uploading Windows profiles with secrets via the /configuration_profiles endpoint
+func (s *integrationMDMTestSuite) TestWindowsConfigSecretVariablesUpload() {
+	tmpl := `
+<Replace>
+    <Item>
+        <Meta>
+            <Format xmlns="syncml:metinf">int</Format>
+        </Meta>
+        <Target>
+            <LocURI>./Device/Vendor/MSFT/Policy/Config/System/DisableOneDriveFileSync</LocURI>
+        </Target>
+        <Data>$FLEET_SECRET_BASH</Data>
+    </Item>
+</Replace>
+`
+
+	newProfileBytes := func(i int) []byte {
+		return []byte(fmt.Sprintf(tmpl, i, i, i))
+	}
+
+	getProfileContents := func(profileUUID string) string {
+		profile, err := s.ds.GetMDMWindowsConfigProfile(context.Background(), profileUUID)
+		require.NoError(s.T(), err)
+		return string(profile.SyncML)
+	}
+
+	s.testSecretVariablesUpload(newProfileBytes, getProfileContents, "xml", "windows")
+
+}

server/service/mdm.go

@@ -1426,12 +1426,12 @@ func (svc *Service) NewMDMWindowsConfigProfile(ctx context.Context, teamID uint,
 	}
 
 	if err := svc.ds.ValidateEmbeddedSecrets(ctx, []string{string(cp.SyncML)}); err != nil {
-		return nil, ctxerr.Wrap(ctx, err, "validating fleet secrets")
+		return nil, ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("profile", err.Error()))
 	}
 
 	err = validateWindowsProfileFleetVariables(string(cp.SyncML))
 	if err != nil {
-		return nil, ctxerr.Wrap(ctx, err, "validating Windows profile")
+		return nil, ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("profile", err.Error()))
 	}
 
 	newCP, err := svc.ds.NewMDMWindowsConfigProfile(ctx, cp)