FS#53087 - [go] Change default buildmode to pie

Attached to Project: Arch Linux
Opened by Bartłomiej Piotrowski (Barthalion) - Sunday, 26 February 2017, 10:53 GMT
Last edited by Bartłomiej Piotrowski (Barthalion) - Tuesday, 14 March 2017, 17:55 GMT
Task Type Feature Request
Category Packages: Extra
Status Closed
Assigned To Bartłomiej Piotrowski (Barthalion)
Architecture All
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 1
Private No

Details

Go by default doesn't use PIE for amd64/i686 executables. This can be specified explicitly with -buildmode or changed globally with simple patch[1].

The advantages are obvious; the disadvantage is that debugging code might be harder, unless -buildmode=default is specified. We have many Go projects packaged though, so it would be beneficial to just have PIE by default.

[1] http://git.alpinelinux.org/cgit/aports/tree/community/go/default-buildmode-pie.patch
This task depends upon

Closed by  Bartłomiej Piotrowski (Barthalion)
Tuesday, 14 March 2017, 17:55 GMT
Reason for closing:  Deferred
Comment by Alexander F. Rødseth (xyproto) - Monday, 27 February 2017, 10:42 GMT
Thanks for reporting. I see that Fedora and Debian have implemented similar changes.

While I think it's a bit un-Arch-like to patch the executable instead of pestering upstream, I think it's a good idea, security-wise.

I will add the proposed patch to the package.
Comment by Alexander F. Rødseth (xyproto) - Wednesday, 01 March 2017, 14:48 GMT
When applying the patch, TestDisasmExtld and several other tests fail.

The Debian patch for this does not involve patching Go itself, but rather passing different arguments to the Go executable. I think this is the way to go (no pun intended).
Comment by Levente Polyak (anthraxx) - Wednesday, 01 March 2017, 15:47 GMT
the difference is that both have macro constructs to pass parameters to go in a uniform way without the need to pass them explicitly for every single project that uses go.
As there is no way for us to do this in a uniform way (like CFLAGS) sane defaults are the better way to choose. The problem with the tests should be fixed and maybe upstream contacted, so IMO the way to go in our environment is to get those flags as defaults.
Comment by Alexander F. Rødseth (xyproto) - Thursday, 02 March 2017, 10:03 GMT
I think upstream should fix this, either by making position independent executables the default on Linux, or by reading an environment variable (like CFLAGS) that we can set globally, as a sane default.
Comment by Bartłomiej Piotrowski (Barthalion) - Thursday, 02 March 2017, 10:10 GMT
One thing is what upstream should do, the other what we, as downstream, are supposed to proactively care about – security what is in the repositories. This is hardly a major patch and it's already used in different distributions.
Comment by Alexander F. Rødseth (xyproto) - Thursday, 02 March 2017, 10:14 GMT
The other distributions (like the Alpine link you posted) does not apply the patch to Go 1.8, so the situation is not comparable.

Until upstream makes it easier to generate position independent executables by default, it's fully possible for packages that depend on Go to supply the correct flags in the build process.
Comment by Levente Polyak (anthraxx) - Thursday, 02 March 2017, 19:01 GMT
you can't be serious to expect that we change all go packages to add special cli commands rather then switch to it in a default way and deactivate where it doesn't work.
Comment by Daniel M. Capella (polyzen) - Thursday, 02 March 2017, 19:57 GMT
Go packaging is already tricky as it is: another vote to patch.
Comment by Bartłomiej Piotrowski (Barthalion) - Thursday, 02 March 2017, 20:04 GMT
Alpine does not ship 1.8, so obviously they can't apply it for unreleased package.

I don't understand why anything about the go package has to be so troublesome. We're not some YOLO GNU/Linux kind of distribution, let's take security seriously.
Comment by Alexander F. Rødseth (xyproto) - Thursday, 02 March 2017, 20:09 GMT
If there is a working patch for 1.8 and someone that is willing to maintain the patch, I will apply it.

Barthalion, after making a lot of fuzz over a non-issue in the previous .install file for go and being unwilling to file a bug report for it, I think especially you should try to stay on topic here.
Comment by Bartłomiej Piotrowski (Barthalion) - Thursday, 02 March 2017, 20:42 GMT
Given how hard you try to make me lose my temper, instead of maintaining a patch for package I do not maintain currently, I will just move it to [extra] as I said previously on IRC. Closing.
Comment by Alexander F. Rødseth (xyproto) - Thursday, 02 March 2017, 21:57 GMT
Your temper is your own, keep it to yourself.

I think there might be better solutions for making go create position independent executables than taking the role of upstream and patching the Go sources. It's not the Arch way. This should be fixed by upstream.

While waiting for the upstream changes (if they ever happen), I think creating a wrapper script, or even modifying how go-related packages are built, are better temporary solutions.
Comment by Alexander F. Rødseth (xyproto) - Thursday, 02 March 2017, 22:22 GMT
Will look into the feasability of creating a wrapper script for passing -buildmode=pie to go by default.
Comment by Daniel M. Capella (polyzen) - Thursday, 02 March 2017, 22:40 GMT
If that patch is all that's needed, it seems more feasible to make an exception and use it.
Comment by Alexander F. Rødseth (xyproto) - Thursday, 02 March 2017, 22:56 GMT
Daniel, that patch breaks the self-testing of Go when building the package. Why do you think patching the Go sources is better than all other possible solutions?
Comment by Daniel M. Capella (polyzen) - Friday, 03 March 2017, 00:19 GMT
Applying a small patch to one package seems simpler, of course, than making a wrapper and requiring all these pkgbuilds be modified to use it. Sorry, the test breakage had slipped my mind.

That being said, would it make sense to do this with the hardening-wrapper?
Comment by Pierre Neidhardt (Ambrevar) - Friday, 03 March 2017, 09:43 GMT
I don't think so. This was discussed extensively here:

https://groups.google.com/forum/#!msg/golang-nuts/Jd9tlNc6jUE/Z9ldF6vPEAAJ;context-place=forum/golang-nuts

It's a a rather long discussion... See the beginning and the end at least.
Comment by Pierre Neidhardt (Ambrevar) - Friday, 03 March 2017, 09:44 GMT
@Daniel: Why is go packaging tricky?
Comment by Alexander F. Rødseth (xyproto) - Friday, 03 March 2017, 11:07 GMT
Created a feature request for hardening-wrapper: https://github.com/thestinger/hardening-wrapper/issues/9
Comment by Alexander F. Rødseth (xyproto) - Thursday, 09 March 2017, 11:59 GMT
Other people also have issues applying the pie patch to Go 1.8: https://github.com/golang/go/issues/19132 (as opposed to Go 1.7, like in Alpine).

Seems to me like upstream is planning to solve this for Go 1.8.1, since the issue is added to the "Go1.8.1 milestone".

Will wait for upstream to release Go 1.8.1.
Comment by Bartłomiej Piotrowski (Barthalion) - Tuesday, 14 March 2017, 17:55 GMT
I took the liberty to remove the childish comment posted here in the morning. Because any discussion on this is going nowhere and I already moved the package to [extra] where I will maintain it, I am closing the issue as well.

I will apply buildmode=pie patch once I rework PKGBUILD so the package builds reliably.

Loading...