cmd/ld: add PAX header

[gopherbot]

by jameshubbard:

What steps will reproduce the problem?
1.Run all.bash on an SELinux enabled Fedora (using F12 Rawhide)
2. Fails during build process. 
3.

What is the expected output? What do you see instead?
hello, world

Error message received
/hello: error while loading shared libraries:
/home/jhubbard/go/pkg/linux_386/libcgo.so: cannot enable executable stack
as shared object requires: Permission denied

I changed the context for the hello world program and it ran. 

What is your $GOOS?  $GOARCH?
linux 
386

Which revision are you sync'ed to?  (hg log -l 1)

changeset:   3975:b51fd2d6c160

Please provide any additional information below.

[agl]

Comment 1:

Owner changed to [email protected].

Status changed to Accepted.

[agl]

Comment 2:

See http://code.google.com/p/go/source/detail?r=6bfe1e03b36c8284bcf471f4b5ec3a4a33703e50
The magic command is # setsebool -P allow_execstack 1
make.bash: detect and warn about SELinux policy that crashes us.
The default SELinux policy on Fedora 12 (at least) disallows stack
pages to be +x. This causes all binaries written by 6g/6l to segfault
immedately. The 'true' way to fix this issue is to mark binaries with
the correct type. However, that assumes that 6l is going to detect
SELinux, figure out the correct type for the current distribution and
set the type (without libselinux).
For now we'll warn users and point them towards the way to enable
execstack for the whole system.
https://golang.org/issue/47
R=rsc
CC=golang-dev
http://codereview.prom.corp.google.com/1026041

[agl]

Comment 3:

Additional tasks for this bug:
  1) Maybe figure out if 6l can set the attributes needed to allow w+x for output
binaries.
  2) Detect the mmap failure in the generated code.

[gopherbot]

Comment 4 by jameshubbard:

Enabling an executable stack is the wrong way to fix the issue.  It's going to be a
problem for almost all selinux enabled distros.  I'm guessing that it will fail RHEL
5.x as well.
Here's a discussion of the general problem.
http://people.redhat.com/drepper/selinux-mem.html

[agl]

Comment 5:

Thanks for the pointer. I know it's not ideal.
Drepper is suggesting creating an anonymous file and mapping it twice: once W and once 
X. We're not going to do that.
The best answer may be a policy change to make it so that the output files of 6l are 
correctly typed so that they can execmem.

[agl]

Comment 6:

Status changed to Thinking.

[dhobsd]

Comment 7:

I think this may end up being an issue on OpenBSD as well.

[dhobsd]

Comment 8:

Actually, there, we can use mprotect

[rsc]

Comment 9:

The gc implementation is going to continue to depend on W+X memory.
Gccgo does too, though work on gcc may provide an
answer later.
The so-called security protections here are trying to
guard against badly written C programs.  There's no
good reason to force these restrictions onto a safe
language like Go.
If someone wants to do the work to make 6l generate
binaries that are blessed enough to do W+X mappings,
patches welcome.  But I think this is a bug in SELinux
and its ilk more than in Go.

Owner changed to [email protected].

Status changed to WorkingAsIntended.

[adg]

Comment 10:

Issue #1669 has been merged into this issue.

[gopherbot]

Comment 11 by powerman.asdf:

This patch needed to compile and run go on amd64 Hardened Gentoo Linux. Probably it
should be modified to work both on amd64 and x86.

Attachments:

1. go-hardened.patch (777 bytes)

[gopherbot]

Comment 12 by powerman.asdf:

Updated patch to fix broken Go compile on Hardened Gentoo (actually, on kernel with PaX
patch) and force Go linker to generate paxmarked binaries. Support both x86 and amd64.

Attachments:

1. go-hardened.patch (1194 bytes)

[gopherbot]

Comment 13 by w.d.hubbs:

@RSC:
I am the developer for gentoo linux who is working to get the go language into our
distribution.
I could condissionally apply the patch in comment 12. But, I would like your opinion as
well. Is something like this a safe temporary fix until 6l/8l are fixed to write the
binaries correctly?
Thanks much,
William

[ianlancetaylor]

Comment 14:

That patch seems to be more or less what 8l and 6l would do anyhow.  It looks as safe as
anything.

[gopherbot]

