[PR #6389] [proxy] notify certificate ready for domains covered by the static certificate #25611

Open
opened 2026-08-05 07:06:14 -04:00 by saavagebueno · 0 comments
Owner

Original Pull Request: https://github.com/netbirdio/netbird/pull/6389

State: closed
Merged: Yes


Describe your changes

When ACME is disabled (NB_PROXY_ACME_CERTIFICATES=false) the proxy serves a static certificate via the certwatch watcher, but setupHTTPMapping only sent NotifyCertificateIssued through the ACME manager's wildcard-hit path. With s.acme nil the notification never fired, so management never set the service's certificate_issued_at and the dashboard showed the domain stuck on "Issuing certificate..." forever, even though TLS worked fine (meta_status was correctly active, observed live on a 0.72.2 self-hosted deployment).

This change:

  • keeps a reference to the static certificate watcher on Server (previously a discarded local in configureTLS),
  • treats a domain covered by the loaded static certificate as a wildcard hit in setupHTTPMapping, mirroring the ACME path's behavior for WildcardDir hits, so the existing NotifyCertificateIssued call fires and the dashboard clears,
  • logs a warning when the static certificate does not cover a mapped domain — previously silent, and clients connecting to such a domain get TLS errors.

Coverage is checked with x509.Certificate.VerifyHostname on the watcher's leaf, so SAN wildcard semantics (no label spanning) match standard TLS validation.

Unit tests cover the new coverage-check helper (wildcard match, exact SAN, no label spanning, nil watcher); the notification wiring itself reuses the existing wildcard-hit path and is not separately tested.

Fixes #5384 (also reported as #5517)

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • This change does not modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — OR I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See CONTRIBUTING.md.

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why): fixes the existing documented behavior of NB_PROXY_ACME_CERTIFICATES=false; no new flags or behavior to document.

Docs PR URL (required if "docs added" is checked)

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes

  • Static certificates are now properly recognized as covering their configured domains, ensuring consistent certificate readiness handling across both static and ACME-based deployments.
**Original Pull Request:** https://github.com/netbirdio/netbird/pull/6389 **State:** closed **Merged:** Yes --- ## Describe your changes When ACME is disabled (`NB_PROXY_ACME_CERTIFICATES=false`) the proxy serves a static certificate via the `certwatch` watcher, but `setupHTTPMapping` only sent `NotifyCertificateIssued` through the ACME manager's wildcard-hit path. With `s.acme` nil the notification never fired, so management never set the service's `certificate_issued_at` and the dashboard showed the domain stuck on "Issuing certificate..." forever, even though TLS worked fine (`meta_status` was correctly `active`, observed live on a 0.72.2 self-hosted deployment). This change: - keeps a reference to the static certificate watcher on `Server` (previously a discarded local in `configureTLS`), - treats a domain covered by the loaded static certificate as a wildcard hit in `setupHTTPMapping`, mirroring the ACME path's behavior for `WildcardDir` hits, so the existing `NotifyCertificateIssued` call fires and the dashboard clears, - logs a warning when the static certificate does **not** cover a mapped domain — previously silent, and clients connecting to such a domain get TLS errors. Coverage is checked with `x509.Certificate.VerifyHostname` on the watcher's leaf, so SAN wildcard semantics (no label spanning) match standard TLS validation. Unit tests cover the new coverage-check helper (wildcard match, exact SAN, no label spanning, nil watcher); the notification wiring itself reuses the existing wildcard-hit path and is not separately tested. ## Issue ticket number and link Fixes #5384 (also reported as #5517) ## Stack <!-- branch-stack --> ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) - [x] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why): fixes the existing documented behavior of `NB_PROXY_ACME_CERTIFICATES=false`; no new flags or behavior to document. ### Docs PR URL (required if "docs added" is checked) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Bug Fixes * Static certificates are now properly recognized as covering their configured domains, ensuring consistent certificate readiness handling across both static and ACME-based deployments. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
saavagebueno added the pull-request label 2026-08-05 07:06:14 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#25611