Skip to content
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

cmd/go: Can't override linkmode when setting buildmode=pie #44480

Closed
Foxboron opened this issue Feb 21, 2021 · 27 comments
Closed

cmd/go: Can't override linkmode when setting buildmode=pie #44480

Foxboron opened this issue Feb 21, 2021 · 27 comments

Comments

@Foxboron
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version go1.16 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/fox/.cache/go-build"
GOENV="/home/fox/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/fox/.go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/fox/.go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1989593130=/tmp/go-build -gno-record-gcc-switches"

What did you do?

The commit 6c0135d breaks RELRO as there is no way to override which linker to use when -buildmode=pie is set and the target is linux/amd64. This makes it impossible to build hardened binaries on most Linux platforms.

I'm not sure if it's intentional to force the internal linker? Shoulnd't -linkmode=external ensure we don't use the internal linker?

https://www.redhat.com/en/blog/hardening-elf-binaries-using-relocation-read-only-relro

What did you expect to see?

λ fzf » export GOFLAGS="-buildmode=pie -trimpath -ldflags=-linkmode=external"
λ fzf » export CGO_LDFLAGS="-Wl,-z,relro,-z,now"

λ fzf » CGO_ENABLED=0 go build -o fzf                                         
# github.com/junegunn/fzf
loadinternal: cannot find runtime/cgo
λ fzf » checksec --file=./fzf         
RELRO           STACK CANARY      NX            PIE             RPATH      RUNPATH	Symbols		FORTIFY	Fortified	Fortifiable	FILE
Partial RELRO   No canary found   NX enabled    PIE enabled     No RPATH   No RUNPATH   5316) Symbols	  No	0		0	./fzf

λ fzf » CGO_ENABLED=1 go build -o fzf           
λ fzf » checksec --file=./fzf                   
RELRO           STACK CANARY      NX            PIE             RPATH      RUNPATH	Symbols		FORTIFY	Fortified	Fortifiable	FILE
Full RELRO      Canary found      NX enabled    PIE enabled     No RPATH   No RUNPATH   5396) Symbols	  No	0		2	./fzf

What did you see instead?

λ fzf » export GOFLAGS="-buildmode=pie -trimpath -ldflags=-linkmode=external"
λ fzf » export CGO_LDFLAGS="-Wl,-z,relro,-z,now"


λ fzf » CGO_ENABLED=0 go build -o fzf 
# github.com/junegunn/fzf
loadinternal: cannot find runtime/cgo
λ fzf » checksec --file=./fzf         
RELRO           STACK CANARY      NX            PIE             RPATH      RUNPATH	Symbols		FORTIFY	Fortified	Fortifiable	FILE
Partial RELRO   No canary found   NX enabled    PIE enabled     No RPATH   No RUNPATH   4594) Symbols	  No	0		0	./fzf

λ fzf » CGO_ENABLED=1 go build -o fzf 
# github.com/junegunn/fzf
loadinternal: cannot find runtime/cgo
λ fzf » checksec --file=./fzf         
RELRO           STACK CANARY      NX            PIE             RPATH      RUNPATH	Symbols		FORTIFY	Fortified	Fortifiable	FILE
Partial RELRO   No canary found   NX enabled    PIE enabled     No RPATH   No RUNPATH   4594) Symbols	  No	0		0	./fzf
@cherrymui
Copy link
Member

How do you conclude that external linking is not used?

I think the fact that it printed loadinternal: cannot find runtime/cgo indicated that external linking is actually used, in both cases. You can check it by passing -ldflags=-v. If it includes a line of host link, it means external linking is used, and the external linker's invocation is printed.

@cherrymui
Copy link
Member

If the program doesn't use cgo but you want to pass flags to external linker, you can pass -ldflags=-extldflags=....

@Foxboron
Copy link
Contributor Author

λ fzf » export GOFLAGS="-buildmode=pie -trimpath -ldflags=-linkmode=external -ldflags=-v"

