Crypt::OpenSSL::DSA bug
Brad Fitzpatrick
brad at danga.com
Sun May 22 13:32:26 PDT 2005
Okay, good fix attached.
Attached patch brings Crypt::OpenSSL::DSA from 0.11 to 0.12, fixes all
strlen/SvLEN issues, and updates the test suite to validate a bunch of
null and non-null sha1 digests against both itself and the openssl binary
found on your machine.
I've mailed the maintainer the patch.
Future versions of Net::OpenID::Consumer will depend on
Crypt::OpenSSL::DSA 0.12.
- Brad
On Sun, 22 May 2005, Karl Koscher wrote:
>
> >Let me know how it works for you.
> >
> >
> It's failing on everything. It looks like SvLEN(dgst) says that it's 21
> bytes long, not 20 bytes. If I force it to use 20 byte digests, it
> works, at least for verification. Perhaps SvLEN is including a final NUL
> character?
>
> - Karl
>
>
-------------- next part --------------
diff -ur Crypt-OpenSSL-DSA-0.11/Changes Crypt-OpenSSL-DSA-0.12/Changes
--- Crypt-OpenSSL-DSA-0.11/Changes 2003-01-06 11:05:38.000000000 -0800
+++ Crypt-OpenSSL-DSA-0.12/Changes 2005-05-22 13:19:05.000000000 -0700
@@ -1,5 +1,10 @@
Revision history for Perl extension Crypt::OpenSSL::DSA.
+0.12 May 22, 2005
+ - Removed all use of strlen() in DSA.xs so signatures with nulls,
+ as commonly generated with sha1, could be signed/verified
+ (Brad Fitzpatrick <brad at danga.com>)
+
0.11 Jan 6th, 2003
- Added -DPERL5 to Makefile.PL required for perl-5.8/gcc-3.2
diff -ur Crypt-OpenSSL-DSA-0.11/DSA.xs Crypt-OpenSSL-DSA-0.12/DSA.xs
--- Crypt-OpenSSL-DSA-0.11/DSA.xs 2002-09-25 17:15:02.000000000 -0700
+++ Crypt-OpenSSL-DSA-0.12/DSA.xs 2005-05-22 13:14:52.000000000 -0700
@@ -40,17 +40,19 @@
DSA_free(dsa);
DSA *
-generate_parameters(CLASS, bits, seed = "")
+generate_parameters(CLASS, bits, seed = NULL)
char * CLASS
int bits
- char *seed
+ SV * seed
PREINIT:
DSA * dsa;
int seed_len = 0;
+ char * seedpv = NULL;
CODE:
- if(seed)
- seed_len = strlen(seed);
- dsa = DSA_generate_parameters(bits, seed, seed_len, NULL, NULL, NULL, NULL);
+ if (seed) {
+ seedpv = SvPV(seed, seed_len);
+ }
+ dsa = DSA_generate_parameters(bits, seedpv, seed_len, NULL, NULL, NULL, NULL);
if (!dsa)
croak(ERR_reason_error_string(ERR_get_error()));
RETVAL = dsa;
@@ -66,14 +68,17 @@
RETVAL
DSA_SIG *
-do_sign(dsa, value)
+do_sign(dsa, dgst)
DSA * dsa
- char *value
+ SV * dgst
PREINIT:
DSA_SIG * sig;
char * CLASS = "Crypt::OpenSSL::DSA::Signature";
+ char * dgst_pv = NULL;
+ int dgst_len = 0;
CODE:
- if (!(sig = DSA_do_sign((const unsigned char *)value, strlen(value), dsa))) {
+ dgst_pv = SvPV(dgst, dgst_len);
+ if (!(sig = DSA_do_sign((const unsigned char *) dgst_pv, dgst_len, dsa))) {
croak("Error in dsa_sign: %s",ERR_error_string(ERR_get_error(), NULL));
}
RETVAL = sig;
@@ -83,14 +88,20 @@
SV *
sign(dsa, dgst)
DSA * dsa
- char *dgst
+ SV * dgst
PREINIT:
unsigned char *sigret;
unsigned int siglen;
+ char * dgst_pv = NULL;
+ int dgst_len = 0;
CODE:
siglen = DSA_size(dsa);
sigret = malloc(siglen);
- if (!(DSA_sign(0, (const unsigned char *)dgst, strlen(dgst), sigret, &siglen, dsa))) {
+
+ dgst_pv = SvPV(dgst, dgst_len);
+ /* warn("Length of sign [%s] is %d\n", dgst_pv, dgst_len); */
+
+ if (!(DSA_sign(0, (const unsigned char *) dgst_pv, dgst_len, sigret, &siglen, dsa))) {
croak("Error in DSA_sign: %s",ERR_error_string(ERR_get_error(), NULL));
}
RETVAL = newSVpvn(sigret, siglen);
@@ -101,22 +112,33 @@
int
verify(dsa, dgst, sigbuf)
DSA * dsa
- char *dgst
+ SV *dgst
SV *sigbuf
- CODE:
- RETVAL = DSA_verify(0, dgst, strlen(dgst), SvPV(sigbuf, SvLEN(sigbuf)), SvLEN(sigbuf), dsa);
+ PREINIT:
+ char * dgst_pv = NULL;
+ int dgst_len = 0;
+ char * sig_pv = NULL;
+ int sig_len = 0;
+ CODE:
+ dgst_pv = SvPV(dgst, dgst_len);
+ sig_pv = SvPV(sigbuf, sig_len);
+ RETVAL = DSA_verify(0, dgst_pv, dgst_len, sig_pv, sig_len, dsa);
if (RETVAL == -1)
- croak("Error in DSA_verify: %s",ERR_error_string(ERR_get_error(), NULL));
+ croak("Error in DSA_verify: %s",ERR_error_string(ERR_get_error(), NULL));
OUTPUT:
RETVAL
int
do_verify(dsa, dgst, sig)
DSA *dsa
- char *dgst
+ SV *dgst
DSA_SIG *sig
+ PREINIT:
+ char * dgst_pv = NULL;
+ int dgst_len = 0;
CODE:
- RETVAL = DSA_do_verify(dgst, strlen(dgst), sig, dsa);
+ dgst_pv = SvPV(dgst, dgst_len);
+ RETVAL = DSA_do_verify(dgst_pv, dgst_len, sig, dsa);
OUTPUT:
RETVAL
Only in Crypt-OpenSSL-DSA-0.12/: Makefile.old
diff -ur Crypt-OpenSSL-DSA-0.11/README Crypt-OpenSSL-DSA-0.12/README
--- Crypt-OpenSSL-DSA-0.11/README 2003-01-06 11:06:56.000000000 -0800
+++ Crypt-OpenSSL-DSA-0.12/README 2005-05-22 13:20:35.000000000 -0700
@@ -1,4 +1,4 @@
-Crypt::OpenSSL::DSA version 0.11
+Crypt::OpenSSL::DSA version 0.12
================================
DESCRIPTION
Only in Crypt-OpenSSL-DSA-0.12/: crypt-openssl-fixes.txt
diff -ur Crypt-OpenSSL-DSA-0.11/lib/Crypt/OpenSSL/DSA.pm Crypt-OpenSSL-DSA-0.12/lib/Crypt/OpenSSL/DSA.pm
--- Crypt-OpenSSL-DSA-0.11/lib/Crypt/OpenSSL/DSA.pm 2003-01-06 11:06:35.000000000 -0800
+++ Crypt-OpenSSL-DSA-0.12/lib/Crypt/OpenSSL/DSA.pm 2005-05-22 13:21:15.000000000 -0700
@@ -7,7 +7,7 @@
use vars qw(@ISA $VERSION);
@ISA = qw(DynaLoader);
-$VERSION = '0.11';
+$VERSION = '0.12';
bootstrap Crypt::OpenSSL::DSA $VERSION;
diff -ur Crypt-OpenSSL-DSA-0.11/t/sign.t Crypt-OpenSSL-DSA-0.12/t/sign.t
--- Crypt-OpenSSL-DSA-0.11/t/sign.t 2002-09-25 17:16:56.000000000 -0700
+++ Crypt-OpenSSL-DSA-0.12/t/sign.t 2005-05-22 13:16:10.000000000 -0700
@@ -4,8 +4,9 @@
use Test;
use Crypt::OpenSSL::DSA;
+use Digest::SHA1 qw(sha1);
-BEGIN { plan tests => 30 }
+BEGIN { plan tests => 111 }
my $message = "foo bar";
@@ -101,6 +102,51 @@
ok($dsa5->verify($message, $dsa_sig3), 1);
ok($dsa6->verify($message, $dsa_sig3), 1);
+# tests added by Brad Fitzpatrick in response to bugs found by Karl Koscher
+
+# test suite now requires openssl binary available to sanity check our results
+my $OPEN_SSL = `which openssl` || "/usr/bin/openssl";
+chomp $OPEN_SSL;
+ok(-x $OPEN_SSL, 1);
+
+my $dsa_pub = Crypt::OpenSSL::DSA->read_pub_key("dsa.pub.pem");
+my $dsa_priv = Crypt::OpenSSL::DSA->read_priv_key("dsa.priv.pem");
+
+my %done; # { zero => $ct, nonzero => $ct }
+
+my $to_do = 500;
+my $of_each = 20;
+
+for (1..$to_do) {
+ my $plain = "This is test number $_";
+ $message = sha1($plain);
+ my $type = ($message =~ /\x00/) ? "zero" : "nonzero";
+ next if $done{$type}++ >= $of_each;
+
+ my $sig = $dsa_priv->sign($message);
+
+ my $we_think = $dsa_pub->verify($message, $sig);
+ my $openssl_think = openssl_verify("dsa.pub.pem", $sig, $plain);
+
+ ok($we_think, 1);
+ ok($openssl_think, 1);
+}
+
unlink("dsa.param.pem");
unlink("dsa.priv.pem");
unlink("dsa.pub.pem");
+
+sub openssl_verify {
+ my ($public_pem_file, $sig, $msg_plain) = @_;
+
+ require File::Temp;
+ my $sig_temp = new File::Temp(TEMPLATE => "tmp.signatureXXXX") or die;
+ my $msg_temp = new File::Temp(TEMPLATE => "tmp.msgXXXX") or die;
+ syswrite($sig_temp,$sig);
+ syswrite($msg_temp,$msg_plain);
+ # FIXME: shutup openssl from spewing to STDOUT the "Verification OK".
+ my $rv = system("openssl", "dgst", "-dss1", "-verify", $public_pem_file, "-signature", "$sig_temp", "$msg_temp");
+ return 0 if $rv;
+ return 1;
+}
+
Only in Crypt-OpenSSL-DSA-0.12/: tmp.msgT1mI
Only in Crypt-OpenSSL-DSA-0.12/: tmp.signatureu51u
More information about the yadis
mailing list