Repository navigation
Host identity cert renewal: added IANA number and jitter - #31579
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis update adds a new documentation file about custom OID extensions, updates an existing doc to clarify certificate renewal timing and OID usage, introduces random jitter to the certificate renewal timer in code, and updates the renewal extension OID to use Fleet's assigned IANA number in both code and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Fleetd
participant Timer
participant TPM
Note over Fleetd: On startup or after renewal
Fleetd->Timer: Calculate renewal time (expiry - 180d + 1min)
Timer->Timer: Add random jitter (0-30min)
Timer->Fleetd: Wait until renewal time
Timer->Fleetd: Trigger renewal
Fleetd->TPM: Use TPM to generate new CSR with custom OID extension
Fleetd->Fleet Server: Submit CSR for renewal
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Member
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
ee/orbit/pkg/hostidentity/host_identity.go (1)
184-187: optional: include jitter in log fieldlogging the actual
jittervalue can help when debugging scheduling:logger.Info(). Dur("renewal_in", renewalTime). + Dur("jitter", jitter). Time("cert_expires", clientCert.NotAfter). Msg("Scheduling host identity certificate renewal timer")docs/Contributing/product-groups/orchestration/tpm-backed-http-signing.md (1)
334-358: duplicate / conflicting renewal descriptionlines 334-335 already describe the new “1 hour or 180 days + jitter” rule, while lines 355-357 restate the old “180 days” rule and then add jitter. the two sections read as contradictory.
suggest keeping a single, clear paragraph:
-The certificate is issued with a validity period of 365 days. Orbit attempts to renew it within 180 days of expiration. - -To prevent thundering herd issues when multiple hosts need to renew certificates simultaneously, Orbit adds a random jitter of 0-30 minutes to the renewal timer. This ensures that certificate renewal requests are spread out over time, reducing load on the Fleet server. +The certificate is valid for 365 days. Orbit schedules renewal for **the later of** +• one hour from now, or +• 180 days before expiration, +plus a random 0–30 minute jitter. +This spreads renewal traffic and avoids thundering-herd spikes.docs/Contributing/product-groups/orchestration/custom-oid-extensions.md (1)
7-19: minor style nitmarkdown-lint points out mixed “dash” vs “asterisk” list markers; the rest of the repo generally uses dashes. no functional impact, but worth normalizing when you next touch the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/Contributing/product-groups/orchestration/custom-oid-extensions.md(1 hunks)docs/Contributing/product-groups/orchestration/tpm-backed-http-signing.md(3 hunks)ee/orbit/pkg/hostidentity/host_identity.go(2 hunks)ee/server/service/hostidentity/types/host_identity_certificates.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: getvictor
PR: fleetdm/fleet#30589
File: ee/server/service/hostidentity/depot/depot.go:104-119
Timestamp: 2025-07-08T16:06:54.576Z
Learning: In ee/server/service/hostidentity/depot/depot.go, the security concern where shared challenges allow certificate revocation (lines 104-119) is a known issue that will be addressed in a later feature, not an immediate concern to fix.
📚 Learning: in ee/server/service/hostidentity/depot/depot.go, the security concern where shared challenges allow...
Learnt from: getvictor
PR: fleetdm/fleet#30589
File: ee/server/service/hostidentity/depot/depot.go:104-119
Timestamp: 2025-07-08T16:06:54.576Z
Learning: In ee/server/service/hostidentity/depot/depot.go, the security concern where shared challenges allow certificate revocation (lines 104-119) is a known issue that will be addressed in a later feature, not an immediate concern to fix.
Applied to files:
docs/Contributing/product-groups/orchestration/tpm-backed-http-signing.mdee/orbit/pkg/hostidentity/host_identity.goee/server/service/hostidentity/types/host_identity_certificates.go
🪛 markdownlint-cli2 (0.17.2)
docs/Contributing/product-groups/orchestration/tpm-backed-http-signing.md
371-371: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: test-go (vuln, ubuntu-latest, mysql:8.0.36, false)
- GitHub Check: test-go (main, ubuntu-latest, mysql:8.0.36, false)
- GitHub Check: test-go (vuln, ubuntu-latest, mysql:9.3.0, false)
- GitHub Check: test-go (main, ubuntu-latest, mysql:9.3.0, false)
- GitHub Check: test-go (service, ubuntu-latest, mysql:9.3.0, false)
- GitHub Check: test-go (scripts, ubuntu-latest, mysql:8.0.36, false)
- GitHub Check: test-go (mysql, ubuntu-latest, mysql:9.3.0, false)
- GitHub Check: test-go (service, ubuntu-latest, mysql:8.0.36, false)
- GitHub Check: test-go (mysql, ubuntu-latest, mysql:8.0.36, false)
- GitHub Check: test-go (integration-enterprise, ubuntu-latest, mysql:8.0.36, false)
- GitHub Check: test-go (fleetctl, ubuntu-latest, mysql:8.0.36, false)
- GitHub Check: test-go (fleetctl, ubuntu-latest, mysql:9.3.0, false)
- GitHub Check: test-go (integration-enterprise, ubuntu-latest, mysql:9.3.0, false)
- GitHub Check: test-go (integration-mdm, ubuntu-latest, mysql:8.0.36, false)
- GitHub Check: test-go (fast, ubuntu-latest, mysql:8.0.36, false)
- GitHub Check: test-go (integration-core, ubuntu-latest, mysql:8.0.36, false)
- GitHub Check: test-go (integration-core, ubuntu-latest, mysql:9.3.0, false)
- GitHub Check: test-go (integration-mdm, ubuntu-latest, mysql:9.3.0, false)
- GitHub Check: markdown-link-check
- GitHub Check: build (20.x)
- GitHub Check: test-db-changes
- GitHub Check: lint (windows-latest)
- GitHub Check: build-binaries
- GitHub Check: lint (ubuntu-latest)
- GitHub Check: Analyze (go)
- GitHub Check: lint (macos-latest)
- GitHub Check: Analyze (javascript)
- GitHub Check: publish
🔇 Additional comments (1)
ee/server/service/hostidentity/types/host_identity_certificates.go (1)
13-15: renewal OID looks goodthe constant now references Fleet’s real IANA PEN (
1.3.6.1.4.1.63991.1.1). that lines up with the docs and removes the placeholder. nothing else to flag here.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #31579 +/- ##
=======================================
Coverage 63.69% 63.69%
=======================================
Files 1960 1960
Lines 191086 191089 +3
Branches 6128 6128
=======================================
+ Hits 121716 121719 +3
- Misses 59824 59825 +1
+ Partials 9546 9545 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
getvictor
marked this pull request as ready for review
August 4, 2025 18:28
getvictor
requested review from
a team,
lukeheath and
sharon-fdm
as code owners
August 4, 2025 18:28
sharon-fdm
reviewed
Aug 7, 2025
|
|
||
| The certificate is issued with a validity period of 365 days. Orbit attempts to renew it within 180 days of expiration. | ||
|
|
||
| To prevent thundering herd issues when multiple hosts need to renew certificates simultaneously, Orbit adds a random jitter of 0-30 minutes to the renewal timer. This ensures that certificate renewal requests are spread out over time, reducing load on the Fleet server. |
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #30476
Minor tweaks to host identity cert renewal feature: added IANA number and jitter
Mostly doc changes
Summary by CodeRabbit
Documentation
Bug Fixes
New Features