| Subject: | $parser is cached globally, so $self->{"_error"} is never reset, preventing loops; with patch |
| Date: | Mon, 29 Apr 2019 20:02:36 +0200 |
| To: | bug-Crypt-X509-CRL [...] rt.cpan.org |
| From: | Markus Wernig <public [...] wernig.net> |
Hi all
Using version 0.1 of the module on Gentoo Linux.
The module defines (line numbers prepended):
18: my $error = undef;
In the constructor it checks if the parser is already set and uses the
old one if present:
147: if ( not defined ( $parser ) ) {
148: $parser = _init();
149: }
It appears that the $parser from Convert::ASN1 that is used here does
not reset its internal error state when being called a second time.
So while this seems a good idea wrt performance, it breaks whenever
looping over multiple CRLs, one of which fails to parse.
The solution is to not check for a cached $parser, just call _init every
time (remove lines 147 and 149 in the constructor quoted above).
This is in fact a duplicate of
https://rt.cpan.org/Public/Bug/Display.html?id=118487, which has gone
unanswered for almost 3 years.
Here's some sample code to illustrate the problem:
#!/usr/bin/perl
use strict;
use warnings;
use Crypt::X509::CRL;
use MIME::Base64;
my $crldir = "/path/to/dir/containing/crl/files";
#
# Note:
# Of the files in $crldir, one is a corrupt CRL that cannot be parsed
#
if( $crldir && -d "$crldir" && opendir (CRLDIR, "$crldir")) {
while(my $file = readdir CRLDIR) {
next if($file =~ /^\./);
next unless($file =~ /\.(pem|crl|der)$/i);
print("Loading CRL from $crldir/$file\n");
if(open FHC, "<$crldir/$file") {
my $crl;
my $fmt = "DER";
while(<FHC>) {
if($_ =~ /(BEGIN|END).*CRL/) {
# remove PEM header/trailer
$fmt = "PEM";
next;
}
$crl .= $_;
}
close FHC;
if($fmt eq "PEM") {
$crl = decode_base64($crl);
}
my $crlobj = undef;
if($crlobj = Crypt::X509::CRL->new(crl => $crl)) {
if($crlobj->error()) {
# error() is now always set for each CRL after the corrupt
# one, even if the CRL is OK
print("Failed to parse CRL from " . $crlobj->issuer_cn() .": "
. $crlobj->error());
} else {
print("Parsed CRL from " . $crlobj->issuer_cn());
}
} else {
print("Failed to parse CRL $crldir/$file\n");
}
} else {
print("Failed to open CRL $crldir/$file\n");
}
}
}
Here's the patch to fix this problem:
--- /usr/local/lib64/perl5/5.24.1/Crypt/X509/CRL.pm 2019-04-29
19:59:41.689237937 +0200
+++ CRL.pm 2019-04-29 20:00:14.817439892 +0200
@@ -144,9 +144,7 @@
sub new {
my ( $class , %args ) = @_;
- if ( not defined ( $parser ) ) {
- $parser = _init();
- }
+ $parser = _init();
my $self = $parser->decode( $args{'crl'} );
Thanks for looking into it.