Fix "400 bad request" from SCEP PKIOperation when base64 message contains "+"#43319
Fix "400 bad request" from SCEP PKIOperation when base64 message contains "+"#43319sergey-cheperis wants to merge 10 commits intofleetdm:mainfrom
Conversation
Go's url.Query() (via url.QueryUnescape) follows the application/x-www-form-urlencoded convention and converts every literal '+' in a query string to a space. The SCEP 'message' parameter for PKIOperation carries standard base64-encoded DER data where '+' is a valid and common character. iOS sends '+' unencoded in the URL (correct per RFC 3986 – '+' is only special in form-encoded request bodies), so every '+' in the certificate payload becomes a space before base64.StdEncoding. DecodeString is called, causing a 400 "illegal base64 data" error and preventing devices from enrolling. The fix inserts strings.ReplaceAll(msg2, " ", "+") immediately before the decode call. Standard base64 has no spaces, so every space at that point is a '+' that was corrupted by url.Query(). Replacing it back is safe and correct. Also present on the main branch as of 2026-04-09.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #43319 +/- ##
==========================================
- Coverage 66.86% 66.85% -0.02%
==========================================
Files 2587 2589 +2
Lines 207476 207428 -48
Branches 9314 9202 -112
==========================================
- Hits 138737 138670 -67
- Misses 56109 56127 +18
- Partials 12630 12631 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
sergey-cheperis
changed the title
There was a problem hiding this comment.
Pull request overview
Fixes Apple SCEP PKIOperation GET handling where + characters in base64 payloads are interpreted as spaces, causing base64 decode failures during iOS/macOS MDM enrollment.
Changes:
- Replace spaces with
+before base64 decoding themessagequery parameter forPKIOperation. - Add a release note entry describing the SCEP decoding fix.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| server/mdm/scep/server/transport.go | Adjusts PKIOperation query message handling prior to base64 decode. |
| changes/43319-fix-scep-pkiop-url-query-plus-sign | Adds release note for the SCEP decoding fix. |
Comments suppressed due to low confidence (1)
server/mdm/scep/server/transport.go:192
- The 400 error returned on base64 decode failure includes the full
msg2payload. That value comes from the request and can be very large; echoing it back to the client and into logs viahttp.Erroris both noisy and risks exposing request contents unnecessarily. Consider removingmsg2from the error message or truncating/sanitizing it (e.g., include only length / a short prefix).
decoded, err := base64.StdEncoding.DecodeString(msg2)
if err != nil {
return nil, &BadRequestError{Message: fmt.Sprintf("failed to base64 decode message: %s: %s", err.Error(), msg2)}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Remove url.PathUnescape no-op
| return nil, &BadRequestError{Message: "missing PKIOperation message"} | ||
| } | ||
|
|
||
| msg2, err := url.PathUnescape(msg) |
There was a problem hiding this comment.
PathUnescape() removed because Query() already provides unescaped values.
Related issue: none
Problem
Apple MacOS devices fail SCEP enrollment with a 400. The proxy sees the request arrive with
+signs in the base64 payload:Fleet logs show those
+signs are interpreted as spaces, and the decode fails:Root cause
message()inserver/mdm/scep/server/transport.goreads the query parameter viar.URL.Query(), which internally callsurl.QueryUnescapeand converts every+to a space.The bug is present on
mainas of 2026-04-09.