Skip to content

crypto/internal/fips140: segfault from hmac memmove #70880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
n2vi opened this issue Dec 17, 2024 · 10 comments
Closed

crypto/internal/fips140: segfault from hmac memmove #70880

n2vi opened this issue Dec 17, 2024 · 10 comments
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. OS-OpenBSD
Milestone

Comments

@n2vi
Copy link

n2vi commented Dec 17, 2024

Go version

go version go1.24rc1 openbsd/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN='/home/eric/go/bin'
GOCACHE='/home/eric/.cache/go-build'
GOENV='/home/eric/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='openbsd'
GOINSECURE=''
GOMODCACHE='/home/eric/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='openbsd'
GOPATH='/home/eric/go'
GOPRIVATE=''
GOPROXY='proxy.golang.org,direct'
GOROOT='/home/eric/go/root'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/eric/go/root/pkg/tool/openbsd_amd64'
GOVCS=''
GOVERSION='go1.23.4'
GODEBUG=''
GOTELEMETRY='on'
GOTELEMETRYDIR='/home/eric/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3116840763=/tmp/go-build -gno-record-gcc-switches'

What did you do?

On a Lenovo Carbon X1 running openbsd-current Dec 13,

git checkout go1.24rc1; cd src
bash ./all.bash

What did you see happen?

git checkout go1.24rc1
cd src; ./all.bash

...

ok      crypto/sha256   0.053s
ok      crypto/sha3     1.089s
ok      crypto/sha512   0.030s
ok      crypto/subtle   0.220s
ok      crypto/tls      2.386s
?       crypto/tls/internal/fips140tls  [no test files]
ok      crypto/x509     0.363s
?       crypto/x509/pkix        [no test files]

##### Testing without libgcc.
ok      net     0.180s
ok      os/user 0.086s

