Bug #63801 for PAR-Packer: setuid pp'ed scripts: 1st invocation fails, 2nd+ call ok

Fri Dec 10 13:44:41 2010 bitcard [...] post2.25u.com - Ticket created

Subject:

setuid pp'ed scripts: 1st invocation fails, 2nd+ call ok

Dear maintainers, setuid pp'ed binaries fail upon first invocation. Here's how to reproduce: /tmp$ cat > suidtest.pl #!/usr/bin/perl print "hello, world, this is $< running as $>!\n"; /tmp$ pp -o suidtest suidtest.pl /tmp$ sudo chown root:root suidtest /tmp$ sudo chmod 6755 suidtest /tmp$ sudo mv suidtest / /tmp$ ls -l /suidtest -rwsr-sr-x 1 root root 3792430 Dec 10 19:23 /suidtest /tmp$ /suidtest Insecure dependency in utime while running setuid at /home/userid/.lib/perl-5.12.2/lib/site_perl/5.12.2/Archive/Zip/Directory Member.pm line 63. eagle323:/tmp$ /suidtest hello, world, this is 32288 running as 0! Depending on what other files are included in the pp'ed archive, the failure appears in different locations. Could you please have a look at this?

Sun Dec 19 11:16:26 2010 RSCHUPP [...] cpan.org - Correspondence added

On 2010-12-10 13:44:41, arost wrote: Show quoted text

> /tmp$ sudo chown root:root suidtest > /tmp$ sudo chmod 6755 suidtest

As I already wrote in #62552, packed executables are not meant to be run setuid and - as far as I'm concerned - never will be. So don't do that then. Cheers, Roderich

Sun Dec 19 11:16:27 2010 The RT System itself - Status changed from 'new' to 'open'

Sun Dec 19 11:18:11 2010 RSCHUPP [...] cpan.org - Severity Important changed to Wishlist

Mon Dec 20 06:28:54 2010 bitcard [...] post2.25u.com - Correspondence added

From:

bitcard [...] post2.25u.com

On Sun Dec 19 11:16:26 2010, RSCHUPP wrote: Show quoted text

> > As I already wrote in #62552, packed executables are not > meant to be run setuid and - as far as I'm concerned - > never will be. So don't do that then.

This fact is clear now, thanks. I don't understand the rationale, though - to users of the pp "compiler", this sounds like the gcc people saying that they don't care about gcc generating broken setuid binaries, since they cannot guarantee that gcc does not introduce security leaks in such binaries. Some background would be helpful here also for others who discover that pp does not support setuid execution. In any case: I do appreciate your contribution to this tremendously useful package, so no need to argue here. We will try switching to sudo now (but, read on please, since there may be similar problems with that). As a matter of fact, pp'ed binaries _do_ run setuid, they just suffer from some rather smaller glitches resulting from ownership of cached files. I wonder if those problems will also appear when running a pp'ed program via sudo (and then changing the effective user id during execution): 1/ cache directory is created as /tmp/par-<real username> 2/ but the owner of the cache entries is root 3/ it seems that PAR tries to put some file into the cache at a later point during the program's lifetime 4/ but if the effective user id is not root at that time, then this *fails* due to "2". Making the cache world-writable fixes the problem under 4/, and the pp'ed program runs fine even though it's setuid root. It looks like a rather straightforward solution for this is to ensure that any cache files will *always* be read and written with the *real user id*, not the effective one. If this is not the case, then also users of sudo (who switch the effective user id in their program) will run into the same problem, since the cache belongs to root, and it's not writeable by non-root users. Cheers/alex

Mon Dec 20 07:51:04 2010 roderich.schupp [...] googlemail.com - Correspondence added

