Skip to content

Cert services: Self-signed and ACME with DNS challenge - #2

Open
mvdschee wants to merge 34 commits into
mainfrom
feature/cert-services-self-signe-and-acme
Open

mvdschee wants to merge 34 commits into
mainfrom
feature/cert-services-self-signe-and-acme

Conversation

@mvdschee

@mvdschee mvdschee commented Aug 10, 2026 •

Copy link
Copy Markdown
Owner

This PR adds the long awaited feature to support real SSL certificates without exposing a local server to the internet, it only supports DNS challenge, which is automated for CloudFlare (will add more later on) so we dont need to do anything ourself every 30 days.

@mvdschee
mvdschee marked this pull request as ready for review October 7, 2026 19:38
@mvdschee
mvdschee requested a balanced review from Copilot October 7, 2026 19:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Certificate handling has unresolved TLS correctness, renewal recovery, and secret-permission issues.

9 open findings
What changed in this PR

Adds ACME DNS-01 certificate issuance and renewal to the reverse proxy, alongside self-signed certificates and live TLS updates.

Changes:

  • Adds Cloudflare DNS challenges and persistent ACME accounts.
  • Integrates background renewal and an atomically replaceable TLS store.
  • Updates configuration, dependencies, examples, and documentation.
File Description
src/​utils.rs Adds reusable string-newtype macro.
src/​services/​mod.rs Registers service modules.
src/​services/​http/​mod.rs Creates shared HTTP clients.
src/​services/​dns/​utils.rs Builds challenge record names.
src/​services/​dns/​resolver.rs Looks up challenge TXT records.
src/​services/​dns/​mod.rs Registers DNS modules.
src/​services/​dns/​cloudflare.rs Creates and updates challenge records.
src/​services/​certs/​self_signed.rs Generates self-signed certificate files.
src/​services/​certs/​mod.rs Registers certificate services.
src/​services/​certs/​acme.rs Manages ACME accounts and orders.
src/​main.rs Connects certificate renewal to proxy startup.
src/​error.rs Adds service error types.
src/​core/​setup.rs Initializes TLS storage and renewal.
src/​core/​models/​tasks.rs Documents interval units.
src/​core/​models/​routes.rs Adds DNS-provider configuration.
src/​core/​models/​proxy.rs Removes per-route certificate paths.
src/​core/​models/​mod.rs Exposes new model modules.
src/​core/​models/​filesystem.rs Reuses string-newtype macro.
src/​core/​models/​dns.rs Defines DNS providers and credentials.
src/​core/​models/​config.rs Centralizes configuration models.
src/​core/​models/​certs.rs Adds renewal state and shared TLS storage.
src/​core/​handlers/​proxy.rs Uses live certificates and background renewal.
src/​core/​handlers/​certs.rs Implements certificate issuance and renewal.
src/​config.rs Adds ACME settings and renewal intervals.
README.md Documents certificate workflows and deployment.
makefile Updates local development settings.
example/​example.toml Demonstrates Cloudflare ACME configuration.
docker-compose.yml Updates comments and removes temporary mount.
Cargo.toml Updates dependencies and package version.
Cargo.lock Records dependency changes.
.rustfmt.toml Configures import grouping.
.foxguard/​baseline.json Changes trailing newline only.
.foxguard.yml Disables a dependency-scanning rule.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/core/handlers/certs.rs Outdated
Comment thread src/core/handlers/certs.rs
Comment thread src/core/handlers/certs.rs
Comment thread src/core/handlers/certs.rs
Comment thread src/core/handlers/certs.rs Outdated
Comment thread src/core/handlers/certs.rs
Comment thread src/core/handlers/certs.rs
Comment thread src/core/handlers/certs.rs
Comment thread src/services/dns/cloudflare.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

ACME account recovery and certificate publication can cause repeated registrations or leave unusable certificate files.

1 open finding
9 resolved since last review
Previously missed (3)

In code that hasn't changed since last review

Medium severity Reject invalid ACME environment values

src/​services/​certs/​acme.rs:39

Every value except exactly production silently selects staging. A typo such as prodution therefore issues an untrusted staging certificate and takes the route offline instead of reporting bad configuration. Accept the two documented values explicitly and reject anything else.

Medium severity Check Cloudflare response status before JSON parsing

src/​services/​dns/​cloudflare.rs:143

Unlike the lookup and update paths, record creation does not inspect the HTTP status. Cloudflare 4xx/5xx responses are consequently fed to serde_json and reported as misleading parse errors instead of the API failure status. Check response.status() before consuming the body.

Low severity Correct ACME environment documentation and precedence

README.md:114

This contradicts the implementation: ACME_ENV=staging selects Let's Encrypt staging, while production is only the default. Documenting this incorrectly encourages testing against production rate limits; also note that an existing account file takes precedence over the environment setting.

🧠 Review effort: Balanced

Comment thread src/core/handlers/certs.rs Outdated
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.

2 participants