From 205fe9059a3e1b5b26b38d60ffe2602cd9d9c1e8 Mon Sep 17 00:00:00 2001 From: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:52:58 -0400 Subject: [PATCH] fix(config): treat allow_unsynced_clock as live in requires_reboot The signer reads pki.allow_unsynced_clock from the stored config on every signing request, and nothing reads it at boot, so a change to it alone takes effect at once. ConfigStore.Apply still answered requires_reboot=true for it. Class it as hot next to allow_unverified_revocation_url. Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> --- docs/time-sync.md | 4 ++- internal/config/reboot.go | 7 +++-- internal/config/reboot_test.go | 7 +++++ internal/init/run.go | 4 +-- internal/node/apply_live_test.go | 48 ++++++++++++++++++++++++++++++++ internal/node/providers.go | 4 +-- 6 files changed, 66 insertions(+), 8 deletions(-) create mode 100644 internal/node/apply_live_test.go diff --git a/docs/time-sync.md b/docs/time-sync.md index 9fb1ba2..6c5f512 100644 --- a/docs/time-sync.md +++ b/docs/time-sync.md @@ -68,7 +68,9 @@ good sync and stays open for the rest of the boot, even if a later poll fails. responder, the EST server and the SCEP RA certificates are minted when their listeners start, often before a first sync, so those listeners can come up. - `pki.allow_unsynced_clock: true` lifts the gate. Every certificate signed that - way is logged as a warning. Leave it off in production. + way is logged as a warning. Leave it off in production. The signer reads it on + every request, so a change to it alone takes effect at once and `config apply` + reports `requires_reboot=false`. ```yaml pki: diff --git a/internal/config/reboot.go b/internal/config/reboot.go index aaccca4..1de6303 100644 --- a/internal/config/reboot.go +++ b/internal/config/reboot.go @@ -27,9 +27,9 @@ import ( // // The CA signer reads the live config on every operation, so a change limited // to the cert profiles (pki.profiles), the irreversible root-leaf-issuance -// acknowledgement (pki.root_leaf_issuance) or the revocation preflight override -// (pki.allow_unverified_revocation_url) takes effect immediately for signing — -// no reboot. Every other field (network, install disk, role, state key, the +// acknowledgement (pki.root_leaf_issuance), the revocation preflight override +// (pki.allow_unverified_revocation_url) or the clock-gate override +// (pki.allow_unsynced_clock) takes effect immediately for signing — no reboot. Every other field (network, install disk, role, state key, the // revocation endpoint that gates the boot-time CRL/OCSP listener, hostname, // management link) is consumed at boot, so a change there still needs a // reboot. This is deliberately conservative: only the proven-hot fields are @@ -67,5 +67,6 @@ func hotNormalized(c *Config) Config { n.PKI.Profiles = nil n.PKI.RootLeafIssuance = "" n.PKI.AllowUnverifiedRevocationURL = false + n.PKI.AllowUnsyncedClock = false return n } diff --git a/internal/config/reboot_test.go b/internal/config/reboot_test.go index 9a6319b..30bbec4 100644 --- a/internal/config/reboot_test.go +++ b/internal/config/reboot_test.go @@ -189,4 +189,11 @@ func TestNeedsRebootLiveFields(t *testing.T) { t.Error("an allow_unverified_revocation_url-only change must not require a reboot") } }) + + t.Run("allow_unsynced_clock is read live by the signer (no reboot)", func(t *testing.T) { + old, next := roundTrip(t, func(c *Config) { c.PKI.AllowUnsyncedClock = true }) + if NeedsReboot(old, next) { + t.Error("an allow_unsynced_clock-only change must not require a reboot") + } + }) } diff --git a/internal/init/run.go b/internal/init/run.go index eb55072..a537173 100644 --- a/internal/init/run.go +++ b/internal/init/run.go @@ -474,8 +474,8 @@ func boot(ctx context.Context, shutdown *shutdownRequests) (err error) { } // The signer reads the LIVE on-disk config, not the boot snapshot, so an // ApplyConfig change on a running node (a new/updated cert profile, the - // root-leaf-issuance acknowledgement, the revocation preflight override) - // takes effect for signing immediately + // root-leaf-issuance acknowledgement, the revocation preflight override, + // the clock-gate override) takes effect for signing immediately // without a reboot. Fall back to the boot config if the store is briefly // unreadable; install-level fields (network/disk/role/state key) are still // only consumed at boot, so reading them live here is harmless. diff --git a/internal/node/apply_live_test.go b/internal/node/apply_live_test.go new file mode 100644 index 0000000..3c0d616 --- /dev/null +++ b/internal/node/apply_live_test.go @@ -0,0 +1,48 @@ +package node + +/* +Copyright The CryptOS Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import ( + "context" + "testing" +) + +// The signer reads pki.allow_unsynced_clock from the stored config on every +// signing request and nothing reads it at boot, so switching the override on +// or off alone takes effect at once and must not ask for a reboot. +func TestConfigStoreApply_AllowUnsyncedClockIsLive(t *testing.T) { + ctx := context.Background() + fs, cs := seededStore(t, protocolsOffSeed) + + for _, on := range []bool{true, false} { + current, err := cs.Current(ctx) + if err != nil { + t.Fatalf("Current: %v", err) + } + current.Pki.AllowUnsyncedClock = on + resp, err := cs.Apply(ctx, current) + if err != nil { + t.Fatalf("Apply (allow_unsynced_clock=%t): %v", on, err) + } + if resp.GetRequiresReboot() { + t.Errorf("setting only allow_unsynced_clock=%t answered requires_reboot", on) + } + if got := storedConfig(t, fs).PKI.AllowUnsyncedClock; got != on { + t.Errorf("stored allow_unsynced_clock = %t, want %t", got, on) + } + } +} diff --git a/internal/node/providers.go b/internal/node/providers.go index 942396b..efb9573 100644 --- a/internal/node/providers.go +++ b/internal/node/providers.go @@ -294,8 +294,8 @@ func (c *ConfigStore) Apply(ctx context.Context, cfg *cryptosv1.MachineConfig) ( // // Classify BEFORE overwriting: a change limited to the hot-reconfigurable // fields (cert profiles, root-leaf-issuance acknowledgement, revocation - // preflight override) takes effect live for signing, so the caller need - // not reboot. Any other change — or a + // preflight override, clock-gate override) takes effect live for signing, + // so the caller need not reboot. Any other change — or a // first apply with no prior config — requires a reboot. Fail safe to reboot // if the current config cannot be read or parsed. var oldCfg *config.Config