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

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

Merged
merged 6 commits into from
Dec 30, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be a new pattern? Or is this just for this one case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DATETIME uses more storage than TIMESTAMP. We can discuss in backend sync.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, is this change for the next PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I just wanted to get it in since I wasn't sure if the next PR would make the cut/release.

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`,
Expand Down
4 changes: 2 additions & 2 deletions server/datastore/mysql/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1660,8 +1660,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;
Expand Down
22 changes: 12 additions & 10 deletions server/service/apple_mdm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
189 changes: 189 additions & 0 deletions server/service/integration_mdm_profiles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

}
4 changes: 2 additions & 2 deletions server/service/mdm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading