Commit Graph

2646 Commits

Author SHA1 Message Date
Jakub Jelen
6cf5f0e340 sftp: Cap maximum SFTP write
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>
2023-09-15 18:04:36 +02:00
Jakub Jelen
4e56c5c956 poll: Avoid passing other events to callbacks when called recursively
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>
2023-09-13 14:10:30 +02:00
Jakub Jelen
f86bec735b poll: Drop all events except POLLOUT when called recursively
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>
2023-09-13 14:10:30 +02:00
Sahana Prasad
adfb2bcc75 Revert the control flow callback in commit
6f029598c7

Signed-off-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2023-09-08 08:51:45 +02:00
Tom Deseyn
6a64f5a11a Allow sending data payloads of remote_maxpacket length.
Signed-off-by: Tom Deseyn <tom.deseyn@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2023-09-08 08:48:40 +02:00
Tom Deseyn
60db508054 channel: use a larger window size to increase receive throughput.
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>
2023-09-01 19:32:12 +02:00
Simon Josefsson
504faca67a crypto: Add ssh_crypto_free().
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>
2023-08-25 15:13:31 +02:00
Eshan Kelkar
85d7cc5cf2 misc.c : Introduce ssh_writen()
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>
2023-08-22 14:22:14 +02:00
Eshan Kelkar
9c8486aafb misc.c : Introduce ssh_readn()
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>
2023-08-22 14:22:14 +02:00
Simon Josefsson
3417161b81 mbedcrypto: Make bignum_bn2dec() return char*.
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>
2023-08-22 14:05:29 +02:00
Jakub Jelen
00bafe0a82 channels: Do not be so picky about the extended data type
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>
2023-08-07 11:08:03 +02:00
Jakub Jelen
d0ffe917fb channels: Fix reading stderr from channels
broken in 4b8db203b0

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-08-07 11:08:03 +02:00
Ahsen Kamal
15dbf3ace7 add control master and path option
Signed-off-by: Ahsen Kamal <itsahsenkamal@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-08-02 15:37:17 +02:00
Eshan Kelkar
5726af1956 priv.h : Add compatibility code for Windows
Compatibility code for mapping open, read, write, close and unlink
to _open, _read, _write, _close and _unlink respectively on Windows
was repeated in a lot of .c files.

This commit adds that compatibility code to include/libssh/priv.h
and removes it from the .c files (while ensuring that those .c
files include priv.h) so that the compatibility code stays in one
place, can be maintained easily and can be added easily to another
source file by including priv.h in that file.

Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2023-07-11 17:47:34 +02:00
Jakub Jelen
fe80f47b0a sftpserver: Add missing allocation check that might cause NULL dereference
Originally reported by Wei Chong Tan <shellcurity at protonmail.com>

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-07-11 17:45:45 +02:00
Jakub Jelen
cba1dfac6c gssapi: Rewrite allocation check to avoid zero_structpt
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-07-11 17:45:39 +02:00
Jakub Jelen
6e016c1c54 misc: Reformat allocation checks
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-07-11 17:45:35 +02:00
Jakub Jelen
96faaeea03 pcap: Reformat
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-07-11 17:45:31 +02:00
Jakub Jelen
a7f85944c8 ecdh_crypto: Avoid memory leak on error condition
CID 1034574

Thanks coverity

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-06-28 12:25:58 +02:00
Jakub Jelen
f8a7571a91 scp: Make sure arguments are sane
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-06-28 12:22:27 +02:00
Jakub Jelen
9c19ba7f33 channels: Avoid out-of-bounds writes
CID 1470005

Thanks coverity

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-06-28 12:14:44 +02:00
Jakub Jelen
ebea7d9023 session: Avoid potential null dereference on low-memory conditions
CID 1500478

Thanks coverity

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-06-28 10:43:51 +02:00
Jakub Jelen
62f013ae96 sftpserver: Check return value ssh_buffer_get_u32
CID 1513157

Thanks coverity

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-06-28 10:18:06 +02:00
Jakub Jelen
20dcb8b830 sftpserver: Reformat remaining condition
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-06-28 10:17:47 +02:00
Jakub Jelen
9709a466d7 sftpserver: Set OOM only if allocation fails
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-06-28 10:17:25 +02:00
Jakub Jelen
ddfc2e08b9 sftpserver: Initialize pointers
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-06-28 10:14:15 +02:00
Jakub Jelen
9847f3f638 Deprecate SSH_BIND_OPTIONS_{RSA,ECDSA}KEY in favor of generic HOSTKEY
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-06-26 15:10:52 +02:00
Jakub Jelen
5b2957f0a7 sftpserver: Avoid unreachable code line
CID 1513155

Thanks coverity

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
2023-06-22 17:29:39 +02:00
Jakub Jelen
812ba3b717 sftp: Check return value of ssh_buffer_add_data
CID 1513156

Thanks coverity

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
2023-06-22 17:29:39 +02:00
Jakub Jelen
4b8db203b0 channels: Refactor channel_rcv_data, check for errors and report more useful errors
CID 1513157

Thanks coverity

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
2023-06-22 17:29:39 +02:00
Jakub Jelen
a45b9938fe channels: Reformat channel_rcv_change_window
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
2023-06-20 15:20:01 +02:00
Jakub Jelen
e6d2b6c713 sftpserver: Avoid leaking fd and dir on allocation error
CID 1513160 and CID 1513159

Thanks coverity

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
2023-06-20 15:20:01 +02:00
Jakub Jelen
7009df7b04 sftp: Refromat sftp_open, sftp_opendir
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
6bebac10b7 sftp: Avoid leaking sftp attributes when opening file
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
c024280669 sftp: Clarify the order of arguments for symlink
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
baa18d3712 sftp: Properly check bounds of incoming packet
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
cd54390188 sftpserver: Standardize logging on errors
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
f09b475c4b sftpserver: Properly handle empty files and EOF while reading
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
3fc30681f4 sftpserver: Properly handle mkdir modes
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
2a0d6d854a sftpserver: Properly handle open modes
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
423bb3c8f0 Deprecate untested function sftp_server_init
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
492317efe9 Rename the sftp_process_init_packet()
... to better describe the function, which is only replying to the client with
our version and extensions.

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
1fe98800d2 sftpserver: Implement stat and realpath
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
7427090a9f sftpserver: Improve logging
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
ff897165ca Reformat most of the sftpserver.c
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
d0bfab2549 sftpserver: Fix reading and writing if buffering occurs
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
691105e93b Remove needless new symbols and add required to API
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
5ea54c8159 sftpserver: Move duplicate code handling SFTP operations to library
These can be replaced by user-provided functions when needed.

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
Jakub Jelen
0affa5d705 sftp: Remove duplicate code handling packet types
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00
tatataeki
8104c19013 sftp: fix problems in sftp APIs and example
Signed-off-by: tatataeki <shengzeyu19_98@163.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2023-06-06 10:54:31 +02:00