cmd/go: enable ASLR on Windows -buildmode=c-shared

[qmuntal]

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

go version go1.15.1 windows/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\muntal\AppData\Local\go-build
set GOENV=C:\Users\muntal\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\muntal\go\pkg\mod
set GONOPROXY=*.hp.com
set GONOSUMDB=*.hp.com
set GOOS=windows
set GOPATH=C:\Users\muntal\go
set GOPRIVATE=*.hp.com
set GOPROXY=direct
set GOROOT=c:\go
set GOSUMDB=off
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\muntal\AppData\Local\Temp\go-build392009438=/tmp/go-build -gno-record-gcc-switches

What did you do?

go build -buildmode=c-shared -o libfoo.dll .
dumpbin /headers libfoo.dll

What did you expect to see?

...
OPTIONAL HEADER VALUES
...
            8160 DLL characteristics
                   High Entropy Virtual Addresses
                   Dynamic base
                   NX compatible
                   Terminal Server Aware
...

I expected the dll to have IMAGE_DLLCHARACTERISTICS_DYNAMIC_BASE and IMAGE_DLLCHARACTERISTICS_HIGH_ENTROPY_VA flags set just as if I would be building a binary with -buildmode=pie (implemented in #35192).

What did you see instead?

...
OPTIONAL HEADER VALUES
...
            8100 DLL characteristics
                   NX compatible
                   Terminal Server Aware
...

Current implementation doesn't use ASLR nor High Entropy which causes go dll's to by flagged by cyber-security as insecure.

This is the follow up of golang-nuts/hVK37aucmco.
@alexbrainman

[gopherbot]

Change https://golang.org/cl/255259 mentions this issue: cmd/link: enable ASLR on windows binaries built with -buildmode=c-shared

[alexbrainman]

@qmuntal thank you for your report and CL.

But CL 255259 makes every DLL use ASLR. What about, if some DLLs needs to be compiled the old way without ASLR? How would the developers build DLL without ASLR?

We already had reports that some packages are broken when building executables with Go 1.15. These developers just use -buildmode=exe to force no ASLR build. So this problem is real. We need something similar for DLLs before we can proceed with any fix here.

Alex

/cc @zx2c4

[qmuntal]

What about, if some DLLs needs to be compiled the old way without ASLR? How would the developers build DLL without ASLR?

Good point @alexbrainman , enabling ASLR on pie binaries could be justified because there is the exe workaround, but mapping optional linker flags to -buildmode does not scale nor make much sense.

One option could be to define an env var, p.e. GOLINK, that allow developers to opt-in or opt-out linker features, in a similar manner we already do on GOARM and GOWASM. The GOLINK values would be a predefined list of well known features, such as aslr or dep, that each GOOS/GOARCH, or even linker if #20982 finally lands, would implement as needed. In case of opt-out features one could just negate the feature name: GOLINK=aslr,-dep.

[alexbrainman]

One option could be to define an env var, p.e. GOLINK, that allow developers to opt-in or opt-out linker features

I cannot make that decision. You need to take this proposal with Go Team.

Personally I think there are too many GO* variables already.

Alex

[qmuntal]

@alexbrainman I´ve updated the CL and removed the --export-all-symbols flag from -buildmode=c-shared as for DLLs relocs are not stripped thus there is no need for this hack.

With this change the only new flags added to -buildmode=c-shared are --dynamicbase and --high-entropy-va, which have been enabled by default on msvc since 2008, therefore I think it is safe to use them without having an opt-out mechanism, just as --nxcompat.

Also the issues I´ve seen related to go1.15 defaulting to -pie and ASLR seem to be related to being a PIE binary (#41329) or --export-all-symbols (#40795). Yet obviously the risk is there, so instead of defining a new GOLINK flag we could just gate aslr behind GODEBUG=aslroff=1.

[alexbrainman]

With this change the only new flags added to -buildmode=c-shared are --dynamicbase and --high-entropy-va, which have been enabled by default on msvc since 2008, therefore I think it is safe to use them without having an opt-out mechanism, just as --nxcompat.

What happened to msvc in 2008 is not relevant here, because Go users don't use msvc, they use gcc. And most of these are very very out of date.

Yet obviously the risk is there, so instead of defining a new GOLINK flag we could just gate aslr behind GODEBUG=aslroff=1.

Not my call, sorry. Go team need to decide that is what they want to do. Cherry can, probably, make that call.

Alex

[qmuntal]

Thanks for the feedback and the review Alex. Lets see if @rsc or @cherrymui can add something to this issue.

[ianlancetaylor]

Seems like we can already get a DSL with ASLR on Windows by running something like -buildmode=c-shared -extldflags=-Wl,--dynamicbase,--high-entropy-va. Or, if we make that the default, perhaps there is -extldflags option we can use to disable ALSR. Perhaps we just need to document that, rather than introduce a new build mode or a new environment variable.

[qmuntal]

Seems like we can already get a DSL with ASLR on Windows by running something like -buildmode=c-shared -extldflags=-Wl,--dynamicbase,--high-entropy-va

Good shoot, it does add ASLR and the required dllcharacteristics to the output DLL.

if we make that the default, perhaps there is -extldflags option we can use to disable ALSR. Perhaps we just need to document that, rather than introduce a new build mode or a new environment variable

Unfortunately mingw-w64 does not have an option to disable dynamicbase nor high-entropy-va. See Patch 1 of https://sourceware.org/bugzilla/show_bug.cgi?id=19011, it adds the necessary flags to disable them but it is still not merged.

[alexbrainman]

Good shoot, it does add ASLR and the required dllcharacteristics to the output DLL.

Option 1: We could just leave things as they are and update the documentation on how to build ASLR DLL. It is only that the flags are bit complicated. And we also want ASLR to be the default.

Unfortunately mingw-w64 does not have an option to disable dynamicbase nor high-entropy-va. See Patch 1 of https://sourceware.org/bugzilla/show_bug.cgi?id=19011, it adds the necessary flags to disable them but it is still not merged.

That is unfortunate indeed. It would be nice, if we could make ASLR the default, and document how to disable the ASLR.

rather than introduce a new build mode or a new environment variable.

I don't want to introduce new build mode or a new environment variable too. But that only leaves us with Option 1. But maybe option 1 is good enough.

Alex

[ianlancetaylor]

We could add a Go linker option that disables ASLR, by telling the linker to not add the options that are passed to the system linker. This would then be available using something like -buildmode=c-shared -ldflags=-no-aslr.

[qmuntal]

We could add a Go linker option that disables ASLR, by telling the linker to not add the options that are passed to the system linker. This would then be available using something like -buildmode=c-shared -ldflags=-no-aslr

Should -no-aslr flag also disable ASLR on -buildmode=pie? I see no reason why it shouldn't, but just double checking with you all.

[ianlancetaylor]

I don't know what -buildmode=pie means if we disable ASLR. Is that exactly the same as -buildmode=exe? If so I would be inclined to make it an error. For a tool it seems confusing to have two different ways to do exactly the same thing.

[alexbrainman]

I don't know what -buildmode=pie means if we disable ASLR. Is that exactly the same as -buildmode=exe? If so I would be inclined to make it an error. For a tool it seems confusing to have two different ways to do exactly the same thing.

I agree with Ian. Lets only allow -no-aslr flag together with -buildmode=c-shared. And make it error with any other buildmode value. Hopefully no one needs to use -no-aslr flag - -no-aslr flag should be an exception.

Alex

[qmuntal]

I'll use the positive flag -aslr instead of -no-aslr as per @cherrymui comment in Gerrit if this is also ok to you.

[alexbrainman]

I'll use the positive flag -aslr instead of -no-aslr as per @cherrymui comment in Gerrit if this is also ok to you.

-aslr is fine with me too. As long as you set its default value correctly.

Thank you.

Alex