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)
Ensure ssh and sftp error codes are consistently set across all
failure paths in sftp_init(). One branch previously returned
-1 leading to ambiguous error reporting.
Signed-off-by: Manas Trivedi <manas.trivedi.020@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit 8dc59ae8ac)
The RFC 4253, Section 8 says that the
Values of 'e' or 'f' that are not in the range [1, p-1] MUST NOT be
sent or accepted by either side. If this condition is violated, the
key exchange fails.
Originally reported by Oren Yomtov
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Aris Adamantiadis <aris@0xbadc0de.be>
(cherry picked from commit d8ccda1d6f)
When using OpenSSH scp to read files larger than a few hundred
kilobytes, downloads stall and never finish. A workaround is to
pass -Xnrequests=1 to scp, which will cause scp to only do a
single concurrent SFTP read request at a time.
The cause for the problem is that if SFTP client messages are
received at a high rate, sftp_channel_default_data_callback() will
potentially be called with multiple messages in the incoming data
buffer, but only the first message will be extracted and handled.
So add a loop to extract as many SFTP client messages as available
from the incoming data buffer.
Signed-off-by: Colin Baumgarten <colin.baumgarten@hubersuhner.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(cherry picked from commit 3154a4ab8d)