λ fzf » CGO_ENABLED=0 go build -o fzf                                                    
# github.com/junegunn/fzf
HEADER = -H5 -T0x401000 -R0x1000
96932 symbols, 27963 reachable
	1 package symbols, 36428 hashed symbols, 54613 non-package symbols, 5890 external symbols
135636 liveness data

λ fzf » CGO_ENABLED=1 go build -o fzf                                         
# github.com/junegunn/fzf
HEADER = -H5 -T0x401000 -R0x1000
96932 symbols, 27963 reachable
	1 package symbols, 36428 hashed symbols, 54613 non-package symbols, 5890 external symbols
135636 liveness data

host link is in neither outputs?

@Foxboron
Copy link
Contributor Author

Foxboron commented Feb 21, 2021

Okay, messing up the GOFLAGS env variable (It seems like it doesn't like having multiple -ldflags...)

This is with patch 6c0135d reverted

λ fzf » export GOFLAGS="-buildmode=pie -trimpath"
λ fzf » CGO_ENABLED=0 /home/fox/Git/prosjekter/Go/go/bin/go build -ldflags '-linkmode=external -v' -o fzf
# github.com/junegunn/fzf
HEADER = -H5 -T0x401000 -R0x1000
loadinternal: cannot find runtime/cgo
host link: "gcc" "-m64" "-Wl,-z,relro" "-pie" "-o" "/tmp/go-build3834599149/b001/exe/a.out" "-rdynamic" "-Wl,--compress-debug-sections=zlib-gnu" "/tmp/go-link-286472386/go.o"
96924 symbols, 40660 reachable
	1 package symbols, 36428 hashed symbols, 54613 non-package symbols, 5882 external symbols
135636 liveness data

λ fzf » CGO_ENABLED=1 /home/fox/Git/prosjekter/Go/go/bin/go build -ldflags '-linkmode=external -v' -o fzf
# github.com/junegunn/fzf
HEADER = -H5 -T0x401000 -R0x1000
host link: "gcc" "-m64" "-Wl,-z,relro" "-pie" "-o" "/tmp/go-build1452289956/b001/exe/a.out" "-rdynamic" "-Wl,--compress-debug-sections=zlib-gnu" "/tmp/go-link-964871768/go.o" "/tmp/go-link-964871768/000000.o" "/tmp/go-link-964871768/000001.o" "/tmp/go-link-964871768/000002.o" "/tmp/go-link-964871768/000003.o" "/tmp/go-link-964871768/000004.o" "/tmp/go-link-964871768/000005.o" "/tmp/go-link-964871768/000006.o" "/tmp/go-link-964871768/000007.o" "/tmp/go-link-964871768/000008.o" "/tmp/go-link-964871768/000009.o" "/tmp/go-link-964871768/000010.o" "/tmp/go-link-964871768/000011.o" "/tmp/go-link-964871768/000012.o" "-g" "-O2" "-lpthread"
97087 symbols, 40831 reachable
	1 package symbols, 36438 hashed symbols, 54654 non-package symbols, 5994 external symbols
135848 liveness data

This is with 1.16

λ fzf » export GOFLAGS="-buildmode=pie -trimpath"
λ fzf » CGO_ENABLED=1 go build -ldflags '-linkmode=external -v' -o fzf                                   
# github.com/junegunn/fzf
HEADER = -H5 -T0x401000 -R0x1000
loadinternal: cannot find runtime/cgo
host link: "gcc" "-m64" "-Wl,-z,relro" "-pie" "-o" "/tmp/go-build3793652355/b001/exe/a.out" "-rdynamic" "-Wl,--compress-debug-sections=zlib-gnu" "/tmp/go-link-943503868/go.o"
96924 symbols, 40660 reachable
	1 package symbols, 36428 hashed symbols, 54613 non-package symbols, 5882 external symbols
135636 liveness data

λ fzf » CGO_ENABLED=0 go build -ldflags '-linkmode=external -v' -o fzf
# github.com/junegunn/fzf
HEADER = -H5 -T0x401000 -R0x1000
loadinternal: cannot find runtime/cgo
host link: "gcc" "-m64" "-Wl,-z,relro" "-pie" "-o" "/tmp/go-build3793652355/b001/exe/a.out" "-rdynamic" "-Wl,--compress-debug-sections=zlib-gnu" "/tmp/go-link-943503868/go.o"
96924 symbols, 40660 reachable
	1 package symbols, 36428 hashed symbols, 54613 non-package symbols, 5882 external symbols
135636 liveness data

@cherrymui
Copy link
Member

As you posted, host like appears in both cases, and the change you reverted doesn't affect this (it changes whether to include runtime/cgo package, but doesn't affect linkmode). So -linkmode=external does work. Is there any more problem?

