Skip Menu |
 

This queue is for tickets about the Convert-PEM CPAN distribution.

Report information
The Basics
Id: 27574
Status: open
Priority: 0/
Queue: Convert-PEM

People
Owner: Nobody in particular
Requestors: ANDK [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: 0.07
Fixed in: (no value)

Attachments
0001-Do-not-test-the-reason-for-decryption-failure-on-bad.patch



Subject: encode test occasionlly fails
Download (untitled) / with headers
text/plain 263b
You may have seen the FAIL report http://www.nntp.perl.org/group/perl.cpan.testers/2007/05/msg479865.html This happens from time to time in my smoke testing. I have about 300 test runs and about 20 of them failed. Let me know if you need further info. Thanks,
From: cpan [...] pjedwards.co.uk
On Thu Jun 14 03:07:06 2007, ANDK wrote: Show quoted text
> You may have seen the FAIL report > http://www.nntp.perl.org/group/perl.cpan.testers/2007/05/msg479865.html > > This happens from time to time in my smoke testing. I have about 300 > test runs and about 20 of them failed. > > Let me know if you need further info. > > Thanks,
I've also experienced intermittent test case failures in smoke testing. I've isolated the failure to 01-readwrite.t, the line: ok($pem->errstr =~ /^Decryption failed/); fails on occasion when attempting to read without a password, it fails because the errstr is: ASN decode failed: decode error at <removed>perl-5.8.7/lib/site_perl/5.8.7/Convert/ASN1/_decode.pm line 57. I've edited the test case to write the .pem file it's created and here are 2 examples that cause this failure: Show quoted text
-----BEGIN TEST OBJECT----- Proc-Type: 4,ENCRYPTED DEK-Info: DES-EDE3-CBC,EB4200ECBD69983E T4LHHdmRVRw=
-----END TEST OBJECT-----
-----BEGIN TEST OBJECT----- Proc-Type: 4,ENCRYPTED DEK-Info: DES-EDE3-CBC,760F432BC1DC730E qMZIumorEEA=
-----END TEST OBJECT----- Hope this is useful. Peter (Stig) Edwards
Download (untitled) / with headers
text/plain 1.5k
On Mon Jan 28 12:24:18 2008, cpan@pjedwards.co.uk wrote: Show quoted text
> On Thu Jun 14 03:07:06 2007, ANDK wrote:
> > You may have seen the FAIL report > > http://www.nntp.perl.org/group/perl.cpan.testers/2007/05/msg479865.html > > > > This happens from time to time in my smoke testing. I have about 300 > > test runs and about 20 of them failed. > > > > Let me know if you need further info. > > > > Thanks,
> > I've also experienced intermittent test case failures in smoke testing. > I've isolated the failure to 01-readwrite.t, the line: > > ok($pem->errstr =~ /^Decryption failed/); > > fails on occasion when attempting to read without a password, it > fails because the errstr is: > > ASN decode failed: decode error at > <removed>perl-5.8.7/lib/site_perl/5.8.7/Convert/ASN1/_decode.pm line 57. > > I've edited the test case to write the .pem file it's created and here > are 2 examples that cause this failure: > > -----BEGIN TEST OBJECT----- > Proc-Type: 4,ENCRYPTED > DEK-Info: DES-EDE3-CBC,EB4200ECBD69983E > > T4LHHdmRVRw= > -----END TEST OBJECT----- > > -----BEGIN TEST OBJECT----- > Proc-Type: 4,ENCRYPTED > DEK-Info: DES-EDE3-CBC,760F432BC1DC730E > > qMZIumorEEA= > -----END TEST OBJECT----- > > Hope this is useful. > > Peter (Stig) Edwards
I did some more digging, it fails to fail in the anticipated way when $cut in: Convert::PEM::CBC::decrypt(<removd>perl-5.8.7/lib/site_perl/5.8.7/Convert/PEM/CBC.pm:85): 85: return $cbc->error("Bad key/passphrase") 86: if $cut > $bs; is not > $bs (8), so in the 2 examples I posted above, cut is 6 (ACK) and 7 (BELL).
Download (untitled) / with headers
text/plain 1.4k
This defect still exists, and is quite reproducible on my machines just by running the test a number of times. Usually within 20 tries it will fail. For instance: for i in {1..1000}; do perl -Iblib/lib -Iblib/arch t/01-readwrite.t | grep -v '^ok' done The simple but flawed answer is to get rid of the regex test -- the read failed so we know it didn't correctly decode. Another (also flawed) possibility is: ok !defined $obj2, 'cannot read encrypted file'; my $ctr = 0; while ($pem->errstr !~ /^Decryption failed/ && $ctr++ < 100) { $obj2 = $pem->read( Filename => $objfile, Password => "yy$ctr" ); } like $pem->errstr, qr/^Decryption failed/, 'errstr matches decryption failed'; where we try other incorrect passwords if necessary, This seems to work at first blush, but let it run enough times and eventually Convert::ASN1 will go crazy spewing: substr outside of string at <path>/Convert/ASN1/_decode.pm line 634. Use of uninitialized value in concatenation (.) or string at <path>/Convert/ASN1/_decode.pm line 634. ... this last line keeps repeating ... We could hope for better behavior from Convert::ASN1 here. I'm not sure how robust that module intends to be. Given this, I wonder if the entire "read encrypted file with wrong password" test should be moved to an author test. Unless there is a more robust way to determine incorrect decryption. Since this module is useful for Crypt::DSA, it'd be nice to have this fixed.
Download (untitled) / with headers
text/plain 205b
The github version of Convert::ASN1 now has fixes for unsafe decoding which should make this test possible. The loop version of the decryption test should be a viable solution once Convert::ASN1 is fixed.
From: ppisar [...] redhat.com
Download (untitled) / with headers
text/plain 2.1k
Dne Pá 04.říj.2013 03:13:15, DANAJ napsal(a): Show quoted text
> This defect still exists, and is quite reproducible on my machines > just by running the test a number of times. Usually within 20 tries > it will fail. For instance: > > for i in {1..1000}; do > perl -Iblib/lib -Iblib/arch t/01-readwrite.t | grep -v '^ok' > done > > The simple but flawed answer is to get rid of the regex test -- the > read failed so we know it didn't correctly decode. > > Another (also flawed) possibility is: > > ok !defined $obj2, 'cannot read encrypted file'; > my $ctr = 0; > while ($pem->errstr !~ /^Decryption failed/ && $ctr++ < 100) { > $obj2 = $pem->read( Filename => $objfile, Password => "yy$ctr" ); > } > like $pem->errstr, qr/^Decryption failed/, 'errstr matches decryption > failed'; > > where we try other incorrect passwords if necessary, This seems to > work at first blush, but let it run enough times and eventually > Convert::ASN1 will go crazy spewing: > > substr outside of string at <path>/Convert/ASN1/_decode.pm line 634. > Use of uninitialized value in concatenation (.) or string at > <path>/Convert/ASN1/_decode.pm line 634. > ... this last line keeps repeating ... > > We could hope for better behavior from Convert::ASN1 here. I'm not > sure how robust that module intends to be. Given this, I wonder if > the entire "read encrypted file with wrong password" test should be > moved to an author test. Unless there is a more robust way to > determine incorrect decryption. >
Convert-ASN1-0.27 does not stuck probably anymore, but I think the real bug is in the Convert-PEM test: Some bad keys can fool the "Bad key/passphrase" heuristic in the Convert::PEM::CBC::decode() because it's a feature a stream cipher to spit out bad output on bad key. So the heuristic is just a kind service to point to the cause of the decoding failure (i.e. bad key). By probabilistic nature of the heuristic, even 100 cycles trying different bad keys is not enough. (You will just get random test failures 100 less times.) Therefore I propose to remove the like $pem->errstr, qr/^Decryption failed/, 'errstr matches decrypt failed'; test completely. -- Petr
From: ppisar [...] redhat.com
Download (untitled) / with headers
text/plain 231b
Dne Pá 05.zář.2014 05:00:09, ppisar napsal(a): Show quoted text
> Therefore I propose to remove the > > like $pem->errstr, qr/^Decryption failed/, 'errstr matches decrypt > failed'; > > test completely. >
Implementation is attached. -- Petr
Subject: 0001-Do-not-test-the-reason-for-decryption-failure-on-bad.patch
From 2d6ec9bc13a0994ba5a5911c1c210082ed4da09f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com> Date: Fri, 5 Sep 2014 11:04:30 +0200 Subject: [PATCH] Do not test the reason for decryption failure on bad key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests checking error message after using bad decryption key fails randomly. Some bad keys can fool the "Bad key/passphrase" heuristic in the Convert::PEM::CBC::decode() because it's a feature a stream cipher to spit out bad output on bad key. So the heuristic is just a kind service to point to the cause of the decoding failure (i.e. bad key). By probabilistic nature of the heuristic, we cannot rely on the /^Decryption failed/ error message. This patch removes these tests. CPAN RT#27574 Signed-off-by: Petr Písař <ppisar@redhat.com> --- t/01-readwrite.t | 3 +-- t/02-encode.t | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/t/01-readwrite.t b/t/01-readwrite.t index 42a4416..208eba1 100644 --- a/t/01-readwrite.t +++ b/t/01-readwrite.t @@ -1,5 +1,5 @@ use strict; -use Test::More tests => 16; +use Test::More tests => 15; use Test::Exception; use Convert::PEM; @@ -29,7 +29,6 @@ lives_ok { $pem->write( Filename => $objfile, Content => $obj, Password => 'xx' ok -e $objfile, 'output file exists'; lives_ok { $obj2 = $pem->read( Filename => $objfile ) } 'can read'; ok !defined $obj2, 'cannot read encrypted file'; -like $pem->errstr, qr/^Decryption failed/, 'errstr matches decryption failed'; lives_ok { $obj2 = $pem->read( Filename => $objfile, Password => 'xx') } 'can read'; is $obj->{TestObject}{int}, $obj2->{TestObject}{int}, 'input matches output'; unlink $objfile; diff --git a/t/02-encode.t b/t/02-encode.t index 37aa987..9c6ab4c 100644 --- a/t/02-encode.t +++ b/t/02-encode.t @@ -1,5 +1,5 @@ use strict; -use Test::More tests => 9; +use Test::More tests => 8; use Convert::PEM; use Math::BigInt; @@ -25,7 +25,6 @@ $blob = $pem->encode( Content => $obj, Password => 'xx' ); ok $blob, 'encode gave us something'; $obj2 = $pem->decode( Content => $blob ); ok !defined $obj2, 'decode fails on encrypted input'; -like $pem->errstr, qr/^Decryption failed/, 'errstr matches decrypt failed'; $obj2 = $pem->decode( Content => $blob, Password => 'xx' ); is $obj->{TestObject}{int}, $obj2->{TestObject}{int}, 'input matches output'; -- 1.9.3


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.