diff --git a/CHANGELOG.md b/CHANGELOG.md index 659bf3c..a72aa86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,8 +38,35 @@ mark the stability commitment. `examples` too, while `examples/go.mod` still asks for 0.54.0 — and CI runs readonly, so that is a hard error before any package loads. +### Added + +- **`InstallTerminationWipeNoExit`** — `InstallTerminationWipe` without the + forced exit, for callers whose own handler owns termination. Adding exported + API makes the next core release a minor bump. + ### Fixed +- **`InstallTerminationWipe` now terminates the process on Windows instead of + wiping and running on.** `os.Process.Signal` there implements only `os.Kill` + and rejects `os.Interrupt` and SIGTERM, and the console event that triggered + the handler has already been consumed — so the re-raise was a guaranteed + no-op. Measured with a real `CTRL_C_EVENT` delivered to a child in its own + process group: the first Ctrl-C wiped every secret and the process kept + running, exiting only on a *second* one. + + That left it in the one state `WipeAllSecrets` does not support. The wipe + deliberately leaves regions mapped so a late read returns zeros rather than + faulting, a trade justified entirely by imminent termination — and reads + still **succeed**. A surviving process therefore holds every key buffer + readable and full of zeros, so an application treating the signal as "begin + shutdown" can sign with an all-zero key or derive from zeros and be told it + worked. Worse than either terminating or never wiping. + + secmem now exits with `0xC000013A` (`STATUS_CONTROL_C_EXIT`) — verified + identical to the status Windows produces for an un-intercepted Ctrl-C, so no + parent, batch file or CI step can tell a wrapped process from an unwrapped + one. Unix is unchanged: the re-raise is a real `kill(2)` and already worked. + - **The emergency wipe could zero a different, live buffer without holding its lock.** The janitor keyed each registration by its mapping's base address. That is unique for a mapping's lifetime but not across lifetimes: free a diff --git a/terminationexit_other.go b/terminationexit_other.go new file mode 100644 index 0000000..3ee62e3 --- /dev/null +++ b/terminationexit_other.go @@ -0,0 +1,14 @@ +//go:build !windows + +package secmem + +// forcedExitStatus is the process status used when [InstallTerminationWipe] has +// to terminate the process itself because the signal cannot be re-raised. +// +// Unreachable in practice here: everywhere except Windows the re-raise is a real +// kill(2) against a restored default disposition, so it terminates the process +// and this constant is never consulted. It exists so the fallback is defined +// rather than platform-conditional at the call site. +// +// 130 is the shell convention for "terminated by SIGINT" (128 + 2). +const forcedExitStatus = 130 diff --git a/terminationexit_windows.go b/terminationexit_windows.go new file mode 100644 index 0000000..bb11acf --- /dev/null +++ b/terminationexit_windows.go @@ -0,0 +1,17 @@ +//go:build windows + +package secmem + +// forcedExitStatus is the process status used when [InstallTerminationWipe] has +// to terminate the process itself because the signal cannot be re-raised. +// +// 0xC000013A is STATUS_CONTROL_C_EXIT — exactly what Windows produces when a +// console Ctrl-C terminates a process that did not intercept it. Matching it is +// the point: a parent, a batch file or a CI step must not be able to tell a +// secmem-wrapped process apart from an un-wrapped one by its exit status, or +// installing the wipe would silently change how every caller's tooling reads a +// cancellation. +// +// Written as the signed 32-bit value so the constant fits an int on 386 as well +// as amd64; os.Exit narrows to int32 and Windows receives 0xC000013A either way. +const forcedExitStatus = -1073741510 diff --git a/terminationwipe.go b/terminationwipe.go index beb6c32..fd9043b 100644 --- a/terminationwipe.go +++ b/terminationwipe.go @@ -16,6 +16,44 @@ import ( "syscall" ) +// reraiseSignal re-delivers sig to this process. It is the step that terminates +// the process on every platform that can do it. +func reraiseSignal(sig os.Signal) error { + proc, err := os.FindProcess(os.Getpid()) + if err != nil { + return err + } + return proc.Signal(sig) +} + +// completeTermination runs after the wipe: re-raise, and if that is impossible, +// either exit or report, per forceExit. +// +// reraise and exit are parameters rather than direct calls so the decision is +// testable without delivering a real signal — a test that actually re-raised +// would terminate the test binary, and one that actually exited would take the +// suite with it. The live behaviour is covered separately by a harness that +// delivers a genuine console Ctrl-C to a child in its own process group. +func completeTermination(sig os.Signal, forceExit bool, reraise func(os.Signal) error, exit func(int)) { + if err := reraise(sig); err == nil { + return // re-raised; the restored disposition or a co-handler owns the exit + } else if !forceExit { + slog.Warn("secmem: could not re-raise the termination signal — secrets are wiped, but this process will NOT exit on its own", + slog.String("signal", sig.String()), + slog.Any("error", err), + slog.String("advice", "exit from your own handler; InstallTerminationWipe (without NoExit) exits for you"), + ) + return + } else { + slog.Warn("secmem: termination signal could not be re-raised; exiting after the wipe", + slog.String("signal", sig.String()), + slog.Any("error", err), + slog.Int("status", forcedExitStatus), + ) + } + exit(forcedExitStatus) +} + // InstallTerminationWipe installs a cooperative signal handler that calls // [WipeAllSecrets] when the process receives a termination signal, then lets the // process terminate as it otherwise would. It returns a function that uninstalls @@ -30,27 +68,57 @@ import ( // explicit signals to override — note that adding SIGQUIT both suppresses Go's // default SIGQUIT goroutine dump and re-raises to a core-dumping disposition. // -// # Windows does not self-terminate -// -// The re-raise that makes the process exit is a no-op on Windows: -// [os.Process.Signal] there supports only [os.Kill] and rejects both -// [os.Interrupt] and SIGTERM outright. So on Windows this installer wipes and -// then RETURNS — every secret is gone, but the process keeps running and must -// exit on its own. The failure is logged at warn level rather than swallowed. -// Handle the exit in your own handler if you need one on that platform. -// // It does NOT clobber other signal handling. It registers its own channel with // [signal.Notify] (which is additive: a handler you installed with // signal.Notify still receives the signal too). On the signal it wipes, // deregisters ONLY its own channel with [signal.Stop] — never the process-global -// signal.Reset/signal.Ignore — and re-raises the signal: if secmem is the only -// handler the default disposition is restored and the process terminates; if you -// have your own handler it receives the signal and decides when to exit, so -// secmem never forces the exit out from under your graceful shutdown. +// signal.Reset/signal.Ignore — and re-raises the signal, so if secmem is the +// only handler the restored default disposition terminates the process, and if +// you have your own handler it receives the signal and decides when to exit. +// +// # The process always terminates +// +// Where the signal cannot be re-raised, secmem exits the process itself with a +// status indistinguishable from the un-intercepted signal. That is Windows: +// os.Process.Signal there implements only [os.Kill] and rejects os.Interrupt and +// SIGTERM outright, and the console event that triggered the handler has already +// been consumed, so there is nothing left to re-deliver. +// +// Verified behaviour before this was so: a real Ctrl-C wiped every secret and +// the process kept running, exiting only on a SECOND Ctrl-C. That left it in the +// one state [WipeAllSecrets] does not support. The wipe deliberately leaves +// regions MAPPED so a late read returns zeros instead of faulting — a trade +// justified entirely by "the process is terminating imminently". A process that +// survives instead keeps every key buffer readable and full of zeros, and reads +// still SUCCEED, so an application that treats the signal as "begin shutdown" +// can go on to sign with an all-zero key or derive from zeros, each call +// reporting success. That is worse than either terminating or never wiping. +// +// Use [InstallTerminationWipeNoExit] if your own handler owns the exit. // // If you already have a termination handler, prefer calling WipeAllSecrets from // inside it rather than using this installer. func InstallTerminationWipe(signals ...os.Signal) (uninstall func()) { + return installTerminationWipe(true, signals...) +} + +// InstallTerminationWipeNoExit is [InstallTerminationWipe] without the forced +// exit: if the signal cannot be re-raised, it wipes, logs, and returns, leaving +// termination entirely to the caller. +// +// Choose it when your own handler performs a graceful shutdown that must not be +// truncated — flushing logs, draining connections — and will exit on its own. +// Read the warning above first: after the wipe your secrets are gone but still +// READABLE as zeros, so a shutdown path that keeps doing cryptography will get +// silent success on zeroed key material. Exit promptly. +// +// It has no effect anywhere except Windows, since every other platform can +// re-raise and the process terminates through the normal disposition. +func InstallTerminationWipeNoExit(signals ...os.Signal) (uninstall func()) { + return installTerminationWipe(false, signals...) +} + +func installTerminationWipe(forceExit bool, signals ...os.Signal) (uninstall func()) { if len(signals) == 0 { signals = []os.Signal{os.Interrupt, syscall.SIGTERM} } @@ -89,17 +157,7 @@ func InstallTerminationWipe(signals ...os.Signal) (uninstall func()) { // so any such handler already received this signal independently. // On Windows the exit is therefore the application's job, and the // log line says so instead of leaving it to be discovered. - proc, err := os.FindProcess(os.Getpid()) - if err == nil { - err = proc.Signal(sig) - } - if err != nil { - slog.Warn("secmem: could not re-raise the termination signal — secrets are wiped, but this process will NOT exit on its own", - slog.String("signal", sig.String()), - slog.Any("error", err), - slog.String("advice", "exit from your own signal handler; on Windows os.Process.Signal supports only Kill"), - ) - } + completeTermination(sig, forceExit, reraiseSignal, os.Exit) case <-done: signal.Stop(ch) } diff --git a/terminationwipe_exit_test.go b/terminationwipe_exit_test.go new file mode 100644 index 0000000..52ed0b0 --- /dev/null +++ b/terminationwipe_exit_test.go @@ -0,0 +1,83 @@ +package secmem + +import ( + "errors" + "os" + "runtime" + "testing" +) + +// TestCompleteTermination_ExitsWhenReraiseImpossible pins the behaviour chosen +// after measuring the Windows one. +// +// os.Process.Signal on Windows implements only os.Kill and rejects os.Interrupt +// and SIGTERM, and the console event that triggered the handler has already been +// consumed — so there is nothing to re-deliver. Before this, the failure was +// swallowed and the process ran on with every secret wiped: reads still succeed +// and return zeros, so an application that treats the signal as "begin shutdown" +// could sign with an all-zero key and be told it worked. That is the one state +// WipeAllSecrets does not support, since its leave-mapped design is justified +// entirely by imminent termination. +// +// The hooks are injected because a test that really re-raised would kill the +// test binary and one that really exited would take the suite with it. Live +// delivery of a genuine console Ctrl-C is covered by a separate manual harness. +func TestCompleteTermination_ExitsWhenReraiseImpossible(t *testing.T) { + t.Parallel() + notSupported := errors.New("not supported by windows") + + cases := []struct { + name string + reraiseErr error + forceExit bool + wantExit bool + }{ + {"re-raise works: the disposition owns the exit", nil, true, false}, + {"re-raise works, NoExit: still not ours to force", nil, false, false}, + {"re-raise impossible, default: secmem exits", notSupported, true, true}, + {"re-raise impossible, NoExit: caller owns the exit", notSupported, false, false}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + var exited bool + var status int + completeTermination( + os.Interrupt, + c.forceExit, + func(os.Signal) error { return c.reraiseErr }, + func(code int) { exited, status = true, code }, + ) + if exited != c.wantExit { + t.Fatalf("exit called = %v, want %v", exited, c.wantExit) + } + if exited && status != forcedExitStatus { + t.Errorf("exit status = %d, want %d", status, forcedExitStatus) + } + }) + } +} + +// TestForcedExitStatus_MatchesUninterceptedSignal pins the status itself. A +// parent, a batch file or a CI step must not be able to tell a secmem-wrapped +// process apart from an un-wrapped one by how it died — otherwise installing the +// wipe silently changes how every caller's tooling reads a cancellation. +// +// 0xC000013A is STATUS_CONTROL_C_EXIT, confirmed against a real console Ctrl-C: +// a process left to Windows' own default and a process exited by this constant +// both report 0xc000013a. +func TestForcedExitStatus_MatchesUninterceptedSignal(t *testing.T) { + t.Parallel() + if runtime.GOOS != "windows" { + t.Skipf("status only has to match the OS default on Windows (GOOS=%s uses %d)", runtime.GOOS, forcedExitStatus) + } + // Typed, not an untyped literal: passed to Errorf an untyped 0xC000013A + // defaults to int and overflows on 386, which is what the 32-bit jobs + // caught. Going through a variable for the narrowing likewise, since + // uint32(constant) of a negative constant does not compile. + const statusControlCExit uint32 = 0xC000013A + narrowed := int32(forcedExitStatus) + if got := uint32(narrowed); got != statusControlCExit { + t.Errorf("forcedExitStatus = %#x, want %#x (STATUS_CONTROL_C_EXIT)", got, statusControlCExit) + } +}