For the runtime/cgo package, #31544 is related. cmd/go has logic to include runtime/cgo if external linking is requested, but somehow that logic doesn't work (unrelated to the change you reverted).

@Foxboron
Copy link
Contributor Author

Should we rename this bugreport to be about the patch altering the runtime/cgo inclusion or? It seems weird to me that the patch is unrelated. It has to trigger the mentioned bug in some weird case e.g by changing the code path?

@cherrymui
Copy link
Member

It is "related" in that PIE used to always include runtime/cgo, and that change stops doing that. That is intentional, and I don't think we want to revert that. Now it's up to -linkmode=external to include runtime/cgo, but that somehow doesn't always work.

Why would you want to force the inclusion of runtime/cgo?

Should we rename this bugreport to be about the patch altering the runtime/cgo inclusion or?

I think we can combine it to #31544 and close this.

@Foxboron
Copy link
Contributor Author

Right, I get the issue now. However I think the patch should be reverted since this is quite a severe regression for binary hardening for distributions and introduces faulty/buggy behavior. What would be the downside of reverting the patch?

@cherrymui
Copy link
Member

You haven't explain why you'd want to force the inclusion of runtime/cgo. Thanks.

@cherrymui
Copy link
Member

Have you tried -ldflags=-extldflags=... as mentioned in #44480 (comment) ? It looks to me that you might want something cgo is included so you can use CGO_LDFLAGS (I may be wrong). If it is only about flags, try that. Thanks.

@Foxboron
Copy link
Contributor Author

I thought it was clear. Sorry. This solves binary hardening for Arch Linux as passing CGO_LDFLAGS is easier to bypass build systems then passing -ldflags through GOFLAGS as the go compiler does not merge ldflags. It was to my understanding they where equivalent but testing now everything work with -extldflags:

go build -ldflags '-linkmode=external -extldflags=-Wl,-z,relro,-z,now' -o fzf 

This works but passing the equivalent through GOFLAGS is something I have not figured out. I should point out that not all CGO_* flags have replacement flags.

@cherrymui
Copy link
Member

Thanks for the reply. So -extldflags works as expected.

GOFLAGS may have problems for parsing flags with spaces. But command line is always available.

PIE implying cgo is never the intention. Forcing the inclusion of C has never been supported (feel free to raise a feature request if you need it). Thanks.

@Foxboron
Copy link
Contributor Author

It still leaves out a lot to be desired though. You can't realistically pass $GOFLAGS for the options to enable RELRO as ldflags are skipped when upstreams provides their own -X options. But all of this is better suited for it's own issue?

@mvdan
Copy link
Member

mvdan commented Feb 22, 2021

@Foxboron from memory, you might be interested in #38522.

@Foxboron
Copy link
Contributor Author

@mvdan Ah yes. That is the crux of the flag issues.

But then I'll close this issue and contemplate reverting the commit downstream in Arch. It's annoying that intended and unintended usage of these flags is never defined and just breaks between releases without any notes in the release notes.

@ianlancetaylor
Copy link
Contributor

Sorry for the breakage. The intended use of CGO_LDFLAGS is to pass linker flags to programs that use cgo, as documented at https://golang.org/cmd/cgo. Merely using -buildmode=pie is not meant to imply that the program uses cgo. The fact that it used to imply that was a bug.

