Hi Michael,
Michael Harris wrote:
> I discovered what I think might be a bug in sftp.c;
Unfortunately it's a problem in libssh2 design and implementation
which runs deeper than that.
> I thought I would canvas it here before putting it into the bug
> tracker to make sure it is not just a misunderstanding on my part.
Hmm, I'm not sure if this is in the tracker, feel free to create a
ticket for it.
> The API for libssh2_sftp_write says that if the return value is
> positive, it is the number of bytes actually written.
Your understanding of the API is correct, but the implementation is
only correct if you try to write less than 32768 - n bytes at a time,
where n is the bytes needed by libssh2 internally to create the
actual packets that will be sent to the kernel.
This is indeed a serious bug, and there doesn't seem to be a simple
fix for it. Until recently the limit was at 1024 because of simple
(yet tedious to find) bugs within the implementation, but we worked
to increase the buffer size that libssh2 could work with in order to
allow for better throughput.
The current recommendation for a workaround is to never give libssh2
more than 32500 bytes to send out, through any function. Then the
implementation should not exhibit any problem.
> I think the problem is in the following part of sftp.c
..
> My packet_len is 32768+handle_len(4)+25 = 32797, and
> _libssh2_channel_write can only write 32768 bytes at a time,
The transport layer (ie. _libssh2_channel_write) also suffers from
the deficiency outlined above.
> so rc is 32768 (the actual amount of the packet written so far).
This isn't true unfortunately. The logic in _libssh2_channel_write
simply fails when more data is coming in from the application than
there is room for in buffers within the function. The fact that
_write() doesn't have the same type of API as system write() (return
number of bytes consumed) further complicates this for the
implementation.
There is a disconnect in API and in implementation between number of
bytes submitted by application, and number of bytes sent to operating
system on the socket. One idea to resolve this situation is described
in http://libssh2.stuge.se/ticket/161 but I also want to fix the
current implementation so that the library actually works correctly
regardless of implementation details.
> But anyway, until the whole packet is sent, we probably can't
> conclude that any of the file is written anyway?
That's correct. SFTP is a packet protocol so until the full packet
has been received, no data at all will have been received.
I believe that passing 32768 bytes into libssh2 will derail the SSH
protocol communication and in effect make the library lock up at some
point. (In non-blocking mode it would just return EAGAIN ad infitium.)
> So maybe line 1474 should return EAGAIN instead?
Unfortunately I believe this issue requires more work throughout the
library.
> This Communication is Confidential.
Please discuss this matter with your security officer. Consider how
Ericsson looks when sending confidential messages to a mailing list
which is open to the public, as well as archived and indexed by
search engines on the world wide web. As I am sure you agree, it is
absolute nonsense. Please challenge your security officer with the
importance of correctly classifying confidential and public
communication within email. I would suggest automatic encryption for
confidential communication - a policy I have seen in other companies
- rather than relying on third party adherence to some disclaimer.
Kind regards from Malmö Sweden, a stone's throw from Lund :)
//Peter
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2010-02-27