Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions .github/scripts/release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,32 @@ esac
# Parsed via `go mod edit -json` rather than by reading go.mod as text — the
# require block has several legal shapes and a regex over it is exactly the
# kind of thing that quietly matches nothing and reports success.
# latest_version extracts .Version from a proxy @latest JSON body on stdin.
latest_version() {
python3 -c 'import json,sys
try:
print(json.load(sys.stdin).get("Version", ""))
except Exception:
print("")' 2>/dev/null || printf ""
}

# version_lt reports whether $1 is an older semver than $2, i.e. whether this
# module's go.mod still points at a superseded release. SECMEM_ALLOW_STALE_DEP=1
# disables the refusal for a deliberately older floor.
#
# An unparseable version on either side reports "not older": pseudo-versions and
# +incompatible are shapes this comparison does not understand, and refusing on
# something it cannot read would be a different kind of wrong answer.
version_lt() {
[ "${SECMEM_ALLOW_STALE_DEP:-0}" = "1" ] && return 1
python3 -c 'import re, sys
def parse(v):
m = re.match(r"^v(\d+)\.(\d+)\.(\d+)", v or "")
return tuple(int(x) for x in m.groups()) if m else None
a, b = parse(sys.argv[1]), parse(sys.argv[2])
sys.exit(0 if (a and b and a < b) else 1)' "$1" "$2"
}

repo_root_module=$(GOWORK=off go list -m)
deps=$(cd "$dir" && GOWORK=off go mod edit -json | python3 -c '
import json,sys
Expand All @@ -123,6 +149,38 @@ else
Tagging now would publish a release pointing at a version that
cannot be resolved. Release $req $reqver first, then re-run this."
ok "in-repo dependency $req $reqver is published"

# "Published" is NOT the invariant this gate exists for. The failure that
# actually happened — secmem-crypto/v0.3.0, permanently inert — had a
# go.mod requiring a version that was published just fine. It was simply
# the OLD one, because the tag was cut before the floor-raise PR merged.
# Checking only for publication passes that case and reports success, which
# is the one outcome this script must never produce.
#
# So the required version must also be the newest published one. If the
# dependency has moved ahead, this module's go.mod has not caught up and the
# tag would claim a dependency it does not have.
latestbody=$(curl -s -w '
%{http_code}' "https://proxy.golang.org/${reqesc}/@latest" || printf '
000')
latestcode=$(printf '%s' "$latestbody" | tail -1)
latest=$(printf '%s' "$latestbody" | sed '$d' | latest_version)
if [ "$latestcode" != "200" ] || [ -z "$latest" ]; then
# Fail closed. An unreachable proxy is exactly when a human is most
# tempted to shrug and tag anyway.
die "cannot determine the latest published $req (HTTP $latestcode).
Refusing to tag: this is the check that catches a stale go.mod, and
it does not get skipped because the proxy is unreachable. Retry
when proxy.golang.org answers."
fi
if version_lt "$reqver" "$latest"; then
die "$dir/go.mod requires $req $reqver, but $latest is published.
This is the ordering footgun: the tag would claim a dependency it
does not have, permanently. Merge the go.mod floor raise and
re-run. If the older floor is deliberate, re-run with
SECMEM_ALLOW_STALE_DEP=1 and record why in CHANGELOG.md."
fi
ok "in-repo dependency $req $reqver is the newest published"
done <<EOF
$deps
EOF
Expand Down
30 changes: 30 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@ mark the stability commitment.

### Changed

- **`release.sh` now refuses a stale in-repo dependency instead of passing it.**
The ordering check only asked whether the required version was *published*.
The failure it exists to prevent — the permanently inert
`secmem-crypto/v0.3.0` — required a version that was published perfectly well;
it was simply the previous one, because the tag was cut before the floor-raise
PR merged. The gate therefore reported success on exactly the case it was
written to catch. It now also requires that version to be the newest published
one, fails closed when the proxy cannot be reached, and honours
`SECMEM_ALLOW_STALE_DEP=1` for a deliberately older floor.

- **`secmem-crypto`, `examples`: `golang.org/x/crypto` 0.54.0 → 0.55.0.**
Maintenance, not a fix: the `vuln` job was green against 0.54.0, so nothing
outstanding was reachable. Recorded because a `require` change in
Expand All @@ -30,6 +40,26 @@ mark the stability commitment.

### Fixed

- **`memfd_secret` descriptors are now close-on-exec.** The fd was created with
no flags and stayed inheritable across `ftruncate`, the guard reservation and
the `MAP_FIXED` — so a `fork`+`exec` from any other goroutine in that window
handed the child a live descriptor to the secret pages, making the strongest
allocation tier the one that leaked across `exec`. The flag is `O_CLOEXEC`,
not the `FD_CLOEXEC` the man page names: the kernel tests `flags & O_CLOEXEC`
and `EINVAL`s anything else, so the wrong bit would have silently dropped
every allocation to the weaker anon path. An `EINVAL` is retried bare with
`fcntl(F_SETFD)` instead, so a kernel that disagrees still gets close-on-exec
and never a tier downgrade.

- **`InstallTerminationWipe` no longer claims to terminate the process on
Windows.** `os.Process.Signal` there supports only `os.Kill` and rejects both
`os.Interrupt` and SIGTERM, so the re-raise was a guaranteed no-op whose error
was discarded: the process ran on past Ctrl-C with every secret already
zeroed, while the documentation said it exited. The failure is now logged and
the platform limit is documented. The behaviour is deliberately not escalated
to a forced `os.Exit`, because the installer promises never to take the exit
away from a co-installed graceful shutdown.

- `WipeAllSecrets` no longer lets one borrowed buffer strand another's secret.
The emergency wipe's second pass blocked on the deferred regions sequentially,
in map-iteration order. Because `tryWipeInPlace` defers a region whose lock is
Expand Down
31 changes: 30 additions & 1 deletion mlock_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,36 @@ func allocMemfdSecret(pageSize, rounded, total int) (region secRegion, noFork bo
if unsafe.Sizeof(uintptr(0)) != 8 {
return secRegion{}, false, errors.New("memfd_secret: requires a 64-bit architecture")
}
fd, _, errno := unix.Syscall(sysMemfdSecret, 0, 0, 0)
// Close-on-exec, asked for at creation. Without it the descriptor stays
// inheritable for the whole window below — ftruncate, the guard
// reservation, and the MAP_FIXED — and a fork+exec from any other
// goroutine during it hands the child a live descriptor to the secret
// pages. secretmem is readable through that fd, so the strongest tier
// would be the one that leaks across exec.
//
// The bit is O_CLOEXEC, not FD_CLOEXEC. memfd_secret(2)'s man page names
// FD_CLOEXEC, but the kernel tests `flags & O_CLOEXEC` and returns EINVAL
// for anything outside SECRETMEM_FLAGS_MASK|O_CLOEXEC. Passing FD_CLOEXEC
// (bit 0) would therefore not merely fail to set close-on-exec — it would
// fail the syscall outright and silently drop every allocation to the
// weaker L3 path, which is worse than the leak being fixed.
//
// That distinction is kernel-version sensitive and this file cannot be
// executed from the maintainer's platform, so EINVAL is not trusted to
// mean "flag unsupported" and nothing else: it retries bare and sets
// close-on-exec with fcntl instead. Slightly larger window than the
// atomic form, still far smaller than none, and a tier downgrade is
// impossible either way.
fd, _, errno := unix.Syscall(sysMemfdSecret, uintptr(unix.O_CLOEXEC), 0, 0)
if errno == unix.EINVAL {
fd, _, errno = unix.Syscall(sysMemfdSecret, 0, 0, 0)
if errno == 0 {
if _, ferr := unix.FcntlInt(fd, unix.F_SETFD, unix.FD_CLOEXEC); ferr != nil {
_ = unix.Close(int(fd))
return secRegion{}, false, fmt.Errorf("memfd_secret: set FD_CLOEXEC: %w", ferr)
}
}
}
if errno != 0 {
return secRegion{}, false, errno // ENOSYS = kernel too old / not built; EPERM = lockdown
}
Expand Down
26 changes: 24 additions & 2 deletions scrub_frame_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package secmem

import (
"runtime/debug"
"testing"
"unsafe"
)
Expand Down Expand Up @@ -63,8 +64,13 @@ func plantMarkers(depth int) uintptr {

// countMarkers reads scrubPad bytes of (dead) stack at addr and counts markers.
// nocheckptr: reading an address held across calls is exactly what checkptr
// forbids; it is intentional here and safe (stack segments are pooled, not
// unmapped).
// forbids, and it is intentional here.
//
// It is NOT unconditionally safe, which an earlier version of this comment
// claimed: "stack segments are pooled, not unmapped" holds only while the
// segment is still this goroutine's. After a stack shrink it returns to the
// pool and may be scavenged, so the caller is responsible for keeping the
// collector out of the window — see TestScrub_ScrubsShallowCallTree.
//
//go:nocheckptr
//go:noinline
Expand All @@ -81,6 +87,22 @@ func countMarkers(addr uintptr) int {
// TestScrub_ScrubsShallowCallTree verifies Scrub scrubs the stack residue
// its own shallow call tree leaves — without any manual pre-growth.
func TestScrub_ScrubsShallowCallTree(t *testing.T) {
// The markers are addressed by a raw uintptr into this goroutine's stack,
// which the GC neither tracks nor adjusts. A stack shrink between planting
// and reading frees that segment back to the pool, and the read then lands
// on whatever now occupies the address:
//
// - unrelated memory, which holds no 0xA5 markers, so countMarkers
// returns 0 and the assertion PASSES having tested nothing — a
// vacuous green on a security regression test, which is worse than a
// failure;
// - or a scavenged span, which faults.
//
// shrinkstack only runs while the collector is scanning this goroutine, so
// turning the collector off for the window closes both. Done before the
// control read so it covers that too.
defer debug.SetGCPercent(debug.SetGCPercent(-1))

// Control: confirm markers are observable on dead stack when nothing scrubs
// them. If a future toolchain zeros eagerly, the subject would be vacuous.
if countMarkers(plantMarkers(4)) == 0 {
Expand Down
38 changes: 36 additions & 2 deletions terminationwipe.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
package secmem

import (
"log/slog"
"os"
"os/signal"
"sync"
Expand All @@ -29,6 +30,15 @@ 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,
Expand Down Expand Up @@ -63,8 +73,32 @@ func InstallTerminationWipe(signals ...os.Signal) (uninstall func()) {
// disposition until this Stop, so a second signal arriving mid-wipe
// could not kill us early — no global Ignore is needed.
signal.Stop(ch)
if p, err := os.FindProcess(os.Getpid()); err == nil {
_ = p.Signal(sig)
// Re-raise so the now-default disposition terminates the process.
//
// This cannot work on Windows: os.Process.Signal rejects everything
// except Kill with "not supported by windows" — verified on go1.26,
// windows/amd64, for both os.Interrupt and SIGTERM. Discarding that
// error left the worst of the three possible outcomes: the process
// sails past Ctrl-C still running, with every secret already zeroed
// (reads return zeros, mutations return ErrWiped), while this
// function's documentation says it terminates.
//
// Reported rather than escalated to a forced os.Exit, because the
// installer promises never to take the exit out from under a
// co-installed graceful shutdown — and signal.Notify is additive,
// 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"),
)
}
case <-done:
signal.Stop(ch)
Expand Down
Loading