From f0494961d5f83c756682aa68e02a31bbb83b92cb Mon Sep 17 00:00:00 2001 From: Konstantin Sykulev Date: Mon, 1 Dec 2025 19:10:55 -0600 Subject: [PATCH] validating certificate template subjectname fleet vars (#36377) **Related issue:** Resolves #36289 ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually --- server/datastore/mysql/teams.go | 17 ++++--- server/service/certificate_templates.go | 62 +++++++++++++++++++++++++ server/service/certificates.go | 40 ++++------------ server/service/client.go | 7 +++ server/service/integration_core_test.go | 14 +++++- 5 files changed, 102 insertions(+), 38 deletions(-) create mode 100644 server/service/certificate_templates.go diff --git a/server/datastore/mysql/teams.go b/server/datastore/mysql/teams.go index 8b73595713..6d00314d24 100644 --- a/server/datastore/mysql/teams.go +++ b/server/datastore/mysql/teams.go @@ -134,6 +134,7 @@ var teamRefs = []string{ "mdm_android_configuration_profiles", "software_title_icons", "software_title_display_names", + "certificate_templates", } func (ds *Datastore) DeleteTeam(ctx context.Context, tid uint) error { @@ -145,6 +146,15 @@ func (ds *Datastore) DeleteTeam(ctx context.Context, tid uint) error { return ctxerr.Wrapf(ctx, err, "deleting policies for team %d", tid) } + // Delete related records from teamRefs tables before deleting the team itself + // to avoid foreign key constraint violations + for _, table := range teamRefs { + _, err = tx.ExecContext(ctx, fmt.Sprintf(`DELETE FROM %s WHERE team_id=?`, table), tid) + if err != nil { + return ctxerr.Wrapf(ctx, err, "deleting %s for team %d", table, tid) + } + } + _, err = tx.ExecContext(ctx, `DELETE FROM teams WHERE id = ?`, tid) if err != nil { return ctxerr.Wrapf(ctx, err, "delete team %d", tid) @@ -155,13 +165,6 @@ func (ds *Datastore) DeleteTeam(ctx context.Context, tid uint) error { return ctxerr.Wrapf(ctx, err, "deleting pack_targets for team %d", tid) } - for _, table := range teamRefs { - _, err = tx.ExecContext(ctx, fmt.Sprintf(`DELETE FROM %s WHERE team_id=?`, table), tid) - if err != nil { - return ctxerr.Wrapf(ctx, err, "deleting %s for team %d", table, tid) - } - } - return nil }) } diff --git a/server/service/certificate_templates.go b/server/service/certificate_templates.go new file mode 100644 index 0000000000..db17c71e58 --- /dev/null +++ b/server/service/certificate_templates.go @@ -0,0 +1,62 @@ +package service + +import ( + "context" + "fmt" + "slices" + + "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" + "github.com/fleetdm/fleet/v4/server/fleet" + "github.com/fleetdm/fleet/v4/server/variables" +) + +// Fleet variables supported in certificate template subject names. +var fleetVarsSupportedInCertificateTemplates = []fleet.FleetVarName{ + fleet.FleetVarHostUUID, + fleet.FleetVarHostHardwareSerial, + fleet.FleetVarHostEndUserIDPUsername, +} + +func validateCertificateTemplateFleetVariables(subjectName string) error { + fleetVars := variables.Find(subjectName) + if len(fleetVars) == 0 { + return nil + } + + for _, fleetVar := range fleetVars { + if !slices.Contains(fleetVarsSupportedInCertificateTemplates, fleet.FleetVarName(fleetVar)) { + return fmt.Errorf("Fleet variable $FLEET_VAR_%s is not supported in certificate templates", fleetVar) + } + } + + return nil +} + +// replaceCertificateVariables replaces FLEET_VAR_* variables in the subject name with actual host values +func (svc *Service) replaceCertificateVariables(ctx context.Context, subjectName string, host *fleet.Host) (string, error) { + fleetVars := variables.Find(subjectName) + if len(fleetVars) == 0 { + return subjectName, nil + } + + result := subjectName + for _, fleetVar := range fleetVars { + switch fleetVar { + case string(fleet.FleetVarHostUUID): + result = fleet.FleetVarHostUUIDRegexp.ReplaceAllString(result, host.UUID) + case string(fleet.FleetVarHostHardwareSerial): + result = fleet.FleetVarHostHardwareSerialRegexp.ReplaceAllString(result, host.HardwareSerial) + case string(fleet.FleetVarHostEndUserIDPUsername): + users, err := fleet.GetEndUsers(ctx, svc.ds, host.ID) + if err != nil { + return "", ctxerr.Wrapf(ctx, err, "getting host end users for variable %s", fleetVar) + } + if len(users) == 0 || users[0].IdpUserName == "" { + return "", ctxerr.Errorf(ctx, "host %s does not have an IDP username for variable %s", host.UUID, fleetVar) + } + result = fleet.FleetVarHostEndUserIDPUsernameRegexp.ReplaceAllString(result, users[0].IdpUserName) + } + } + + return result, nil +} diff --git a/server/service/certificates.go b/server/service/certificates.go index 5871293809..4e97237622 100644 --- a/server/service/certificates.go +++ b/server/service/certificates.go @@ -3,12 +3,12 @@ package service import ( "context" "errors" + "fmt" "strconv" "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" hostctx "github.com/fleetdm/fleet/v4/server/contexts/host" "github.com/fleetdm/fleet/v4/server/fleet" - "github.com/fleetdm/fleet/v4/server/variables" ) type createCertificateTemplateRequest struct { @@ -46,6 +46,10 @@ func (svc *Service) CreateCertificateTemplate(ctx context.Context, name string, return nil, err } + if err := validateCertificateTemplateFleetVariables(subjectName); err != nil { + return nil, &fleet.BadRequestError{Message: err.Error()} + } + certTemplate := &fleet.CertificateTemplate{ Name: name, TeamID: teamID, @@ -285,6 +289,11 @@ func (svc *Service) ApplyCertificateTemplateSpecs(ctx context.Context, specs []* teamID = uint(parsed) } + // Validate Fleet variables in subject name + if err := validateCertificateTemplateFleetVariables(spec.SubjectName); err != nil { + return &fleet.BadRequestError{Message: fmt.Sprintf("%s (certificate %s)", err.Error(), spec.Name)} + } + cert := &fleet.CertificateTemplate{ Name: spec.Name, CertificateAuthorityID: spec.CertificateAuthorityId, @@ -325,35 +334,6 @@ func (svc *Service) DeleteCertificateTemplateSpecs(ctx context.Context, certific return svc.ds.BatchDeleteCertificateTemplates(ctx, certificateTemplateIDs) } -// replaceCertificateVariables replaces FLEET_VAR_* variables in the subject name with actual host values -func (svc *Service) replaceCertificateVariables(ctx context.Context, subjectName string, host *fleet.Host) (string, error) { - fleetVars := variables.Find(subjectName) - if len(fleetVars) == 0 { - return subjectName, nil - } - - result := subjectName - for _, fleetVar := range fleetVars { - switch fleetVar { - case "HOST_UUID": - result = fleet.FleetVarHostUUIDRegexp.ReplaceAllString(result, host.UUID) - case "HOST_HARDWARE_SERIAL": - result = fleet.FleetVarHostHardwareSerialRegexp.ReplaceAllString(result, host.HardwareSerial) - case "HOST_END_USER_IDP_USERNAME": - users, err := fleet.GetEndUsers(ctx, svc.ds, host.ID) - if err != nil { - return "", ctxerr.Wrapf(ctx, err, "getting host end users for variable %s", fleetVar) - } - if len(users) == 0 || users[0].IdpUserName == "" { - return "", ctxerr.Errorf(ctx, "host %s does not have an IDP username for variable %s", host.UUID, fleetVar) - } - result = fleet.FleetVarHostEndUserIDPUsernameRegexp.ReplaceAllString(result, users[0].IdpUserName) - } - } - - return result, nil -} - type updateCertificateStatusRequest struct { CertificateTemplateID uint `url:"id"` NodeKey string `json:"node_key"` diff --git a/server/service/client.go b/server/service/client.go index 54d9c58c29..61bb5a72ad 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -2920,6 +2920,13 @@ func (c *Client) doGitOpsAndroidCertificates(config *spec.GitOps, logFn func(for ) } + // Validate Fleet variables in subject name + if err := validateCertificateTemplateFleetVariables(certificates[i].SubjectName); err != nil { + return newGitOpsValidationError( + fmt.Sprintf(`Invalid Fleet variable in certificate %q: %s`, certificates[i].Name, err.Error()), + ) + } + caID, ok := caIDsByName[certificates[i].CertificateAuthorityName] if !ok { return fmt.Errorf("certificate authority %q not found for certificate %q", diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index 7f30023a0e..eb34fed2c2 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -7978,6 +7978,18 @@ func (s *integrationTestSuite) TestCertificatesSpecs() { // apply certificate templates var applyResp applyCertificateTemplateSpecsResponse + s.DoJSON("POST", "/api/latest/fleet/spec/certificates", applyCertificateTemplateSpecsRequest{ + Specs: []*fleet.CertificateRequestSpec{ + { + Name: "Invalid Template", + Team: fmt.Sprint(team.ID), + CertificateAuthorityId: ca.ID, + SubjectName: "CN=$FLEET_VAR_NOT_VALID/OU=$FLEET_VAR_HOST_UUID", + }, + }, + }, http.StatusBadRequest, &applyResp) + + // valid templates s.DoJSON("POST", "/api/latest/fleet/spec/certificates", applyCertificateTemplateSpecsRequest{ Specs: []*fleet.CertificateRequestSpec{ { @@ -7990,7 +8002,7 @@ func (s *integrationTestSuite) TestCertificatesSpecs() { Name: "Template 2", Team: fmt.Sprint(team.ID), CertificateAuthorityId: ca.ID, - SubjectName: "CN=$FLEET_VAR_HOST_END_USER_IDP_USERNAME/OU=$FLEET_VAR_HOST_UUID/ST=$FLEET_VAR_HOST_HARDWARE_SERIAL", + SubjectName: "CN=$FLEET_VAR_HOST_END_USER_IDP_USERNAME/OU=$FLEET_VAR_HOST_UUID", }, }, }, http.StatusOK, &applyResp)