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/ld: add PAX header #47

Closed
gopherbot opened this issue Nov 11, 2009 · 37 comments
Closed

cmd/ld: add PAX header #47

gopherbot opened this issue Nov 11, 2009 · 37 comments
Assignees

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 11, 2009

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
Copy link
Contributor

@agl agl commented Nov 11, 2009

Comment 1:

Owner changed to a...@golang.org.

Status changed to Accepted.

@agl
Copy link
Contributor

@agl agl commented Nov 11, 2009

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
Copy link
Contributor

@agl agl commented Nov 11, 2009

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
Copy link
Author

@gopherbot gopherbot commented Nov 11, 2009

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
Copy link
Contributor

@agl agl commented Nov 11, 2009

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
Copy link
Contributor

@agl agl commented Nov 13, 2009

Comment 6:

Status changed to Thinking.

@dhobsd
Copy link
Contributor

@dhobsd dhobsd commented Nov 18, 2009

Comment 7:

I think this may end up being an issue on OpenBSD as well.
@dhobsd
Copy link
Contributor

@dhobsd dhobsd commented Nov 18, 2009

Comment 8:

Actually, there, we can use mprotect
@rsc
Copy link
Contributor

@rsc rsc commented Apr 27, 2010

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 r...@golang.org.

Status changed to WorkingAsIntended.

@adg
Copy link
Contributor

@adg adg commented Apr 25, 2011

Comment 10:

Issue #1669 has been merged into this issue.

@gopherbot
Copy link
Author

@gopherbot gopherbot commented Feb 16, 2012

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
Copy link
Author

@gopherbot gopherbot commented May 11, 2012

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
Copy link
Author

@gopherbot gopherbot commented May 14, 2012

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
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 14, 2012

Comment 14:

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

@gopherbot gopherbot commented May 14, 2012

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
Copy link
Contributor

@rsc rsc commented May 17, 2012

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
Copy link
Author

@gopherbot gopherbot commented May 17, 2012

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
Copy link
Contributor

@rsc rsc commented May 17, 2012

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
Copy link
Author

@gopherbot gopherbot commented May 17, 2012

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 pageexec@freemail.hu
@gopherbot
Copy link
Author

@gopherbot gopherbot commented May 17, 2012

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
Copy link
Author

@gopherbot gopherbot commented May 17, 2012

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
Copy link
Contributor

@rsc rsc commented May 17, 2012

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
Copy link
Author

@gopherbot gopherbot commented May 17, 2012

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
Copy link
Contributor

@rsc rsc commented May 17, 2012

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
Copy link
Author

@gopherbot gopherbot commented May 17, 2012

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 <pageexec@freemail.hu>
- 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 <pageexec@freemail.hu>
- 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
Copy link
Author

@gopherbot gopherbot commented May 19, 2012

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
Copy link
Contributor

@rsc rsc commented May 21, 2012

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
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 21, 2012

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
Copy link
Author

@gopherbot gopherbot commented Jun 1, 2012

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
Copy link
Contributor

@rsc rsc commented Jun 26, 2012

Comment 30:

Status changed to Accepted.

@rsc
Copy link
Contributor

@rsc rsc commented Jun 26, 2012

Comment 31:

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

@gopherbot gopherbot commented Jul 30, 2012

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
Copy link
Author

@gopherbot gopherbot commented Jul 31, 2012

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
Copy link
Contributor

@rsc rsc commented Aug 3, 2012

Comment 34:

This issue was closed by revision 3f34248.

Status changed to Fixed.

@minux
Copy link
Member

@minux minux commented Aug 18, 2012

Comment 35:

Issue #3329 has been merged into this issue.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Apr 5, 2013

Comment 36:

Issue #5223 has been merged into this issue.

@davecheney
Copy link
Contributor

@davecheney davecheney commented Apr 5, 2013

Comment 37:

Issue #5223 has been merged into this issue.

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants
You can’t perform that action at this time.