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: default to -buildmode=pie on Windows #35192

Closed
zx2c4 opened this issue Oct 27, 2019 · 22 comments
Closed

cmd/go: default to -buildmode=pie on Windows #35192

zx2c4 opened this issue Oct 27, 2019 · 22 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Milestone

Comments

@zx2c4
Copy link
Contributor

zx2c4 commented Oct 27, 2019

It's 2019. ASLR has been well supported in Windows for a while. We should enable this by default or all the time and address any weird Go bugs that come up as a result. So far I haven't seen any.

ARM on Windows has had ASLR since its inception, so I imagine things are probably fine.

Personally, I'd prefer only allowing ASLR and not having the option in there to avoid it. I think that'd make things a bit simpler and less confusing too, a la v1 of the aslr cl: https://go.googlesource.com/go/+/c04035be04c3a0ef0e44c1e3674485048df5d327%5E%21/

CC @alexbrainman @ianlancetaylor @bradfitz @FiloSottile

@gopherbot gopherbot added this to the Proposal milestone Oct 27, 2019
@zx2c4 zx2c4 changed the title proposal: cmd/link: enable Windows ASLR by default cmd/link: enable Windows ASLR by default Oct 27, 2019
@zx2c4 zx2c4 changed the title cmd/link: enable Windows ASLR by default cmd/link: enable Windows ASLR by default or all the time Oct 27, 2019
@gopherbot
Copy link

Change https://golang.org/cl/203606 mentions this issue: cmd/link: enable ASLR by default on Windows

@alexbrainman
Copy link
Member

I would also like to add that this proposal assumes that #27144 is implemented as -buildmode=pie. So this current proposal is effectively means that -buildmode=pie will become default on Windows (as suggested by @ianlancetaylor at #27144 (comment)).

Alex

@FiloSottile
Copy link
Contributor

I am not convinced by the idea of overloading -buildmode with fundamental security features. Compiling with -buildmode=exe is currently the default, so it's reasonable to expect it to be a no-op, not a security downgrade.

I think we should just turn on ASLR everywhere, becoming consistent with arm. Losing the distinction between -buildmode=exe and -buildmode=pie sounds much more natural than making -buildmode=exe a security switch: features move into default modes making explicit switches no-ops all the time.

If we are concerned about encountering bugs, I think we should go about it as we go about everything else: testing during the freeze, and making a GODEBUG (or equivalent) opt-out (or temporary opt-in). Likewise, if we need to make it possible to disable ASLR to debug, that sounds like it should be a scary, debug-branded ldflag, not -buildmode=exe. How necessary is it, though?

@zx2c4
Copy link
Contributor Author

zx2c4 commented Oct 28, 2019

I am not convinced by the idea of overloading -buildmode with fundamental security features. Compiling with -buildmode=exe is currently the default, so it's reasonable to expect it to be a no-op, not a security downgrade.

I think we should just turn on ASLR everywhere, becoming consistent with arm. Losing the distinction between -buildmode=exe and -buildmode=pie sounds much more natural than making -buildmode=exe a security switch: features move into default modes making explicit switches no-ops all the time.

Fully agree.

If we are concerned about encountering bugs, I think we should go about it as we go about everything else: testing during the freeze, and making a GODEBUG (or equivalent) opt-out (or temporary opt-in). Likewise, if we need to make it possible to disable ASLR to debug, that sounds like it should be a scary, debug-branded ldflag, not -buildmode=exe. How necessary is it, though?

All the debuggers I use have no problem dynamically rebasing. Otherwise, how would I possibly debug any real world applications or reverse engineer stuff?

@ianlancetaylor
Copy link
Contributor

Perhaps we should indeed make -buildmode=exe be a synonym for -buildmode=pie on Windows, but we still must have a way to generate a non-PIE binary. I have seen far too many cases where non-PIE executables are required, albeit on non-Windows systems. How should people select a non-PIE binary? This should not be a scary, debug-branded, flag. There are legitimate choices here. The added security of ASLR, which in my personal opinion is quite small for a language like Go, can be reasonably traded off against the costs of ASLR.

@aarzilli
Copy link
Contributor

All the debuggers I use have no problem dynamically rebasing. Otherwise, how would I possibly debug any real world applications or reverse engineer stuff?

