Previously, SHA-1 was used always.
BUG: https://red.libssh.org/issues/148
Signed-off-by: Alan Dunn <amdunn@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The commit fixes this build warning:
====
src/server.c:223:8: warning: ‘privkey’ may be used uninitialized in this function [-Wmaybe-uninitialized]
rc = ssh_pki_export_privkey_to_pubkey(*privkey, &pubkey);
^
src/server.c:243:11: note: ‘privkey’ was declared here
ssh_key privkey;
====
Signed-off-by: Luka Perkov <luka.perkov@sartura.hr>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
In 'ssh_bind_listen', move setting of 'sshbind->bindfd' to only happen after
the listen call: otherwise 'bindfd' can be set to a bogus descriptor for the
case that listen fails.
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
When accepting a new connection, a forking server based on libssh forks
and the child process handles the request. The RAND_bytes() function of
openssl doesn't reset its state after the fork, but simply adds the
current process id (getpid) to the PRNG state, which is not guaranteed
to be unique.
This can cause several children to end up with same PRNG state which is
a security issue.
After discussion with Aris and it was not obvious enough to understand
the issue we decided to refactor it.
Reviewd-by: Aris Adamantiadis <aris@0xbadc0de.be>
Right now the behavior of packet_{en,de}crypt on len == 0 depends on
the behavior of malloc. Instead, make these consistently fail based
on what I assume the desired behavior is due to the first error
message in each.
Signed-off-by: Alan Dunn <amdunn@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
If we receive a packet of length exactly blocksize, then
packet_decrypt gets called on a buffer of size 0. The check at the
beginning of packet_decrypt indicates that the function should be
called on buffers of at least one blocksize, though the check allows
through zero length. As is packet_decrypt can return -1 when len is 0
because malloc can return NULL in this case: according to the ISO C
standard, malloc is free to return NULL or a pointer that can be freed
when size == 0, and uclibc by default will return NULL here (in
"non-glibc-compatible" mode). The net result is that when using
uclibc connections with libssh can anomalously fail.
Alternatively, packet_decrypt (and probably packet_encrypt for
consistency) could be made to always succeed on len == 0 without
depending on the behavior of malloc.
Thanks to Josh Berlin for bringing conneciton failures with uclibc to
my attention.
Signed-off-by: Alan Dunn <amdunn@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Follow-up to 4e04ec8, which caused a regression on OS X.
Checking the value of CMAKE_THREAD_LIBS_INIT to decide whether any threading
library is present on a system turns out to be wrong -- in OS X, for
example, usage of pthreads does not depend on any additional linker or
compiler flags, so CMAKE_THREAD_LIBS_INIT is empty and our check in
src/CMakeLists.txt failed (it used to work before 4e04ec8 because
CMAKE_HAVE_THREADS_LIBRARY is set).
Instead, just look for Threads_FOUND, which FindThreads sets just like any
other Find module when it has found what it was looking for.
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Fix bug #142
The mode does need to be an octal numeric string. Mode 0600 now gets sent on the wire as 0384, triggering a "scp: protocol error: bad mode" response, and an "scp status code 1d not valid" message from libssh.
As mentioned in the previous commit, there are cases where
CMAKE_HAVE_THREADS_LIBRARY is not set and pthreads _is_ being used: one can
pass -DTHREADS_HAVE_PTHREAD_ARG=1 to CMake directly so that it just passes
-pthread to the compiler/linker and does not set CMAKE_HAVE_THREADS_LIBRARY.
Since we are only interested in knowing whether any threading library has
been found, we should use CMAKE_THREAD_LIBS_INIT instead (Threads_FOUND
would also work).
Note that, at the moment, there is only a pthreads backend available in
threads/, so if it is not found configuration will fail because CMake will
try to create a library from an empty set of source files.
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
libssh is primarily interested in whether pthreads is present and can be
used. Checking for CMAKE_HAVE_THREADS_LIBRARY is not the same thing, as
there are cases where pthread exists but CMAKE_HAVE_THREADS_LIBRARY is not
set (for example, FreeBSD passes -DTHREADS_HAVE_PTHREAD_ARG=1 to CMake by
default as a way to skip the checks for -lpthread, -lpthreads and others and
tell the build system that -pthread is the one expected to be used).
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Ensure to check whether the socket at hand is indeed still connected
throughout POLLIN processing in ssh_socket_pollcallback.
Before this change, the POLLIN block in ssh_socket_pollcallback is
predicated against the condition (s->state == SSH_SOCKET_CONNECTED).
Once entered, data from the socket is consumed through the data
callback in this loop:
do {
r = s->callbacks->data(buffer_get_rest(s->in_buffer),
buffer_get_rest_len(s->in_buffer),
s->callbacks->userdata);
buffer_pass_bytes(s->in_buffer,r);
} while (r > 0);
However, it is possible for the socket data callback to change the
state of the socket (closing it, for example). Fix the loop to only
continue so long as the socket remains connected: this also entails
setting the ssh_socket state to SSH_SOCKET_CLOSED upon close.
The bug can be observed before the change by sending a bogus banner
to the server: 'echo -e "A\r\nB\r\n" | nc localhost 22'. Each of
'A' and 'B' will be processed by 'callback_receive_banner', even
though the client socket is closed after rejection of 'A'.
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
So that it would match ssh_channel_pty_request_callback as well as the documentation
Signed-off-by: Audrius Butkevicius <audrius.butkevicius@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Fix the DSA portion of 'pki_signature_to_blob': before this change, it
is possible to sometimes observe DSA signature validation failure when
testing with OpenSSH clients. The problem ended up being the following
snippet which did not account for the case when 'ssh_string_len(x)' may
be less than 20:
r = make_bignum_string(sig->dsa_sig->r);
...
memcpy(buffer,
((char *) ssh_string_data(r)) + ssh_string_len(r) - 20,
20);
Above consider the case that ssh_string_len(r) is 19; in that case the
memcpy unintentionally starts in the wrong place. The same situation
can happen for value 's' in this code.
To fix, adjust the offsets used for the input and output pointers, taking
into account that the lengths of 'r' and 's' can be less than 20. With
the fix I am no longer able to reproduce the original failure mode.
BUG: https://red.libssh.org/issues/144
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
