"Copying a file to the remote computer" is not an appropriate heading
for a subsection that describes how to open a remote file and write
"Hello World" to it.
That heading is not appropriate as the subsection does not show how
to copy a file from local to remote computer. Hence, this commit changes
that heading to a more suitable heading.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
benchmarks_async_sftp_aio_up() has been added in
tests/benchmarks/bench_sftp.c to obtain the performance
metrics of a upload using the low level async sftp aio
api.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
benchmarks_async_sftp_aio_down() has been added in
tests/benchmarks/bench_sftp.c to obtain the performance
metrics of a download using the low level async sftp aio
api.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
benchmark code present in tests/benchmarks/ directory
was linked with libssh dynamically due to which it
could use only the functions exposed in the public API
of libssh.
To be able to use those functions in the benchmark
code which are a part of libssh api but not a part of
the public api for libssh (examples of such functions
are ssh_list api functions), the benchmark code needs
to be linked statically to libssh.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This commit fixes connect_host() such that if
ssh_new() fails, connect_host() fails and provides
the reason for failure. Prior to this commit if
ssh_new() failed, connect_host() failed but did
not provide the reason for failure to connect to
the host.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
According to libssh coding conventions, function
return values must not be directly passed to if-
or while- conditions. This rule was not being followed
in connect_host(). A helper variable has been introduced
which stores the return code of the functions which
is then passed to the if- conditions.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
torture_sftp_aio.c has been added in tests/client/ directory.
It contains torture_sftp_aio_read(), torture_sftp_aio_write()
and torture_sftp_aio_negative().
torture_sftp_aio_read() tests sftp_aio_begin_read() and
sftp_aio_wait_read() to perform async reads.
torture_sftp_aio_write() tests sftp_aio_begin_write() and
sftp_aio_wait_write() to perform async writes.
torture_sftp_aio_negative() performs negative tests on the
sftp aio read/write API.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The existing sftp async read api has two problems :
1. sftp_async_read() assumes that the value of the third
parameter count is same as the number of bytes requested
to read in the corresponding call to sftp_async_read_begin().
But the documentation of sftp_async_read() allows the value of
count parameter to be more than that requested length. If value
of count parameter is more than that requested length then
sftp_async_read() updates the file->offset incorrectly which
leads to further read/writes occuring from incorrect offsets.
The problem here is that sftp_async_read() doesn't know about
the number of bytes requested to read specified in the call to
sftp_async_read_begin(), and it wrongly assumes the value
of its count parameter (which is actually the size of the buffer
to store the read data) to be the same as the number of bytes
requested to read.
2. sftp_async_read_begin() returns an uint32_t type value type
casted to int as a request identifier, whereas sftp_async_read()
expects an uint32_t type value as a request identifier. Due to this
the user has to typecast the identifier returned by sftp_async_read_begin()
from int to uint32_t and then pass it to sftp_async_read(). This
type casting is cumbersome for the user and hence the approach is
not user-friendly.
This commit solves the above two problems by introducing a new
sftp aio api.
The sftp_aio_begin_*() functions in the api send an i/o request to
the sftp server and provide the caller a dynamically allocated
structure storing information about the sent request. Information
like number of bytes requested for i/o, id of sent request etc is
stored in the structure.
That structure should be provided to the sftp_aio_wait_*() functions
in the api which wait for the response corresponding to the request whose
info is stored in the provided structure.
The libssh user is supposed to handle that structure through an
opaque type sftp_aio.
Since the structure stores the number of bytes requested for i/o,
sftp_aio_wait_*() knows about the number of bytes requested for i/o
(specified in the call to sftp_aio_begin_*()) and hence updates the
file->offset correctly solving problem #1 present in the existing
async api.
Since the structure provided by sftp_aio_begin_*() (containing the
request id) is supplied to sftp_aio_wait_*(), no casting of id's
needs to be done by the user solving problem #2 of the existing
async api.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Currently the sftp api code is limited to sftp.c, sftpserver.c
In future it can be required to add new sftp related APIs which
are present in their own separate source files instead of adding
their code to the already large sftp.c file.
Those new hypothetical or existing (in sftpserver.c) sftp API
functions present in the source files other than sftp.c will
need to call certain functions present in sftp.c which are not
provided in the public api as they are for internal use (by other
sftp related functions) only. Some of these sftp.c functions have
external linkage, some of them don't and cannot be currently accessed
outside sftp.c
This commit :
1. Moves such functions along with the functions they depend on
from sftp.c to a new file sftp_common.c, to seperate them out
from other sftp api functions.
2. Makes necessary changes to make required functions visible
outside sftp_common.c
3. Uses the header file sftp_priv.h for necessary declarations
(and not sftp.h) since these functions are not to be provided
in the public sftp api.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
ssh_channel_read() reads the data into the buffer, but doesn't
append a '\0' after it. When the buffer is asserted to be equal to
a string further in the test, the assertion could fail if the byte
after the data stored in the buffer doesn't contain '\0' (and it mayn't)
This commit appends a '\0' after the data read into the buffer before
comparing it with a string.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The curl does not do any (or enough) chunking when writing large files using the
sftp_write() function which causes some servers to choke [1]. The simplest
solution is to limit the SFTP packet size according the SFTP specification
recommendation which is 32768 B and not write more.
This means the function will not write the whole amount of data it was asked to
write and the calling applications are required to handle the return values
correctly.
More complicated solution would be to send several SFTP packet from the single
sftp_write() function by iterating over the all data passed.
The next improvement in the long term should be respecting the value reported by
the server in the limits@openssh.com extension, which specifies the maximum
packet size and reads/writes explicitly (if supported).
[1] https://github.com/curl/curl/pull/11804
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Some architectures (s390x) provide different poll events such as POLLHUP in case
the remote end closed the connection (and they keep reporting this forever).
This is an issue when the user provided callback registering this event as an
error and tries to send some reply (for example EOF) using
`ssh_channel_send_eof()` which will lead to infinite recursion and sefgaults.
This was not solved by the 30b5a2e33b because the
POLLHUP event is not provided by the poll in events bitfield, but only reported
by the poll in revents bit field thus we need to filter these events later on
when the poll is recursively.
Fixes#202
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The FD locking was modified in 30b5a2e33b but it
caused some weird issues on s390x in Debian tests, which were getting POLLHUP,
causing infinite recursion while the callback tried to close socket.
Previously, the lock blocked only the POLLIN events as we believed these were
the only events we could get recursively that could cause issues. But it looks
like more sane behavior will be blocking everything but POLLOUT to allow the
buffers to be flushed.
Fixes#202
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The window size controls how much data the peer can send before
we send back a message to to increase the window.
This changes the default window from 1.28MB to 2MiB. 2MiB matches
the OpenSSH default session size.
The code is also refactored to grow the windows on code paths
where data is consumed, and move the condition that checks
if the growing the window is needed into the grow method.
Signed-off-by: Tom Deseyn <tom.deseyn@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
We use string(APPEND) from version 3.4 for 5 years and CMake is deprecating
support for versions before 3.5 so bumping one more version.
Fixes: #209
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Aris Adamantiadis <aris@0xbadc0de.be>
The intention is that this releases memory allocated by the crypto
library, for functions like bignum_bn2hex() and bignum_bn2dec().
Consequently, ssh_gcry_bn2dec and ssh_mbedcry_bn2num should use
gcry_malloc() and mbedtls_calloc() respectively to allocate
memory since it will/should be released by ssh_crypto_free() so
that the internal APIs are consistent between crypto libraries.
Signed-off-by: Simon Josefsson <simon@josefsson.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
A call to write() may perform a short write on a local file.
To avoid short writes, ssh_writen() can be used.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
A call to read() may peform a short read from a local file even when
sufficient data is present in the file. ssh_readn() can be used
instead of read() to avoid such short reads.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
This aligns it with libgcrypt/OpenSSL backends which uses char*.
It also aligns mbedcrypto's bignum_bn2hex() to use an unsigned
cast just like OpenSSL backend.
Signed-off-by: Simon Josefsson <simon@josefsson.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
assume stderr by default and log only warning in case the data type is
non-standard.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
