DCIT / perl-CryptX

https://metacpan.org/pod/CryptX
Other
35 stars 23 forks source link

DSA/RSA/ECC/DH key2hash - hexadecimal numbers are missing leading zero #20

Closed allanoepping closed 8 years ago

allanoepping commented 8 years ago

Net::SSH's Perl/Key/DSA.pm sub equal calls key2hash on the key of the connected and known host and then compares p,q,g,y all match except for the y of the known host parameter has an extra '0' at the end making it 257 bytes. The known hosts entry is loaded via Net::SSH Perl/Key/DSA.pm's sub init using import_key.

allanoepping commented 8 years ago

I'm not sure if this is a CryptX bug or a problem with the way Net::SSH is initializing CryptX. Sorry if this is the wrong place to report this.

allanoepping commented 8 years ago

Here are the y parameters:

Active host: 4FBD403CB1E022A427593F306C5DC93A126BC9E24DA89516F5332F546A47E800A53537419978675C46FD84DB5C5A9D0A56499E4C030838CDF47957050BB0E2DFE3AE103AE565E8BCC05AE5282D3161C133C204F1AB8B64C5B0F4520419E93E782F8529C6AC0E0EDCD07E1849B3E8BAAEE9860372124E791AF5A04732CA6A118

known: 4FBD403CB1E022A427593F306C5DC93A126BC9E24DA89516F5332F546A47E800A53537419978675C46FD84DB5C5A9D0A56499E4C030838CDF47957050BB0E2DFE3AE103AE565E8BCC05AE5282D3161C133C204F1AB8B64C5B0F4520419E93E782F8529C6AC0E0EDCD07E1849B3E8BAAEE9860372124E791AF5A04732CA6A1180

Thanks.

karel-m commented 8 years ago

I am not sure how the appended zero happened.

There is one a bit unfortunate/unexpected feature of key2hash (not only DSA but also RSA, ECC and DH) - exported hex numbers are missing leading zeroes so it might have odd number of hexadecimal digits (as in your example 4FBD40...CA6A118 is 255 hexadecimal digits representing 256 bytes starting with 0x04 0xFB 0xD4).

Which is fine unless somebody tries to feed it to pack("H*", $key->key2hash->{y}) which I see at several places in Net/SSH/Perl/Key/DSA.pm

Perhaps patching key2hash so that it adds missing leading 0 might fix it.

Cc: @lkinley

allanoepping commented 8 years ago

Oddly enough calling dump_public on the two keys produces results that do "eq" each other. Since dump_public calls as_blob(b64 encoding the result) which calls $b->put_bignum2_bytes(pack('H*',$hash->{y}));

karel-m commented 8 years ago

OK, key2hash needs a fix. Unfortunately it is a XS function so it'll need some work.

allanoepping commented 8 years ago

sub xkey2hash { my ($self) = @_; my $hash = $self->key2hash(); if(length($hash->{y}) % 2) { $hash->{y} = '0' . $hash->{y}; } return $hash; }

Used the above as a workaround and it fixed this issue, but I had to remove the offending entry from the known_hosts2 file.

karel-m commented 8 years ago

I have push an initial idea here https://github.com/DCIT/perl-CryptX/commit/39f9a6fcca5b5393d7d3653c2526a119db8bd9a8 - to test try to install the latest CryptX from github.

But it only makes sure that the exported hex form has even number of digits/chars.

In theory a member of key2hash can have a zero byte at the beginning e.g. 00E587AB.. which will be exported as E587AB.. (I am not sure if it might be an issue).

allanoepping commented 8 years ago

Thanks!

That fixes the issue.

karel-m commented 8 years ago

Great. For consistency I should also "fix" ECC key2hash part exporting curve parameters.

karel-m commented 8 years ago

fix released in CryptX-0.038