Expose the configured IdentityAgent and IdentitiesOnly options - #15
Open
alexey-milovidov wants to merge 795 commits into
Open
Expose the configured IdentityAgent and IdentitiesOnly options#15alexey-milovidov wants to merge 795 commits into
IdentityAgent and IdentitiesOnly options#15alexey-milovidov wants to merge 795 commits into
Conversation
In cases where this is the initial allocation, the shrinking of the polltrs buffer would result in 0B realloc, which really does not make sense. Also, when this second realloc fails, the memory is never freed as the outer code believes there is nothing allocated on the poll_ctx Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Since 07cb0be we are not closing the user provided FDs, but the above change also resulted in memory leak during ssh_disconnect that left the poll_handle allocated during reset. Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
…essed wrongly Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
…ent's wrong KEX guess Signed-off-by: Francesco Rollo <eferollo@gmail.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Andreas Schneider <asn@cryptomilk.org> Reviewed-by: Jakub Jelen <jjelen@redhat.com>
As it may a cause a use after free if `send` fails when ssh_poll_ctx_dopoll does its callback ssh_poll_ctx_dopoll still wants to use the poll object later Signed-off-by: Philippe Antoine <p.antoine@catenacyber.fr> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com> Reviewed-by: Jakub Jelen <jjelen@redhat.com>
…dized The specification is now in the last call, data point is allocated so there is no need to stick to the namespaces alias anymore https://datatracker.ietf.org/doc/draft-ietf-sshm-ntruprime-ssh/ Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Sahana Prasad <sahana@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Sahana Prasad <sahana@redhat.com>
Signed-off-by: Francesco Rollo <eferollo@gmail.com> Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Philippe Antoine <p.antoine@catenacyber.fr> Reviewed-by: Andreas Schneider <asn@cryptomilk.org> Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com> Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com> Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Define a new constant for the default RSA key size for consistency. Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com> Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com> Reviewed-by: Jakub Jelen <jjelen@redhat.com>
… function The logic for creating the buffer to be verified from an sk signature from ssh_pki_signature_verify has been separated into a new function named pki_prepare_sk_signature_buffer to allow for convenient reuse of this logic. Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com> Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The libssh requires mbedTLS to have support for threading. Given the way how the mbedTLS builds are configured (at least to my limited understanding), by modifying mbedtls_config.h header file, this changes the silent failure to a message on stderr pointing the user in the right direction. Fixes: #304 Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Pavol Žáčik <pzacik@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Pavol Žáčik <pzacik@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com> Reviewed-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Addresses issue #319 The commit description explains: 1. Fix for sftp aio + read 2. Fix for sftp aio + write 1. Fix for sftp aio + read ------------------------- The reproducer provided in the issue description had a model as follows (with one jump host): fd_1---(socket_pair)---fd_2---(connector)----channel(fd_3)-----server Via debugging, it was noticed that the channel connected directly to the server stored a lot of unbuffered data (received from the server) that wasn't being written to fd_2 via the connector API. (Here on, channel refers to the channel(fd_3) in the diagram connected directly to the server) Consider the situation, where after a bit of progress in the transfer, the server has sent all the requested data (requested via outstanding requests) and all of that data is stored in channel->stdout_buffer. Say this data is 10,000 bytes. At this point, all the client (fd_1) is doing is waiting for all outstanding requests. (and processing thei responses) - POLLOUT event callback gets generated indicating that fd_2 is available for writing. - ssh_connector_fd_out_cb() gets called to handle the POLLOUT. - Assuming connector->in_available was true, 4096 (CHUNKSIZE) bytes get read from the channel. (really channel->stdout_buffer) leaving 10,000 - 4096 = 5904 bytes unread in the channel. - The read bytes are sent via fd_2 (so that fd_1 can recv them) - After this, the callback sets connector->in_available to 0 and connector->out_wontblock to 0. - Since out_wontblock has been set to 0 ssh_connector_reset_pollevents() (called after the callback returns) will consider POLLOUT events on the connector output. - (Based on assumption before) Since the client (fd_1) is eagerly awaiting responses and processing them, the received data gets processed quickly and fd_2 is available for sending/writing. - POLLOUT event gets generated for fd_2 indicating that its available for writing/sending to fd_1 - ssh_connector_fd_out_cb() gets called to handle the POLLOUT - Since connector->in_available is 0 (and ssh_connector_channel_data_cb() has not been trigerred in between as we have assumed before that all the data has already been received on the channel and is stored in the channel->stdout_buffer), ssh_connector_fd_out_cb() does nothing besides setting connector->out_wontblock to 1. - Since out_wontblock has been set to 1 ssh_connector_reset_pollevents() (called after the callback returns) will IGNORE POLLOUT events on the connector output. - So, at this point, the channel->buffer contains 5706 bytes and the fd_2 is available for writing/sending (out_wontblock is 1), but nothing happens and the transfer gets stalled/hanged. In my opinion, this hanging occurs because connector->in_available was incorrectly set to 0 despite the channel buffer having 5706 bytes in it. This commit changes that code to consider the data available to read on the channel (includes buffered data as well as polled data on channel's internal fd) and taking that into consideration to set in_available appropriately. (Instead of unconditionally setting it to 0 as the current code does) so that the next time POLLOUT gets received on fd_2 the ssh_connector_fd_out_cb() does read from the channel and write to fd_2 (as the connector->in_available flag would be set). 2. Fix for sftp aio + write ------------------------------------- On writing tests for sftp aio + proxyjump, it was encountered that file uploads were also hanging. Though I was not able to pin point the exact cause for this hanging, the nature of hanging was observed to be as follows: - sftp aio write + proxyjump blocks/hangs occasionally (not always) - It hangs at different points in the test - hang point 1: Sometimes it hangs after sending the first write request (i.e. the second write request call hangs and never returns, at this point we are not even waiting for response, just sending data). A lot of pending data to write to socket/fd was noticed at this hang point. - hang point 2: Sometimes it hangs while waiting for the second write request response. - It hangs at ssh_handle_packets_termination (i.e. this is the call that never returns), in context to hang point 1, this occurs due to trying to flush the channel during sftp_packet_write, and in context to hang point 2, this occurs due to trying to read an sftp response packet. - Not sure why, but more the verbose logging/printing I do, the lesser occasionally test hangs (e.g. 1 test in 6-7 test runs), maybe this could be a hint for a race condition / thread interaction related bug, but am not sure. Fix: On modifying the connector code to mark out_wontblock to 0 in case of output channel only when the channel's remote window is 0, the hanging no longer occured. Though, as mentioned before, I don't know the exact problem (i.e. case causing hanging) the fix addresses, but the fix is logical (if remote window is +ve data can still be written to channel and hence out_wontblock should not be reset to 0, it should be set to 1) and fixes the issue hence is added to this commit. Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com> Reviewed-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
This affects libgcrypt and mbedTLS backends. The OpenSSL backend is using OpenSSL implementation of the Ed25519 which is compared correctly. Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The specially crafted patterns (from configuration files) could cause exhaustive search or timeouts. Previous attempts to fix this by limiting recursion to depth 16 avoided stack overflow, but not timeouts. This is due to the backtracking, which caused the exponential time complexity O(N^16) of existing algorithm. This is code comes from the same function from OpenSSH, where this code originates from, which is not having this issue (due to not limiting the number of recursion), but will also easily exhaust stack due to unbound recursion: openssh/openssh-portable@05bcd0c This is an attempt to simplify the algorithm by preventing the backtracking to previous wildcard, which should keep the same behavior for existing inputs while reducing the complexity to linear O(N*M). This fixes the long-term issue we had with fuzzing as well as recently reported security issue by Kang Yang. Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
…e configuration files Changes also the reading of known_hosts to use the new helper function Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Andreas Schneider <asn@cryptomilk.org> Reviewed-by: Jakub Jelen <jjelen@redhat.com>
thanks oss-fuzz https://issues.oss-fuzz.com/u/1/issues/482613826 Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Pavol Žáčik <pzacik@redhat.com> Reviewed-by: Sahana Prasad <sahana@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Pavol Žáčik <pzacik@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
|
|
alexey-milovidov
added a commit
to ClickHouse/ClickHouse
that referenced
this pull request
Aug 20, 2026
…ests Addresses the review of #115072: - `IdentityFile` and `IdentityAgent` now expand the whole token set that `ssh` supports (`%C`, `%d`, `%h`, `%i`, `%L`, `%l`, `%n`, `%p`, `%r`, `%u`, `%%`), instead of leaving the unknown ones as is and silently probing a file that cannot exist. A token that is not supported is now an error. - `IdentitiesOnly yes` is honored: the ssh-agent is no longer asked for an arbitrary key when none of the configured identity files exists. Retrieval of the option is added to `libssh` (ClickHouse/libssh#15). - Reading the configuration, which used to happen up to three times per connection, happens once, in `getSSHClientConfiguration`. - Every failure to talk to the ssh-agent is reported as `SSH_AGENT_ERROR`. A socket name that is too long for a Unix socket used to escape as a `Poco::Exception` and abort the client instead of letting it use the key file. - `03780_failed_ssh` no longer expects the passphrase prompt, which is now asked only for a key that is really encrypted. - `04908_ssh_key_file_lookup`: the heading of the last case was printed before the previous one, and the new cases for `%r` and `IdentitiesOnly` are added.
IdentityAgent optionIdentityAgent and IdentitiesOnly options
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Expose
SSH_OPTIONS_IDENTITY_AGENTandSSH_OPTIONS_IDENTITIES_ONLYthroughssh_options_get, so that consumers can use the values resolved by libssh's full SSH configuration parser instead of parsingssh_configthemselves.IdentitiesOnlyis a boolean, and it is reported the same way it is spelled in the configuration file:yesorno.Used by ClickHouse/ClickHouse#115072, which looks up the SSH key of
clickhouse-clientthe same waysshdoes it.