I guess I should point out that debug_frame is broken on macOS with buildmode=pie. I haven't checked the state of debug symbols on windows with buildmode=pie.

@zx2c4
Copy link
Contributor Author

zx2c4 commented Oct 28, 2019

I have seen far too many cases where non-PIE executables are required, albeit on non-Windows systems.

Can you point out a Windows case where non-PIE executables are required? Seems like we should base this on real actual cases. With auditors mechanically searching for them and flagging them in reports, they have been moving toward extinction for a long time now.

@ianlancetaylor
Copy link
Contributor

Let's please take the conservative approach here. We know how to generate a non-PIE. If we change the default to be PIE, I see no reason at all to make it hard to generate a non-PIE.

@alexbrainman
Copy link
Member

I am not convinced by the idea of overloading -buildmode with fundamental security features.

I am not married to making -buildmode=pie default idea. If you have better suggestion I am fine with that. -buildmode=pie will not require us to add any new flags.

I think we should just turn on ASLR everywhere, becoming consistent with arm.

I doubt we have many windows-arm Go users. We don't even have windows-arm builder. So we should treat ASLR as completely new feature.

I haven't checked the state of debug symbols on windows with buildmode=pie.

I agree, we should make sure Delve works with ASLR enabled windows binaries. We might discover that "build non ASLR executable" flag will get handy with Delve. Thank you @aarzilli for reminding us about Delve.

Let's please take the conservative approach here. We know how to generate a non-PIE. If we change the default to be PIE, I see no reason at all to make it hard to generate a non-PIE.

I completely agree.

Alex

@aarzilli
Copy link
Contributor

Out of curiosity, when was buildmode=pie support added to windows? It wasn't in Go 1.11 because I remember I couldn't test Delve with it but I can't find mention of it in either 1.12 or 1.13 release notes.

@ianlancetaylor
Copy link
Contributor

@aarzilli As far as I know -buildmode=pie does not yet work on Windows. This discussion seems to be about adding it.

@aarzilli
Copy link
Contributor

In that case I don't understand how https://go-review.googlesource.com/c/go/+/203606 is passing trybots.

@ianlancetaylor
Copy link
Contributor

Those CLs are changes to the linker to make it generate PIE. They don't add support for go - buildmode=pie. They also don't change cmd/dist to test PIE.

@rsc
Copy link
Contributor

rsc commented Oct 30, 2019

On various Android systems we already set the default buildmode to pie in cmd/go.
Setting the default to pie on Windows too would be consistent with that.

@FiloSottile, I hear what you are saying, but this is an implementation mechanism and a security detail. I would argue it is not primarily about security. The bug says ASLR but really this is just about making a position-independent binary. There are other reasons to have one of those besides ASLR.

Does anyone object to changing the default buildmode for Windows to pie in Go 1.15?
(It seems too late in the cycle for Go 1.14.)

@rsc rsc changed the title cmd/link: enable Windows ASLR by default or all the time proposal: cmd/link: enable Windows ASLR by default or all the time Oct 30, 2019
@networkimprov
Copy link

networkimprov commented Oct 30, 2019

Hasn't this been working for a while? I think 1.14 is a reasonable target, esp as it's a security improvement. There's the whole freeze period to turn up any problems.

EDIT: sorry, got this mixed up with #27144

@alexbrainman
Copy link
Member

Does anyone object to changing the default buildmode for Windows to pie in Go 1.15?
(It seems too late in the cycle for Go 1.14.)

