Re: [PATCH] ExtUtils::MakeMaker and world writable files in dists

2008-11-12 Thread demerphq
2008/11/13 chromatic <[EMAIL PROTECTED]>:
> On Wednesday 12 November 2008 22:36:31 demerphq wrote:
>
>> > I really, really, really don't want PAUSE modifying my stuff after it's
>> > uploaded.  Oh god the mysterious bugs.  And then there's the fact that
>> > the code I've put my name and signature on is not the same code as is
>> > being distributed!  That's a trust violation as well as maybe a license
>> > violation.
>
>> Oh please, save me the drama. We aren't talking about modifying "your
>> stuff" we are talking about twiddling some bits in a tar file.
>
> I can only think of several ways that could possibly go wrong.

Pray tell, what are they?

> I understand why PAUSE enforces the policy that it won't index anything it
> can't index, but I don't understand what permission bits that may or may not
> be set have to do with indexing.
>
> I realize the longstanding Perl cultural view of encapsulation is, to put it
> mildly, highly voluntary -- but the first time I catch a naked, drunk
> neighbor rifling through my closet is the last time any naked, drunk neighbor
> rifles through my closet, regardless of sincerity of intent.

So you equate PAUSE unpacking the tar file, chmod'ing to not be world
writable and then retarring it to a naked drunk neighbor rifling
through your closet? I don't get it really, and I'm wondering what
kind of neighborhood you live in!. And presumably this would never
happen to you right? Being a switched on unix guy you wouldnt roll a
world writable CPAN package anyway would you?

If there is any comparison its like the library putting durable
binding and a security strip on a book before it hits the shelves.

Cheers,
yves
-- 
perl -Mre=debug -e "/just|another|perl|hacker/"


Re: [PATCH] ExtUtils::MakeMaker and world writable files in dists

2008-11-12 Thread chromatic
On Wednesday 12 November 2008 22:36:31 demerphq wrote:

> > I really, really, really don't want PAUSE modifying my stuff after it's
> > uploaded.  Oh god the mysterious bugs.  And then there's the fact that
> > the code I've put my name and signature on is not the same code as is
> > being distributed!  That's a trust violation as well as maybe a license
> > violation.

> Oh please, save me the drama. We aren't talking about modifying "your
> stuff" we are talking about twiddling some bits in a tar file.

I can only think of several ways that could possibly go wrong.

I understand why PAUSE enforces the policy that it won't index anything it 
can't index, but I don't understand what permission bits that may or may not 
be set have to do with indexing.

I realize the longstanding Perl cultural view of encapsulation is, to put it 
mildly, highly voluntary -- but the first time I catch a naked, drunk 
neighbor rifling through my closet is the last time any naked, drunk neighbor 
rifles through my closet, regardless of sincerity of intent.

-- c


Re: [PATCH] ExtUtils::MakeMaker and world writable files in dists

2008-11-12 Thread demerphq
2008/11/13 Michael G Schwern <[EMAIL PROTECTED]>:
> Jonathan Rockway wrote:
>> * On Wed, Nov 12 2008, David Golden wrote:
>>> On Wed, Nov 12, 2008 at 3:17 PM, demerphq <[EMAIL PROTECTED]> wrote:
 IMO if the toolchain is to work this should happen at PAUSE (if it can
 detect this problem IMO it should just damn well fix it itself) or at
 extraction.
>>> It *is* being fixed at extraction.  But it requires people to upgrade
>>> CPAN and CPANPLUS (maybe Archive::Extract as well).  It was a faster
>>> fix to close the PAUSE indexing door than to get those fixes released.
>>
>> I agree with demerphq here, why can't PAUSE just fix this?  It won't
>> break signatures (since they sign file content, not file metadata),
>
> Maybe they should start signing the metadata then!
>
>
>> and
>> it won't break the CHECKSUMS file (since that could be generated after
>> the tarball is fixed).
>>
>> It could be weird if what you upload to CPAN isn't what you
>> download... but it fixes a security problem, and it doesn't require
>> authors to know that this problem exists.  Abstraction++
>
> -100_000_000
>
> I really, really, really don't want PAUSE modifying my stuff after it's
> uploaded.  Oh god the mysterious bugs.  And then there's the fact that the
> code I've put my name and signature on is not the same code as is being
> distributed!  That's a trust violation as well as maybe a license violation.

Oh please, save me the drama. We aren't talking about modifying "your
stuff" we are talking about twiddling some bits in a tar file.

Bits in a tar file mind you that mean nothing to the system that the
tar file was created on.

And if you really do want to be picky about this, then it could be
voluntary as was already suggested.

