Skip to content

Normalizes CRLF line endings at certificate input boundary#787

Merged
hemanthnakkina merged 1 commit intocanonical:mainfrom
Raven-182:cert-normalize
Apr 25, 2026
Merged

Normalizes CRLF line endings at certificate input boundary#787
hemanthnakkina merged 1 commit intocanonical:mainfrom
Raven-182:cert-normalize

Conversation

@Raven-182
Copy link
Copy Markdown

CRLF line endings are surviving validation, causing duplicate entries in ca_bundle.pem and inconsistent data stored in keystone.
Fixes by normalizing CRLF to LF at two input boundaries:

  • validate_ca_certificate / validate_ca_chain: strips CRLF from --ca and --ca-chain
  • ConfigureTLSCertificatesStep.prompt() in common.py: strips CRLF from signed leaf certs

0bb84d3 partially fixed this but missed sanitizing the --ca which gets carried to keystone and causes duplication later in ca_bundle.pem. With normalization at the input boundary, this fix is no longer needed

CRLF line endings are surviving validation, causing duplicate
entries in ca_bundle.pem and inconsistent data stored
in keystone.
Fixes by normalizing CRLF to LF at two input boundaries:
 - validate_ca_certificate / validate_ca_chain: strips
   CRLF from --ca and --ca-chain
 - ConfigureTLSCertificatesStep.prompt() in common.py: strips CRLF from
   signed leaf certs

0bb84d3 partially fixed this but missed
sanitizing the --ca which gets carried to keystone and causes duplication
later in ca_bundle.pem. With normalization at the input boundary, this
fix is no longer needed

Signed-off-by: Raven Kaur <raven.kaur@canonical.com>
@Raven-182 Raven-182 requested review from ahmad-can and Copilot April 24, 2026 22:15
Copy link
Copy Markdown

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 addresses inconsistent certificate storage caused by CRLF line endings by normalizing PEM content to LF at key certificate input boundaries, preventing downstream duplication (e.g., in ca_bundle.pem) and inconsistent Keystone data.

Changes:

  • Added a shared normalize_pem() helper and applied it in validate_ca_certificate / validate_ca_chain, returning canonicalized base64.
  • Normalized signed leaf certificates in ConfigureTLSCertificatesStep.prompt() before persisting answers.
  • Updated/added unit tests covering CRLF normalization behavior in the validators and prompt flow.

Reviewed changes

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

File Description
sunbeam-python/sunbeam/features/interface/utils.py Introduces PEM normalization and canonicalizes CA/chain validator outputs; removes prior normalization from generate_ca_chain().
sunbeam-python/sunbeam/features/tls/common.py Normalizes leaf certs (base64 decode → PEM normalize → base64 encode) before storing in answers/state.
sunbeam-python/tests/unit/sunbeam/features/test_utils.py Adds unit tests asserting CRLF normalization for CA certificate/chain validators.
sunbeam-python/tests/unit/sunbeam/features/test_tls.py Adjusts fixtures to accommodate new base64/normalization behavior in the TLS prompt path.
Comments suppressed due to low confidence (1)

sunbeam-python/sunbeam/features/interface/utils.py:225

  • generate_ca_chain() no longer normalizes CRLF/CR before checking whether ca_certificate_decoded is already present in ca_chain_decoded. There are existing callers (e.g., load balancer certificate prompts) that don’t appear to normalize CA cert/chain inputs, so this can reintroduce duplicated CA certs when line endings differ. Consider either keeping normalization inside generate_ca_chain() (to make it robust for all callers) or normalizing CA cert/chain at every input boundary that can reach this function.
    certificate_decoded = decode_base64_as_string(certificate)
    ca_certificate_decoded = decode_base64_as_string(ca_certificate)
    ca_chain_decoded = decode_base64_as_string(ca_chain)

    if not certificate_decoded or not ca_certificate_decoded or not ca_chain_decoded:
        raise binascii.Error("Unable to decode one of the certificates")

    # If ca_certificate is already part of ca_chain, do not add it to the final ca chain
    # manual-tls-certificates checks if the final ca_chain is in proper order and each
    # certificate is signed by the successor one.
    if ca_certificate_decoded in ca_chain_decoded:
        chain_parts = [certificate_decoded, ca_chain_decoded]
    else:
        chain_parts = [certificate_decoded, ca_certificate_decoded, ca_chain_decoded]

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread sunbeam-python/tests/unit/sunbeam/features/test_tls.py
@hemanthnakkina hemanthnakkina merged commit cb6fc7e into canonical:main Apr 25, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants