crypto/ed25519: Sign panics with private key allocated in memory-mapped region

[rkerno]

Go version

go version go1.25.1 linux/amd64

Output of go env in your module/workspace:

go env
AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE='on'
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/home/rkerno/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/rkerno/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build4067861915=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/home/rkerno/src/m/golib/go.mod'
GOMODCACHE='/home/rkerno/go/pkg/mod'
GONOPROXY='gitlab.projectcatalysts.prv'
GONOSUMDB='gitlab.projectcatalysts.prv'
GOOS='linux'
GOPATH='/home/rkerno/go'
GOPRIVATE='gitlab.projectcatalysts.prv'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/rkerno/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.25.1'
GOWORK='/home/rkerno/src/m/go.work'
PKG_CONFIG='pkg-config'
uname -sr: Linux 6.6.87.2-microsoft-standard-WSL2
/lib/x86_64-linux-gnu/libc.so.6: GNU C Library (Debian GLIBC 2.31-13+deb11u7) stable release version 2.31.

What did you do?

After creating an ed25519.PrivateKey ([]byte) from memory backed by a guarded memory page under my control (not part of Go's memory allocations), ed25519.Sign() panics because the pointer in the slice is not allocated by go.

This is a issue that was introduced after version 1.23.1 (I've just upgraded my project).

This is the function that panics:

//go:linkname internal_weak_runtime_registerWeakPointer weak.runtime_registerWeakPointer
func internal_weak_runtime_registerWeakPointer(p unsafe.Pointer) unsafe.Pointer {
	return unsafe.Pointer(getOrAddWeakHandle(unsafe.Pointer(p)))
}

If I hold my cursor over the variable p, this message is reported:

(unreadable could not find loclist entry at 0x2d2d1 for address 0x483bb3)

The issue is caused by the use of the fips140 private key cache:

func sign(signature []byte, privateKey PrivateKey, message []byte) {
k, err := privateKeyCache.Get(&privateKey[0], func() (*ed25519.PrivateKey, error) {

The reason for using a guarded page is so that I have explicit control over the lifetime of the private key, can prevent it from being included in crash dumps, and can ensure the backing memory is scrubbed when the key is no longer required. Introducing an (undocumented) requirement that the private key must be backed by go controlled memory is a regression,

What did you see happen?

fatal error: getWeakHandle on invalid pointer

goroutine 6 gp=0xc0001a2380 m=4 mp=0xc00004f808 [running]:
runtime.throw({0x88d1e1?, 0x498497?})
/usr/local/go/src/runtime/panic.go:1094 +0x48 fp=0xc0001bea80 sp=0xc0001bea50 pc=0x484b48
runtime.getWeakHandle(0x750d2c4f0040)
/usr/local/go/src/runtime/mheap.go:2639 +0xf2 fp=0xc0001bead0 sp=0xc0001bea80 pc=0x43a9d2
runtime.getOrAddWeakHandle(0x750d2c4f0040)
/usr/local/go/src/runtime/mheap.go:2568 +0x2b fp=0xc0001beb20 sp=0xc0001bead0 pc=0x43a76b
weak.runtime_registerWeakPointer(0x8b7430?)
/usr/local/go/src/runtime/mheap.go:2451 +0x13 fp=0xc0001beb38 sp=0xc0001beb20 pc=0x483bb3
weak.Make...
/usr/local/go/src/weak/pointer.go:74 +0x65 fp=0xc0001beb88 sp=0xc0001beb38 pc=0x63e9a5
crypto/internal/fips140cache.(*Cache[...]).Get(0x8bce80, 0x750d2c4f0040, 0xc0001bed88, 0xc0001bed68)
/usr/local/go/src/crypto/internal/fips140cache/cache.go:33 +0x76 fp=0xc0001bed00 sp=0xc0001beb88 pc=0x63de36
crypto/ed25519.sign({0xc0000ba040, 0x40, 0x40}, {0x750d2c4f0040, 0x40, 0x40}, {0xc0001bf847, 0x19, 0x19})
/usr/local/go/src/crypto/ed25519/ed25519.go:189 +0x173 fp=0xc0001bee58 sp=0xc0001bed00 pc=0x63d393
crypto/ed25519.Sign({0x750d2c4f0040, 0x40, 0x40}, {0xc0001bf847, 0x19, 0x19})
/usr/local/go/src/crypto/ed25519/ed25519.go:184 +0xde fp=0xc0001beef8 sp=0xc0001bee58 pc=0x63d19ec

What did you expect to see?

This call worked in 1.23.1.

[mateusz834]

Didn't know we have such good commit SHA1: ed25519 😄.

This is surely: CL 654096

CC @FiloSottile

[FiloSottile]

We are just using the weak package, so I think this is a decision for the @golang/runtime team. If storing values in non-Go allocated memory is a supported use case, then weak should support it. If not, our keys are not special.

[mateusz834]

We are just using the weak package, so I think this is a decision for the @golang/runtime team. If storing values in non-Go allocated memory is a supported use case, then weak should support it. If not, our keys are not special.

FYI there is also a runtime.AddCleanup call there, which surely is not going to work with a non-GC managed memory.

[mknyszek]

Oof, yeah, this is not a use-case supported by the weak package. The crypto libraries could ask the runtime if the slice is Go-managed, and if not, skip caching, maybe?

Disclaimer: I have very little context on what the crypto packages are caching.

[mateusz834]

The crypto libraries could ask the runtime if the slice is Go-managed, and if not, skip caching, maybe?

Is there a public API to do so? Just curious.

[mknyszek]

FWIW, having something like #70224 (which is not going to be trivial) would make this possible, but there is still a hazard here of unmapping the memory prematurely (before the runtime stops tracking it).

A more narrow version of #70224, like an os package for making memory maps managed by the Go runtime, would be easier (but still not trivial). (If we cheat and make the minimum mmap size 8 KiB and force it to be aligned, that would be MUCH easier.) We could even execute cleanups and clear weak pointers on deterministic destruction, for example.

Is there a public API to do so? Just curious.

No, it would have to be an internal linkname.

[mateusz834]

I am not sure if this is worth fixing, since a workaround is a 32B alloc+copy, but maybe just turning off the cache in non-fips mode would be fine? If i understand it correctly it is not needed then.

[FiloSottile]

I prefer to avoid as much as possible divergence between FIPS and non-FIPS mode. If we start accruing small little codepath differences, eventually it will get confusing to reason about behavior when debugging etc.

[gabyhelp]

Related Issues

• crypto/ecdsa: Sign() panics if public key is not set #70468
• crypto/internal/fips140: segfault from hmac memmove #70880 (closed)
• crypto/cipher: out of bounds write #21104 (closed)
• cmd/go: panic in cmd/go/internal/cache.(*DiskCache).markUsed #70600 (closed)
• weak: `Make` panics on pointers to objects not allocated in heap #71726 (closed)
• unsafe: store invalid address in `unsafe.Pointer` cause panic #75360 (closed)
• crypto/internal/fips140/subtle: Go 1.25 subtle.xorBytes panic on MIPS #74998
• unique: Fatal errors (found bad pointer in Go heap, found pointer to free object) and memory corruption #69643 (closed)
• With go 1.20 enters an infinite loop while trying to generate (a deterministic) ECDSA key #69248 (closed)

Related Code Changes

• crypto/ecdsa,crypto/ed25519: cache FIPS private keys

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[rkerno]

If a pairwise consistency test (PCT), as required by FIPS 140, is meant to verify that a private key and its corresponding public key form a valid keypair, how is that expected to work in this code path where only the private key is provided? I am just trying to use Go's standard library to sign some data using ed25519, and this use case shouldn't mandate FIPS 140 requirements.

[rkerno]

Some other thoughts...allocating a heap object for each signing call is inefficient, plus has the side effect that there would be a new allocation for each call, it would run through the slow path for the check for every signing operation, and each key copy would be added to the cache which would be guaranteed to grow for every signing call until the GC kicks in.

In any case, it doesn't actually make any sense in this context to derive a public key from the private key and then verify it, because all you're proving is your math is working. I understand you probably "have to" do this in order to tick the FIPS 140 box, but in my use case I am actually looking to place extra safeguards around the key itself.

I also don't know how this is supposed to work if you have keys managed externally via CGO - I imagine this use case would also fail.

[mknyszek]

Looking back at your original post, it sounds like your main use-case for using a guarded page is to make sure the memory gets scrubbed and the private key doesn't leak in various diagnostics. It sounds like the proposal in #21865 would be helpful to you.

Some other thoughts...allocating a heap object for each signing call is inefficient, plus has the side effect that there would be a new allocation for each call, it would run through the slow path for the check for every signing operation, and each key copy would be added to the cache which would be guaranteed to grow for every signing call until the GC kicks in.

That's true, but my impression is (correct me if I'm wrong) that you're already reusing a pointer to a private key value that's just allocated specially. And again, this isn't my area so correct me if I'm wrong, but given that the API's cache is based on pointers, it seems to expect you to do the same. This doesn't seem totally unreasonable to me, but I don't know the provenance of these private keys are (you're probably reading them from somewhere -- are you re-reading them every time?).

I also don't know how this is supposed to work if you have keys managed externally via CGO - I imagine this use case would also fail.

That's also true. Personally, supporting this in the crypto APIs specifically this doesn't sound unreasonable to me, because there seem to be additional motivations for using non-Go memory in crypto APIs specifically. So for that reason, I am going to punt this back to @FiloSottile for a final decision on what the crypto APIs should support.

From the perspective of the runtime, it's unfortunate but also OK to have these APIs just ask if it's in non-Go memory before checking the cache.

@FiloSottile: to your question about what the weak package supports, it just can't support non-Go memory at all. There's a lot of missing infrastructure to make that work.

I think @rkerno's use-case is another good reason to have some kind Go runtime managed mmap-style API. You'd get some chunk of mapped memory, you'd be allowed to do stuff to it like mlock or whatever it is you need, and the runtime will track pointers into it and free it if it's no longer in use. With this tracking would also come all the infrastructure needed to make things like weak pointers and cleanups work out of the box.