Bug #118335 for Net-SSH-Perl: Net::SSH::Perl 2.01 - available length miscalculation in Packet.pm

Tue Oct 11 18:03:34 2016 allan [...] oepping.com - Ticket created

CC:	allan-cpan [...] oepping.com
Subject:	Net::SSH::Perl 2.01 - available length miscalculation in Packet.pm - patch included
Date:	Tue, 11 Oct 2016 16:03:21 -0600
To:	bug-Net-SSH-Perl [...] rt.cpan.org
From:	Allan Oepping <allan [...] oepping.com>

Version string in Packet.pm: Packet.pm,v 1.25 2008/09/24 19:21:20 turnstep Exp In Packet.pm in fuction read_poll_ssh2 if $mac && $mac->enabled but not $mac->etm then $block_size bytes are also read from the $incoming buffer but the $incoming->length may not be large enough to include a full read of $stored_mac later in the code. I added the a return if the length including block_size is not available (Line after the comment "RETURN IF INSUFFICIENT BUF") and that seems to fix the issue. if ($mac && $mac->enabled) { if ($mac->etm) { $p_str = $incoming->bytes(0, $aadlen + $need, ''); $macbuf = $mac->hmac(pack("N", $seqnr) . $p_str); # remove packet length bytes substr($p_str,0,$aadlen,''); } else { #RETURN IF INSUFFICIENT BUF return if $incoming->length < $aadlen + $need + $authlen + $maclen + $block_size; # not etm mac, append bytes already decrypted above $buffer->append( $incoming->bytes(0, $block_size, '') ); $p_str = $incoming->bytes(0, $need, ''); } Unified diff/patch attached. Thanks, Allan Oepping

Message body is not shown because sender requested not to inline it.

Thu Oct 13 07:04:34 2016 schwigon [...] cpan.org - Correspondence added

Thanks! If by any chance you are able to try your patch against lkinley's github repository and send him a pull request, that would be great. lkinley is currently doing the actual work on this lib and I plan to merge and upload his new version soon. His repo is https://github.com/lkinley/Net-SSH-Perl This would help because I'm just co-maintaining the CPAN release but don't really know the internals. Thanks and kind regards, Steffen On Tue Oct 11 18:03:34 2016, allan@oepping.com wrote: Show quoted text

> Version string in Packet.pm: Packet.pm,v 1.25 2008/09/24 19:21:20 > turnstep Exp > > In Packet.pm in fuction read_poll_ssh2 if $mac && $mac->enabled but not > $mac->etm then $block_size bytes are also read from the $incoming buffer > but the $incoming->length may not be large enough to include a full read > of $stored_mac later in the code. > > I added the a return if the length including block_size is not available > (Line after the comment "RETURN IF INSUFFICIENT BUF") and that seems to > fix the issue. > > > if ($mac && $mac->enabled) { > if ($mac->etm) { > $p_str = $incoming->bytes(0, $aadlen + $need, ''); > $macbuf = $mac->hmac(pack("N", $seqnr) . $p_str); > # remove packet length bytes > substr($p_str,0,$aadlen,''); > } else { > #RETURN IF INSUFFICIENT BUF > return if $incoming->length < $aadlen + $need + $authlen > + $maclen + $block_size; > # not etm mac, append bytes already decrypted above > $buffer->append( $incoming->bytes(0, $block_size, '') ); > $p_str = $incoming->bytes(0, $need, ''); > } > > Unified diff/patch attached. > > Thanks, > > Allan Oepping

-- Steffen Schwigon <ss5@renormalist.net> Dresden Perl Mongers <http://dresden-pm.org/>

Thu Oct 13 07:04:34 2016 The RT System itself - Status changed from 'new' to 'open'

Mon Jun 12 02:33:29 2017 schwigon [...] cpan.org - Correspondence added

Hi, is this still an issue with the v2.12 that I just uploaded to CPAN? Thanks for checking. Kind regards, Steffen On Thu Oct 13 07:04:34 2016, SCHWIGON wrote: Show quoted text

> Thanks! > > If by any chance you are able to try your patch against lkinley's > github > repository and send him a pull request, that would be great. lkinley > is currently > doing the actual work on this lib and I plan to merge and upload his > new version soon. > > His repo is https://github.com/lkinley/Net-SSH-Perl > > This would help because I'm just co-maintaining the CPAN release but > don't really know the internals. > > Thanks and kind regards, > Steffen > > On Tue Oct 11 18:03:34 2016, allan@oepping.com wrote:

> > Version string in Packet.pm: Packet.pm,v 1.25 2008/09/24 19:21:20 > > turnstep Exp > > > > In Packet.pm in fuction read_poll_ssh2 if $mac && $mac->enabled but > > not > > $mac->etm then $block_size bytes are also read from the $incoming > > buffer > > but the $incoming->length may not be large enough to include a full > > read > > of $stored_mac later in the code. > > > > I added the a return if the length including block_size is not > > available > > (Line after the comment "RETURN IF INSUFFICIENT BUF") and that seems > > to > > fix the issue. > > > > > > if ($mac && $mac->enabled) { > > if ($mac->etm) { > > $p_str = $incoming->bytes(0, $aadlen + $need, ''); > > $macbuf = $mac->hmac(pack("N", $seqnr) . $p_str); > > # remove packet length bytes > > substr($p_str,0,$aadlen,''); > > } else { > > #RETURN IF INSUFFICIENT BUF > > return if $incoming->length < $aadlen + $need + $authlen > > + $maclen + $block_size; > > # not etm mac, append bytes already decrypted above > > $buffer->append( $incoming->bytes(0, $block_size, '') ); > > $p_str = $incoming->bytes(0, $need, ''); > > } > > > > Unified diff/patch attached. > > > > Thanks, > > > > Allan Oepping

-- Steffen Schwigon <ss5@renormalist.net> Dresden Perl Mongers <http://dresden-pm.org/>

Mon Jun 12 13:18:55 2017 allan [...] oepping.com - Correspondence added

Subject:	Re: [rt.cpan.org #118335] Net::SSH::Perl 2.01 - available length miscalculation in Packet.pm - patch included
Date:	Mon, 12 Jun 2017 11:18:35 -0600
To:	bug-Net-SSH-Perl [...] rt.cpan.org
From:	Allan Oepping <allan [...] oepping.com>

A connection worked, but the conditions of the original problem are no longer there, and running the original modules also did not present the problem. We are using it with the Net::SFTP module and one intermittent issue is fixed "Key verification failed for server host key" no longer occurs. But Net::SFTP depends on Net::SSH::Perl::Util SSH2MP.pm mp2bin and bin2mp and v2.12 no longer contains that module, so I had to modify Net::SFTP in order to test. It looks like you are also the maintainer for that package. I will run it in rotation and see if the problem presents itself again. Thanks, Allan On 06/12/2017 12:33 AM, Steffen Schwigon via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=118335 > > > Hi, > > is this still an issue with the v2.12 that I just uploaded to CPAN? > > Thanks for checking. > > Kind regards, > Steffen > > On Thu Oct 13 07:04:34 2016, SCHWIGON wrote:

>> Thanks! >> >> If by any chance you are able to try your patch against lkinley's >> github >> repository and send him a pull request, that would be great. lkinley >> is currently >> doing the actual work on this lib and I plan to merge and upload his >> new version soon. >> >> His repo is https://github.com/lkinley/Net-SSH-Perl >> >> This would help because I'm just co-maintaining the CPAN release but >> don't really know the internals. >> >> Thanks and kind regards, >> Steffen >> >> On Tue Oct 11 18:03:34 2016, allan@oepping.com wrote:

>>> Version string in Packet.pm: Packet.pm,v 1.25 2008/09/24 19:21:20 >>> turnstep Exp >>> >>> In Packet.pm in fuction read_poll_ssh2 if $mac && $mac->enabled but >>> not >>> $mac->etm then $block_size bytes are also read from the $incoming >>> buffer >>> but the $incoming->length may not be large enough to include a full >>> read >>> of $stored_mac later in the code. >>> >>> I added the a return if the length including block_size is not >>> available >>> (Line after the comment "RETURN IF INSUFFICIENT BUF") and that seems >>> to >>> fix the issue. >>> >>> >>> if ($mac && $mac->enabled) { >>> if ($mac->etm) { >>> $p_str = $incoming->bytes(0, $aadlen + $need, ''); >>> $macbuf = $mac->hmac(pack("N", $seqnr) . $p_str); >>> # remove packet length bytes >>> substr($p_str,0,$aadlen,''); >>> } else { >>> #RETURN IF INSUFFICIENT BUF >>> return if $incoming->length < $aadlen + $need + $authlen >>> + $maclen + $block_size; >>> # not etm mac, append bytes already decrypted above >>> $buffer->append( $incoming->bytes(0, $block_size, '') ); >>> $p_str = $incoming->bytes(0, $need, ''); >>> } >>> >>> Unified diff/patch attached. >>> >>> Thanks, >>> >>> Allan Oepping

> >

Thu Aug 24 02:22:51 2017 schwigon [...] cpan.org - Correspondence added

I think the trouble with SFTP got sorted out in the meantime, so I'm closing this ticket. If I'm wrong please re-open it. Thanks for your report. Steffen -- Steffen Schwigon <ss5@renormalist.net> Dresden Perl Mongers <http://dresden-pm.org/>

Thu Aug 24 02:22:53 2017 schwigon [...] cpan.org - Status changed from 'open' to 'resolved'

Bug #118335 for Net-SSH-Perl: Net::SSH::Perl 2.01 - available length miscalculation in Packet.pm - patch included