Then when PAUSE bounces my package it can say "We've rejected your
package for blah blah blah, but we can fix it for you if you visit
this [link], or if you reupload a new package with SPECIALFLAG set in
the FNORBLE file."

> This security check has sent CPAN on the slippery slope of security.  Until
> now CPAN has been a common carrier.  Pretty much anything was allowed, stuff
> was only rejected for extreme reasons and always on a case-by-case basis and
> always by human judgment.  Now we've put in an automatic filter to reject some
> vaguely insecure code.  CPAN is no longer a common carrier.  Once that line
> has been crossed, all sorts of attempts will be made to add more filtering,
> such as the suggestion above.
>
> They will be well intentioned and they will add complications and generate
> false negatives and get in people's way and continue to erode CPAN's policy of
> being a common carrier.
>
> Now that the CPAN shells and archiving modules are handling it at their end, I
> think the PAUSE filter should be removed.  It's not PAUSE's job to be the code
> police.

I agree with this. However we are where we are, and PAUSE fixing the
package in a way that doesn't require windows users to get annoyed is
a good solution.

-- 
perl -Mre=debug -e "/just|another|perl|hacker/"


Re: [PATCH] ExtUtils::MakeMaker and world writable files in dists

2008-11-12 Thread Michael G Schwern
Andreas J. Koenig wrote:
>> On Wed, 12 Nov 2008 19:13:40 -0800, Michael G Schwern <[EMAIL 
>> PROTECTED]> said:
> 
>   > Now that the CPAN shells and archiving modules are handling it at their 
> end, I
>   > think the PAUSE filter should be removed.  It's not PAUSE's job to be the 
> code
>   > police.
> 
> It is 'tar xzf CPANFILE.tar.gz' which is exploitable. No CPAN shell
> and archiving module involved.

What I was expressing is that the CPAN shell can do the twiddling to strip
flags at the point of extraction, rather than PAUSE stopping it at the gate.
Archive::Tar already does this (see $Archive::Tar::INSECURE_EXTRACT_MODE).
The important distinction being that it's done under the user's control and
not by PAUSE fiat.  PAUSE shouldn't be playing security nanny or any other 
nanny.

It's not even necessary or effective.  Because there's already a perfectly
sensible and universal way to avoid this problem and that's to set your umask
to something sensible.  Then no matter what the archive's internal permissions
are set to they'll be stripped when it's extracted.

Most systems already do this by default, because it's good security practice.
 If you don't have a umask set, that's a basic vulnerability *at the user's
end*.  No amount of hand-holding from CPAN will protect the user without a
umask.  Some other system will ship a world writable file or a setuid
executable or something.  Then you're hosed all over again.

We are trying to fix a basic, wide-spread, user-end security hole, one that is
not at all specific to Perl, at too high a level and too specific a system.

It's like plugging one hole in a screen door.


-- 
Insulting our readers is part of our business model.
http://somethingpositive.net/sp07122005.shtml


Re: [PATCH] ExtUtils::MakeMaker and world writable files in dists

2008-11-12 Thread Michael G Schwern
David Golden wrote:
> On Wed, Nov 12, 2008 at 3:17 PM, demerphq <[EMAIL PROTECTED]> wrote:
>> I rather strongly object to this change.
> 
> I totally understand -- but keep in mind that this was in response to
> someone flagging this as a potential (if highly unlikely) security
> hole, forwarding it to some security-watchdog site, etc.  So the rapid
> response was "close the hole so no one can say CPAN creates a security
> risk".  (Other than the usual, obvious one of running arbitrary
> code...)
> 
> So it causes some pain, but in my view, it's in the interest of the
> Perl community to be seen as vigilant.

I'm sorry, you'll have to remove your shoes before you can post to this
mailing list.

http://en.wikipedia.org/wiki/Security_theater


-- 
Stabbing you in the face for your own good.


Re: [PATCH] ExtUtils::MakeMaker and world writable files in dists

2008-11-12 Thread Michael G Schwern
Jonathan Rockway wrote:
> * On Wed, Nov 12 2008, David Golden wrote:
>> On Wed, Nov 12, 2008 at 3:17 PM, demerphq <[EMAIL PROTECTED]> wrote:
>>> IMO if the toolchain is to work this should happen at PAUSE (if it can
>>> detect this problem IMO it should just damn well fix it itself) or at
>>> extraction.
>> It *is* being fixed at extraction.  But it requires people to upgrade
>> CPAN and CPANPLUS (maybe Archive::Extract as well).  It was a faster
>> fix to close the PAUSE indexing door than to get those fixes released.
> 
> I agree with demerphq here, why can't PAUSE just fix this?  It won't
> break signatures (since they sign file content, not file metadata),

Maybe they should start signing the metadata then!


