Validate certificate template names (#37765)
Fix for unreleased bug: - Enforce validation rules for certificate template names (e.g., length, allowed characters). - Implement duplicate name error handling in MySQL datastore. <!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #37761 # Checklist for submitter ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually For unreleased bug fixes in a release candidate, one of: - [x] Confirmed that the fix is not expected to adversely impact load test results <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Certificate template naming now enforced with validation rules: no empty or whitespace-only names, character restrictions, and maximum length limits. * Validation occurs on both client and server sides. * **Bug Fixes** * Improved error handling and messaging for duplicate certificate templates. * Corrected certificate validation error message text. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
+1
-1
@@ -37,7 +37,7 @@ export const generateFormValidations = (
|
||||
{
|
||||
name: "required",
|
||||
isValid: (formData: IAddCertFormData) => {
|
||||
return formData.name.length > 0;
|
||||
return formData.name.trim().length > 0;
|
||||
},
|
||||
},
|
||||
{
|
||||
|
||||
@@ -162,6 +162,9 @@ func (ds *Datastore) CreateCertificateTemplate(ctx context.Context, certificateT
|
||||
) VALUES (?, ?, ?, ?)
|
||||
`, certificateTemplate.Name, certificateTemplate.TeamID, certificateTemplate.CertificateAuthorityID, certificateTemplate.SubjectName)
|
||||
if err != nil {
|
||||
if IsDuplicate(err) {
|
||||
return nil, ctxerr.Wrap(ctx, alreadyExists("CertificateTemplate", certificateTemplate.Name), "inserting certificate_template")
|
||||
}
|
||||
return nil, ctxerr.Wrap(ctx, err, "inserting certificate_template")
|
||||
}
|
||||
|
||||
|
||||
@@ -85,6 +85,88 @@ func TestCreateCertificateTemplate(t *testing.T) {
|
||||
// Check that the error is about invalid CA type
|
||||
require.Contains(t, err.Error(), "not found")
|
||||
})
|
||||
|
||||
t.Run("Empty or whitespace-only name", func(t *testing.T) {
|
||||
whitespaceNames := []string{"", " ", " ", "\t", "\n", " \t\n "}
|
||||
for _, name := range whitespaceNames {
|
||||
_, err := svc.CreateCertificateTemplate(ctx, name, TeamID, uint(ValidCATypeID), "CN=$FLEET_VAR_HOST_UUID")
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "Certificate template name is required")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Name too long", func(t *testing.T) {
|
||||
longName := string(make([]byte, 256))
|
||||
for i := range longName {
|
||||
longName = longName[:i] + "a" + longName[i+1:]
|
||||
}
|
||||
_, err := svc.CreateCertificateTemplate(ctx, longName, TeamID, uint(ValidCATypeID), "CN=$FLEET_VAR_HOST_UUID")
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "Certificate template name is too long")
|
||||
})
|
||||
|
||||
t.Run("Name with invalid characters", func(t *testing.T) {
|
||||
testCases := []struct {
|
||||
name string
|
||||
}{
|
||||
{name: "template@name"},
|
||||
{name: "template#name"},
|
||||
{name: "template$name"},
|
||||
{name: "template%name"},
|
||||
{name: "template.name"},
|
||||
{name: "template/name"},
|
||||
{name: "template\\name"},
|
||||
{name: "template!name"},
|
||||
{name: "template?name"},
|
||||
{name: "template*name"},
|
||||
{name: "template+name"},
|
||||
{name: "template=name"},
|
||||
{name: "template<name>"},
|
||||
{name: "template(name)"},
|
||||
{name: "template[name]"},
|
||||
{name: "template{name}"},
|
||||
{name: "template|name"},
|
||||
{name: "template;name"},
|
||||
{name: "template:name"},
|
||||
{name: "template'name"},
|
||||
{name: "template\"name"},
|
||||
{name: "template`name"},
|
||||
{name: "template~name"},
|
||||
{name: "template^name"},
|
||||
{name: "template name"},
|
||||
}
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
_, err := svc.CreateCertificateTemplate(ctx, tc.name, TeamID, uint(ValidCATypeID), "CN=$FLEET_VAR_HOST_UUID")
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "Invalid certificate template name")
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Name with valid characters", func(t *testing.T) {
|
||||
validNames := []string{
|
||||
"my template",
|
||||
" my template ",
|
||||
"my-template",
|
||||
"my_template",
|
||||
"MyTemplate123",
|
||||
"Template 1",
|
||||
"UPPERCASE",
|
||||
"lowercase",
|
||||
"Mix-Ed_Case 123",
|
||||
"a",
|
||||
"1",
|
||||
"a1",
|
||||
"1a",
|
||||
}
|
||||
for _, name := range validNames {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
_, err := svc.CreateCertificateTemplate(ctx, name, TeamID, uint(ValidCATypeID), "CN=$FLEET_VAR_HOST_UUID")
|
||||
require.NoError(t, err)
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestApplyCertificateTemplateSpecs(t *testing.T) {
|
||||
@@ -236,4 +318,56 @@ func TestApplyCertificateTemplateSpecs(t *testing.T) {
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "not found")
|
||||
})
|
||||
|
||||
t.Run("Empty name", func(t *testing.T) {
|
||||
err := svc.ApplyCertificateTemplateSpecs(ctx, []*fleet.CertificateRequestSpec{
|
||||
{
|
||||
Name: "",
|
||||
CertificateAuthorityId: 1,
|
||||
SubjectName: "foo",
|
||||
},
|
||||
})
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "Certificate template name is required")
|
||||
})
|
||||
|
||||
t.Run("Whitespace-only name", func(t *testing.T) {
|
||||
err := svc.ApplyCertificateTemplateSpecs(ctx, []*fleet.CertificateRequestSpec{
|
||||
{
|
||||
Name: " ",
|
||||
CertificateAuthorityId: 1,
|
||||
SubjectName: "foo",
|
||||
},
|
||||
})
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "Certificate template name is required")
|
||||
})
|
||||
|
||||
t.Run("Name with invalid characters", func(t *testing.T) {
|
||||
err := svc.ApplyCertificateTemplateSpecs(ctx, []*fleet.CertificateRequestSpec{
|
||||
{
|
||||
Name: "template@name",
|
||||
CertificateAuthorityId: 1,
|
||||
SubjectName: "foo",
|
||||
},
|
||||
})
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "Invalid certificate template name")
|
||||
})
|
||||
|
||||
t.Run("Name too long", func(t *testing.T) {
|
||||
longName := string(make([]byte, 256))
|
||||
for i := range longName {
|
||||
longName = longName[:i] + "a" + longName[i+1:]
|
||||
}
|
||||
err := svc.ApplyCertificateTemplateSpecs(ctx, []*fleet.CertificateRequestSpec{
|
||||
{
|
||||
Name: longName,
|
||||
CertificateAuthorityId: 1,
|
||||
SubjectName: "foo",
|
||||
},
|
||||
})
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "Certificate template name is too long")
|
||||
})
|
||||
}
|
||||
|
||||
@@ -4,6 +4,8 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"regexp"
|
||||
"strings"
|
||||
|
||||
"github.com/fleetdm/fleet/v4/server/authz"
|
||||
"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
|
||||
@@ -12,6 +14,32 @@ import (
|
||||
"github.com/go-kit/kit/log/level"
|
||||
)
|
||||
|
||||
// Certificate template name validation constants
|
||||
const (
|
||||
maxCertificateTemplateNameLength = 255
|
||||
)
|
||||
|
||||
// certificateTemplateNameRegex allows only letters, numbers, spaces, dashes, and underscores
|
||||
var certificateTemplateNameRegex = regexp.MustCompile(`^[a-zA-Z0-9 \-_]+$`)
|
||||
|
||||
// validateCertificateTemplateName validates the certificate template name.
|
||||
// Returns a BadRequestError if validation fails.
|
||||
func validateCertificateTemplateName(name string) error {
|
||||
if strings.TrimSpace(name) == "" {
|
||||
return &fleet.BadRequestError{Message: "Certificate template name is required."}
|
||||
}
|
||||
|
||||
if len(name) > maxCertificateTemplateNameLength {
|
||||
return &fleet.BadRequestError{Message: fmt.Sprintf("Certificate template name is too long. Maximum is %d characters.", maxCertificateTemplateNameLength)}
|
||||
}
|
||||
|
||||
if !certificateTemplateNameRegex.MatchString(name) {
|
||||
return &fleet.BadRequestError{Message: "Invalid certificate template name. Only letters, numbers, spaces, dashes, and underscores are allowed."}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
type createCertificateTemplateRequest struct {
|
||||
Name string `json:"name"`
|
||||
TeamID uint `json:"team_id"` // If not provided, intentionally defaults to 0 aka "No team"
|
||||
@@ -47,6 +75,11 @@ func (svc *Service) CreateCertificateTemplate(ctx context.Context, name string,
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Validate certificate template name
|
||||
if err := validateCertificateTemplateName(name); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if err := validateCertificateTemplateFleetVariables(subjectName); err != nil {
|
||||
return nil, &fleet.BadRequestError{Message: err.Error()}
|
||||
}
|
||||
@@ -387,6 +420,11 @@ func (svc *Service) ApplyCertificateTemplateSpecs(ctx context.Context, specs []*
|
||||
|
||||
var certificates []*fleet.CertificateTemplate
|
||||
for _, spec := range specs {
|
||||
// Validate certificate template name
|
||||
if err := validateCertificateTemplateName(spec.Name); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Get the CA to validate its existence and type.
|
||||
ca, ok := casByID[spec.CertificateAuthorityId]
|
||||
if !ok {
|
||||
@@ -394,7 +432,7 @@ func (svc *Service) ApplyCertificateTemplateSpecs(ctx context.Context, specs []*
|
||||
}
|
||||
|
||||
if ca.Type != string(fleet.CATypeCustomSCEPProxy) {
|
||||
return &fleet.BadRequestError{Message: fmt.Sprintf("Ccertificate `%s`: Currently, only the custom_scep_proxy certificate authority is supported.", spec.Name)}
|
||||
return &fleet.BadRequestError{Message: fmt.Sprintf("Certificate `%s`: Currently, only the custom_scep_proxy certificate authority is supported.", spec.Name)}
|
||||
}
|
||||
|
||||
// Validate Fleet variables in subject name
|
||||
|
||||
@@ -186,7 +186,7 @@ func (s *integrationMDMTestSuite) TestCertificateTemplateLifecycle() {
|
||||
})
|
||||
|
||||
// Step: Create a certificate template
|
||||
certTemplateName := t.Name() + "-CertTemplate"
|
||||
certTemplateName := strings.ReplaceAll(t.Name(), "/", "-") + "-CertTemplate"
|
||||
var createResp createCertificateTemplateResponse
|
||||
s.DoJSON("POST", "/api/latest/fleet/certificates", createCertificateTemplateRequest{
|
||||
Name: certTemplateName,
|
||||
@@ -307,7 +307,7 @@ func (s *integrationMDMTestSuite) TestCertificateTemplateSpecEndpointAndAMAPIFai
|
||||
caID := ca.ID
|
||||
|
||||
// Step: Create certificate template via spec/certificates endpoint with $FLEET_VAR_HOST_UUID
|
||||
certTemplateName := t.Name() + "-CertTemplate"
|
||||
certTemplateName := strings.ReplaceAll(t.Name(), "/", "-") + "-CertTemplate"
|
||||
var applyResp applyCertificateTemplateSpecsResponse
|
||||
s.DoJSON("POST", "/api/latest/fleet/spec/certificates", applyCertificateTemplateSpecsRequest{
|
||||
Specs: []*fleet.CertificateRequestSpec{
|
||||
@@ -429,7 +429,7 @@ func (s *integrationMDMTestSuite) TestCertificateTemplateNoTeamWithIDPVariable()
|
||||
caID := ca.ID
|
||||
|
||||
// Step: Create certificate template for "no team" (team_id = 0) with IDP_USERNAME variable
|
||||
certTemplateName := t.Name() + "-CertTemplate"
|
||||
certTemplateName := strings.ReplaceAll(t.Name(), "/", "-") + "-CertTemplate"
|
||||
var createResp createCertificateTemplateResponse
|
||||
subjectName := "CN=$FLEET_VAR_HOST_END_USER_IDP_USERNAME"
|
||||
s.DoJSON("POST", "/api/latest/fleet/certificates", createCertificateTemplateRequest{
|
||||
@@ -586,7 +586,7 @@ func (s *integrationMDMTestSuite) TestCertificateTemplateUnenrollReenroll() {
|
||||
require.NoError(t, s.ds.UpdateHost(ctx, host))
|
||||
|
||||
// Step: Create the first certificate template (while host is enrolled)
|
||||
certTemplateName := t.Name() + "-CertTemplate1"
|
||||
certTemplateName := strings.ReplaceAll(t.Name(), "/", "-") + "-CertTemplate1"
|
||||
var createResp createCertificateTemplateResponse
|
||||
s.DoJSON("POST", "/api/latest/fleet/certificates", createCertificateTemplateRequest{
|
||||
Name: certTemplateName,
|
||||
@@ -614,7 +614,7 @@ func (s *integrationMDMTestSuite) TestCertificateTemplateUnenrollReenroll() {
|
||||
require.Equal(t, 0, enrolledStatus, "Host should be marked as unenrolled in host_mdm")
|
||||
|
||||
// Step: Create a second certificate template while host is unenrolled
|
||||
certTemplateName2 := t.Name() + "-CertTemplate2"
|
||||
certTemplateName2 := strings.ReplaceAll(t.Name(), "/", "-") + "-CertTemplate2"
|
||||
s.DoJSON("POST", "/api/latest/fleet/certificates", createCertificateTemplateRequest{
|
||||
Name: certTemplateName2,
|
||||
TeamID: teamID,
|
||||
|
||||
Reference in New Issue
Block a user