hselomein
2 days ago
committed by
GitHub
GitHub
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 186 additions and 12 deletions
-
150ISSUE_DESCRIPTION.md
-
48acme.sh
@ -0,0 +1,150 @@ |
|||
# Fix: Honor `--days` flag for short-lived certificates |
|||
|
|||
## Issue Description |
|||
|
|||
The `--days` flag in acme.sh is currently ignored for short-lived certificates (typically 5-50 days validity), causing unexpected renewal behavior that differs from user expectations and normal certificate handling. |
|||
|
|||
### Problem Details |
|||
|
|||
**Current Behavior:** |
|||
- For **normal certificates** (90+ days): `--days 60` works as expected, scheduling renewal ~60 days after issuance |
|||
- For **short-lived certificates** (5-50 days): `--days` value is completely ignored, renewal is automatically set to 1 day before expiration regardless of user preference |
|||
|
|||
**Root Cause:** |
|||
The renewal calculation logic has two separate code paths: |
|||
1. **When `_notAfter` is NOT set** (normal certs): Uses `Le_RenewalDays` from `--days` flag |
|||
2. **When `_notAfter` IS set** (short-lived certs): Ignores `Le_RenewalDays` and uses hardcoded fallback logic |
|||
|
|||
This inconsistency exists in the certificate processing logic around lines 5388-5411, where short-lived certificates follow a different renewal calculation that doesn't consider the user's `--days` setting. |
|||
|
|||
### Impact |
|||
|
|||
This affects users working with: |
|||
- Short-lived certificates from CAs like Sectigo InCommon, DigiCert, etc. |
|||
- Testing environments with brief certificate lifespans |
|||
- High-security environments requiring frequent certificate rotation |
|||
- Upcoming industry standards (200 days by 2026, 100 days by 2027, 47 days by 2029) |
|||
|
|||
**Example Issue:** |
|||
```bash |
|||
./acme.sh --issue -d example.com --valid-to "+20d" --days 7 |
|||
# Expected: Renew 7 days after issuance |
|||
# Actual: Renews 1 day before expiration (19 days after issuance), ignoring --days 7 |
|||
``` |
|||
|
|||
## Solution |
|||
|
|||
### Changes Made |
|||
|
|||
**1. Enhanced Renewal Logic for Short-lived Certificates** |
|||
- Modified the `_notAfter` code path to respect user's `--days` setting first |
|||
- Added safety check to prevent renewal after certificate expiration |
|||
- Maintained fallback logic for cases where user's setting is invalid |
|||
|
|||
**2. Added Parameter Validation** |
|||
- Early validation for `--days` parameter (1-398 days range) |
|||
- Aligned maximum with current industry standard (398 days) |
|||
- Clear error messages for invalid values |
|||
|
|||
**3. Improved User Feedback** |
|||
- Added informational messages explaining which renewal logic is being used |
|||
- Warning messages when fallback logic is triggered |
|||
- Clear indication when user's setting conflicts with certificate validity |
|||
|
|||
### Technical Implementation |
|||
|
|||
**Core Logic Change** (lines ~5388-5427): |
|||
```bash |
|||
if [ "$_notAfter" ]; then |
|||
Le_CertExpireTime=$(_date2time "$_notAfter") |
|||
|
|||
# NEW: Calculate renewal time based on user's --days setting first |
|||
Le_UserRenewTime=$(_math "$Le_CertCreateTime" + "$Le_RenewalDays" \* 24 \* 60 \* 60) |
|||
|
|||
# NEW: Check if user's renewal time is after certificate expiration |
|||
if [ "$Le_UserRenewTime" -ge "$Le_CertExpireTime" ]; then |
|||
# Fallback to safe defaults with clear messaging |
|||
# (existing logic for 1 day/1 hour before expiration) |
|||
else |
|||
# NEW: Use user's setting when valid |
|||
Le_NextRenewTime="$Le_UserRenewTime" |
|||
_info "Using user-specified renewal time: $Le_RenewalDays days after issuance" |
|||
fi |
|||
fi |
|||
``` |
|||
|
|||
**Validation Enhancement** (lines ~5351-5364): |
|||
```bash |
|||
# NEW: Reasonable validation for --days parameter |
|||
if [ "$Le_RenewalDays" -gt "398" ]; then |
|||
_err "Invalid --days value: $Le_RenewalDays. Maximum supported is 398 days (current industry standard)." |
|||
return 1 |
|||
fi |
|||
if [ "$Le_RenewalDays" -eq "0" ]; then |
|||
_err "Invalid --days value: $Le_RenewalDays. Minimum supported is 1 day." |
|||
return 1 |
|||
fi |
|||
``` |
|||
|
|||
### Logic Flow |
|||
|
|||
1. **Early Validation**: Check `--days` parameter for reasonable range (1-398 days) |
|||
2. **User Preference**: Calculate renewal time based on user's `--days` setting |
|||
3. **Safety Check**: Verify user's setting doesn't cause renewal after expiration |
|||
4. **Fallback Logic**: Use safe defaults only when user's setting is invalid |
|||
5. **Clear Feedback**: Inform user which logic was applied and why |
|||
|
|||
### Behavior Examples |
|||
|
|||
**Valid User Settings (respected):** |
|||
```bash |
|||
--days 7 with 20-day cert → Renews 7 days after issuance ✓ |
|||
--days 15 with 30-day cert → Renews 15 days after issuance ✓ |
|||
--days 30 with 90-day cert → Renews 30 days after issuance ✓ |
|||
``` |
|||
|
|||
**Invalid User Settings (fallback with warning):** |
|||
```bash |
|||
--days 25 with 20-day cert → Warning + fallback to 1 day before expiration |
|||
--days 0 → Error: "Minimum supported is 1 day" |
|||
--days 500 → Error: "Maximum supported is 398 days" |
|||
``` |
|||
|
|||
### Backward Compatibility |
|||
|
|||
- **Existing behavior preserved** when no `--days` flag is used |
|||
- **Normal certificates** maintain existing renewal logic (with 1-day safety buffer) |
|||
- **Short-lived certificates without `--days`** continue using current fallback logic |
|||
- **No breaking changes** to configuration file format or cron behavior |
|||
|
|||
### Future-Proofing |
|||
|
|||
This fix prepares acme.sh for the industry's transition to shorter certificate lifespans: |
|||
- Current: 398 days maximum |
|||
- March 2026: 200 days maximum |
|||
- March 2027: 100 days maximum |
|||
- March 2029: 47 days maximum |
|||
|
|||
The validation and logic gracefully handle these transitions while respecting user intent. |
|||
|
|||
## Testing |
|||
|
|||
**Test Case 1: Short-lived certificate with valid --days** |
|||
```bash |
|||
./acme.sh --issue -d test.example.com --valid-to "+20d" --days 7 |
|||
Result: Le_RenewalDays='7', renewal scheduled exactly 7 days after issuance ✓ |
|||
``` |
|||
|
|||
**Test Case 2: Short-lived certificate with invalid --days** |
|||
```bash |
|||
./acme.sh --issue -d test.example.com --valid-to "+15d" --days 20 |
|||
Result: Warning message + fallback to 1 day before expiration ✓ |
|||
``` |
|||
|
|||
**Test Case 3: Normal certificate behavior unchanged** |
|||
```bash |
|||
./acme.sh --issue -d test.example.com --days 60 |
|||
Result: Renewal scheduled 59 days after issuance (existing behavior) ✓ |
|||
``` |
|||
|
|||
This fix ensures the `--days` flag works consistently across all certificate types while maintaining safety and providing clear user feedback. |
|||
Write
Preview
Loading…
Cancel
Save
Reference in new issue