> and
> it won't break the CHECKSUMS file (since that could be generated after
> the tarball is fixed).
> 
> It could be weird if what you upload to CPAN isn't what you
> download... but it fixes a security problem, and it doesn't require
> authors to know that this problem exists.  Abstraction++

-100_000_000

I really, really, really don't want PAUSE modifying my stuff after it's
uploaded.  Oh god the mysterious bugs.  And then there's the fact that the
code I've put my name and signature on is not the same code as is being
distributed!  That's a trust violation as well as maybe a license violation.

This security check has sent CPAN on the slippery slope of security.  Until
now CPAN has been a common carrier.  Pretty much anything was allowed, stuff
was only rejected for extreme reasons and always on a case-by-case basis and
always by human judgment.  Now we've put in an automatic filter to reject some
vaguely insecure code.  CPAN is no longer a common carrier.  Once that line
has been crossed, all sorts of attempts will be made to add more filtering,
such as the suggestion above.

They will be well intentioned and they will add complications and generate
false negatives and get in people's way and continue to erode CPAN's policy of
being a common carrier.

Now that the CPAN shells and archiving modules are handling it at their end, I
think the PAUSE filter should be removed.  It's not PAUSE's job to be the code
police.


-- 
Reality is that which, when you stop believing in it, doesn't go away.
-- Phillip K. Dick


Re: [PATCH] ExtUtils::MakeMaker and world writable files in dists

2008-11-12 Thread Jonathan Rockway
* On Wed, Nov 12 2008, David Golden wrote:
> On Wed, Nov 12, 2008 at 3:17 PM, demerphq <[EMAIL PROTECTED]> wrote:
>> IMO if the toolchain is to work this should happen at PAUSE (if it can
>> detect this problem IMO it should just damn well fix it itself) or at
>> extraction.
>
> It *is* being fixed at extraction.  But it requires people to upgrade
> CPAN and CPANPLUS (maybe Archive::Extract as well).  It was a faster
> fix to close the PAUSE indexing door than to get those fixes released.

I agree with demerphq here, why can't PAUSE just fix this?  It won't
break signatures (since they sign file content, not file metadata), and
it won't break the CHECKSUMS file (since that could be generated after
the tarball is fixed).

It could be weird if what you upload to CPAN isn't what you
download... but it fixes a security problem, and it doesn't require
authors to know that this problem exists.  Abstraction++

Regards,
Jonathan Rockway

--
print just => another => perl => hacker => if $,=$"


Re: [PATCH] ExtUtils::MakeMaker and world writable files in dists

2008-11-12 Thread demerphq
2008/11/12 David Golden <[EMAIL PROTECTED]>:
> On Wed, Nov 12, 2008 at 3:17 PM, demerphq <[EMAIL PROTECTED]> wrote:
>> I rather strongly object to this change.
>
> I totally understand -- but keep in mind that this was in response to
> someone flagging this as a potential (if highly unlikely) security
> hole, forwarding it to some security-watchdog site, etc.  So the rapid
> response was "close the hole so no one can say CPAN creates a security
> risk".  (Other than the usual, obvious one of running arbitrary
> code...)
>
> So it causes some pain, but in my view, it's in the interest of the
> Perl community to be seen as vigilant.

Ah well fair enough. Writing my rant was cathartic. :-)

>> this silly test. What really gets me going tho is I WASNT TOLD THIS
>> ABOUT 1.51_01 or 1.51_02 or 1.51_03 or (do you detect a pattern here?)
>> 1.51_04 or 1.51_05, all of which i uploaded in the last few days in
>> the exact same way!!!
>
> That's kind of a loophole, since development versions aren't indexed.
> I think any upload that fails a security test should probably be
> rejected, whether development or full release.

Or at least the author should be notified about it.

>> IMO if the toolchain is to work this should happen at PAUSE (if it can
>> detect this problem IMO it should just damn well fix it itself) or at
>> extraction.
>
> It *is* being fixed at extraction.  But it requires people to upgrade
> CPAN and CPANPLUS (maybe Archive::Extract as well).  It was a faster
> fix to close the PAUSE indexing door than to get those fixes released.

Just curious whats wrong with PAUSE repacking the file with the required perms?

