D30 rotation hardened after review: the key file is replaced by an fsynced rename over it (never missing or empty), the first old key stays private.key.prev, later ones dated (3 kept), the once-an-hour bound survives a restart; red-proved; CONTEXT; unreleased
gates / gates (push) Successful in 2m24s
gates / gates (push) Successful in 2m24s
This commit is contained in:
1 parent
ae69ad4512
commit
39fb9cbf6c
4 files changed
+145
-7
No files matched your search
@@ -7,6 +7,8 @@ import (
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-agent/internal/hub"
|
||||
@@ -66,6 +68,9 @@ func (m *Manager) clockAheadRekeyLocked(ctx context.Context, block *hub.WireWire
|
||||
if now.Sub(m.futureSince) < m.rekeyAfter {
|
||||
return false
|
||||
}
|
||||
if m.lastRekey.IsZero() {
|
||||
m.lastRekey = m.loadLastRekey()
|
||||
}
|
||||
if !m.lastRekey.IsZero() && now.Sub(m.lastRekey) < rekeyMinGap {
|
||||
return false
|
||||
}
|
||||
@@ -77,9 +82,11 @@ func (m *Manager) clockAheadRekeyLocked(ctx context.Context, block *hub.WireWire
|
||||
if err != nil {
|
||||
m.logger.Error("wgtunnel: re-key after a future handshake FAILED — the old key stays", "err", err)
|
||||
m.lastRekey = now // not again before rekeyMinGap
|
||||
m.saveLastRekey(now)
|
||||
return false
|
||||
}
|
||||
m.lastRekey, m.futureSince = now, time.Time{}
|
||||
m.saveLastRekey(now)
|
||||
m.logger.Warn("wgtunnel: RE-KEYED after a handshake stamped in the future (D30) — registering the new key; the hub re-keys the peer in place and the endpoint takes the new key",
|
||||
"old_pubkey", pub, "new_pubkey", newPub, "future_s", fut)
|
||||
m.nextRegAt = time.Time{} // register now, not after an older backoff
|
||||
@@ -87,8 +94,11 @@ func (m *Manager) clockAheadRekeyLocked(ctx context.Context, block *hub.WireWire
|
||||
return true
|
||||
}
|
||||
|
||||
// rotateKey makes a new key pair: the new private key is written next to the old one, the old one is kept as
|
||||
// private.key.prev, then the new one takes its place (renames in one directory). Returns the new PUBLIC key only.
|
||||
// rotateKey makes a new key pair, never leaving the box without a readable key (review 2026-10-23 night, N6): the new
|
||||
// key is written and fsynced to a temp file; the current key gets a second name by a HARD LINK (the escrow may hold
|
||||
// it — `private.key.prev` keeps the first such key, later ones `private.key.prev.<unix>`, the newest three kept, N7);
|
||||
// then the temp file is renamed OVER the current name (atomic) and the directory fsynced. At every instant
|
||||
// `private.key` is a complete key. Returns the new PUBLIC key only.
|
||||
func rotateKey(stateDir string) (string, error) {
|
||||
dir := filepath.Join(stateDir, "wg")
|
||||
cur := filepath.Join(dir, keyFileName)
|
||||
@@ -105,18 +115,67 @@ func rotateKey(stateDir string) (string, error) {
|
||||
return "", err
|
||||
}
|
||||
tmp := filepath.Join(dir, keyFileName+".new")
|
||||
if err := os.WriteFile(tmp, []byte(base64.StdEncoding.EncodeToString(priv)+"\n"), 0o600); err != nil {
|
||||
f, err := os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("writing the new key: %w", err)
|
||||
}
|
||||
if err := os.Rename(cur, filepath.Join(dir, prevKeyName)); err != nil {
|
||||
_, werr := f.Write([]byte(base64.StdEncoding.EncodeToString(priv) + "\n"))
|
||||
serr := f.Sync()
|
||||
cerr := f.Close()
|
||||
if werr != nil || serr != nil || cerr != nil {
|
||||
os.Remove(tmp)
|
||||
return "", fmt.Errorf("writing the new key: %v %v %v", werr, serr, cerr)
|
||||
}
|
||||
prev := filepath.Join(dir, prevKeyName)
|
||||
if _, err := os.Lstat(prev); err == nil {
|
||||
prev = filepath.Join(dir, fmt.Sprintf("%s.%d", prevKeyName, rekeyNow().Unix()))
|
||||
}
|
||||
if err := os.Link(cur, prev); err != nil {
|
||||
os.Remove(tmp)
|
||||
return "", fmt.Errorf("keeping the old key: %w", err)
|
||||
}
|
||||
if err := os.Rename(tmp, cur); err != nil {
|
||||
// put the old key back — never leave the box without a key
|
||||
os.Rename(filepath.Join(dir, prevKeyName), cur)
|
||||
os.Remove(tmp)
|
||||
return "", fmt.Errorf("installing the new key: %w", err)
|
||||
}
|
||||
if d, err := os.Open(dir); err == nil {
|
||||
d.Sync()
|
||||
d.Close()
|
||||
}
|
||||
prunePrevKeys(dir, 3)
|
||||
return pub, nil
|
||||
}
|
||||
|
||||
// rekeyNow is the clock for the kept-key names (a seam).
|
||||
var rekeyNow = time.Now
|
||||
|
||||
// prunePrevKeys keeps the newest `keep` of the `private.key.prev.<unix>` files (`private.key.prev` itself always stays).
|
||||
func prunePrevKeys(dir string, keep int) {
|
||||
m, _ := filepath.Glob(filepath.Join(dir, prevKeyName+".*"))
|
||||
if len(m) <= keep {
|
||||
return
|
||||
}
|
||||
sort.Strings(m) // the same digit count until 2286: lexical = time order
|
||||
for _, p := range m[:len(m)-keep] {
|
||||
os.Remove(p)
|
||||
}
|
||||
}
|
||||
|
||||
// lastRekeyFile records the last re-key's time across agent restarts (N7: the once-an-hour bound was in memory only).
|
||||
const lastRekeyFile = "last-rekey"
|
||||
|
||||
func (m *Manager) loadLastRekey() time.Time {
|
||||
b, err := os.ReadFile(filepath.Join(m.wgDir(), lastRekeyFile))
|
||||
if err != nil {
|
||||
return time.Time{}
|
||||
}
|
||||
var u int64
|
||||
if _, err := fmt.Sscanf(strings.TrimSpace(string(b)), "%d", &u); err != nil {
|
||||
return time.Time{}
|
||||
}
|
||||
return time.Unix(u, 0)
|
||||
}
|
||||
|
||||
func (m *Manager) saveLastRekey(t time.Time) {
|
||||
_ = os.WriteFile(filepath.Join(m.wgDir(), lastRekeyFile), []byte(fmt.Sprintf("%d\n", t.Unix())), 0o600)
|
||||
}
|
||||
@@ -3,6 +3,7 @@ package wgtunnel
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
@@ -144,3 +145,64 @@ func TestD30_AFailedRegistrationIsRetriedByTheExistingPath(t *testing.T) {
|
||||
t.Fatalf("the existing re-key-in-place path must register the new key: %+v", mk)
|
||||
}
|
||||
}
|
||||
|
||||
// Review 2026-10-23 night. N7: a second rotation keeps the FIRST old key as `private.key.prev` (the escrow may hold it)
|
||||
// and the later one beside it; N6: the current key is replaced by a rename OVER it, so `private.key` is never missing.
|
||||
// RED-PROOF: with the „prev exists → a dated name" branch removed, TwoRotations fails (the first key overwritten) —
|
||||
// seen failing 2026-10-23 night.
|
||||
func TestD30_TwoRotationsKeepTheFirstOldKey(t *testing.T) {
|
||||
fh := &fakeHub{}
|
||||
m, _, active, _ := testManager(t, fh)
|
||||
establishWG(t, m, active)
|
||||
first := readKey(t, m, keyFileName)
|
||||
old := rekeyNow
|
||||
defer func() { rekeyNow = old }()
|
||||
rekeyNow = func() time.Time { return time.Unix(hs, 0) }
|
||||
if _, err := rotateKey(m.stateDir); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
second := readKey(t, m, keyFileName)
|
||||
rekeyNow = func() time.Time { return time.Unix(hs+7200, 0) }
|
||||
if _, err := rotateKey(m.stateDir); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if readKey(t, m, prevKeyName) != first {
|
||||
t.Fatal("private.key.prev must keep the FIRST old key")
|
||||
}
|
||||
if readKey(t, m, fmt.Sprintf("%s.%d", prevKeyName, hs+7200)) != second {
|
||||
t.Fatal("the second old key must be kept beside it")
|
||||
}
|
||||
if cur := readKey(t, m, keyFileName); cur == first || cur == second {
|
||||
t.Fatal("the current key must be new")
|
||||
}
|
||||
}
|
||||
|
||||
// N7: the once-an-hour bound survives an agent restart (it was in memory only). RED-PROOF: with loadLastRekey removed
|
||||
// from clockAheadRekeyLocked, this fails — seen failing 2026-10-23 night.
|
||||
func TestD30_TheHourBoundSurvivesARestart(t *testing.T) {
|
||||
fh := &fakeHub{}
|
||||
m, rr, active, _ := testManager(t, fh)
|
||||
pub := establishWG(t, m, active)
|
||||
ctx := context.Background()
|
||||
t0 := int64(hs - 3*3600)
|
||||
at(m, t0)
|
||||
m.Watchdog(ctx, true, testBlock(pub))
|
||||
at(m, t0+16*60)
|
||||
m.Watchdog(ctx, true, testBlock(pub))
|
||||
newPub := localPub(t, m)
|
||||
if newPub == pub {
|
||||
t.Fatal("fixture: the first re-key must happen")
|
||||
}
|
||||
// The agent restarts: a NEW manager on the same state; the served block now names the new key; the sign is back.
|
||||
m2 := NewManager(rr, fh, m.stateDir, m.logger)
|
||||
m2.isActive, m2.wgPresent, m2.resolver = m.isActive, m.wgPresent, m.resolver
|
||||
m2.Apply(ctx, true, testBlock(newPub))
|
||||
at(m2, t0+20*60)
|
||||
m2.Watchdog(ctx, true, testBlock(newPub))
|
||||
at(m2, t0+40*60)
|
||||
m2.Watchdog(ctx, true, testBlock(newPub))
|
||||
if localPub(t, m2) != newPub {
|
||||
t.Fatal("a restart must not reset the once-an-hour bound")
|
||||
}
|
||||
_ = active
|
||||
}
|
||||
Reference in new issue
Block a user