Commit Graph

162 Commits

Author SHA1 Message Date
Andreas Schneider
65f363c9e3 CVE-2025-8114: Fix NULL pointer dereference after allocation failure
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit 53ac23ded4)
2025-08-14 11:02:39 +02:00
Jakub Jelen
697650caa9 CVE-2025-4878 Initialize pointers where possible
This is mostly mechanical change initializing all the pointers I was able to
find with some grep and manual review of sources and examples.

Used the following greps (which yield some false positives though):

    git grep "    \w* *\* *\w*;$"
    git grep " ssh_session \w*;"
    git grep " ssh_channel \w*;"
    git grep " struct ssh_iterator \*\w*;"
    git grep " ssh_bind \w*;"
    git grep " ssh_key \w*;"
    git grep " ssh_string \w*;"
    git grep " ssh_buffer \w*;"
    git grep " HMACCTX \w*;"
    git grep " SHACTX \w*;"
    grep -rinP '^(?!.*=)\s*(?:\w+\s+)*\w+\s*\*\s*\w+\s*;'

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2025-06-24 09:27:52 +02:00
Jakub Jelen
eeb498c0e3 Make sure we pass right parameters to buffer_pack
Fixes: #299

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Norbert Pocs <norbertpocs0@gmail.com>
(cherry picked from commit d00f7b1bf9)
2025-06-23 13:22:47 +02:00
Jakub Jelen
51f4a5743d kex: Avoid unused variable when built without server
Fixes: #267

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Eshan Kelkar <eshankelkar@galorithm.com>
(cherry picked from commit 71e1baeb5f)
2024-08-19 15:16:58 +02:00
Eshan Kelkar
d41a0aaa13 Move ssh_match_group() from misc.c to match.c
ssh_match_group() has been moved from misc.c to match.c, because it fits
better with other match_*() functions in match.c

The name of the function has also been changed from "ssh_match_group" to
"match_group" to be consistent with the naming of the other match.c
functions.

Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
2024-06-20 13:46:24 +02:00
Jakub Jelen
0882338142 Detect blowfish in mbedtls and skip it if not found
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
2024-05-16 09:56:38 +02:00
Jakub Jelen
a8b7e17aa0 kex: Avoid trailing comma in cipher list
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
2024-04-09 12:46:08 +02:00
Jakub Jelen
bdcdf92096 CVE-2023-48795: Strip extensions from both kex lists for matching
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2023-12-18 17:35:04 +01:00
Aris Adamantiadis
3876976ced CVE-2023-48795: Server side mitigations
Signed-off-by: Aris Adamantiadis <aris@0xbadc0de.be>
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2023-12-18 17:35:04 +01:00
Aris Adamantiadis
7ecc6a704b CVE-2023-48795: client side mitigation
Signed-off-by: Aris Adamantiadis <aris@0xbadc0de.be>
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2023-12-18 17:35:04 +01:00
Jakub Jelen
7ea71ead61 kex: Avoid NULL pointer dereference (GHSL-2023-032)
Thanks Phil Turnbull from Github

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2023-05-04 11:51:17 +02:00
Jakub Jelen
08386d4787 CVE-2023-1667:kex: Add support for sending first_kex_packet_follows flag
This is not completely straightforward as it requires us to do some state
shuffling.

We introduce internal flag that can turn this on in client side, so far for
testing only as we do not want to universally enable this. We also repurpose the
server flag indicating the guess was wrong also for the client to make desired
decisions.

If we found out our guess was wrong, we need to hope the server was able to
figure out this much, we need to revert the DH FSM state, drop the callbacks
from the "wrong" key exchange method and initiate the right one.

The server side is already tested by the pkd_hello_i1, which is executing tests
against dropbrear clients, which is using this flag by default out of the box.

Tested manually also with the pkd_hello --rekey to make sure the server is able
to handle the rekeying with all key exchange methods.

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2023-05-04 11:51:17 +02:00
Jakub Jelen
8dbe055328 CVE-2023-1667:kex: Correctly handle last fields of KEXINIT also in the client side
Previously, the last two fields of KEXINIT were considered as always zero for
the key exchange. This was true for the sending side, but might have not been
true for the received KEXINIT from the peer.

This moves the construction of these two fields closer to their reading or
writing, instead of hardcoding them on the last possible moment before they go
as input to the hashing function.

This also allows accepting the first_kex_packet_follows on the client side, even
though there is no kex algorithm now that would allow this.

It also avoid memory leaks in case the server_set_kex() or ssh_set_client_kex()
gets called multiple times, ensuring the algorithms will not change under our
hands.

It also makes use of a new flag to track if we sent KEXINIT.

Previously, this was tracked only implicitly by the content of the
session->next_crypto->{server,client}_kex (local kex). If it was not set, we
considered it was not send. But given that we need to check the local kex even
before sending it when we receive first_kex_packet_follows flag in the KEXINIT,
this can no longer be used.

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2023-05-04 11:51:17 +02:00
Jakub Jelen
cd0aa0bd91 CVE-2023-1667:dh: Expose the callback cleanup functions
These will be helpful when we already sent the first key exchange packet, but we
found out that our guess was wrong and we need to initiate different key
exchange method with different callbacks.

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2023-05-04 11:51:17 +02:00
Jakub Jelen
f455ffe8b8 CVE-2023-1667:kex: Factor out the kex mapping to internal enum
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2023-05-04 11:51:17 +02:00
Jakub Jelen
1c85acb6e6 CVE-2023-1667:kex: Remove needless function argument
The information if the session is client or server session is already part of
the session structure so this argument only duplicated information.

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2023-05-04 11:51:17 +02:00
Jakub Jelen
3981aeede2 CVE-2023-1667:kex: Properly conditionalize server code
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2023-05-04 11:51:17 +02:00
Jakub Jelen
070f679767 kex: Reformat ssh_kex_select_methods
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2023-05-04 11:51:17 +02:00
Jakub Jelen
1d6f2e4d9b Reformat ssh_packet_kexinit()
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2023-05-04 11:51:17 +02:00
Jakub Jelen
e6cc8dfef5 kex: Reformat ssh_send_kex
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2023-05-04 11:51:17 +02:00
Jakub Jelen
8cdf602330 kex: Clarify the comment
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2023-05-04 11:51:17 +02:00
khalid
f9147a3cf4 Remove zlib from the default compression methods and fips methods
Signed-off-by: Khalid Mamdouh <khalidmamdou7@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2023-03-28 11:36:44 +02:00
Mohammad Shehar Yaar Tausif
a3a13eb3a8 Remove support for DSA Keys
Solving issue #110. The original work is at !231
Some changes were needed because the newly added features in master through time

Signed-off-by: Mohammad Shehar Yaar Tausif <sheharyaar48@gmail.com>
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2023-02-02 14:49:06 +01:00
Jakub Jelen
cc1b021153 kex: Fix typo
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-01-25 16:53:19 +01:00
Jakub Jelen
492f5d82b8 Clean up usage of HAVE_ECC and HAVE_ECDH
they might be turned off and on independenty and each of them affects different
part of libssh, authentication and key exchange respectively. But only HAVE_ECC
is defined by the cmake.

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
2023-01-25 16:53:19 +01:00
Norbert Pocs
96ad1b380d Add support for sk-keys through configuration
To be able to enable sk-ecdsa, sk-edd25519 key usage from the config file
the algorithms are needed to be listed in the algorithm lists.

Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2023-01-09 17:02:30 +01:00
Norbert Pocs
039d1b2775 kex: Add functions for openssh +,-,^ features
The funcions can:
- add a list to the default list
- remove a list from the default list
- prepend a list to the default list

Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2022-11-18 20:28:30 +01:00
Norbert Pocs
657d9143d1 SSH_LOG_DEBUG: Recategorize loglevels
Loglevel INFO is the default openssh configuration setting which does not print
redundant information. On a system using openssh with loglevels set by the
terms of openssh will cause unwanted log lines in the output.
recategorized based on - SSH_LOG_DEBUG are informational debug logs (no error)

Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2022-10-07 09:13:19 +02:00
Norbert Pocs
7ea75cda45 SSH_LOG_TRACE: Recategorize loglevels
Do not print out logs when no fatal error happens.
This approach is similiar to openssh, when Error/Fatal does not print
recoverable error logs.
recategorized based on - SSH_LOG_TRACE are debug logs when error happens

Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2022-10-07 09:13:19 +02:00
Norbert Pocs
20c13a2c76 Change const bignum to bignum
Openssl3.0 API retrieves bignum variables from a key.

Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2022-07-07 08:39:39 +02:00
Junda Ai
41e2d17119 Fix multiple spelling and grammar mistakes
Signed-off-by: Junda Ai <aijunda29@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2022-05-02 19:05:51 +02:00
Fabian Vogt
14991ad071 Soften behaviour of the Compression=no/yes option
Currently Compression=no (the default) force-disables zlib algos, while
Compression=yes force-enables it. This means that mismatching options between
client and server lead to connection failure. This can easily happen if the
server has default settings but the client specifies Compression=yes.

OpenSSH treats the option as a "prefer compression" setting:
Compression=no  -> none,zlib@openssh.com,zlib (default)
Compression=yes -> zlib@openssh.com,zlib,none

This commit changes the libssh behaviour to the same as OpenSSH.

Signed-off-by: Fabian Vogt <fabian@ritter-vogt.de>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2022-01-05 11:26:30 +01:00
Jakub Jelen
f5211239f9 CVE-2021-3634: Create a separate length for session_id
Normally, the length of session_id and secret_hash is the same,
but if we will get into rekeying with a peer that changes preference
of key exchange algorithm, the new secret hash can be larger or
smaller than the previous session_id causing invalid reads or writes.