Comment 15 by w.d.hubbs:

I am considering extending it so that it would be activated from the command line of
make.bash as follows:
make.bash --pax-kernel
or with a similar switch.
That way it could always be applied instead of being condissionally applied.
If we do that, is there a chance this would be accepted into the repository?

[rsc]

Comment 16:

I'd rather not make wrappers for 6l and 8l.  What do they need to do
differently?
Failing that, is there some way for cmd/go to understand that it needs
to run paxctl and then work that into the build after 6l or 8l is run?
Why can paxctl do something 6l/8l cannot?
Russ

[gopherbot]

Comment 17 by powerman.asdf:

Some ways to detect PaX kernel are listed at
https://bugs.gentoo.org/show_bug.cgi?id=342505#c67
Other ways may be as simple as checking for paxctl existence on target system.
Also, in this case make sure paxctl also will be used while building go itself.

[rsc]

Comment 18:

Reading /proc/self/status for PaX seems like a decent signal.
It's fine to invoke paxctl in cmd/go after the ordinary linker.
What does paxctl do that 6l or 8l cannot?
Russ

[gopherbot]

Comment 19 by powerman.asdf:

> What does paxctl do that 6l or 8l cannot?
I think go linkers may do the same paxctl does, but in case there are subtle issues
you'd better ask PaX developer [email protected]

[gopherbot]

Comment 20 by w.d.hubbs:

Hi Russ,
In the wrappers, we use the -c and -m flags for paxctl.
According to the man page, these do the following:
-c creates the PT_PAX_FLAGS program header if it does not exist by converting the
PT_GNU_STACK program header if it exists.
-m does not enforce secure memory protections (NOMPROTECT)
In otherwords, I think you would want to add a PT_PAX_FLAGS header to the binaries with
the NOMPROTECT setting.
I'm not an expert on how to do this sort of thing, but I don't know why 6l/8l (and maybe
even the arm linker) couldn't do this.
William

[gopherbot]

Comment 21 by w.d.hubbs:

Hi Russ,
Comment 5 seems to be refering to the same thing; changing the linkers to output
binaries with the correct flags to allow them to execmem.
William

[rsc]

Comment 22:

It looks possible that we could add a PT_PAX_FLAGS program header
(instead of replacing PT_GNU_STACK) and end up with binaries that work
on both kinds of Linux systems.  Of course it is also possible that
the non-PaX Linux systems will reject such binaries.

[gopherbot]

Comment 23 by powerman.asdf:

> Of course it is also possible that the non-PaX Linux systems will reject such binaries.
I don't think so. Paxmarked binaries works just fine with vanilla kernels.
Also, paxctl can both convert PT_GNU_STACK into PT_PAX_FLAGS (with -c option)
and just add PT_PAX_FLAGS (with -C option).
In practice, only real difference between these was for skype binary (which has
it's own protection against unauthorized modification of skype binary) - using
paxctl -c will break skype protection, while using paxctl -C won't break it and so
it's only way to paxmark skype.

[rsc]

Comment 24:

Can you compile a simple hello world C program, mark it with
appropriate pax markings, and attach it to this issue?  Thanks.

[gopherbot]

Comment 25 by powerman.asdf:

powerman@home ~ $ paxctl -v hello
PaX control v0.7
Copyright 2004,2005,2006,2007,2009,2010,2011,2012 PaX Team <[email protected]>
- PaX flags: -------x-e-- [hello]
        RANDEXEC is disabled
        EMUTRAMP is disabled
powerman@home ~ $ paxctl -v hello_paxmarked 
PaX control v0.7
Copyright 2004,2005,2006,2007,2009,2010,2011,2012 PaX Team <[email protected]>
- PaX flags: -----m-x-e-- [hello_paxmarked]
        MPROTECT is disabled
        RANDEXEC is disabled
        EMUTRAMP is disabled

Attachments:

1. hello (7833 bytes)
2. hello_paxmarked (7833 bytes)

[gopherbot]

Comment 26 by anthony.g.basile:

I maintain the hardened (ie pax) kernel for gentoo.  Some observations
1) applying pax markings base on whether the *current* running kernel is pax enabled or
not is a bad idea.  Consider: we build under a vanilla kernel and don't pax mark, we
boot into a pax kernel and we get breakage.
2a) i don't want to get into a full out debate, but i can't see how a language that
needs an rwx mmap can be safe.  you have to write to an area of memory presumeably
because of some conditions external to the program, like user input.  you then turn
around and execute that.  how is this not a vector for exploit?
2b) i have written a utility to drop the x from an executable stack, but my guess is
that it will just break stuff, given that wx memory is consciously part of go's design.
3) anytime you try to mangle an elf as paxctl -c does, you can expect breakage.  on
gentoo, we patch binutils so all our binaries have a PT_PAX program header.  so the only
things that stick out are "foreign" elfs like skype (which is also self checking etc etc
making paxctl -c a really bad idea there).  however, i think if you start doing paxctl
-cm on elfs where you are not certain of the internals, you need to test/convince
yourself that you are not introducing some badness.  converting PT_GNU_STACK to PT_PAX
may not be harmless.  i'd feel a lot better if you could get away with just paxctl -m.
i don't use go and i'm not familiar with it, so i really can't say where to go from here.

