Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 18 Packages Projects Releases Wiki Activity

Reverting printableCharacters SCEP validation (#49758)

Browse Source 
<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #49756

# Checklist for submitter

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files)
for more information.

## Testing

- [x] Added/updated automated tests
- [x] QA'd all new/changed functionality manually


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

## Summary by CodeRabbit

* **Bug Fixes**
* Custom SCEP proxy challenges can again include characters such as
underscores.
  * Apple device enrollment works again with these challenges.
* Removed the overly strict printable-character validation from the
Custom SCEP configuration form.
* The Challenge field now only enforces the required-value rule and no
longer shows printable-character validation errors.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Victor Lyuboslavsky
2026-07-22 13:46:47 -05:00
committed by GitHub
parent 8b49181a14
commit 95b535a622
6 changed files with 26 additions and 174 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changes/49756-revert-scep-challenge-printable-validation
+1
View File
@@ -0,0 +1 @@
- Removed the validation, added in Fleet 4.89.0, that rejected custom SCEP proxy certificate authority challenges containing characters outside the ASN.1 PrintableString set (for example, an underscore). Apple devices can enroll certificates using such challenges, so they are accepted again. A fix for Windows certificate enrollment failing with these challenges will ship separately.
ee/server/service/certificate_authorities.go
+3 -37
View File
@@ -127,8 +127,7 @@ func (svc *Service) NewCertificateAuthority(ctx context.Context, p fleet.Certifi
if p.CustomSCEPProxy != nil {
p.CustomSCEPProxy.Preprocess()
// New CA: the challenge is always being set, so validate its characters.
if err := svc.validateCustomSCEPProxy(ctx, p.CustomSCEPProxy, true, errPrefix); err != nil {
if err := svc.validateCustomSCEPProxy(ctx, p.CustomSCEPProxy, errPrefix); err != nil {
return nil, err
}
@@ -393,27 +392,7 @@ func (svc *Service) validateNDESSCEPProxy(ctx context.Context, ndesSCEP *fleet.N
return nil
}
// printableStringChallengeRegexp matches challenges containing only characters that are valid in an ASN.1 PrintableString, minus
// the space. Windows encodes the SCEP challenge password as a PrintableString, so a challenge containing any other character
// (most commonly "_") makes Windows certificate enrollment fail with "The string contains a non-printable character." The space
// is a valid PrintableString character but is disallowed here because leading/trailing spaces are an invisible footgun. Keep in
// sync with PRINTABLE_STRING_REGEX in the CustomSCEPForm frontend helpers.
var printableStringChallengeRegexp = regexp.MustCompile(`^[A-Za-z0-9'()+,./:=?-]*$`)
// scepChallengePrintableErrMsg is returned when a custom SCEP proxy challenge contains characters that Windows cannot use.
const scepChallengePrintableErrMsg = `Custom SCEP Proxy challenge can only contain letters, numbers, and the characters ' ( ) + , - . / : = ?. Certificate enrollment rejects other characters, such as "_".`
// challengeHasAllowedChars reports whether the challenge contains only the characters Fleet allows in a SCEP challenge: the ASN.1
// PrintableString set minus the space (see printableStringChallengeRegexp).
func challengeHasAllowedChars(challenge string) bool {
return printableStringChallengeRegexp.MatchString(challenge)
}
// validateCustomSCEPProxy validates a custom SCEP proxy CA payload. validateChallengeChars controls whether
// the challenge is checked for Windows-incompatible (non-PrintableString) characters; callers should only
// set it when the challenge is being created or changed, so that challenges stored before this validation
// existed continue to work.
func (svc *Service) validateCustomSCEPProxy(ctx context.Context, customSCEP *fleet.CustomSCEPProxyCA, validateChallengeChars bool, errPrefix string) error {
func (svc *Service) validateCustomSCEPProxy(ctx context.Context, customSCEP *fleet.CustomSCEPProxyCA, errPrefix string) error {
if err := validateCAName(customSCEP.Name, errPrefix); err != nil {
return err
}
@@ -423,9 +402,6 @@ func (svc *Service) validateCustomSCEPProxy(ctx context.Context, customSCEP *fle
if customSCEP.Challenge == "" || customSCEP.Challenge == fleet.MaskedPassword {
return fleet.NewInvalidArgumentError("challenge", fmt.Sprintf("%sCustom SCEP Proxy challenge cannot be empty", errPrefix))
}
if validateChallengeChars && !challengeHasAllowedChars(customSCEP.Challenge) {
return fleet.NewInvalidArgumentError("challenge", fmt.Sprintf("%s%s", errPrefix, scepChallengePrintableErrMsg))
}
if err := svc.scepConfigService.ValidateSCEPURL(ctx, customSCEP.URL); err != nil {
svc.logger.ErrorContext(ctx, "Failed to validate custom SCEP URL", "err", err)
return &fleet.BadRequestError{Message: fmt.Sprintf("%sInvalid SCEP URL. Please correct and try again.", errPrefix)}
@@ -814,11 +790,7 @@ func (svc *Service) processCustomSCEPProxyCAs(ctx context.Context, batchOps *fle
}
for name, incoming := range incomingByName {
// Only validate the challenge characters when the challenge is new or changed, so that challenges stored before this validation
// existed continue to work.
existing, exists := existingByName[name]
challengeChanged := !exists || existing == nil || incoming.Challenge != existing.Challenge
if err := svc.validateCustomSCEPProxy(ctx, incoming, challengeChanged, "certificate_authorities.custom_scep_proxy: "); err != nil {
if err := svc.validateCustomSCEPProxy(ctx, incoming, "certificate_authorities.custom_scep_proxy: "); err != nil {
return err
}
// create the payload to be added or updated
@@ -1506,12 +1478,6 @@ func (svc *Service) validateCustomSCEPProxyUpdate(ctx context.Context, customSCE
Message: fmt.Sprintf("%sCustom SCEP Proxy challenge cannot be empty", errPrefix),
}
}
// Only validate the challenge characters when a new challenge value is provided. A nil or masked challenge means it is unchanged,
// so challenges stored before this validation existed keep working.
if customSCEP.Challenge != nil && *customSCEP.Challenge != fleet.MaskedPassword &&
!challengeHasAllowedChars(*customSCEP.Challenge) {
return &fleet.BadRequestError{Message: fmt.Sprintf("%s%s", errPrefix, scepChallengePrintableErrMsg)}
}
return nil
}
ee/server/service/certificate_authorities_test.go
+17 -96
View File
@@ -412,20 +412,24 @@ func TestCreatingCertificateAuthorities(t *testing.T) {
verifyNilFieldsForType(t, createdCA)
})
t.Run("Create Custom SCEP CA - challenge with non-printable character is rejected", func(t *testing.T) {
t.Run("Create Custom SCEP CA - challenge with non-PrintableString characters is accepted", func(t *testing.T) {
// Regression test for the reverted PrintableString challenge validation (#49756): characters outside the
// ASN.1 PrintableString set (such as "_" and "@") must be accepted.
svc, ctx := baseSetupForCATests()
createRequest := fleet.CertificateAuthorityPayload{
CustomSCEPProxy: &fleet.CustomSCEPProxyCA{
Name: "CustomSCEPWIFI",
URL: "https://customscep.example.com",
Challenge: "bad_challenge", // underscore is not a valid ASN.1 PrintableString character
Challenge: "base64url_style@challenge",
},
}
_, err := svc.NewCertificateAuthority(ctx, createRequest)
require.ErrorContains(t, err, scepChallengePrintableErrMsg)
require.Empty(t, createdCAs)
require.EqualError(t, err, "mock error to avoid NewActivity panic")
require.Len(t, createdCAs, 1)
require.NotNil(t, createdCAs[0].Challenge)
assert.Equal(t, createRequest.CustomSCEPProxy.Challenge, *createdCAs[0].Challenge)
})
t.Run("Create NDES SCEP CA - Happy path", func(t *testing.T) {
@@ -1597,28 +1601,13 @@ func TestUpdatingCertificateAuthorities(t *testing.T) {
require.EqualError(t, err, "mock error to avoid NewActivity panic")
})
t.Run("Challenge with non-printable character is rejected", func(t *testing.T) {
t.Run("Challenge with non-PrintableString characters is accepted", func(t *testing.T) {
// Regression test for the reverted PrintableString challenge validation (#49756).
svc, ctx := baseSetupForCATests()
payload := fleet.CertificateAuthorityUpdatePayload{
CustomSCEPProxyCAUpdatePayload: &fleet.CustomSCEPProxyCAUpdatePayload{
Challenge: new("bad_challenge"), // underscore is not a valid ASN.1 PrintableString character
},
}
err := svc.UpdateCertificateAuthority(ctx, scepID, payload)
require.ErrorContains(t, err, scepChallengePrintableErrMsg)
})
t.Run("Masked (unchanged) challenge skips character validation", func(t *testing.T) {
// Backward compatibility: an unchanged challenge is submitted as the masked placeholder, so it must
// not be re-validated. Otherwise editing a CA whose challenge predates this validation would break.
svc, ctx := baseSetupForCATests()
payload := fleet.CertificateAuthorityUpdatePayload{
CustomSCEPProxyCAUpdatePayload: &fleet.CustomSCEPProxyCAUpdatePayload{
URL: new("https://customscep.example.com"),
Challenge: new(fleet.MaskedPassword),
Challenge: new("updated_challenge@with_special_chars"),
},
}
@@ -2029,85 +2018,17 @@ func TestDeleteCertificateAuthority(t *testing.T) {
})
}
func TestChallengeHasAllowedChars(t *testing.T) {
tests := []struct {
name string
challenge string
want bool
}{
{"alphanumeric", "FleetSCEPtest2026", true},
{"empty", "", true},
{"allowed punctuation", "Fleet-SCEP.2026(test)+,/:=?'", true},
{"hyphen only", "abc-def-123", true},
{"underscore rejected", "Fleet_SCEP", false},
{"at sign rejected", "fleet@scep", false},
{"asterisk rejected", "fleet*scep", false},
{"base64url with underscore rejected", "JURAzXStYElNpVi63B_ps6D0WxF7b3Gv", false},
{"base64url with hyphen only allowed", "i-8MPPQ85Ux3uqNptijN53Ru3KYIIgEI", true},
{"hash rejected", "fleet#scep", false},
{"tilde rejected", "fleet~scep", false},
{"internal space rejected", "fleet scep", false},
{"leading space rejected", " fleetscep", false},
{"trailing space rejected", "fleetscep ", false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
require.Equal(t, tt.want, challengeHasAllowedChars(tt.challenge))
})
}
}
// TestProcessCustomSCEPProxyCAsChallengeValidation covers the GitOps/batch path, which (unlike the UI
// update path) provides the challenge unmasked and detects "unchanged" by comparing the incoming
// challenge to the existing one. A pre-existing challenge with otherwise-disallowed characters must keep
// working when it is re-applied unchanged.
func TestProcessCustomSCEPProxyCAsChallengeValidation(t *testing.T) {
// TestProcessCustomSCEPProxyCAsChallengeChars is a regression test for the reverted PrintableString challenge validation
// (#49756): the GitOps/batch path must accept challenges containing characters outside the ASN.1 PrintableString set.
func TestProcessCustomSCEPProxyCAsChallengeChars(t *testing.T) {
svc := &Service{
logger: slog.New(slog.NewTextHandler(os.Stdout, nil)),
scepConfigService: &scep_mock.SCEPConfigService{
ValidateSCEPURLFunc: func(_ context.Context, _ string) error { return nil },
},
}
const url = "https://customscep.example.com"
tests := []struct {
name string
existing []fleet.CustomSCEPProxyCA
incoming []fleet.CustomSCEPProxyCA
wantErr bool
}{
{
name: "new CA with disallowed challenge is rejected",
incoming: []fleet.CustomSCEPProxyCA{{Name: "SCEP1", URL: url, Challenge: "bad_challenge"}},
wantErr: true,
},
{
name: "unchanged disallowed challenge is skipped (backward compatible)",
existing: []fleet.CustomSCEPProxyCA{{Name: "SCEP1", URL: url, Challenge: "legacy_challenge"}},
incoming: []fleet.CustomSCEPProxyCA{{Name: "SCEP1", URL: url, Challenge: "legacy_challenge"}},
wantErr: false,
},
{
name: "challenge changed to a disallowed value is rejected",
existing: []fleet.CustomSCEPProxyCA{{Name: "SCEP1", URL: url, Challenge: "goodchallenge"}},
incoming: []fleet.CustomSCEPProxyCA{{Name: "SCEP1", URL: url, Challenge: "new_bad"}},
wantErr: true,
},
{
name: "challenge changed to an allowed value succeeds",
existing: []fleet.CustomSCEPProxyCA{{Name: "SCEP1", URL: url, Challenge: "goodchallenge"}},
incoming: []fleet.CustomSCEPProxyCA{{Name: "SCEP1", URL: url, Challenge: "new-good.value"}},
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := svc.processCustomSCEPProxyCAs(t.Context(), &fleet.CertificateAuthoritiesBatchOperations{}, tt.incoming, tt.existing)
if tt.wantErr {
require.ErrorContains(t, err, scepChallengePrintableErrMsg)
} else {
require.NoError(t, err)
}
})
}
incoming := []fleet.CustomSCEPProxyCA{{Name: "SCEP1", URL: "https://customscep.example.com", Challenge: "base64url_style@challenge"}}
err := svc.processCustomSCEPProxyCAs(t.Context(), &fleet.CertificateAuthoritiesBatchOperations{}, incoming, nil)
require.NoError(t, err)
}
frontend/pages/admin/IntegrationsPage/cards/CertificateAuthorities/components/CustomSCEPForm/CustomSCEPForm.tests.tsx
+4 -22
View File
@@ -3,8 +3,6 @@ import { noop } from "lodash";
import { render, screen } from "@testing-library/react";
import { renderWithSetup } from "test/test-utils";
import { UNCHANGED_PASSWORD_API_RESPONSE } from "utilities/constants";
import CustomSCEPForm, { ICustomSCEPFormData } from "./CustomSCEPForm";
const createTestFormData = (overrides?: Partial<ICustomSCEPFormData>) => ({
@@ -97,32 +95,16 @@ describe("CustomSCEPForm", () => {
expect(screen.getByRole("button", { name: "Submit" })).toBeEnabled();
});
it("rejects a challenge with non-printable characters", () => {
render(
<CustomSCEPForm
formData={createTestFormData({ challenge: "bad_challenge" })}
isSubmitting={false}
submitBtnText="Submit"
isDirty
onChange={noop}
onSubmit={noop}
onCancel={noop}
/>
);
expect(screen.getByText(/Invalid characters/)).toBeVisible();
expect(screen.getByRole("button", { name: "Submit" })).toBeDisabled();
});
it("does not block an unchanged (masked) challenge when editing", () => {
it("accepts a challenge with non-PrintableString characters", () => {
// Regression test for the reverted PrintableString challenge validation (#49756): characters
// such as "_" and "@" must not block submission.
render(
<CustomSCEPForm
formData={createTestFormData({
challenge: UNCHANGED_PASSWORD_API_RESPONSE,
challenge: "base64url_style@challenge",
})}
isSubmitting={false}
submitBtnText="Submit"
isEditing
isDirty
onChange={noop}
onSubmit={noop}
frontend/pages/admin/IntegrationsPage/cards/CertificateAuthorities/components/CustomSCEPForm/CustomSCEPForm.tsx
-1
View File
@@ -95,7 +95,6 @@ const CustomSCEPForm = ({
label="Challenge"
name="challenge"
value={challenge}
error={formValidation.challenge?.message}
onChange={onInputChange}
parseTarget
helpText="Password to authenticate with a SCEP server."
frontend/pages/admin/IntegrationsPage/cards/CertificateAuthorities/components/CustomSCEPForm/helpers.ts
+1 -18
View File
@@ -1,21 +1,16 @@
import { ICertificateAuthorityPartial } from "interfaces/certificates";
import { UNCHANGED_PASSWORD_API_RESPONSE } from "utilities/constants";
import valid_url from "components/forms/validators/valid_url";
import { ICustomSCEPFormData } from "./CustomSCEPForm";
// Windows encodes the SCEP challenge password as an ASN.1 PrintableString, so a challenge with any character outside that set (most
// commonly "_") fails. Keep in sync with printableStringChallengeRegexp in ee/server/service/certificate_authorities.go.
const PRINTABLE_STRING_REGEX = /^[A-Za-z0-9'()+,./:=?-]*$/;
// TODO: create a validator abstraction for this and the other form validation files
export interface ICustomSCEPFormValidation {
isValid: boolean;
name?: { isValid: boolean; message?: string };
scepURL?: { isValid: boolean; message?: string };
challenge?: { isValid: boolean; message?: string };
challenge?: { isValid: boolean };
}
type IMessageFunc = (formData: ICustomSCEPFormData) => string;
@@ -95,18 +90,6 @@ export const generateFormValidations = (
return formData.challenge.length > 0;
},
},
{
name: "printableCharacters",
isValid: (formData: ICustomSCEPFormData) => {
// Skip an unchanged (masked) challenge, so editing a CA whose challenge predates this validation isn't blocked.
return (
formData.challenge === UNCHANGED_PASSWORD_API_RESPONSE ||
PRINTABLE_STRING_REGEX.test(formData.challenge)
);
},
message:
"Invalid characters. Certificate enrollment only supports letters, numbers, and ' ( ) + , - . / : = ?",
},
],
},
};