##### external linking, -buildmode=exe
--- FAIL: TestFIPSCheckVerify (0.06s)
    check_test.go:50: GODEBUG=fips140=on [/tmp/go-build4015808559/b001/fips140test.test -test.v -test.run=TestFIPSCheck] failed: exit status 2
        unexpected fault address 0x5ad020
        fatal error: fault
        [signal SIGSEGV: segmentation violation code=0x2 addr=0x5ad020 pc=0x47df7a]
        
        goroutine 1 gp=0xc000004380 m=0 mp=0x64aba0 [running, locked to thread]:
        runtime.throw({0x25f729?, 0x418a6f?})
                /home/eric/go/root/src/runtime/panic.go:1099 +0x48 fp=0xc000063c90 sp=0xc000063c60 pc=0x475d28
        runtime.sigpanic()
                /home/eric/go/root/src/runtime/signal_unix.go:939 +0x26a fp=0xc000063cf0 sp=0xc000063c90 pc=0x4778ca
        runtime.memmove()
                /home/eric/go/root/src/runtime/memmove_amd64.s:205 +0x17a fp=0xc000063cf8 sp=0xc000063cf0 pc=0x47df7a
        crypto/internal/fips140/sha256.(*Digest).Write(0xc0000d8280, {0x5ad020, 0x5c780, 0x5c780})
                /home/eric/go/root/src/crypto/internal/fips140/sha256/sha256.go:165 +0x7b fp=0xc000063d50 sp=0xc000063cf8 pc=0x5ade7b
        crypto/internal/fips140/hmac.(*HMAC).Write(...)
                /home/eric/go/root/src/crypto/internal/fips140/hmac/hmac.go:73
        crypto/internal/fips140/check.init.0()
                /home/eric/go/root/src/crypto/internal/fips140/check/check.go:127 +0x258 fp=0xc000063e28 sp=0xc000063d50 pc=0x5bf8b8
        runtime.doInit1(0x60e790)
                /home/eric/go/root/src/runtime/proc.go:7350 +0xc7 fp=0xc000063f50 sp=0xc000063e28 pc=0x450e07
        runtime.doInit(...)
                /home/eric/go/root/src/runtime/proc.go:7317
        runtime.main()
                /home/eric/go/root/src/runtime/proc.go:254 +0x330 fp=0xc000063fe0 sp=0xc000063f50 pc=0x442670
        runtime.goexit({})
                /home/eric/go/root/src/runtime/asm_amd64.s:1700 +0x1 fp=0xc000063fe8 sp=0xc000063fe0 pc=0x47d021
        
        goroutine 2 gp=0xc000004e00 m=nil [force gc (idle)]:
        runtime.gopark(0x60e9c0?, 0x64aba0?, 0x0?, 0x0?, 0x0?)
                /home/eric/go/root/src/runtime/proc.go:435 +0xce fp=0xc00004e7a8 sp=0xc00004e788 pc=0x475e4e
        runtime.goparkunlock(...)
                /home/eric/go/root/src/runtime/proc.go:441
        runtime.forcegchelper()
                /home/eric/go/root/src/runtime/proc.go:348 +0xa5 fp=0xc00004e7e0 sp=0xc00004e7a8 pc=0x4428e5
        runtime.goexit({})
                /home/eric/go/root/src/runtime/asm_amd64.s:1700 +0x1 fp=0xc00004e7e8 sp=0xc00004e7e0 pc=0x47d021
        created by runtime.init.7 in goroutine 1
                /home/eric/go/root/src/runtime/proc.go:336 +0x1a
        
        goroutine 3 gp=0xc000005340 m=nil [GC sweep wait]:
        runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
                /home/eric/go/root/src/runtime/proc.go:435 +0xce fp=0xc00005ef80 sp=0xc00005ef60 pc=0x475e4e
        runtime.goparkunlock(...)
                /home/eric/go/root/src/runtime/proc.go:441
        runtime.bgsweep(0xc00007a000)
                /home/eric/go/root/src/runtime/mgcsweep.go:276 +0x94 fp=0xc00005efc8 sp=0xc00005ef80 pc=0x42b974
        runtime.gcenable.gowrap1()
                /home/eric/go/root/src/runtime/mgc.go:204 +0x25 fp=0xc00005efe0 sp=0xc00005efc8 pc=0x41ff05
        runtime.goexit({})
                /home/eric/go/root/src/runtime/asm_amd64.s:1700 +0x1 fp=0xc00005efe8 sp=0xc00005efe0 pc=0x47d021
        created by runtime.gcenable in goroutine 1
                /home/eric/go/root/src/runtime/mgc.go:204 +0x66
        
        goroutine 4 gp=0xc000005500 m=nil [GC scavenge wait]:
        runtime.gopark(0xc00007a000?, 0x2b6120?, 0x1?, 0x0?, 0xc000005500?)
                /home/eric/go/root/src/runtime/proc.go:435 +0xce fp=0xc000064f78 sp=0xc000064f58 pc=0x475e4e
        runtime.goparkunlock(...)
                /home/eric/go/root/src/runtime/proc.go:441
        runtime.(*scavengerState).park(0x649aa0)
                /home/eric/go/root/src/runtime/mgcscavenge.go:425 +0x49 fp=0xc000064fa8 sp=0xc000064f78 pc=0x429449
        runtime.bgscavenge(0xc00007a000)
                /home/eric/go/root/src/runtime/mgcscavenge.go:653 +0x3c fp=0xc000064fc8 sp=0xc000064fa8 pc=0x42999c
        runtime.gcenable.gowrap2()
                /home/eric/go/root/src/runtime/mgc.go:205 +0x25 fp=0xc000064fe0 sp=0xc000064fc8 pc=0x41fea5
        runtime.goexit({})
                /home/eric/go/root/src/runtime/asm_amd64.s:1700 +0x1 fp=0xc000064fe8 sp=0xc000064fe0 pc=0x47d021
        created by runtime.gcenable in goroutine 1
                /home/eric/go/root/src/runtime/mgc.go:205 +0xa5
        
        goroutine 18 gp=0xc00008a700 m=nil [finalizer wait]:
        runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
                /home/eric/go/root/src/runtime/proc.go:435 +0xce fp=0xc000184e30 sp=0xc000184e10 pc=0x475e4e
        runtime.runfinq()
                /home/eric/go/root/src/runtime/mfinal.go:196 +0x107 fp=0xc000184fe0 sp=0xc000184e30 pc=0x41eec7
        runtime.goexit({})
                /home/eric/go/root/src/runtime/asm_amd64.s:1700 +0x1 fp=0xc000184fe8 sp=0xc000184fe0 pc=0x47d021
        created by runtime.createfing in goroutine 1
                /home/eric/go/root/src/runtime/mfinal.go:166 +0x3d
FAIL
FAIL    crypto/internal/fips140test     0.105s
FAIL
2024/12/16 18:21:47 Failed: exit status 1

##### sync -cpu=10
ok      sync    3.523s

##### Testing cgo
ok      cmd/cgo/internal/test   1.584s
# cmd/cgo/internal/test.test
ld: warning: test.cgo2.c(/tmp/go-link-1852096358/000011.o:(testSendSIG)): warning: rand() may return deterministic values, is that what you want?
ld: warning: test.cgo2.c(/tmp/go-link-1852096358/000011.o:(issue12030conv)): warning: sprintf() is often misused, please use snprintf()
ok      cmd/cgo/internal/test   2.005s
ok      cmd/cgo/internal/testtls        0.032s
ok      cmd/cgo/internal/testtls        0.038s
ok      cmd/cgo/internal/testnocgo      0.067s
ok      cmd/cgo/internal/testnocgo      0.019s
# cmd/cgo/internal/test.test
ld: warning: test.go:252 (/_/GOROOT/src/cmd/cgo/internal/test/test.go:252)(/tmp/go-link-3171303109/000010.o:(testSendSIG)): warning: rand() may return deterministic values, is that what you want?
ld: warning: test.go:634 (/_/GOROOT/src/cmd/cgo/internal/test/test.go:634)(/tmp/go-link-3171303109/000010.o:(issue12030conv)): warning: sprintf() is often misused, please use snprintf()
ok      cmd/cgo/internal/test   1.068s
# cmd/cgo/internal/test.test
ld: warning: test.go:252 (/_/GOROOT/src/cmd/cgo/internal/test/test.go:252)(/tmp/go-link-84946796/000010.o:(testSendSIG)): warning: rand() may return deterministic values, is that what you want?
ld: warning: test.go:634 (/_/GOROOT/src/cmd/cgo/internal/test/test.go:634)(/tmp/go-link-84946796/000010.o:(issue12030conv)): warning: sprintf() is often misused, please use snprintf()
ok      cmd/cgo/internal/test   1.008s