[rsc]

Comment 27:

Thank you for your note on this bug.
I would be happy to add the PT_PAX program header to all our Linux ELF
binaries.  I just need to know what the format is so that the linker
can write it out itself, instead of depending on the presence of the
paxctl binary.  Can you please point me at the spec for what a PT_PAX
program header should contain?
Thanks again.
Russ

[ianlancetaylor]

Comment 28:

The format is straightforward.  It's a program segment with type
PT_PAX_FLAGS == 0x65041580.  The p_flags field is set to a bitmask of:
#define PF_PAGEEXEC     (1U << 4)       /* Enable  PAGEEXEC */
#define PF_NOPAGEEXEC   (1U << 5)       /* Disable PAGEEXEC */
#define PF_SEGMEXEC     (1U << 6)       /* Enable  SEGMEXEC */
#define PF_NOSEGMEXEC   (1U << 7)       /* Disable SEGMEXEC */
#define PF_MPROTECT     (1U << 8)       /* Enable  MPROTECT */
#define PF_NOMPROTECT   (1U << 9)       /* Disable MPROTECT */
#define PF_RANDEXEC     (1U << 10)      /* Enable  RANDEXEC */
#define PF_NORANDEXEC   (1U << 11)      /* Disable RANDEXEC */
#define PF_EMUTRAMP     (1U << 12)      /* Enable  EMUTRAMP */
#define PF_NOEMUTRAMP   (1U << 13)      /* Disable EMUTRAMP */
#define PF_RANDMMAP     (1U << 14)      /* Enable  RANDMMAP */
#define PF_NORANDMMAP   (1U << 15)      /* Disable RANDMMAP */
The PF_X, PF_W, and PF_R fields are for the stack.
For Go we will want to set PF_EMUTRAMP.  I'm not sure what else what we
need.

[gopherbot]

Comment 29 by anthony.g.basile:

here's a link to a patch against binutils-2.19.  its a bit dated, but it answers the
question of what a PT_PAX phdr should contain:
http://pax.grsecurity.net/binutils-2.19-pt-pax-flags-200811041810.patch

[rsc]

Comment 30:

Status changed to Accepted.

[rsc]

Comment 31:

Can you please try http://golang.org/cl/6326054 ?
It is my attempt to add the PAX headers.

[gopherbot]

Comment 32 by w.d.hubbs:

Hi Russ,
I have applied your patch to go-1.0.2 in the gentoo distribution. I can verify via
readelf -l that the generated binaries contain the PaX headers.
Also, they run fine with a normal kernel (I do not run PaX).
Now we need to make sure that things work correctly under a PaX kernel.
Anthony,
can you add your observations wrt our version of go-1.0.2 in gentoo since it now has
this patch applied?
Thanks,
William

[gopherbot]

Comment 33 by anthony.g.basile:

Hi Russ, William.  I just tested that patch and its good.  elf's built with go behave as
expected under a pax enabled kernel.  So go ahead an push it out!

[rsc]

Comment 34:

This issue was closed by revision 3f34248.

Status changed to Fixed.

[minux]

Comment 35:

Issue #3329 has been merged into this issue.

[bradfitz]

Comment 36:

Issue #5223 has been merged into this issue.

[davecheney]

Comment 37:

Issue #5223 has been merged into this issue.