If the function that extracts/computes the public key from a private key
fails the errors it reports were masked by the function calling it. This
patch modifies the key extraction function to return errors using
_libssh_error() function. The error messages are tweaked to contain
reference to the failed operaton in addition to the reason.
* AUTHORS: - add my name
* libgcrypt.c: _libssh2_pub_priv_keyfile(): - return a more verbose
error using
_libssh2_error() func.
* openssl.c: - modify call graph of _libssh2_pub_priv_keyfile() to use
_libssh2_error for error reporting();
* userauth.c: - tweak functions calling _libssh2_pub_priv_keyfile() not
to shadow error messages
Some SFTP servers send SFTP packets larger than 40000. Since the limit
is only present to avoid insane sizes anyway, we can easily bump it.
The define was formerly in the public header libssh2_sftp.h but served
no external purpose and was moved into the source dir.
Bug: http://www.libssh2.org/mail/libssh2-devel-archive-2011-11/0004.shtml
Reported by: Michael Harris
Documentation for libssh2_knownhost_checkp() and related functions
states that the last argument is filled with data if non-NULL.
"knownhost if set to non-NULL, it must be a pointer to a 'struct
libssh2_knownhost' pointer that gets filled in to point to info about a
known host that matches or partially matches."
In this function ext is dereferenced even if set to NULL, causing
segfault in applications not needing the extra data.
In function knownhost_add, memory is alocated for a new entry. If normal
alocation is used, memory is not initialized to 0 right after, but a
check is done to verify if correct key type is passed. This test is done
BEFORE setting the memory to null, and on the error path function
free_host() is called, that tries to dereference unititialized memory,
resulting into a glibc abort().
* knownhost.c - knownhost_add(): - move typemask check before alloc
When cross compiling to Windows, libssh2.h include Windows header files
with upper case filenames : BaseTsd.h and WinSock2.h.
These files have lowercase names with mingw-w64 (iirc, it's the same with
mingw). And as on Windows, being lowercase or uppercase does not matter.
Make sure we don't clear or reset static structs after first init so
that they work fine even when used from multiple threads. Init the
structs in the global init.
Help and assistance by: John Engstrom
Fixes#229 (again)
make_ctr_evp() is changed to take a struct pointer, and then each
_libssh2_EVP_aes_[keylen]_ctr function is made to pass in their own
static struct
Reported by: John Engstrom
Fixes#229
Set read_state back to idle before trying to send anything so that if
the state somehow is wrongly set.
Also, avoid such a case of confusion by resetting the read_state when an
sftp handle is closed.
Removed the automatic window_size adjustments from
_libssh2_channel_read() and instead all channel readers must now make
sure to enlarge the window sizes properly themselves.
libssh2_channel_read_ex() - the public function, now grows the window
size according to the requested buffer size. Applications can still opt
to grow the window more on demand. Larger windows tend to give higher
performance.
sftp_read() now uses the read-ahead logic to figure out a window_size.
A returned READ packet that is short will now only reduce the
offset.
This is a temporary fix as it is slightly better than the previous
approach but still not very good.
When receiving more data than what the window size allows on a
particular channel, make sure that the window size is adjusted in that
case too. Previously it would only adjust the window in the non-error
case.
When seeking to a new position, flush the packetlist and buffered data
to prevent already received or pending data to wrongly get used when
sftp-reading from the new offset within the file.
In the case where a read packet has been received from the server, but
the entire contents couldn't be copied to the user-buffer, the data is
instead buffered and copied to the user's buffer in the next invocation
of sftp_read(). When that "extra" copy is made, the 'offset' pointer was
not advanced accordingly.
The biggest impact of this flaw was that the 'already' variable at the
top of the function that figures out how much data "ahead" that has
already been asked for would slowly go more and more out of sync, which
could lead to the file not being read all the way to the end.
This problem was most noticable in cases where the application would
only try to read the exact file size amount, like curl does. In the
examples libssh2 provides the sftp read function is most often called
with a fixed size large buffer and then the bug would not appear as
easily.
This bug was introduced in the SFTP rewrite in 1.2.8.
Bug: http://curl.haxx.se/mail/lib-2011-08/0305.htmlhttp://www.libssh2.org/mail/libssh2-devel-archive-2011-08/0085.shtml
A sftp session failed with error "failure establishing ssh session" on
Solaris and HP-UX. Sometimes the first recv() function call sets errno
to ENOENT. In the man pages for recv of Solaris and HP-UX the error
ENOENT is not documented.
I tested Solaris SPARC and x86, HP-UX i64, AIX, Windows and Linux.
After an error occurs in libssh2_scp_recv() or libssh2_scp_send(), the
function libssh2_session_last_error() would return
LIBSSH2_ERROR_SOCKET_NONE on error.
Bug: http://trac.libssh2.org/ticket/216
Patch by: "littlesavage"
Fixes#216
Added libraries needed to link whether using openssl dynamically or
statically
Added LIBSSH2DEBUG define to debug versions to enable tracing
URL: http://trac.libssh2.org/ticket/215
Patch by: Mark Smith
Someone on IRC pointed out that we don't have these documented so I
wrote up a first set based on the information in the wiki:
http://trac.libssh2.org/wiki/KeepAlive
Stop using the $VERSION variable as it seems to be magically used by
autoconfig itself and thus gets set to the value set in AC_INIT()
without us wanting that. $LIBSSH2VER is now the libssh2 version as
detected.
Reported by: Paul Howarth
Bug: http://www.libssh2.org/mail/libssh2-devel-archive-2011-04/0008.shtml
Starting now, the NEWS file is generated from git using the git2news.pl
script. This makes it always accurate and up-to-date, even for daily
snapshots etc.
Previously the code assumed either a single host name or a hostname,ip-address pair. However, according to the spec [1], there can be any number of comma separated host names or IP addresses.
[1] http://www.openbsd.org/cgi-bin/man.cgi?query=sshd&sektion=8
Fix the bug that libssh2 could not connect if the sftp server
sends data before sending the version string.
http://tools.ietf.org/html/rfc4253#section-4.2
"The server MAY send other lines of data before sending the version
string. Each line SHOULD be terminated by a Carriage Return and Line
Feed. Such lines MUST NOT begin with "SSH-", and SHOULD be encoded
in ISO-10646 UTF-8 [RFC3629] (language is not specified). Clients
MUST be able to process such lines."
The buffer for the decompression (remote.comp_abstract) is initialised
in time when it is needed. With this fix decompression is disabled when
the buffer (remote.comp_abstract) is not initialised.
Bug: http://trac.libssh2.org/ticket/200
As pointed out in bug #206, if a second invoke of libssh2_sftp_read()
would shrink the buffer size, libssh2 would go nuts and send out read
requests like crazy. This was due to an unsigned variable turning
"negative" by some wrong math, and that value would be the amount of
data attempt to pre-buffer!
Bug: http://trac.libssh2.org/ticket/206
If asked to read data into a buffer and the buffer is too small to hold
the data, this function now returns an error instead of as previously
just copy as much as fits.
As discussed on the mailing list, it was wrong for win64 and using the
VC-provided type is the safest approach instead of second- guessing
which one it should be.
Added crypto.h that is the unified header to include when using crypto
functionality. It should be the only header that needs to adapt to the
underlying crypto library in use. It provides the set of prototypes that
are library agnostic.
Pass a NULL pointer for the publickey parameter of
libssh2_userauth_publickey_fromfile and
libssh2_userauth_hostbased_fromfile functions. In this case, the
functions recompute the public key from the private key file data.
This is work done by Jean-Louis CHARTON
<Jean-Louis.CHARTON@oikialog.com>, then adapted by Mark Smith and
slightly edited further by me Daniel.
WARNING: this does leave the feature NOT WORKING when libssh2 is built
to use libgcrypt instead of OpenSSL simply due to lack of
implementation.
By using a new separate struct for incoming SFTP packets and not sharing
the generic packet struct, we can get rid of an unused field and add a
new one dedicated for holding the request_id for the incoming
package. As sftp_packet_ask() is called fairly often, a "mere" integer
comparison is MUCH faster than the previous memcmp() of (typically) 5
bytes.
Make sure that we cleanup remainders when the handle is closed and when
the subsystem is shutdown.
Existing flaw: if a single handle sends packets that haven't been
replied to yet at the time when the handle is closed, those packets will
arrive later and end up in the generic packet brigade queue and they
will remain in there until flushed. They will use unnecessary memory,
make things slower and they will ruin the SFTP handling if the
request_id counter ever wraps (highly unlikely to every happen).
The SFTP read function now does transfers the same way the SFTP write
function was made to recently: it creates a list of many outgoing
FXP_READ packets that each asks for a small data chunk. The code then
tries to keep sending read request while collecting the acks for the
previous requests and returns the received data.
The loop that waits for remote.close to get set may end up looping
forever since session->socket_state gets set to
LIBSSH2_SOCKET_DISCONNECTED by the packet_add() function called from the
transport_read() function and after having been set to
LIBSSH2_SOCKET_DISCONNECTED, the transport_read() function will only
return 0.
Bug: http://trac.libssh2.org/ticket/198
Split off libssh2_sftp_seek64 from the libssh2_sftp_seek man page, and
mentioned that we consider the latter deprecated. Also added a mention
about the dangers of doing seek during writing or reading.
The new SFTP write code caused a regression as the seek function no
longer worked as it didn't set the write position properly.
It should be noted that seeking is STRONGLY PROHIBITED during upload, as
the upload magic uses two different offset positions and the multiple
outstanding packets etc make them sensitive to change in the midst of
operations.
This functionality was just verified with the new example code
sftp_append. This bug was filed as bug #202:
Bug: http://trac.libssh2.org/ticket/202
I ran SFTP upload tests against localhost. It showed that to make the
app reach really good speeds, I needed to do a little code tweak and
change MAX_SFTP_OUTGOING_SIZE from 4000 to 30000. The tests I did before
with the high latency tests didn't show any real difference whatever I
had that size set to.
This number is the size in bytes that libssh2 cuts off the large input
buffer and sends off as an individual sftp packet.
This is an example that is very similar to sftp_write_nonblock.c, with
the exception that this uses
1 - a larger upload buffer
2 - a sliding buffer mechnism to allow the app to keep sending lots of
data to libssh2 without having to first drain the buffer.
These are two key issues to make libssh2 SFTP uploads really perform
well at this point in time.
The attempts made to have _libssh2_channel_write() accept larger pieces
of data and split up the data by itself into 32700 byte chunks and pass
them on to channel_write() in a loop as a way to do faster operations on
larger data blocks was a failed attempt.
The reason why it is difficult:
The API only allows EAGAIN or a length to be returned. When looping over
multiple blocks to get sent, one block can get sent and the next might
not. And yet: when transport_send() has returned EAGAIN we must not call
it again with new data until it has returned OK on the existing data it
is still working on. This makes it a mess and we do get a much easier
job by simply returning the bytes or EAGAIN at once, as in the EAGAIN
case we can assume that we will be called with the same arguments again
and transport_send() will be happy.
Unfortunately, I think we take a small performance hit by not being able
to do this.
This is a new example snippet. The code is largely based on ssh2_exec,
and is written by Tommy Lindgren. I edited it into C90 compliance and to
conform to libssh2 indent style and some more.
When a piece of data is sent from the send_existing() function we must
make the parent function return afterwards. Otherwise we risk that the
parent function tries to send more data and ends up getting an EGAIN for
that more data and since it can only return one return code it doesn't
return info for the successfully sent data.
As this change is a regression I now added a larger comment explaining
why it has to work like this.
Starting now, we unconditionally use the internal replacement functions
for send() and recv() - creatively named _libssh2_recv() and
_libssh2_send().
On errors, these functions return the negative 'errno' value instead of
the traditional -1. This design allows systems that have no "natural"
errno support to not have to invent it. It also means that no code
outside of these two transfer functions should use the errno variable.
Some checks are better done in _libssh2_channel_write just once per
write instead of in channel_write() since the looping will call the
latter function multiple times per _libssh2_channel_write() invoke.
The SFTP handle struct now buffers number of acked bytes that haven't
yet been returned. The way this is used is as following:
1. sftp_write() gets called with a buffer of let say size 32000. We
split 32000 into 8 smaller packets and send them off one by one. One of
them gets acked before the function returns so 4000 is returned.
2. sftp_write() gets called again a short while after the previous one,
now with a much smaller size passed in to the function. Lets say 8000.
In the mean-time, all of the remaining packets from the previous call
have been acked (7*4000 = 28000). This function then returns 8000 as all
data passed in are already sent and it can't return any more than what
it got passed in. But we have 28000 bytes acked. We now store the
remaining 20000 in the handle->u.file.acked struct field to add up in
the next call.
3. sftp_write() gets called again, and now there's a backlogged 20000
bytes to return as fine and that will get skipped from the beginning
of the buffer that is passed in.
When SCP send or recv fails, it gets a special message from the server
with a warning or error message included. We have no current API to
expose that message but the foundation is there. Removed unnecessary use
of session struct fields.
I added size checks in several places. I fixed the code flow to be easier
to read in some places.
I removed unnecessary zeroing of structs. I removed unused struct fields.
We don't like magic numbers in the code. Now the acceptable failure
codes sent in the SSH_MSG_CHANNEL_OPEN_FAILURE message are added as
defined values in the private header file.
This function now only returns EAGAIN if a lower layer actually returned
EAGAIN to it. If nothing was acked and no EAGAIN was received, it will
now instead return 0.
If _libssh2_wait_socket() gets called but there's no direction set to
wait for, this causes a "hang". This code now detects this situation,
set a 1 second timeout instead and outputs a debug output about it.
SFTP packets come as [32 bit length][payload] and the code didn't
previously handle that the initial 32 bit field was read only partially
when it was read.
While setting up the session, ssh tries to determine the type of
encryption method it can use for the session. This requires looking at
the keys offered by the remote host and comparing these with the methods
supported by libssh2 (rsa & dss). To do this there is an iteration over
the array containing the methods supported by libssh2.
If there is no agreement on the type of encryption we come to the 3rd
entry of the hostkeyp array. Here hostkeyp is valid but *hostkep is
NULL. Thus when we dereference that in (*hostkeyp)->name there is a
crash
There were some chances that they would cause -1 to get returned by
public functions and as we're hunting down all such occurances and since
the underlying functions do return valuable information the code now
passes back proper return codes better.
The man page clearly says it returns 1 for "already authenticated" but
the code said non-zero. I changed the code to use 1 now, as that is also
non-zero but it gets the benefit that it now matches the documentation.
Using 1 instead of non-zero is better for two reasons:
1. We have the opportunity to introduce other return codes in the future for
things like error and what not.
2. We don't expose the internal bitmask variable value.
First I wanted to free the memory in session_free() but then
I had still memory leaks because in my test case the function
userauth_keyboard_interactive() is called twice. It is called
twice perhaps because the server has this authentication
methods available: publickey,gssapi-with-mic,keyboard-interactive
The keyboard-interactive method is successful.
I found an undocumented public function and we can't have it like
that. The description here is incomplete, but should serve as a template
to allow filling in...
The sftp_write function shouldn't assume that the buffer pointer will be
the same in subsequent calls, even if it assumes that the data already
passed in before haven't changed.
The sftp structs are now moved to sftp.h (which I forgot to add before)
sftp_write was rewritten to split up outgoing data into multiple packets
and deal with the acks in a more asynchronous manner. This is meant to
help overcome latency and round-trip problems with the SFTP protocol.
Neither _libssh2_channel_write nor sftp_write now have the 32500 size
limit anymore and instead the channel writing function now has its own
logic to send data in multiple calls until everything is sent.
The new function takes two data areas, combines them and sends them as a
single SSH packet. This allows several functions to allocate and copy
less data.
I also found and fixed a mixed up use of the compression function
arguments that I introduced in my rewrite in a recent commit.
We now allow libssh2_session_flag() to enable compression with a new
flag and I added documentation for the previous LIBSSH2_FLAG_SIGPIPE
flag which I wasn't really aware of!
As a zero return code from channel_read() is not an error we must make
sure that the SCP functions deal with that properly. channel_read()
always returns 0 if the channel is EOFed already so we check for EOF
after 0-reads to be able to return error properly.
In the transport functions we avoid a strcmp() now and just check a
boolean instead.
The compress/decompress function's return code is now acknowledged and
used as actual return code in case of failures.
The function libssh2_session_startup() is now considered deprecated due
to the portability issue with the socket argument.
libssh2_session_handshake() is the name of the replacement.
When a call to _libssh2_transport_write() succeeds, we must return from
_libssh2_channel_write() to allow the caller to provide the next chunk
of data.
We cannot move on to send the next piece of data that may already have
been provided in this same function call, as we risk getting EAGAIN for
that and we can't return information both about sent data as well as
EAGAIN. So, by returning short now, the caller will call this function
again with new data to send.
Replaced -1/SOCKET_NONE errors with appropriate error defines instead.
Made the verbose trace output during banner receiving less annoying for
non-blocking sessions.
In an attempt to make the trace output less cluttered for non-blocking
sessions the error function now avoids calling the debug function if the
error is the EAGAIN and the session is non-blocking.
LIBSSH2_SOCKET_NONE (-1) should no longer be used as error code as it is
(too) generic and we should instead use specific and dedicated error
codes to better describe the error.
The well known and used ssh server Dropbear has a maximum SSH packet
length at 32768 by default. Since the libssh2 design current have a
fixed one-to-one mapping from channel_write() to the packet size created
by transport_write() the previous limit of 32768 in the channel layer
caused the transport layer to create larger packets than 32768 at times
which Dropbear rejected forcibly (by closing the connection).
The long term fix is of course to remove the hard relation between the
outgoing SSH packet size and what the input length argument is in the
transport_write() function call.
When parsing the SCP protocol and verifying that the data looks like a
valid file name, byte values over 126 must not be consider illegal since
UTF-8 file names will use such codes.
Reported by: Uli Zappe
Bug: http://www.libssh2.org/mail/libssh2-devel-archive-2010-08/0112.shtml
No idea why we had this ifdef at all but MSVC, MingW32, Watcom
and Borland all have no sys/uio.h header; so if there's another
Win32 compiler which needs it then it should be added explicitely
instead of this negative list.
Since libssh2 often sets LIBSSH2_ERROR_EAGAIN internally before
_libssh2_wait_socket is called, we can decrease some amount of
confusion in user programs by resetting the error code in this function
to reduce the risk of EAGAIN being stored as error when a blocking
function returns.
As reported on the mailing list, the code path using poll() should
multiple seconds with 1000 to get milliseconds, not divide!
Reported by: Jan Van Boghout
The condition around the ssize_t typedef depended on both LIBSSH2_WIN32
*and* _MSC_VER being defined when it should be enough to depend on
_MSC_VER only. It also makes it nicer so libssh2-using code builds fine
without having custom defines.
As was pointed out in bug #182, we must not return failure from
_libssh2_channel_free() when _libssh2_channel_close() returns an error
that isn't EAGAIN. It can effectively cause the function to never go
through, like it did now in the case where the socket was actually
closed but socket_state still said LIBSSH2_SOCKET_CONNECTED.
I consider this fix the right thing as it now also survives other
errors, even if making sure socket_state isn't lying is also a good
idea.
Fixed a compiler warning I introduced previously when checking input
arguments more. I also added a check for the other pointer to avoid NULL
pointer dereferences.
Make use of the EVP interface for the AES-funktion. Using this method
supports the use of different ENGINES in OpenSSL for the AES function
(and the direct call to the AES_encrypt should not be used according to
openssl.org)
There was a buffer overflow waiting to happen when a debug message was
longer than 1536 bytes.
Thanks to Daniel who spotted that there was a problem with the message
length passed to a trace handler also after commit
0f0652a309.
In KEXINIT messages, the client and server agree on, among other
things, whether to use compression. This method agreement occurs
in src/kex.c's kex_agree_methods() function. However, if
compression is enabled (either client->server, server->client, or
both), then the compression layer is initialized in
kex_agree_methods() -- before NEWKEYS has been received.
Instead, the initialization of the compression layer should
happen after NEWKEYS has been received. This looks to occur
insrc/kex.c's diffie_hellman_sha1(), which even has the comment:
/* The first key exchange has been performed,
switch to active crypt/comp/mac mode */
There, after NEWKEYS is received, the cipher and mac algorithms
are initialized, and that is where the compression should be
initialized as well.
The current implementation fails if server->client compression is
enabled because most server implementations follow OpenSSH's
lead, where compression is initialized after NEWKEYS. Since the
server initializes compression after NEWKEYS, but libssh2
initializes compression after KEXINIT (i.e. before NEWKEYS), they
are out of sync.
Reported in bug report #180
The packet length calculated in src/userauth.c's
userauth_hostbased_fromfile() function is too short by 4 bytes;
it forgets to add four bytes for the length of the hostname.
This causes hostbased authentication to fail, since the server
will read junk data.
verified against proftpd's mod_sftp module
This functions get the method length by looking at the first 32
bit of data, and I now made it not accept method lengths that are
longer than the whole data set is, as given in the dedicated
function argument.
This was detected when the function was given bogus public key
data as an ascii string, which caused the first 32bits to create
a HUGE number.
Sending in NULL as the primary pointer is now dealt with by more
public functions. I also narrowed the userauth.c code somewhat to
stay within 80 columns better.
The LIBSSH2_DEBUG macro, defined in libssh2_priv.h, incorrectly uses the
function variable ssh_msg_disconnect when it should use ssh_msg_debug.
This shows that the LIBSSH2_CALLBACK_DEBUG callback never has worked...
The individual man pages for macros now show the full convenience
macro as defined, and then the man page for the actual function
only shows the function.
sftp_write() now limits how much data it gets at a time even more
than before. Since this function creates a complete outgoing
packet based on what gets passed to it, it is crucial that it
doesn't create too large packets.
With this method, there's also no longer any problem to use very
large buffers in your application and feed that to libssh2. I've
done numerous tests now with uploading data over SFTP using 100K
buffers and I've had no problems with that.
The select() is just to make it nicer so that it doesn't
crazy-loop on EAGAIN. The buffer size thing is mostly to verify
that this really work as supposed.
Transfer timing is just a minor thing, but it can just as well be
there and help us time and work on performance easier using out
of the box examples.
As pointed out in bug report #173, this module basically never
used _libssh2_error() which made it work inconstently with other
parts of the libssh2 code base. This is my first take at making
this code more in line with the rest.
If an application accidentally provides a NULL handle pointer to
the channel or sftp public functions, they now return an error
instead of segfaulting.
'last_errno' holds to the error code from the SFTP protocol and
since that is 32 bits on the wire there's no point in using a
long for this internally which is larger on some platforms.
agent->ops gets initialized by the libssh2_agent_connect() call
but we need to make sure that we don't segfault even if a bad
sequence of function calls is used.
Passing an invalid public key to libssh2_userauth_publickey_fromfile_ex
triggered an assertion. Replaced this with a runtime check that rejects
obviously invalid key data.
Alexander Lamaison filed bug #172
(http://trac.libssh2.org/ticket/172), and pointed out that SFTP
init would do bad if the session isn't yet authenticated at the
time of the call, so we now check for this situation and returns
an error if detected. Calling sftp_init() at this point is bad
usage to start with.
In order to increase portability of this example, I'm bringing
the inclusion of libssh2_config.h back, and I also added an
require that header for this example to compile.
I also made all code lines fit within 80 columns.
As the long-term goal is to get rid of the extensive set of
macros from the API we can just as well start small by not adding
new macros when we add new functions. Therefore we let the
function be libssh2_sftp_statvfs() plainly without using an _ex
suffix.
I also made it use size_t instead of unsigned int for the string
length as that too is a long-term goal for the API.
As pointed out by Grubsky Grigory <g.grubsky@securitycode.ru>, I
made a mistake when I added the _libssh2_store_str() call before
and I made a slightly different patch than what he suggested.
Based purely on taste.
libssh2_knownhost_checkp took 0 as a magic port number that indicated
a 'generic' check should be performed. However, 0 is a valid port
number in its own right so this commit changes the magic value to any
negative int.
OpenSSH has ways to add hosts to the knownhosts file that include
a specific port number which makes the key associated with only
that specific host+port pair. libssh2 previously did not support
this, and I was forced to add a new function to the API to
properly expose this ability to applications:
libssh2_knownhost_checkp()
To *add* such hosts to the knownhosts file, you make sure to pass
on the host name in that manner to the libssh2_knownhost_addc()
function.
There was some stub-like parts of an implementation for
implementing kex language negotiation that caused clang-analyzer
to warn and as it did nothing I've now removed the dead code.
The clang-analyzer report made it look into this function and
I've went through it to remove a potential use of an
uninitialized variable and I also added some validation of input
data received from the server.
In general, lots of more code in this file need to validate the
input before assuming it is correct: there are servers out there
that have bugs or just have another idea of how to do the SFTP
protocol.
The errors mentioned in this man page are possible return codes
but not necessarily the only return codes that this can return.
Also reformatted the typ prototypes somewhat.
To get the blocking vs non-blocking to work as smooth as possible
and behave better internally, we avoid using the external
interfaces when calling functions internally.
Renamed a few internal functions to use _libssh2 prefix when not
being private within a file, and removed the libssh2_ for one
that was private within the file.
This was triggered by a clang-analyzer complaint that turned out
to be valid, and it made me dig deeper and fix some generic non-
blocking problems I disovered in the code.
While cleaning this up, I moved session-specific stuff over to a
new session.h header from the libssh2_priv.h header.
By making 'data' and 'data_len' more local in several places in
this file it will be easier to spot how they are used and we'll
get less risks to accidentally do bad things with them.
clang-analyzer pointed out how 'data' could be accessed as a NULL
pointer if the wrong state was set, and while I don't see that
happen in real-life the code flow is easier to read and follow by
moving the LIBSSH2_FREE() call into the block that is supposed to
deal with the data pointer anyway.
clang-analyzer pointed out how 'data' could be accessed as a NULL
pointer if the wrong state was set, and while I don't see that
happen in real-life the code flow is easier to read and follow by
moving the LIBSSH2_FREE() call into the block that is supposed to
deal with the data pointer anyway.
clang-analyzer pointed this out as a "Pass-by-value argument in
function call is undefined" but while I can't see exactly how
this can ever happen in reality I think a little check for safety
isn't such a bad thing here.
The previously existing libssh2_scp_send_ex() function has no way
to send files that are larger than 'size_t' which on 32bit
systems mean 4GB. This new API uses a libssh2_int64_t type and
should thus on most modern systems be able to send enormous
files.
I'll introduce a new internal function set named
_libssh2_store_u32
_libssh2_store_u64
_libssh2_store_str
That can be used all through the library to build binary outgoing
packets. Using these instead of the current approach removes
hundreds of lines from the library while at the same time greatly
enhances readability. I've not yet fully converted everything to
use these functions.
I've converted LOTS of 'unsigned long' to 'size_t' where
data/string lengths are dealt with internally. This is The Right
Thing and it will help us make the transition to our
size_t-polished API later on as well.
I'm removing the PACKET_* error codes. They were originally
introduced as a set of separate error codes from the transport
layer, but having its own set of errors turned out to be very
awkward and they were then converted into a set of #defines that
simply maps them to the global libssh2 error codes instead. Now,
I'l take the next logical step and simply replace the PACKET_*
defines with the actual LIBSSH2_ERROR_* defines. It will increase
readability and decrease confusion.
I also separated packet stuff into its own packet.h header file.
We reserve ^libssh2_ for public symbols and we use _libssh2 as
prefix for internal ones. I fixed the intendation of all these
edits with emacs afterwards, which then changed it slightly more
than just _libssh2_error() expressions but I didn't see any
obvious problems.
This function no longer has any special purpose code for the
single entry case, as it was pointless.
The previous code would overflow the buffers with an off-by-one
in case the file name or longentry data fields received from the
server were exactly as long as the buffer provided to
libssh2_sftp_readdir_ex.
We now make sure that libssh2_sftp_readdir_ex() ALWAYS zero
terminate the buffers it fills in.
The function no longer calls the libssh2_* function again, but
properly uses the internal sftp_* instead.
When we ignore the EAGAIN from the transport layer within channel_write, we
now drain the outgoing transport layer buffer so that remainders in that
won't cause any problems in the next invoke of _libssh2_transport_write()
When sending data in a loop, we must not return EAGAIN if we
managed to send data in an earlier round. This was reported in
bug #126 => http://libssh2.stuge.se/ticket/126
When _libssh2_channel_write() is asked to send off 9 bytes, the
code needs to deal with the situation where less than 9 bytes
were sent off and prepare to send the remaining piece at a later
time.
As reported in bug report #166http://libssh2.stuge.se/ticket/166
by 'ptjm', the maximum window size must be less crazy for libssh2
to do better with more server implementations. I did not do any
testing to see how this changes raw SCP performance, but the
maximum window size is still almost 4MB. This also has the upside
that libssh2 will use less memory.
commit 7317edab61 cleared the outbound
blocking bit when send_existing() returned PACKET_NONE and *ret=0, as
opposed to before even calling send_existing(), but because *ret=1 when
sending parts 2..n of an existing packet, the bit would only be cleared
when calling libssh2_transport_write() for a new packet.
Clear the direction flag after the final part of a packet has been sent.
Suyog Jadhav pointed out that when receiving a window adjust to
a channel not found, the code would reference a NULL pointer.
Now it will instead output a message about that fact.
Comments in known_hosts file were not handle properly. They were parsed as
part of the key causing key matching to return a mismatch if the entry had a
comment. This adds a new API function that takes an optional comment and
changes libssh2_knownhost_readline to parse the comment as pass it to the
new function.
Fixes#164.
when _libssh2_packet_requirev() returns an error when waiting for
SSH_MSG_USERAUTH_SUCCESS or SSH_MSG_USERAUTH_FAILURE, it is an
error and it should be treated as such
libssh2_error() no longer allocates a string and only accepts a const
error string. I also made a lot of functions use the construct of
return libssh2_error(...) instead of having one call to
libssh2_error() and then a separate return call. In several of those
cases I then also changed the former -1 return code to a more
detailed one - something that I think will not change behaviors
anywhere but it's worth keeping an eye open for any such.
Sending SSH_MSG_CHANNEL_CLOSE without channel EOF is explicitly allowed
in RFC 4254, but some non-conforming servers will hang or time out when
the channel is closed before EOF.
Other common clients send and receive EOF before closing, there are no
drawbacks, and some servers need it to work correctly.
The libssh2 API calls should set the last error code and a message when
returning a failure by calling libssh2_error. This changeset adds these
calls to the libssh2_knownhost_* API as well as libssh2_base64_decode.
This change also makes libssh2_error into a function rather than a macro.
Its implementation is moved to misc.c. This function returns the error
code passed to it allowing callers to return the error value directly
without duplicating the error code.
When removing a known host, libssh2_knownhost_del would remove the node from the linked list, free its memory and then overwrite the struct parameter (which indicated which node to remove) with 0. However, this struct is actually allocated within the just-freed node meaning we're writing to freed memory. This made Windows very upset.
The fix is simply to overwrite the struct first before freeing the memory.
To make sure the public API is functional and that the
BLOCK_ADJUST_ERRNO() macro works correctly we MUST make sure to
call libssh2_error() when we return errors.
Mr anonymous in bug #125 pointed out that the userauth_keyboard_interactive()
function does in fact assign the same pointer a second time to a new allocated
buffer without properly freeing the previous one, which caused a memory leak.
To allow the libssh2_session_last_error() function to work as
documented, userauth_password() now better makes sure to call
libssh2_error() everywhere before it returns error.
Pointed out by mr anonymous in bug #128
all #defined macros in the public headers are considered to be part
of the API and I've generated individual man pages for each of them
to A) make it easier to figure out what each function/macro actually
is for so that automated lookups work better and for B) make sure we
have all public functions document (both macros and functions) to
make it easier for us to work away from all the macros in a future
release.
Fix memoary leak: if there was an "output" still allocated when a
session was torn down it needs to be freed in session_free()
Patch by Yoichi Iwaki in bug #2929647
Building libssh2-1.2.3 on Tru64 fails at line 48 and 166 because socklen_t
isn't defined on Tru64 unless _POSIX_PII_SOCKET is defined.
This patch updates configure.ac to add -D_POSIX_PII_SOCKET when building
on Tru64 platform(s).
Solaris builds of libssh2-1.2.3 failed on both x64 and UltraSPARC
platforms because of two problems:
1) src/agent.c:145 sun is a reserved word when using the SUNWspro compiler
2) example/direct_tcpip.c:84 INADDR_NONE is not defined
Neither libssh2_userauth_password_ex() nor
libssh2_userauth_keyboard_interactive_ex() would return a login failure
error if the server responded with a SSH_MSG_USERAUTH_FAILURE, instead
you would see whatever previous error had occurred, typically
LIBSSH2_ERROR_EAGAIN.
This patch changes error code -18 to LIBSSH2_ERROR_AUTHENTICATION_FAILED
and makes LIBSSH2_ERROR_PUBLICKEY_UNRECOGNIZED an alias for
LIBSSH2_ERROR_AUTHENTICATION_FAILED. In addition, new logic in
userauth_password() properly handles SSH_MSG_USERAUTH_FAILURE and both
this function and userauth_keyboard_interactive() now properly return
LIBSSH2_ERROR_AUTHENTICATION_FAILED.
The trace context is actually a bitmask so that tracing output can be
controlled by setting a bitmask using libssh2_trace(). However, the logic
in libssh2_debug() that converted the context to a string was using the
context value as an array index. Because the code used a bounds check on
the array, there was never a danger of a crash, but you would certainly
either get the wrong string, or "unknown".
This patch adds a lookup that iterates over the context strings and uses
it's index to check for the corresponding bit in the context.
The libssh2_trace_sethandler() call allows the user to handle the output of libssh2 rather than having it written to stderr. This patch updates libssh2_trace_sethandler() to allow a user-defined void* context value to be passed back to the output handler.
buildconf copies the template to example/ and configure makes sure
to generate a proper file from it and the direct_tcpip.c example
is the first one to use it - to make sure it builds fine on more
paltforms
userauth_publickey_fromfile() reads the key from a
file using file_read_publickey() which returns two
allocated strings, the decoded key and the key
method (such as "ssh-dss"). The latter can be
derived from the former but returning both avoids a
later allocation while doing so.
Older versions of userauth_publickey_fromfile() used
this method string directly but when
userauth_publickey() was factored out of
userauth_publickey_fromfile() it derived the method
from the key itself. This resulted in the method
being allocated twice.
This fix, which maintains the optimisation that
avoids an extra allocation, changes
userauth_publickey() so it doesn't allocate and
derive the method when userauth_pblc_method already
has a value.
Signed-off-by: Alexander Lamaison <awl03@doc.ic.ac.uk>
Commit 70b199f476 introduced a parsing
bug in file_read_publickey() which made the algorithm name contain an
extra trailing space character, breaking all publickey authentication.
While this is code not currently in use, it is part of the generic linked
list code and since I found the error I thought I'd better fix it since we
might bring in this function into the code one day.
In case of failure we must make sure that the data we return
doesn't point to a memory area already freed. Reported anonymously
in the bug report #2910103.
Commit 683aa0f6b5 made send_existing() send
more than just the second part of a packet when the kernel did not accept
the full packet, but the function still overlooked the SSH protocol
overhead in each packet, often 48 bytes.
If only the last few bytes of a packet remained, then the packet would
erroneously be considered completely sent, and the next call to write
more data in the session would return a -39 error.
DSA signatures consist of two 160-bit integers called r and s. In ssh-dss
signature blobs r and s are stored directly after each other in binary
representation, making up a 320-bit (40 byte) string. (See RFC4253 p14.)
The crypto wrappers in libssh2 would either pack r and s incorrectly, or
fail, when at least one integer was small enough to be stored in 19 bytes
or less.
The patch ensures that r and s are always stored as two 160 bit numbers.
When libssh2_transport_write() is called to continue sending a
partially sent packet the write direction flag must not be cleared
until the previous packet has been completely sent, or the app would
hang if the packet still isn't sent completely, since select() gets
called by the internal blocking emulation layer in libssh2 but would
then not be watching the socket for writability.
Clear the flag only once processing of previous packet data is
complete and a new packet is about to be prepared.
The forward accepting was not done right before, and the
packet_queue_listener function didn't assign a necessary
variable. All fixed by Juzna. I (Daniel) modified the
forward_accept() change somewhat.
First, win32/msvcproj.{head,foot} are now committed with CRLF line endings,
and .gitattributes specifies that these should not be changed on checkout or
commit. These are win32 files so it makes sense to store them with native
line endings.
Second, the rules for generating libssh2.dsp and libssh2.vcproj are changed
so that the full file contents passes through awk, which strips all CR and
then prints each line with one CRLF line ending. Stripping CR is important
to avoid CRCRLF in case the input already comes with CRLF.
The channel layer sends packets using the transport layer, possibly
calling _libssh2_transport_write() many times for each packet.
The transport layer uses the send_existing() helper to send out any
remaining parts of previous packets before a new packet is started.
The bug made send_existing() consider the entire packet sent as soon as it
successfully sent the second part of a packet, even if the packet was not
completely done yet.
Neil Gierman's patch adds a gettimeofday() function for win32
for the libssh2_trace() functionality. The code originates from
cygwin and was put in the public domain by the author
Danny Smith <dannysmith@users.sourceforge.net>
If the channel is already at EOF or even closed at the end of the
libssh2_channel_read_ex() function and there's no data to return,
we need to signal that back. We may have gotten that info while
draining the incoming transport layer until EAGAIN so we must not
be fooled by that return code.
libssh2_channel_wait_closed() had a bad loop waiting for the
channel to close, as it could easily miss the info and then if
the socket would be silent from that moment the funtion would
hang if in blocking-mode or just return EAGAIN wrongly to the
app. The drain-transport loop now correctly checks if the close
has arrived.
Somehow I had completely missed to make the libssh2_scp_send/recv
functions support the blocking mode the correct way so when I
cleaned up things the other day blocking mode broke for them...
Fixed now.
This keeps all FILE* handling on the OpenSSL side of the DLL boundary avoiding crashes on Windows while removing the need for libssh2 to read the private key file into memory. This is now done by OpenSSL which is likely to do a better job of it.
I don't follow those particular guidelines myself so I think I'd
rather remove them here and keep my style than the opposite. As
I am the most frequent writer of code for the moment.
In theory we could split larger buffers into several smaller
packets to pass to transport_write(), but for now we instead only
deal with the first 32K in this call and assume the app will call
this function again with the rest! The 32K size is a
conservative limit based on the text in RFC4253 section 6.1.
internal code was changed to use that instead of wrongly using
libssh2_channel_read_ex(). Some files now need to include
channel.h to get this proto.
channel_read() calls libssh2_error() properly on transport_read()
failures
channel_read() was adjusted to not "invent" EAGAIN return code in
case the transport_read() didn't return it
channel_close() now returns 0 or error code, as
documented. Previously it would return number of bytes read in
the last read, which was confusing (and useless).
I made this change just to easier grep for "return .*EAGAIN" cases
as they should be very rare or done wrongly. Already worked to find
a flaw, marked with "TODO FIXME THIS IS WRONG" in channel.c. I also
fixed a few cases to become more general returns now when we have
more unified return codes internally.
it is important that only the transport layer can generate EAGAIN
error codes so that we limit where we need to set direction bits
and more. When the local window is too small to send data we simply
stop trying to send and (risk) returning zero in
_libssh2_channel_write()
_libssh2_channel_receive_window_adjust is the new replacement that
is both the correct internal version instead of the external API one,
and it has the return code flaw fixed. I also fixed more return
codes to pass long the correct error found.
On many places in the code there have been laziness return -1
statements lying around that should be fixed to return sensible
error codes. Here's a take at fixing a few offenders.
I added three new public error codes, and then modified the return
codes we use in the transport layer to use the generic error codes
so that there won't be any risk of internal confusions due to
different error code sets.
Steven Van Ingelgem introduces libssh2_socket_t as a generic socket
type to use internally to avoid compiler warnings and mistakes. Also,
the private struct iovec declaration for windows is now made to look
like the POSIX struct does.
Each SFTP file handle is now handled by the "mother-struct"
using the generic linked list functions. The goal is to move
all custom linked list code to use this set of functions.
I also moved the list declarations to the misc.h where they
belong and made misc.h no longer include libssh2_priv.h itself
since now libssh2_priv.h needs misc.h...
In misc.c I added a #if 0'ed _libssh2_list_insert() function
because I ended up writing one, and I believe we may need it here
too once we move over more stuff to use the _libssh2_list* family.
Alexander Lamaison tracked down that my previous commit broke SFTP
reads in some aspects. The reversion now gets back to always recv()
until EAGAIN is returned so that the code no longer treats a short
read as an indication that it is "enough for now".
The bad commit in particular had another independent change included,
which is to clear the direction-bits first in the transport read
and write functions, but this reversion does not revert that change.
Clearing those bits first is a good thing.
Passing a FILE* argument across a DLL boundary causes problems on Windows. Instead the keys are read into memory by libssh2 and passed to the OpenSSL functions as BIO* arguments.
I went over the NEWS and git log outputs and added all mentioned
contributors through the project's history. I want us to make an
effort to give credit to everyone who contributes, big or small.
George Neill pointed out that the C99 vararg marcro use was
wrong, and I edited his suggested patch slightly and unified
both prec99-GCC and C99 into the same macro.
I hope to maintain this file during development so that we also add
changes and bugfixes to it when we change things. Makes the daily
snapshots better and makes less of a hurdle when the release day
comes.
The anonymous bug report #2822910 pointed out that debugdump() was
stupidly called within the send_existing() function. At closer
inspection was the report not only right, but it also revealed
another problem to me: when the _libssh2_send() function returns
after sending only a part of the buffer, it would then misbehave.
This probably is very rare though, which must be the reason we
haven't seen a bigger problem with this.
* make sure the windowing code adapts better to slow situations so that it
doesn't then use as much memory as today
* Introduce a 'libssh2_socket_t' type for sockets in internal code
as we're already doing the correct check further down anyway there's no point
in doing the (wrong) check further up as well. Paul Veldkamp pointed this out.
Makefile changes to generate MSVS project files
- I integrated the libssh2_lib.dsp and libssh2_dll.dsp into a single
libssh2.dsp with different targets for lib vs dll
- Since I run Visual Studio 2008 with VC++9 I did not do vcproj files since
I would have newer vc8proj.head|foot than what others will be running
- My patch only has changes to Makefile.am's. I noticed that Makefile.in's
are included in the daily snapshots but they should be generated from the
.am's
- The 3 new files are msvcproj.head and msvcproj.foot for the beginning and
ending of the dsp file, and a new Makefile.inc that will have the source and
header file names. NOTE: All new source files will need to be added to
Makefile.inc and NOT Makefile.am now.
- I moved the win32 dir before the include dir
- I modified the dsw file so it points to the new libssh2.dsp project file
functions were not given the correct data as argument. This is now fixed even
if I personally don't quite grasp why abstract is passed as a pointer to
pointer all over libssh2...
include the public libssh2.h header as it breaks the compile on window. I'll
adapt to this now, but in the long run I think we should rather fix the
includes so that we _can_ include the public headers properly.
after my initial cleanup that I posted to the list on May 26th 2009. It still
has a few ugly spots that should be cleaned up, but until then it's will at
least be found in the repo. For this reason I don't add this to the makefile.
is then properly thread-safe on that OS. These autuconf macros are straight
from the cURL project and were mostly written by Yang Tse. They were only
very slightly edited by me when imported to here.
know that when this define exists, the API exists. And the version number can
be used for run-time checks. 1.1.1 is not likely to be the release version as
I think we'll go with 1.2 instead but 1.1.1 OR LATER should still work.
application can get a pointer back to the internal representation of the host
it just added. Useful for example when the app wants to add a host, and then
convert that exact same host to a line for storing in a known host file.
'store' can also be set to NULL to simple not care.
host so we now only return pointers to structs instead of having the app
allocate a full struct
I moved the private struct definition into knownhosts.c instead of exposing it
wider in libssh2_priv.h
I thus modified the proto for two functions that previously used 'struct
libssh2_knownhost *' to receive data.
'sftpInit_channel' struct fields within the session struct are used. And made
sure to clear them both correctly when sftp_init() returns instead of at
shutdown time, as it must not touch them at shutdown time. This should make it
possible to properly make more than one SFTP handle.
transport layer better, to avoid draining the window size when sending large
packets. I also fixed the return code for it to return the number of bytes
handled in this single invoke (and not the cumulative amount).
much larger packages only cause internal problems and much larger allocations.
Also fix sftp_write() when _libssh2_channel_write() returns that a packet was
only partially sent as that is not an error.
Fixed a few error messages to more accurately point out the problem
in loops until it returns < 0 when we call it, as if we just call it once we
may drain the socket for data and then leave unused in-memory data that we
won't detect because the socket is back to idle...
until _after_ we're done as otherwise we might not get called again properly
to clean the entire thing since the close state is checked in
libssh2_channel_free
and introduced a transport.h header.
* Fixed the blocking mode to only change behavior not the actual underlying
socket mode so we now always work with non-blocking sockets. This also
introduces a new rule of thumb in libssh2 code: we don't call the
external function calls internally. We use the internal (non-blocking)
ones!
* libssh2_channel_receive_window_adjust2 was added and
libssh2_channel_receive_window_adjust is now deprecated
* Introduced "local" header files with prototypes etc for different parts
instead of cramming everything into libssh2_priv.h. channel.h is the
first.
properly after recv() and send() calls (that internally are now known as
_libssh2_recv() and _libssh2_send()) so that the API and more works fine on
windows too!
SCP recv and send deal with file names containing spaces (and other
shell-special letters) by quoting them before they are passed to the remote
server.
frequent, use a few less struct fields in the channel struct and improved
reading from the network with libssh2_packet_read(). I also modified the
windowing algorithm and now use a much larger window. This greatly enhances
SSH/SCP performance. I also increased the size of the buffer the transport
layer uses from 4k to 16K.
some tests:
* cut off "_ex" from several internal function names
* corrected some log outputs
* simplified libssh2_channel_read_ex() and made it much faster in the process
* cut out {{{ and }}} comments that were incorrect anyway
* fixed sftp_packet_ask() to return the correct packet by using memcmp() and
not strncmp()
* fixed mkdir()'s wait for packet to use the correct request_id - it
semi-worked previously because strncmp() in sftp_packet_ask() made it
match far too easily.
* took away the polling functionality from sftp_packet_ask() since it wasn't
used
libssh2_channel_x11_req_ex() function that made it produce a crappy random
chunk of data. Peter Stuge improved the fix to not do out-of-boundary
writes. I (Daniel Stenberg) replaced the snprintf() with a plain sprintf()
since the size argument wasn't adding anything anyway.
Made the libssh2_sftp_open_ex() deal with servers that first responds with
STATUS OK and then sends the actual HANDLE. It seems openssh does this at
times and it screwed things up. To me it seems like a spec violation...
is private in this file only and a shorter name is nicer
Removed a "flush" of the data in sftp_packet_requirev() that now seems to have
made SFTP operations a lot more reliable. It didn't make much sense to have it
there but if someone can present a reason for one I figure we should carefully
investigate one and only do it conditionally where/when needed.
libssh2_session_block_directions() which returns a bitmask for what
directions the connection blocks. It is to be used applications that use
non-blocking sockets and when a libssh2 function returns
LIBSSH2_ERROR_EAGAIN this function can be used to figure out in which
direction the socket would block and thus it can wait for the socket to
again be ready for communication in that direction before it calls libssh2
again.
libssh2_poll() detecting that there was data ready to be read on
the socket. This is seen when small amounts of data are ready to
be read, typically 1-4 chars worth.
reading both normal and extended data when a non-zero value
is passed in as the 2nd parameter. This now matches the functionallity
described in the documentation.
reason why macros are evil: The string (which is allready char *) should be
cast into char * not the integer.
Signed-off-by: Sebastian Siewior <sebastian@breakpoint.cc>
\fIchannel\fP - Previously opened channel instance such as returned by
.BRlibssh2_channel_open_session(3)
.BRlibssh2_channel_open_ex(3)
\fIvarname\fP - Name of environment variable to set on the remote
channel instance.
@@ -25,8 +23,9 @@ channel instance.
\fIvalue_len\fP - Length of value parameter.
Set an environment variable in the remote channel's process space. Note that this does not make sense for all channel types and may be ignored by the server despite returning success.
Set an environment variable in the remote channel's process space. Note that
this does not make sense for all channel types and may be ignored by the
server despite returning success.
.SHRETURNVALUE
Return 0 on success or negative on failure. It returns
LIBSSH2_ERROR_EAGAIN when it would otherwise block. While
@@ -38,6 +37,5 @@ LIBSSH2_ERROR_EAGAIN is a negative number, it isn't really a failure per se.
\fILIBSSH2_ERROR_SOCKET_SEND\fP - Unable to send data on socket.
.THlibssh2_publickey_list_fetch3"1 Jun 2007""libssh2 0.15""libssh2 manual"
.SHNAME
libssh2_publickey_list_fetch - TODO
.SHSYNOPSIS
.SHDESCRIPTION
.SHRETURNVALUE
.SHERRORS
.SHSEEALSO
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