So I think the intended use of these flags is defined. I agree that unintended use of these flags can change between releases, and I agree that that is unfortunate. But it was documented in the release notes: https://golang.org/doc/go1.15#linker.

@Foxboron
Copy link
Contributor Author

Foxboron commented Feb 23, 2021

If you read over the command output you see CGO_LDFLAGS is not producing the correct binaries with CGO_ENABLED=0 or 1 though. Fine -buildmode=pie shouldn't have implied CGO, but CGO_ENABLED=1 no longer does the correct thing together with CGO_LDFLAGS as we are hitting the 2 year old bug where the go compiler hazards a guess what to do. So this is not just about -buildmode=pie previously implying cgo. It's just broken.

λ fzf » export CGO_LDFLAGS="-Wl,-z,relro,-z,now"
λ fzf » export GOFLAGS="-buildmode=pie -trimpath -ldflags=-linkmode=external"

# We ask go to enable CGO. No dice. Documented bug

λ fzf » CGO_ENABLED=1 go build -buildmode=pie -o fzf
# github.com/junegunn/fzf
loadinternal: cannot find runtime/cgo
λ fzf » checksec --file=./fzf
RELRO           STACK CANARY      NX            PIE             RPATH      RUNPATH	Symbols		FORTIFY	Fortified	Fortifiable	FILE
Partial RELRO   No canary found   NX enabled    PIE enabled     No RPATH   No RUNPATH   4594) Symbols	  No	0		0		./fzf
^^^^^^^^^^^^^

# Works. It uses CGO_LDFLAGS

λ fzf » CGO_ENABLED=1 go build -buildmode=pie -o fzf main.go
λ fzf » checksec --file=./fzf                               
RELRO           STACK CANARY      NX            PIE             RPATH      RUNPATH	Symbols		FORTIFY	Fortified	Fortifiable	FILE
Full RELRO      Canary found      NX enabled    PIE enabled     No RPATH   No RUNPATH   4692) Symbols	  Yes	2		2		./fzf
^^^^^^^^^^

@Foxboron
Copy link
Contributor Author

Foxboron commented Feb 23, 2021

So the issue is that we now have to pass -ldflags=-extldflags=$(LDFLAGS) for the go compiler to do the correct thing. This doesn't work without patching a number of build systems because GOFLAGS usage in Makefiles for Go upstreams predated the inclusion of the env variable in the compiler. And since "right most wins -ldflags" we can't produce hardened binaries without leaping through hoops.

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Feb 24, 2021

I'm not completely sure what you are saying, but I want to clarify that setting CGO_ENABLED=1 does not cause a program to built as a cgo program. It only enables the ability to build cgo programs.

Using -ldflags='-linkmode=external -extldflags=$(LDFLAGS)' has always been the correct and documented way to build Go programs in a specific way with the external linker.

@Foxboron
Copy link
Contributor Author

But you can't pass those flags through the GOFLAGS environment variable which is why the over-reliance on -buildmode=pie to pass CGO_* flags became a thing.

For Linux distributions this is all extremely limiting. I have spent many hours patching GOFLAGS usage in Makefiles and different build systems for go projects to try and get distributed binaries hardened but this all seems like an uphill battle.

CGO_* flags enabled us to largely skip the build system intricacies since upstreams don't use the variables. We can't pass GOFLAGS top-level since we can't realistically have it be passed through peoples build files. The syntax of the env variable versus the command line arguments is different enough that you can't really rely on people dealing with it consistently either.

The original issue was misguided as the compiler is complicated, but the situation is extremely confusing when you try do something that isn't straight forward with the compiler and compile flags 😞

Apologies for the noise

@Foxboron
Copy link
Contributor Author

I figured out that -z,relro is passed by the compiler when -buildmode=pie is specified. I should have looked at the specifics a year ago and it would have saved me quite some work.

The issue is that #15485 failed to mention -z,relro is not enough and should rather pass -z,relro-z,now as partial relro is not really useful for binary hardening. This is what distributions do downstream. It doesn't solve fortify, but that can be fixed in another round.

