fix: Make SSH certificate auto-renewal robust to backdated validity#362
fix: Make SSH certificate auto-renewal robust to backdated validity#362minhtule wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the SSH host certificate auto-renewal loop against certificates whose validity window is backdated or already expired, preventing immediate timer triggers and tight re-sign loops that could hammer the CA.
Changes:
- Floor renewal timers using a new
renewalDelay()helper so past renewal times wait at leastretryInterval. - Adjust
renewTime()to measure the renewal schedule from “now” whenValidAfteris in the past (backdated validity). - Add/expand tests to cover floored renewal behavior and backdated
ValidAfterhandling.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/sshhandler/cert.go | Floors timer delays via renewalDelay() and clamps backdated ValidAfter when computing renewal time. |
| internal/sshhandler/cert_test.go | Adds a regression test for past renew times and expands renewTime() test coverage for backdated validity. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #362 +/- ##
=======================================
Coverage 86.18% 86.19%
=======================================
Files 40 40
Lines 2830 2839 +9
=======================================
+ Hits 2439 2447 +8
- Misses 265 268 +3
+ Partials 126 124 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
minhtule
changed the title
minhtule
force-pushed
the
feat/mt/improve-ssh-cert-renewal-logic
branch
from
6a3a75c to
afc6fb1
Compare
| // renewalDelay returns the delay until t, floored at retryInterval so a renewal time in the past | ||
| // (e.g. from a backdated certificate) cannot busy-loop the timer. | ||
| func renewalDelay(t time.Time) time.Duration { | ||
| return max(time.Until(t), retryInterval) | ||
| } |
There was a problem hiding this comment.
The floor only affects renewals under ~12.5s out (0.8 × lifetime < retryInterval). The shortest configured TTL is 5m, which renews ~240s out, so a legitimate near-term renewal is never delayed in practice. The floor is also an intentional debounce: it caps re-signs at one per retryInterval, matching the error-retry path and preventing CA hammering if a CA keeps issuing near-expired certs. Clamping only non-positive durations would remove that guard.
…interval Surface a misbehaving CA that issues certificates already at or near their renewal point, which forces renewal onto the retry interval.
Measure remaining lifetime from now rather than the certificate's ValidAfter, so a backdated start no longer drags the renewal point earlier. Drops the now-unneeded ValidAfter clamp and overflow guard.
Related Tickets & Documents
Changes
ValidAfteris backdated, so an already-started validity window doesn't drag the schedule into the pastretryInterval