This adds a test to check if the internal implementation is not used
when it is not supposed to be used.
To be able to override functions using LD_PRELOAD, a shared version of
the torture library was added, as well as a shared library for each
of the algorithms implemented internally (ChaCha20, Poly1305,
curve25519, and ed25519).
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Previously, the mbed TLS implementation wouldn't be use at all when
available, being the internal implementation always used instead.
This corrects few bugs and makes the mbed TLS implementation to be used
when ChaCha20 and Poly1305 are available.
This also makes the constant time comparison to be used when checking
the authentication tag.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
When checking the authentication tag, use secure_memcmp() instead of
memcmp().
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
When comparing the authentication tag for chacha20-poly1305, use the
constant time CRYPTO_memcmp() instead of memcmp().
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Move the secure_memcmp() function to a shared source to make it
available internally for other crypto implementations.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The test for delayed close asks for the execution of a command that
generates big output (larger than the default window) to make data to
remain in buffers while the close message arrives, triggering the
delayed channel closure.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
If the SSH2_MSG_CHANNEL_CLOSE was previously received, change the
channel state to SSH_STATE_CHANNEL_CLOSED in
ssh_channel_read_timeout() after reading all data available.
Fixes T31
Signed-off-by: Tom Deseyn <tom.deseyn@gmail.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
OpenSSL 1.0.1 has support for CTR & GCM modes so the checks here are no
longer needed. This allows for a bunch of additional cleanup of the old
code.
As for old MacOS versions etc, LibreSSL is a kind of compatibility layer
there but things already don't work anyway with that, so it doesn't
break anything that isn't already broken. OpenSSL is needed on MacOS
separately anyway (like installed with Homebrew).
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
CRYPTO_THREADID_set_callback is available since 1.0.1 which is the
oldest supported version. This means the check and compatibility code
can be removed.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Since OpenSSL 1.0.1 is the minimum version, this function is always
available so no compatibility check is needed anymore.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This can be implemented with the init directly when the context is
reused. When a new cipher context is allocated, no initialization call
is needed either so this moves the logic to one place as well.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Now that the minimum OpenSSL version is 1.0.1, we know that the EVP HMAC
API is always available. This switches to this API. The existing API is
deprecated for OpenSSL 3.0.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This fixes the formatting for src/libcrypto.c for the last bits where it
is not correct.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The HMAC_CTX_free function in the compat layer already handles this so
there's no need to add conditional logic to the code here.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This specific cleanup function describes better what happens here and is
available for older OpenSSL releases.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This isn't referenced anywhere outside of the compatibility layer so it
is unneeded.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The EVP_MD_CTX_reset function is not used anywhere outside of the compat
layer and is not needed there. The only usage in the compat layer is for
cleanup, but EVP_MD_CTX_cleanup can be used for that which is availble
at least since OpenSSL 0.9.8.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Older OpenSSL version have a cleanup function that can be used here.
This removes a whole bunch of now no longer needed logic and custom
conditionals.
These functions have existed since 0.9.8 and can be used here.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This function is not needed, because in each case it is used, we follow
it up immediately with an initialization function call. This means that
the zeroing here is unneeded, since the initialization already
guarantees things end up in the right state.
It also swaps the reset call with a simpler init call, also because
reset is implemented as init with a return value that is always 1. That
means the more complex logic is not needed at all.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
EVP_MD_CTX_new / EVP_MD_CTX_free is the current recommended / documented
API. The other names are defined as aliases for backwards compatibility.
The other part here is that EVP_MD_CTX_init is not needed for a context
allocated with EVP_MD_CTX_new. Only for the compatibility path for older
OpenSSL is the init needed if the structure is allocated directly.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This is now the minimum version, so check it in the CMake configuration.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
OpenSSL 0.9.7 is already not supported, so clean up the old legacy bits
for that as well.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This updates the minimum version requirement for OpenSSL in the
documentation to 1.0.1 and also updates the practical minimum CMake
version.
Why pick 1.0.1 as the minimum? Main reason is whatever is still out
there with long term support contracts etc. One of the oldest I could
find is Ubuntu 14.04 which still has paid extended support and is on
1.0.1.
Another reason that 1.0.1 is probably a good minimum is a bit more
involved. 1.0.1 is the first version to add TLS 1.2. Large parts of the
internet have TLS 1.2 as a minimum requirement. This means that systems
with OpenSSL older than 1.0.1 already can't access large parts of the
internet anyway, so not supporting the latest libssh there either is ok
I think.
Bumping minimum support also means things like the HMAC API can be moved
to the more recent EVP style APIs and things can be more easily made
compatible with the deprecated APIs in OpenSSL 3.0.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Another item identified during code review was cases where the return
value of ssh_list_new() was not properly checked and handled. This
updates all cases that were missing this to handle failure to allocate a
new list.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
These calls can fail and the return code should always be checked. These
issues were identified when code review called it out on new code. The
updates here are to existing code with no behavior changes to make
review simpler.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
In https://gitlab.com/libssh/libssh-mirror/-/merge_requests/145#note_463232084
behavior in libssh was identified where it diverges from how for example
OpenSSH behaves. In OpenSSH if a request success of failure message is
received, apart from it being treated as a keepalive message, it is
ignored otherwise.
Libssh does handle the unexpected message and triggers an error
condition internally. This means that with the Dropbear behavior where
it replies to a hostkeys-00@openssh.com message even with a want_reply
= 0 (arguably a bug), libssh enters an error state.
This change makes the libssh behavior match OpenSSH to ignore these
messages. The spec is a bit unclear on whether Dropbear is buggy here or
not, but let's be liberal with the input accepted here in libssh.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
FindOpenSSL.cmake usually defines this synonym, but it doesn't on CMake < 3.16 when building on Windows outside of Cygwin.
Signed-off-by: Kevin Kane <kkane@microsoft.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The build currently breaks when attempting to link libssh.so using a statically-linked OpenSSL. -ldl and -lpthread are required when linking a binary with the static libcrypto.a. The OPENSSL_CRYPTO_LIBRARY does not include these dependencies when linking against static OpenSSL. OPENSSL_CRYPTO_LIBRARIES contains the correct dependencies in both static and shared configurations; -ldl and -lpthread are not required when linking against shared libcrypto.so.
This change changes all uses of OPENSSL_CRYPTO_LIBRARY to OPENSSL_CRYPTO_LIBRARIES to let the FindOpenSSL CMake module always provide the correct libraries at link time.
Signed-off-by: Kevin Kane <kkane@microsoft.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The build currently breaks when attempting to link libssh.so using a
statically-linked OpenSSL. -ldl and -lpthread are required when linking
a binary with the static libcrypto.a. The OPENSSL_CRYPTO_LIBRARY does
not include these dependencies when linking against static OpenSSL.
OPENSSL_CRYPTO_LIBRARIES contains the correct dependencies in both
static and shared configurations; -ldl and -lpthread are not required
when linking against shared libcrypto.so.
This change changes all uses of OPENSSL_CRYPTO_LIBRARY to
OPENSSL_CRYPTO_LIBRARIES to let the FindOpenSSL CMake module always
provide the correct libraries at link time.
Signed-off-by: Kevin Kane <kkane@microsoft.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Stanislav Zidek <szidek@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Fixes T248
Signed-off-by: Stanislav Zidek <szidek@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
These are disabled in latest since Dropbear 2020.79, while
older do not support anything better than aes-ctr ciphers.
We should implement some dynamic algorithm detection for dropbear
too to increase test coverage.
https://bugs.libssh.org/T252
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
There's currently a bug in libssh that a handshake doesn't complete if
there is no overlap between HMAC methods, but when an AEAD cipher is
used.
In case of an AEAD cipher such as chacha20-poly1305 or aes256-gcm, the
HMAC algorithm that is being picked is not relevant. But the problem
here is that the HMAC still needs to have an overlap in the handshake,
even if it is not used afterwards.
This was found with a very strict server side configuration with libssh
where only AEAD ciphers and EtM HMAC modes are accepted. The client
tested against was dropbear.
Dropbear does have support for chacha20-poly1305 and AES GCM modes, but
no support for EtM HMAC modes. This meant that the libssh server in this
case rejected the dropbear client, even though it is perfectly able to
serve it since dropbear supports AEAD algorithms.
The fix implemented here updates the HMAC phase of the handshake to
handle this case. If it detects an AEAD cipher is used, it uses the HMAC
abbreviations for the method instead. This is the same name that is used
in other places as well. It matches the client to server and server to
client values, but it does depend on the order of things in the
ssh_kex_types_e enum, which I'm assuming here is ok since it's explicit.
I've looked at how to add a test for this, but I couldn't really find a
suitable place for it. I would love some tips if this is easily
possible, or if it's easier for someone else to contribute, that's of
course welcome too.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This adds an initial test with all AEAD modes to verify that they work
if there is no overlap in HMAC ciphers.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