##### GOMAXPROCS=2 runtime -cpu=1 -quick
ok      runtime 7.719s

##### GOMAXPROCS=2 runtime -cpu=2 -quick
ok      runtime 11.014s

##### GOMAXPROCS=2 runtime -cpu=4 -quick
skipped due to earlier error

##### ../test
skipped due to earlier error
go tool dist: FAILED

What did you expect to see?

normal successful build

@dr2chase
Copy link
Contributor

Does this repeat easily/often?

@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 17, 2024
@dr2chase
Copy link
Contributor

@golang/openbsd

@n2vi
Copy link
Author

n2vi commented Dec 17, 2024 via email

@FiloSottile
Copy link
Contributor

It's segfaulting when trying to read sections from the binary to hash them, which is an unfortunate FIPS requirement.

I seem to remember OpenBSD was moving to make executable pages unreadable, which would be consistent with this.

We might have to just declare OpenBSD incompatible with FIPS 140 mode.

Unclear why the builders don't catch this.

/cc @rsc @cpu

@n2vi
Copy link
Author

n2vi commented Dec 27, 2024

Fine with me if Go says any OS using the security mitigation of "--execute-only" in the ELF linker from LLVM is ineligible for FIPS 140 mode. I'm no expert, but I gather that in addition to OpenBSD this includes some configurations of Android and Fuchsia.

There is continuing debate whether all the security mitigations that OpenBSD turns on are ultimately worth it. I like simplicity myself so wish it weren't so, but for some years I have voted with my feet: based on the evidence of real-world compromises, the machines I trust only run bare-bones OpenBSD.

I wonder if NIST will hold to that requirement of reading the text page if confronted with the incompatibility with extreme-security-configuration?

@4a6f656c
Copy link
Contributor

4a6f656c commented Jan 1, 2025

(I started raising an issue for this a few weeks back, only now managing to get back to it)

It's segfaulting when trying to read sections from the binary to hash them, which is an unfortunate FIPS requirement.

I seem to remember OpenBSD was moving to make executable pages unreadable, which would be consistent with this.

This is correct - OpenBSD runs with non-readable (execute-only) text segments on many architectures, since OpenBSD 7.3. For Go supported architectures this includes amd64, arm64, ppc64 and riscv64 - this gets more complex since the Go internal linker does not currently produce read-only text, which means failures only show up with external linking. Additionally, the code generated by Go's arm64 assembler still loads some constants from text (rather than rodata). As such, we currently disable execute-only when external linking on openbsd/arm64.

We might have to just declare OpenBSD incompatible with FIPS 140 mode.

I think there are two options - one is to declare OpenBSD incompatible with FIPS 140 mode, the other is to somehow know if FIPS mode is enabled when linking, in which case we can compile with -fno-execute-only when external linking (I had a super quick look a few weeks back, but couldn't see an easy way of detecting this at link time).

Unclear why the builders don't catch this.

Unfortunately the openbsd/amd64 builder is very out of date and runs OpenBSD 7.2 (which is EOL). The openbsd/riscv64 builder does however catch this:

https://build.golang.org/log/3c069cafb6d5d2659ecefc9d8ef47bdf5cc59d92

@FiloSottile
Copy link
Contributor

Thank you @4a6f656c, that all makes sense.

FIPS 140 mode is selected at runtime, so there is no way to enable -fno-execute-only at link time. We could enable it when compiling against a frozen FIPS module, which makes FIPS 140 mode the default, but it would probably be kinda confusing.

We're not validating the module on OpenBSD, at least for now, so it's fine to just mark it unsupported, and we can reassess in the future in case.

I wonder if NIST will hold to that requirement of reading the text page if confronted with the incompatibility with extreme-security-configuration?

Various objections to the integrity check requirement have been raised, and so far they have all failed to change anything. Last I heard, NIST was claiming they are not responsible for it because it's in the ISO standard (although they can and do waive requirements in SP 800-140 or IG documents). If anyone wants to give this a shot, I'd support it.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/639337 mentions this issue: crypto/internal/fips140: mark OpenBSD unsupported

@dr2chase
Copy link
Contributor

dr2chase commented Jan 2, 2025

I had a (horrible) thought, from my checkered past. The signal handler could check for reads from executable pages, and if FIPS mode is enabled and the address is in the range of the FIPS code, then temporarily remap that page, make a copy, and in the future redirect reads from that page to the copy. The shadow page could even be pre-allocated by the linker.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. FixPending Issues that have a fix which has not yet been reviewed or submitted. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 3, 2025
@dmitshur dmitshur added this to the Go1.24 milestone Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. OS-OpenBSD
Projects
None yet
Development

No branches or pull requests

7 participants