Subject:	Re: [rt.cpan.org #63801] setuid pp'ed scripts: 1st invocation fails, 2nd+ call ok
Date:	Mon, 20 Dec 2010 13:50:56 +0100
To:	bug-PAR-Packer [...] rt.cpan.org
From:	Roderich Schupp <roderich.schupp [...] googlemail.com>

On Mon, Dec 20, 2010 at 12:28 PM, Alexander Ost via RT <bug-PAR-Packer@rt.cpan.org> wrote: Show quoted text

> I don't understand the rationale, though - to users of the pp

My thinking here is: The packed executable should strive to exhibit the same behaviour as the original perl script. I have no problem with people writing setuid scripts that have security problems. But a packed executable uses several Perl modules internally (i.e. unaware to user) and all these and their usage would have to be audited not to introduce _additional_ security problems. Moreover, the packed executable uses a custom Perl interpreter with a small C prologue and that would have to be audited as well. Especially cumbersome is the bootstrap process (the packed executable extracts another executable and then execs that). Show quoted text

> As a matter of fact, pp'ed binaries _do_ run setuid, they just suffer > from some rather smaller glitches resulting from ownership of cached > files. I wonder if those problems will also appear when running a pp'ed > program via sudo (and then changing the effective user id during > execution): > 1/ cache directory is created as /tmp/par-<real username>

Yeah, but you can overwite the name of the cache directory (PAR_GLOBAL_TMPDIR IIRC). Show quoted text

> 3/ it seems that PAR tries to put some file into the cache at a later > point during the program's lifetime

Correct. Some stuff will only be extracted on demand. Show quoted text

> Making the cache world-writable fixes the problem under 4/, and the

Thereby creating a security hole you can drive an aicraft carrier through. Cheers, Roderich

Mon Dec 20 08:24:13 2010 bitcard [...] post2.25u.com - Correspondence added

From:

bitcard [...] post2.25u.com

On Mon Dec 20 07:51:04 2010, roderich.schupp@googlemail.com wrote: Show quoted text

> On Mon, Dec 20, 2010 at 12:28 PM, Alexander Ost via RT

> > 3/ it seems that PAR tries to put some file into the cache at a later > > point during the program's lifetime

> > Correct. Some stuff will only be extracted on demand.

I just verified that (as suspected) this breaks any script that changes its effective uid during runtime, because it leads to ownership clashes in the cache directory. No matter if the change is due to setuid or because of some the script itself changes the id. So... is there a) any way to "request" that the whole archive is extracted *completely* into the cache? or b) what would you think about a solution where all cache entry access (esp writing) is ONLY done with the real user id, not the effective user id? Cheers/alex

Mon Dec 20 17:53:46 2010 smueller [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #63801] setuid pp'ed scripts: 1st invocation fails, 2nd+ call ok
Date:	Mon, 20 Dec 2010 23:53:36 +0100
To:	bug-PAR-Packer [...] rt.cpan.org
From:	Steffen Mueller <smueller [...] cpan.org>

On Dec 20, 2010, at 2:24 PM, Alexander Ost via RT wrote: Show quoted text

> Queue: PAR-Packer > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=63801 > > > On Mon Dec 20 07:51:04 2010, roderich.schupp@googlemail.com wrote:

>> On Mon, Dec 20, 2010 at 12:28 PM, Alexander Ost via RT

>>> 3/ it seems that PAR tries to put some file into the cache at a later >>> point during the program's lifetime

>> >> Correct. Some stuff will only be extracted on demand.

> > I just verified that (as suspected) this breaks any script that changes its effective > uid during runtime, because it leads to ownership clashes in the cache directory. > > No matter if the change is due to setuid or because of some the script itself changes > the id. > > So... is there > a) any way to "request" that the whole archive is extracted *completely* into the cache? > or > b) what would you think about a solution where all cache entry access (esp writing) is > ONLY done with the real user id, not the effective user id?

Try installing Archive::Unzip::Burst on the dev machine. If it succeeds, it will be packed into the executable and everything will be extracted on the first invocation. Roderich's concerns, however, remain valid. a setuid pp'd executable is not guaranteed safe.\ Best regards, Steffen

Tue Dec 21 10:56:00 2010 bitcard [...] post2.25u.com - Correspondence added

From:

bitcard [...] post2.25u.com

On Mon Dec 20 17:53:46 2010, SMUELLER wrote: Show quoted text

> > On Dec 20, 2010, at 2:24 PM, Alexander Ost via RT wrote: > [...]

> > a) any way to "request" that the whole archive is extracted

> *completely* into the cache? > [...] > Try installing Archive::Unzip::Burst on the dev machine. If it > succeeds, it will be packed into the executable and everything will be > extracted on the first invocation.

Thanks for the hint. Ironically, even though I switched to using sudo now, this fixes the setuid problem posted originally for this bug. However, for setuid, this doesn't solve the problem completely. Starting my application (including "use Archive::Unzip::Burst") via sudo, I get the following error in the middle of the execution: Permission denied at /home/builder/.lib/perl-5.10.0- nt/lib/site_perl/5.10.0/PAR/Heavy.pm line 160. Compilation failed in require at XML/Parser.pm line 18. BEGIN failed--compilation aborted at XML/Parser.pm line 22. Compilation failed in require at XML/SAX/Expat.pm line 11. BEGIN failed--compilation aborted at XML/SAX/Expat.pm line 11. Compilation failed in require at (eval 71) line 1. Line 160 in Heavy.pm (version 0.12) is an open() statement in sub _dl_extract that requires write access. To find the culprit, I made the PAR cache world writeable and started over. After that the application runs fine, and one file has been added to the cache. That file corresponds to lib/site_perl/5.10.0/i686-linux-thread- multi/auto/XML/Parser/Expat/Expat.so So this file hasn't been extracted at the beginning, and later on, extraction fails, because in the meantime the application has dropped root priviledges. I could now work around this by explicitly require-ing XML::SAX::Expat at the very beginning of the main module, but this is no general solution of course. Would there be any way to enforce a (really) complete "unpack" at the very beginning? Or, as suggested before, maybe it's even better to make cache handling independent of the applications current effective user id. Which makes sense, since the cache handling (I think) is something that should be completely transparent to the application. One last remark, maybe this is relevant: XML::SAX::Expat was added explicitly on the pp command line with -l "/usr/lib/libexpat.so" -M "XML::SAX::Expat" Thanks again for your help, /alex