Resolves https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=35485

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2021-08-18 14:13:56 +02:00
Jakub Jelen
a3b2229a4e More consistent logging
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2021-08-18 14:13:56 +02:00
Dirkjan Bussink
710815674a Cleanup AES compatibility code
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>
2021-01-11 10:45:22 +01:00
Dirkjan Bussink
7e99867533 Cleanup old OpenSSL 0.9.7 compatibility code
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>
2020-12-22 09:15:38 +01:00
Dirkjan Bussink
42741b1883 Fix handshake bug with AEAD ciphers and no HMAC overlap
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>
2020-10-14 11:07:40 +02:00
Sahana Prasad
cc953ff7e4 src/kex.c: removes DES and SHA1 from mac and kex algorithms by default.
Signed-off-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2020-09-29 11:35:51 +02:00
Jakub Jelen
90e56df74e Disable *-cbc ciphers by default
OpenSSH disabled them in 2014 and 2017 for servers and clients so its our
turn to follow the suit.

Fixes T236

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2020-07-16 12:01:04 +02:00
Jakub Jelen
e6aee24a1e Add basic support for none cipher and MACs
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2020-05-05 14:23:06 +02:00
Jakub Jelen
fecdc3cc0e Disable RSA and DSA keys with sha1 by default
Fixes: T218

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
2020-04-20 14:14:33 +02:00
Anderson Toshiyuki Sasaki
9eb1ce88ae kex: Add support for diffie-hellman-group14-sha256
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2020-04-06 09:27:26 +02:00
Jakub Jelen
e2841908fb kex: Avoid always-false comparisons as reported by csbuild
/builds/jjelen/libssh-mirror/src/kex.c:360:17: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits] <--[cc]
  360 |         if (len < 0) {
      |                 ^
/builds/jjelen/libssh-mirror/src/kex.c:372:17: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits] <--[cc]
  372 |         if (len < 0) {
      |                 ^

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2019-12-23 14:31:32 +01:00
Andreas Schneider
3ab8b76921 kex: Use SSH_BUFFER_FREE()
Fixes T183

Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2019-12-09 16:08:03 +01:00
Andreas Schneider
7c3a6ca6bc kex: Use SSH_STRING_FREE()
Fixes T183

Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2019-12-09 16:08:03 +01:00
Andreas Schneider
be34ff967f kex: Use a len variable for length checks in ssh_packet_kexinit()
Fixes T188

Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2019-12-09 16:08:03 +01:00
Jakub Jelen
c8a621c606 Use only one variable denoting the size of methods arrays
Previously, there was non-consistent usage of constans SSH_KEX_METHODS,
KEX_METHODS_SIZE and of magic number 10 to reference the arrays used
for algorithm negotiation by peers. This commit settles down to the single
constant and its usage throughout the whole codebase.

Fixes T195

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2019-11-18 11:59:01 +01:00
Anderson Toshiyuki Sasaki
68baebbd62 Replace ssh_print_hexa() with ssh_log_hexdump()
Replace all occurrences of the deprecated function ssh_print_hexa() with
the introduced ssh_log_hexdump().

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2019-07-30 13:26:29 +02:00
Anderson Toshiyuki Sasaki
f18a7cc17e kex: Do not ignore keys in known_hosts files
Previously, if the SSH_OPTIONS_HOSTKEYS option was set by any mean,
including the client configuration file, the keys in known_hosts files
wouldn't be considered before advertising the list of wanted host keys.

This could result in the client requesting the server to provide a
signature using a key not present in the known_hosts files (e.g. when
the first wanted algorithm in SSH_OPTIONS_HOSTKEYS is not present in the
known_hosts files), causing a host key mismatch and possible key
rejection.

Now, the keys present in the known_hosts files are prioritized over the
other wanted keys.  This do not change the fact that only keys of types
present in the list set in SSH_OPTIONS_HOSTKEYS will be accepted and
prioritized following the order defined by such list.

The new wanted list of hostkeys is given by:
 - The keys present in known_hosts files, ordered by preference defined
   in SSH_OPTIONS_HOSTKEYS.  If the option is not set, a default order
   of preference is used.
 - The other keys present in the same option are appended without adding
   duplicates.  If the option is not set, the default list of keys is
   used.

Fixes: T156

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2019-07-04 10:29:20 +02:00
Anderson Toshiyuki Sasaki
53ae2502f4 kex: Only advertise allowed signature types
Previously, if the client supported rsa-sha2-256 or rsa-sha2-512, the
server would advertise the extensions as supported without checking its
own list of allowed algorithms.  Now the server will only advertise
allowed signature algorithms.

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
2019-06-13 16:28:23 +02:00