I'll figure out a patch for that and see what happens.

@ianlancetaylor
Copy link
Contributor

It sounds like you are saying that GOFLAGS is a poor choice of variable name because existing Makefiles also use that variable name. Is that right? Are there cases where the Makefile use of GOFLAGS interferes with the go tool?

@ianlancetaylor
Copy link
Contributor

Using -z,relro is basically a pure optimization, so there is no reason not to do it. Using -z,now changes the behavior of the program starup, and it's not necessarily the case that everybody wants that. That said, I think it would be reasonable to make -z,now the default but we might want some way to turn it off.

@Foxboron
Copy link
Contributor Author

It sounds like you are saying that GOFLAGS is a poor choice of variable name because existing Makefiles also use that variable name. Is that right?

Yes

Are there cases where the Makefile use of GOFLAGS interferes with the go tool?

This should give you a noisy overview how GOFLAGS is utilized in Makefiles. The usage of this name in build systems predates the introduction of the env variable.

https://github.com/search?q=go+build+GOFLAGS&type=Code

An example from a popular upstream: https://github.com/helm/helm/blob/6092f01fac0770009cd2cdad9541dd475c254d38/Makefile#L17

I found autoconf uses the flag for command-line arguments from 2010. I know this is only for gccgo, and I did spot the author. But it might have given some influence for using GOFLAGS for compiler flags in existing build systems before go upstream adopted it.

https://git.savannah.gnu.org/cgit/autoconf.git/tree/lib/autoconf/go.m4#n33

Changing this everywhere breaks existing build systems since the GOFLAGS env variable is not interchangeable and distributions already work around this or have pre-existing assumptions about what this is used for. This places the burden on upstream which needs to communicate the syntax change.

I think we are way off what the original issue was about. I'm not sure if you want a new issue for this?

That said, I think it would be reasonable to make -z,now the default but we might want some way to turn it off.

I'll submit a patch proposal so we can work something out. But I don't see why you wouldn't want full relro. The original commit didn't really mention why relro was added in the first place.

@ianlancetaylor
Copy link
Contributor

Yes, we should probably have two new issues: one for GOFLAGS confusion, and one for -z,now.

(From my perspective, -z,now is clearly distinct from -z,relro. -z,relro is a simple way to handle dynamic relocations that must always be resolved at program startup time. It's not a real change in functionality. -z,now tells the dynamic linker to resolve all dynamic relocations at program startup time, not just the ones that must be handled anyhow. This can introduce measurable latency to program startup time. It can also cause errors that would not otherwise occur, when there are some dynamic relocations that can't be resolved, but that doesn't matter without -z,now because they wouldn't be called.)

@Foxboron
Copy link
Contributor Author

Ack. Thanks for listening and I'll try hack on the linker!

Foxboron added a commit to Foxboron/go that referenced this issue Apr 21, 2021
Most Linux distributions today enable PIE and full RELRO on all binaries
to make exploitation harder. When buildmode=pie is used we enable full
relro as that is probably what most people want regardless.

This introduces a negligible startup time for binaries.

https://fedoraproject.org/wiki/Changes/Harden_All_Packages
https://www.redhat.com/en/blog/hardening-elf-binaries-using-relocation-read-only-relro

Related golang#44480

Signed-off-by: Morten Linderud <morten@linderud.pw>
Foxboron added a commit to Foxboron/go that referenced this issue Apr 21, 2021
Most Linux distributions today enable PIE and full RELRO on all binaries
to make exploitation harder. When buildmode=pie is used we enable full
relro as that is probably what most people want regardless.

This introduces a negligible startup time for binaries.

https://fedoraproject.org/wiki/Changes/Harden_All_Packages
https://www.redhat.com/en/blog/hardening-elf-binaries-using-relocation-read-only-relro

Related golang#44480
@gopherbot
Copy link

Change https://golang.org/cl/312509 mentions this issue: cmd/link: support full relro

@golang golang locked and limited conversation to collaborators Apr 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants