Skip to content

Add internal CA certificate provider - #490

Draft
cassi-volkova wants to merge 1 commit into
masterfrom
cassi/internal-ca-certificates
Draft

Add internal CA certificate provider#490
cassi-volkova wants to merge 1 commit into
masterfrom
cassi/internal-ca-certificates

Conversation

@cassi-volkova

Copy link
Copy Markdown
Member

Summary

  • add a generic internal_ca certificate method for private service TLS;
  • generate a private CA and DNS-SAN leaf certificates in the certificate backend;
  • keep CA private key material server-side while exposing the public ca_cert for trust distribution;
  • reuse the CA for leaf renewal and rotate it before a new leaf would outlive the CA;
  • persist and reconcile CA certificate state with a backward-compatible migration;
  • document the trust-distribution and rotation contract.

Related to #489

Security and architecture notes

The CA private key is stored only in backend certificate storage and is never returned by the user API or resource renderer. Consumers receive the leaf private key and certificate chain for the service, and the public CA certificate for client trust stores. The generated leaf includes DNS SANs, server-auth EKU, key usage constraints, and a signature from the retained private CA.

The intended migration from build-time certificate bundles uses overlapping trust: distribute the new Core-managed CA first, switch the service leaf certificate second, verify clients, and remove the legacy CA last.

Validation

  • tox -e ruff-check
  • tox -e py312 — 201 passed
  • focused PostgreSQL functional tests for API acceptance and certificate service reconciliation — 2 passed
  • migration upgrade from migration 0064 through the current head; verified ca_cert and ca_key columns
  • mypy -p exordos_core delta check — 603 existing errors on both master and this branch; no new errors

The full functional suite was not completed; only the affected functional paths were rerun successfully after provisioning an isolated PostgreSQL database.

Draft status

This PR is intentionally a draft for architecture and security review. It is not approved or intended for deployment to any current live Core installation. Consumer rollout, backup/restore validation, operational rotation procedures, and end-to-end acceptance remain future work.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for private service certificates through a new internal_ca provider in the Secret Manager service, allowing the generation and rotation of internal CA and server certificates. The changes include backend implementation using the cryptography library, updates to database models, API controllers, documentation, functional tests, and a database migration. The review feedback highlights potential ValueError exceptions during certificate generation if the Common Name (CN) exceeds the 64-character limit specified by RFC 5280, suggesting truncation for both the CA common name and the server certificate's common name.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +62 to +70
subject = x509.Name(
[
x509.NameAttribute(NameOID.ORGANIZATION_NAME, "Exordos Internal"),
x509.NameAttribute(
NameOID.COMMON_NAME,
f"{resource.value['name']} CA",
),
]
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The Common Name (CN) in X.509 certificates has a maximum length of 64 characters per RFC 5280. If resource.value['name'] is long, appending ' CA' to it can exceed this limit and cause cryptography to raise a ValueError during certificate generation. We should truncate the name to ensure the CN is always within the 64-character limit.

Suggested change
subject = x509.Name(
[
x509.NameAttribute(NameOID.ORGANIZATION_NAME, "Exordos Internal"),
x509.NameAttribute(
NameOID.COMMON_NAME,
f"{resource.value['name']} CA",
),
]
)
common_name = f"{resource.value['name']} CA"
if len(common_name) > 64:
common_name = f"{resource.value['name'][:60]} CA"
subject = x509.Name(
[
x509.NameAttribute(NameOID.ORGANIZATION_NAME, "Exordos Internal"),
x509.NameAttribute(NameOID.COMMON_NAME, common_name),
]
)

Comment on lines +122 to +123
domains = resource.value["domains"]
subject = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, domains[0])])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The Common Name (CN) in X.509 certificates is limited to 64 characters. If the first domain name in domains exceeds 64 characters, cryptography will raise a ValueError. We should truncate the domain name to 64 characters when setting it as the Common Name. (Note that modern TLS clients rely on Subject Alternative Names (SAN) anyway, so truncating the CN is safe and standard practice).

    domains = resource.value["domains"]
    common_name = domains[0]
    if len(common_name) > 64:
        common_name = common_name[:64]
    subject = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, common_name)])

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.

1 participant