>> Whats going to happen next, stuff rejected because they don't have
>> *nix line endings? Or *nix style shebangs? Or use perl-qa's preferred
>> indentation style or something? H?!
>
> Maybe instead, at a minimum, every distribution should be run against
> Perl::Critic at severity level 4 and anything that doesn't pass should
> be rejected as well.  ;-)
>
> (THAT'S A JOKE, PEOPLE!)
>
>> /g
>
> Right there with you, except my "/grrr" was back when the "security
> alert" got sent off to the watchdogs while the discussion was still
> going on as to whether this was a significant risk in the first place.

Ah, yes I can imagine that being worth a /grrr or two.

-- 
perl -Mre=debug -e "/just|another|perl|hacker/"


Re: [PATCH] ExtUtils::MakeMaker and world writable files in dists

2008-11-12 Thread David Golden
On Wed, Nov 12, 2008 at 3:17 PM, demerphq <[EMAIL PROTECTED]> wrote:
> I rather strongly object to this change.

I totally understand -- but keep in mind that this was in response to
someone flagging this as a potential (if highly unlikely) security
hole, forwarding it to some security-watchdog site, etc.  So the rapid
response was "close the hole so no one can say CPAN creates a security
risk".  (Other than the usual, obvious one of running arbitrary
code...)

So it causes some pain, but in my view, it's in the interest of the
Perl community to be seen as vigilant.

> this silly test. What really gets me going tho is I WASNT TOLD THIS
> ABOUT 1.51_01 or 1.51_02 or 1.51_03 or (do you detect a pattern here?)
> 1.51_04 or 1.51_05, all of which i uploaded in the last few days in
> the exact same way!!!

That's kind of a loophole, since development versions aren't indexed.
I think any upload that fails a security test should probably be
rejected, whether development or full release.

> IMO if the toolchain is to work this should happen at PAUSE (if it can
> detect this problem IMO it should just damn well fix it itself) or at
> extraction.

It *is* being fixed at extraction.  But it requires people to upgrade
CPAN and CPANPLUS (maybe Archive::Extract as well).  It was a faster
fix to close the PAUSE indexing door than to get those fixes released.

> Whats going to happen next, stuff rejected because they don't have
> *nix line endings? Or *nix style shebangs? Or use perl-qa's preferred
> indentation style or something? H?!

Maybe instead, at a minimum, every distribution should be run against
Perl::Critic at severity level 4 and anything that doesn't pass should
be rejected as well.  ;-)

(THAT'S A JOKE, PEOPLE!)

> /g

Right there with you, except my "/grrr" was back when the "security
alert" got sent off to the watchdogs while the discussion was still
going on as to whether this was a significant risk in the first place.

-- David


Re: [PATCH] ExtUtils::MakeMaker and world writable files in dists

2008-11-12 Thread demerphq
2008/10/1 Andreas J. Koenig <[EMAIL PROTECTED]>:
>> On Tue, 30 Sep 2008 17:11:00 -0500, Jonathan Rockway <[EMAIL PROTECTED]> 
>> said:
>
>  >> Anyway, I think the average CPAN author doesn't
>  >> really know or care about that, sadly.
>  >> See also
>
>  > FWIW, this is true.  I have never thought about it.
>
>  > Personally, I am confused as to why users have programs that do whatever
>  > an input file from the Internet tells them to do.  If you don't want
>  > your tar command to create world-writable files, you should probably
>  > tell your tar command to not create world-writable files... right?  That
>  > is much easier than convincing every person on the Internet to do what
>  > you want.  It is also easier than convincing every CPAN author to
>  > upgrade MakeMaker.
>
> Not true. The tools we are advocating must simply work. If they would
> leave all decisions to the end user we would have no CPAN. MakeMaker
> has always done the right thing, no need to upgrade. There was a bug
> to squash and not to paint over it or bathe in ignorance. Did you even
> notice the bug in the noise?



I rather strongly object to this change.

I just uploaded ExtUtils-Install 1.51, and was rudely told it would
not be indexed. I then spent 15 minutes trying to figure out what
win32 permissions would allow me to create a tarball that would pass
this silly test. What really gets me going tho is I WASNT TOLD THIS
ABOUT 1.51_01 or 1.51_02 or 1.51_03 or (do you detect a pattern here?)
1.51_04 or 1.51_05, all of which i uploaded in the last few days in
the exact same way!!!

IMO if the toolchain is to work this should happen at PAUSE (if it can
detect this problem IMO it should just damn well fix it itself) or at
extraction. It shouldn't be my problem how you nixy people (and I'm
one these days myself) want your files permissioned when they are
installed, especially if those permissions don't make sense on my box
(of the moment) anyway. And at the very least I should find out about
this when I upload a dev release and not be surprised when I make a
production release.

Whats going to happen next, stuff rejected because they don't have
*nix line endings? Or *nix style shebangs? Or use perl-qa's preferred
indentation style or something? H?!

/g



ps: Andreas, don't take this personally I'm just letting off steam and
I still think you are a really nice guy and do a fantastic job with
PAUSE.  :-)
-- 
perl -Mre=debug -e "/just|another|perl|hacker/"