Tue Dec 21 12:56:07 2010 bitcard [...] post2.25u.com - Correspondence added

From:

bitcard [...] post2.25u.com

Oh my, this gets confusing now. The paragraph above should read However, for **sudo**, this doesn't solve the problem completely. So Steffen's workaround does not solve the cache ownership problem that exists when running pp'ed application via *sudo*. Br/alex

Wed Dec 22 05:00:46 2010 RSCHUPP [...] cpan.org - Correspondence added

On 2010-12-21 10:56:00, arost wrote: Show quoted text

> Line 160 in Heavy.pm (version 0.12) is an open() statement in sub > _dl_extract that requires write access.

Which - unfortunately - can't be helped for security reasons (for the non-setuid usecase). Archive::Unzip::Burst has probably done its job and extracted Expat.so - but using its actual name (and path). However, PAR::Heavy insists on looking for Expat.so first in the packed executable (treated as a .par archive) and then extracts it again using a mangled name. Now, if one were to abandon the whole name mangling scheme... Show quoted text

> Or, as suggested before, maybe it's even better to > make cache handling independent of the applications current effective > user id. Which makes sense, since the cache handling (I think) is > something that should be completely transparent to the application.

No, it doesn't make sense: the cache is per user (for a reason) and YOU break transparency by "switching users" in midstream. Sorry, but this not different from e.g. a script manipulating @INC in nasty ways at runtime or invoking $^X. Show quoted text

> One last remark, maybe this is relevant: XML::SAX::Expat was added > explicitly on the pp command line with > > -l "/usr/lib/libexpat.so" -M "XML::SAX::Expat"

No, that shouldn't make any difference. What matters is that XML::SAX::Expat is loaded by your script at runtime (probably required by XML::Simple or similar), instead of compile time (i.e. via "use XML::SAX::Expat"). Show quoted text

> So Steffen's workaround does not solve the cache ownership > problem that exists when running pp'ed application via *sudo*.

Actually, it's not the use of sudo per se - it's your script calling setuid at runtime. Cheers, Roderich Cheers, Roderich

Wed Dec 22 06:24:46 2010 bitcard [...] post2.25u.com - Correspondence added

From:

bitcard [...] post2.25u.com

On Wed Dec 22 05:00:46 2010, RSCHUPP wrote: Show quoted text

> On 2010-12-21 10:56:00, arost wrote: > [...]

> > Or, as suggested before, maybe it's even better to > > make cache handling independent of the applications current effective > > user id. Which makes sense, since the cache handling (I think) is > > something that should be completely transparent to the application.

> > No, it doesn't make sense: the cache is per user (for a reason) > and YOU break transparency by "switching users" in midstream. > Sorry, but this not different from e.g. a script manipulating > @INC in nasty ways at runtime or invoking $^X.

I don't agree to that. For @INC, I know that @INC can be subject to external settings in the environment and that @INC affects the runtime behaviour of my Perl application - so I know that I have to be careful fiddling with that, no matter if pp'ed or not. Also for $^X, I'd _expect_ things to be different in a pp'ed world. setuid(), in contrast, is a systemcall like any other, which normally has no impact on the behaviour of a Perl application (from a Perl point of view), so in that sense, it is completely orthogonal to the world of Perl. In the pp'ed Perl world, this is no longer true. The per-user nature of caching I don't question at all - my suggestion is rather to _enforce_ its per-user nature, by _always_ accessing the cache with the real user id, and _not_ taking into account any later changes to the effective user id. Show quoted text

> Actually, it's not the use of sudo per se - it's your script > calling setuid at runtime.

Yep. So, to summarize, pp users must be aware of these three points then: 1/ do not use setuid 2/ do not use POSIX::setuid(), and do not modify $>/$) 3/ if 2/ cannot be avoided, then workaround the PAR cache problems by a) use Archive::Unzip::Burst, and b) explicitly require any "leftovers" before the first call to setuid() Cheers/alex

Wed Oct 09 08:02:43 2013 RSCHUPP [...] cpan.org - Correspondence added

Creating safe setuid executables is not (and will never be) supported

Wed Oct 09 08:02:44 2013 RSCHUPP [...] cpan.org - Status changed from 'open' to 'rejected'