Skip to content

Conversation

@cjen1-msft
Copy link
Contributor

Currently we don't recognise a PEM with carriage returns as a PEM certificate.

This PR fixes that and adds a test for this.

Copilot AI review requested due to automatic review settings December 4, 2025 11:54
@cjen1-msft cjen1-msft requested a review from a team as a code owner December 4, 2025 11:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enables recognition of PEM certificates that use Windows-style line endings (carriage return + line feed, \r\n) in addition to Unix-style line endings (line feed only, \n).

Key Changes:

  • Modified PEM prefix validation to accept both \n and \r\n line endings by removing the hardcoded newline from the prefix constant
  • Added comprehensive test coverage for PEM certificates with carriage return line endings

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
include/ccf/crypto/openssl/openssl_wrappers.h Removed trailing \n from pem_prefix constant to allow recognition of PEM certificates with both Unix (\n) and Windows (\r\n) line endings
src/crypto/test/crypto.cpp Added new test case "Carriage returns in PEM certificates" that validates both standard and carriage-return PEM formats can be parsed successfully

cjen1-msft and others added 2 commits December 4, 2025 12:51
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@cjen1-msft cjen1-msft enabled auto-merge (squash) December 4, 2025 13:14
@cjen1-msft cjen1-msft merged commit 240ff51 into microsoft:main Dec 4, 2025
17 checks passed
@achamayou achamayou added auto-backport Automatically backport this PR to LTS branch 6.x-todo PRs which should be backported to 6.x labels Dec 4, 2025
cjen1-msft added a commit to cjen1-msft/CCF that referenced this pull request Dec 4, 2025
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
cjen1-msft added a commit that referenced this pull request Dec 4, 2025
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@cjen1-msft cjen1-msft added the backported This PR was successfully backported to LTS branch label Dec 5, 2025
@cjen1-msft cjen1-msft deleted the fix-cr-pem branch December 5, 2025 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.x-todo PRs which should be backported to 6.x auto-backport Automatically backport this PR to LTS branch backported This PR was successfully backported to LTS branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants