Using the FIPS OpenSSL provider, EVP_PKEY_CTX_new_from_pkey
does not return NULL with an ML-KEM key even if the
FIPS provider does not support ML-KEM and the propquery is NULL.
In both ssh_mlkem_encapsulate and ssh_mlkem_decapsulate, we would
therefore initialize the context with the FIPS provider
instead of the fallback (default) provider and the functions
would later fail during EVP_PKEY_encapsulate_init or
EVP_PKEY_decapsulate_init, respectively.
With this change, we determine the correct propquery to use
in FIPS mode by checking whether we can fetch the ML-KEM
implementation from the FIPS provider. The propquery is cached.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Merge-Request: <https://gitlab.com/libssh/libssh-mirror/-/merge_requests/862>
(cherry picked from commit a7ab540c55)
Cover both directions with unit tests: a client receiving
SSH2_MSG_CHANNEL_OPEN_CONFIRMATION with a maximum packet size of 0 must
not transition the channel to the open state, and a server receiving
SSH2_MSG_CHANNEL_OPEN with a maximum packet size of 0 must reject the
request. Both come with a positive control using a valid packet size.
Signed-off-by: Martin Pitt <mpitt@debian.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Merge-Request: <https://gitlab.com/libssh/libssh-mirror/-/merge_requests/863>
(cherry picked from commit 7d7c3a04a2)
Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
Do this both for SSH2_MSG_CHANNEL_OPEN and for
SSH2_MSG_CHANNEL_OPEN_CONFIRMATION. Using the
max packet size 0 would lead to an infinite loop
in channel_write_common.
Originally reported by Rinku Das on on 23th February.
Independently reported by Yi Lin on 26th February and
Haruto Kimura on 22nd March.
We do not consider this as a security issue as connecting
to untrusted servers on the internet brings much worse
security consequences than hanging your clinet.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit 44b186fa17)
Without any special configuration (e.g., in ~/.k5login), we should not
expect the principal alice@REALM to be able to authenticate as bob
against either sshd or a libssh server.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit 098b427c87)
The callback should check whether the principal
is authorized to log in as the user; krb5_kuserok
does exactly that.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit 114b0dfc23)
Like gssapi-with-mic, gssapi-keyex also requires
the server to decide whether the client that's
obtained a valid ticket for a specific principal
is authorized to log in under the requested user,
otherwise any client with a valid ticket would
be able to log in as an arbitrary local user.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit a45d20b752)
After going through the whole list of certificates, the iterator was
set to NULL and at the beginning of the next loop reset to the start
of the list again, which caused infinite loop.
Adding one more intermediate state solves the issue.
Found by AISLE in partnership with Red Hat
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
(cherry picked from commit bcfa191283)
The loop will be infinite only when the server will not drop the connection
after infinite amount of pubkey auth requests.
Found by AISLE in partnership with Red Hat
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
(cherry picked from commit bb96432364)
This adds a new field to sftp_session_struct,
containing a list of outstanding request IDs.
An ID is added to the list when a request
is constructed and removed when the corresponding
request is received. If a client receives a response
with an unknown request ID, it reports an error.
Storing responses with unknown request IDs in
the response queue could be abused by a malicious
SFTP server which could deplete client memory
this way.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit 26147eb476)
EVP_DecryptFinal() returns 0 errors, which was wrongly checked since
its introduction.
Reported by Ben Smyth discuss@bensmyth.com
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
(cherry picked from commit 6d6cb6cb4d)
When an attacker could sneak the dollar sign or backslash into the username
expanded for example in proxy command, it can result in printing environment
variables that might contain secrets.
This is a fixup of CVE-2023-6004 which fixed this for hostnames, but these
two metacharacters were left out from the username filter.
This keeps the list in one place to simplify maintenance.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
(cherry picked from commit 6309df220e)
During execution of proxy command, when fork() fails, its return value
is stored in pid and when the parent process attempts to kill it,
it sends the kill signal to all processes the calling application has
access to (except for init).
This caused nard to debug issues when the system under the load was hitting
fork failures, which resulted in killing of all the system processes
(of given user).
Reported and first patch iteration provided by: Halil Oktay (oblivionsage).
This code missing fork return value check is in libssh since 2010
(f31a14b793), but this issue is exploitable only
since libssh 0.9.0 as previously there was no implementation of killing
ProxyCommand children.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
(cherry picked from commit 92b6fb9c5e)
The client-provided length is directly used in
a malloc in process_read(), so not restricting it
leads to allocations bounded only by UINT32_MAX.
The new cap is the same as the one currently used
by OpenSSH.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit 6dba2e06f0)
Receiving a short Curve25519 public key would cause an
out-of-bounds read, potentially leaking small amounts
of heap data if an attacker sends a prefix of
a valid public key and the key exchange succeeds.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit ed9109dfc6)
ssh_packet_unimplemented() tries to unpack the uint32 sequence number from SSH_MSG_UNIMPLEMENTED. If the packet payload is malformed and does not contain the full sequence number, ssh_buffer_unpack() fails, but the function still logs seq. Return after the failed unpack so the uninitialized local variable is not formatted into the log message.
Signed-off-by: Jiami Lin <jiami3us@icloud.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Merge-Request: <https://gitlab.com/libssh/libssh-mirror/-/merge_requests/834>
(cherry picked from commit 9075952491)
Additional hardening realated to 3ce8bf3289 fix that
switches ssh_string_new() to calloc() so the payload bytes
are zero-initialised. ssh_string is used throughout libssh as a
byte container for wire data and crypto material; the uninitialised
payload is never semantically meaningful, and zeroing it kills the
"forgot to check read_len" class of bugs at the source.
Signed-off-by: David Cermak <d_cermak@centrum.cz>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Merge-Request: <https://gitlab.com/libssh/libssh-mirror/-/merge_requests/829>
(cherry picked from commit d94a96bf23)
ssh_packet_socket_callback() will not return error codes like
SSH_ERROR on failure. Instead it will always return an unsigned
size_t value representing the number of bytes which have been
processed.
So adjust the assert in the test accordingly, making sure that all
input bytes have actually been processed.
Signed-off-by: Colin Baumgarten <colin.baumgarten@hubersuhner.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Merge-Request: <https://gitlab.com/libssh/libssh-mirror/-/merge_requests/824>
(cherry picked from commit a33da974ef)
After processing a packet and before returning the processed size,
ssh_packet_socket_callback() recursively calls itself if there are
still unprocessed bytes remaining in the socket buffer.
This is redundant because this function is already called in a loop
by its only caller until the socket buffer is drained or only a
partial packet remains.
The recursive call was introduced with c92f54102e at a time
where ssh_packet_socket_callback() was not yet called in a loop.
The loop was only introduced later by 186116f34a.
So remove the redundant recursive call, but leave a log message in
place to allow following packet processing from the logs.
Signed-off-by: Colin Baumgarten <colin.baumgarten@hubersuhner.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Merge-Request: <https://gitlab.com/libssh/libssh-mirror/-/merge_requests/824>
(cherry picked from commit 655601d357)
Both callers of ssh_packet_socket_callback() assume that it will
always return the number of bytes processed. They don't properly
handle negative return values like SSH_ERROR, even though the
function's doc-comment mentions them.
Handling negative values would not be straightforward because
the return type is unsigned (size_t).
So instead of fixing the callers, adjust the only place where a
negative value is returned right now (rekey failure) and align
it with the handling of all other possible errors in the same
function.
Signed-off-by: Colin Baumgarten <colin.baumgarten@hubersuhner.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Merge-Request: <https://gitlab.com/libssh/libssh-mirror/-/merge_requests/824>
(cherry picked from commit 1bd5c2b043)
This should not happen because the OID is selected
from a subset of client-supplied OID in gss_acquire_cred,
i.e., it would imply a bug in krb5. But better be safe,
it would later cause an out-of-bounds read in
ssh_gssapi_send_response.
Reported and patch provided by Meta (vulns@meta.com).
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit 81a7e92c44)
ssh_session_get_known_hosts_entry() could return SSH_KNOWN_HOSTS_OK without initializing *pentry, leaving callers with an undefined output pointer.
Initialize *pentry to NULL at function entry so every return path leaves the output in a defined state.
Signed-off-by: Nuhiat-Arefin <nuhiatarefin@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Merge-Request: <https://gitlab.com/libssh/libssh-mirror/-/merge_requests/814>
(cherry picked from commit 6cd0e6e52a)