How about we will make -buildmode=pie work on Windows (issue #27144) for Go 1.14? Then in Go 1.14 release notes we will announce the change and declare that -buildmode=pie will become default in Go 1.15. Gives everyone plenty of time to prepare for the change.

If this suggestion is accepted, I ask for dispensation to implement #27144 after Go 1.14 freeze. It should not be hard to implement, but there are only few days left.

Alex

@ianlancetaylor
Copy link
Contributor

I think that's OK although it needs to be complete by the week after the freeze.

@rsc rsc changed the title proposal: cmd/link: enable Windows ASLR by default or all the time proposal: cmd/go: default to -buildmode=pie on Windows Nov 6, 2019
@rsc
Copy link
Contributor

rsc commented Nov 6, 2019

It looks very much like -buildmode=pie will not be working, reviewed, and submitted by Friday, so we should hold off on implementation until next cycle. Sorry, but we are being extra strict about the freeze to try to make sure we spend November getting the beta ready, not doing more development.

Given that it won't be in Go 1.14, assuming it is ready for the start of the Go 1.15 cycle, it seems fine, as I said last week, to implement -buildmode=pie and make that the default, in the same cycle.

No one has objected to changing the default (once the code works), so this proposal seems like a likely accept.

Leaving open for a week for final comments.

@networkimprov
Copy link

networkimprov commented Nov 6, 2019

Relevant CLs: https://golang.org/cl/203602 & https://golang.org/cl/203606

EDIT: Alas, the first is unfinished and the second turned out to be wrong :-(

@rsc
Copy link
Contributor

rsc commented Nov 13, 2019

Accepted.

@rsc rsc removed this from the Proposal milestone Nov 13, 2019
@rsc rsc added this to the Go1.15 milestone Nov 13, 2019
@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 13, 2019
@rsc rsc added this to Final Comment in Proposals (old) Nov 27, 2019
@rsc rsc changed the title proposal: cmd/go: default to -buildmode=pie on Windows cmd/go: default to -buildmode=pie on Windows Jan 8, 2020
@gopherbot
Copy link

Change https://golang.org/cl/214397 mentions this issue: cmd/go, cmd/link: implement -buildmode=pie on windows

gopherbot pushed a commit that referenced this issue Mar 1, 2020
This CL implements windows version of -buildmode=pie code in both
cmd/go and cmd/link.

Windows executables built with -buildmode=pie set (unlike the one
built with -buildmode=exe) will have extra .reloc PE section, and
will have no IMAGE_FILE_RELOCS_STRIPPED flag set. They will also
have IMAGE_DLLCHARACTERISTICS_DYNAMIC_BASE flag set, and
IMAGE_DLLCHARACTERISTICS_HIGH_ENTROPY_VA flag set for windows/amd64.

Both cgo and non-cgo versions are implemented. And TestBuildmodePIE
is extended to test both cgo and non-cgo versions on windows and
linux.

This CL used some code from CLs 152759 and 203602.

RELNOTE=yes

Fixes #27144
Updates #35192

Change-Id: I1249e4ffbd79bd4277efefb56db321c390c0f76f
Reviewed-on: https://go-review.googlesource.com/c/go/+/214397
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/230217 mentions this issue: cmd/go: use -buildmode=pie as default on window

xujianhai666 pushed a commit to xujianhai666/go-1 that referenced this issue May 21, 2020
This change adjusts go command to pass -buildmode=pie to cmd/link,
if -buildmode is not explicitly provided.

Fixes golang#35192

Change-Id: Iec020131e676eb3e9a2df9eea1929b2af2b6df04
Reviewed-on: https://go-review.googlesource.com/c/go/+/230217
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
sergystepanov added a commit to giongto35/cloud-game that referenced this issue Jan 29, 2021
Since Go version 1.15 ASLR security mode is enabled by default for Windows builds. In order to make it work without random NPE crashes in the cores (as example in PSX games during load), you have to (re)compile every core with ASLR support or just disable this mode.

See: golang/go#35192
sergystepanov added a commit to giongto35/cloud-game that referenced this issue Jan 29, 2021
Since Go version 1.15 ASLR security mode is enabled by default for Windows builds. In order to make it work without random NPE crashes in the cores (as example in PSX games during load), you have to (re)compile every core with ASLR support or just disable this mode.

See: golang/go#35192
sergystepanov added a commit to giongto35/cloud-game that referenced this issue Jan 29, 2021
Since Go version 1.15 ASLR security mode is enabled by default for Windows builds. In order to make it work without random NPE crashes in the cores (as example in PSX games during load), you have to (re)compile every core with ASLR support or just disable this mode.

See: golang/go#35192
sergystepanov added a commit to giongto35/cloud-game that referenced this issue Apr 6, 2021
Since Go version 1.15 ASLR security mode is enabled by default for Windows builds. In order to make it work without random NPE crashes in the cores (as example in PSX games during load), you have to (re)compile every core with ASLR support or just disable this mode.

See: golang/go#35192
@golang golang locked and limited conversation to collaborators Apr 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Projects
None yet
Development

No branches or pull requests

8 participants