Without this, the gcov is crashing with some suspicious coverage reports on
functions like `uint32_divmod_uint14()` from internal sntrup implementation.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The ML-KEMx25519 is now preferred algorithm in OpenSSH so follow the suit
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The threads_pki_rsa was running and working under valgrind for some
time already without anyone noticing this syntax does not work.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
When reusing session structures for multiple
connections, the packet state could be SIZE_READ
before disconnect, causing initial packets of the
next connection to be misinterpreted.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The implementation largely follows that of sntrup761x25519-sha512.
Most of the work was done by Sahana with the help of Claude,
Pavol provided fixes to match specs and did a final clean up.
Co-Authored-By: Sahana Prasad <sahana@redhat.com>
Co-Authored-By: Pavol Žáčik <pzacik@redhat.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The EVP_CIPHER_CTX_init API is deprecated and doesn't exist in some
OpenSSL versions. Switch to EVP_CIPHER_CTX_reset which works with
1.1.x which is the min version libssh requires.
Signed-off-by: Mike Frysinger <vapier@chromium.org>
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>
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>
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>
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>
Since 07cb0be12 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>
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>
and properly check the return value where it is used
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
When allocation during tilde expansion fails, libssh could
leak a memory.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
These tests are flaky because even though the care was taken to guess if
the ssh_channel_free() really freed the channel, it might not always be correct
and call to operation on the freed channel results in use after free.
Generally, no operation should be called after the channel is freed by the user.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
We already assign the correct key type_c using ssh_key_type_to_char before this point.
Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Whenever the pki_pubkey_build_ecdsa and pki_privkey_build_ecdsa functions are called, the key type assignment is already done. So, we don't need to assign it again. Moreover, because the pki_key_ecdsa_nid_to_name function was used, for key types like the SSH_KEYTYPE_SK_ECDSA, we assign the wrong type string to the key, based on the nid.
Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Actually the condition was duplicated at the beginning of the function and this
one could not be hit (again), but it is an error to be fixed anyway.
Thanks Coverity!
CID 1618865
CID 1618864
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This is leaking memory allocated in process_open(), which is stored in the
handles list in the sftpserver session. Given that the data is provided by the
use callbacks, we can not universally free them on our side, but we should, in
the long term, introduce some way for the implementers to free outstanding
handles that were not closed by misbehaving clients.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This was initially in hurry disabled in
ca4c874a9e because dropbear dropped support for
these HMACs. The follow-up commit enabled running these tests on old dropbear in
c17112f070, but still did not run them on openssh,
when the new dropbear was installed.
This fixes up the above commit to run the HMAC-SHA1 tests with OpenSSH even if
the new dropbear is installed.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
See libssh-mirror#311 for background. But in some case, it's possible to
trigger the code in ssh_handle_key_exchange() to move session state
directly to SSH_SESSION_STATE_AUTHENTICATED. The exit condition for this
function is SSH_SESSION_STATE_AUTHENTICATING though, so when it happens,
ssh_handle_key_exchange() will time out eventually.
The fix is straightforward. Tested with the problematic
client (trilead-ssh2) and made sure the bad condition happened (and not
cause timeout)
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The torture_config_make_absolute() and its _no_sshdir() version both
segfault on OpenBSD. The reason for this is that the storage returned
by getpwuid() is backed by mmap and is unapped by the getpwnam() call
in ssh_path_expand_tilde(), so a later access to home segfaults. The
possibility of this happening (getpwnam() overwriting values returned
by getpwuid()) is explicitly called out in POSIX.
A simple fix is to work with copies of username and homedir.
Signed-off-by: Theo Buehler <tb@openbsd.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
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>
On 32b architecture when processing the SFTP packets, the value
0x7ffffffc in the payload_len will overflow to negative integer values,
causing these checks to pass and possibly reading behind the buffer
bounds later.
This affects only SFTP server implementations running on 32b
architecture.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Set maximum input to 256MB to have safe margin to the 1GB trigger point
for 32b arch.
The OOB should not be reachable by any internal code paths as most of
the buffers and strings we use as input for this operation already have
similar limit and none really allows this much of data.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The spread out initialization and variable definition (and alising)
was hell to keep up with and was causing memory issues as reported by valgrind:
==4480== 128 bytes in 1 blocks are definitely lost in loss record 1 of 12
==4480== at 0x48463F3: calloc (vg_replace_malloc.c:1675)
==4480== by 0x487D152: mbedtls_mpi_grow (bignum.c:218)
==4480== by 0x487D6C5: mbedtls_mpi_copy (bignum.c:334)
==4480== by 0x48B9627: mbedtls_rsa_export (rsa.c:899)
==4480== by 0x283955: pki_key_to_blob (pki_mbedcrypto.c:976)
==4480== by 0x24F162: ssh_pki_export_privkey_blob (pki.c:2188)
==4480== by 0x278001: ssh_pki_openssh_privkey_export (pki_container_openssh.c:546)
==4480== by 0x24D7D2: ssh_pki_export_privkey_file_format (pki.c:1122)
==4480== by 0x24D916: torture_pki_rsa_write_privkey_format (torture_pki_rsa.c:895)
==4480== by 0x24D916: torture_pki_rsa_write_privkey (torture_pki_rsa.c:962)
==4480== by 0x4865499: ??? (in /usr/lib64/libcmocka.so.0.8.0)
==4480== by 0x4865C0B: _cmocka_run_group_tests (in /usr/lib64/libcmocka.so.0.8.0)
==4480== by 0x252115: torture_run_tests (torture_pki_rsa.c:1160)
==4480== by 0x2546B8: main (torture.c:1984)
==4480==
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Eshan Kelkar <eshankelkar@galorithm.com>
Originally reported by Till on mailing list here:
https://archive.libssh.org/libssh/2025-05/0000000.html
After some debugging, it turns out the client code does not guarantee
the extensions are processed before making decisions on the signature algorithm
that is being used for authentication, causing false-positive failures.
This does not happen in the tests, where we initially call ssh_userauth_none,
which enumerates authentications methods and as a side effect processes
outstanding packets such as SSH_EXT_INFO message with the server-sig-algs
extension.
When the first function called after `ssh_connect()` is
`ssh_userauth_publickey()`, the `ssh_userauth_request_service()` was wrongly
called only after the signature algorithm compatibility was checked.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <norbertpocs0@gmail.com>
The ssh_channel_is_closed function would crash when
accessing channel->session->alive if session is NULL.
This patch adds a null check before accessing the session
pointer.
- build succeeded
- unit test passed
- no new unit test added
https://gitlab.com/libssh/libssh-mirror/-/issues/239
Signed-off-by: Raviraaja Lakshmanan <mailstoraviraaja@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This option makes padding between macro names and the values based on
the longest macro name in a consecutive list of macro lines.
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This was omitted from the 84d02e74 and caused issues to build when abimap is
not present.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This could happen only if the function would really be called with the NULL
session, but this was never the case as the session is dereferenced already on
all code paths toward this place.
This is just to make the scanner happy that the session can not really be NULL
here.
Thanks coverity!
CID 1593926
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <norbertpocs0@gmail.com>
This commit adds test cases to catch issues where ProxyJump configurations lead to infinite loops or incorrect username usage, as reported in issue #287, and issue #291
Signed-off-by: Yuvraj Saxena <ysaxenax@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Try to find GSSAPI via pkg-config. If found, add the appropriate
module name, depending on the flavor, to the libssh.pc file so that
the pkg-config can report the list of libraries needed when linking
against the static library version of libssh.
Fix#293
Signed-off-by: John Thacker <johnthacker@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Starts a second sshd. This enables to test proxyjump through
multiple servers.
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This will work only with pkcs11 provider. Not tested with engines.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
... instead of keeping around public and private key blobs.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
The p11-kit remoting was initially introduced because softhsm
was crashing during cleanup with OpenSSL 3.0. This was resolved
since then and this code introduces a lot of complexity and
possible bugs, such as when using the mechanisms from PKCS#11 3.0
that are unknown to the p11-kit remoting tool. It decides to remove
them from the list as demonstrated here:
https://github.com/p11-glue/p11-kit/issues/668
This resulted in pkcs11-provider not registering EDDSA siganture
methods to the OpenSSL and failing when asked to provide a singature
by the Ed25519 key from the PKCS#11 token.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Add a cmake option to enable hermetic-usr, i.e., use of config files in /usr/.
If turned on, GLOBAL_*_CONFIG is prepended with /usr/ and defined as
USR_GLOBAL_*_CONFIG. Config lookup follows this path GLOBAL_*_CONFIG ->
USR_GLOBAL_*_CONFIG.
Introduce a ssh_config_parse primitive. This avoids convoluted checks for file
presence (without modifing the behaviour of ssh_config_parse_file) and allows
marking whether the config is global at the call site.
Signed-off-by: Lucas Mulling <lucas.mulling@suse.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The GSSAPI find module sets GSSAPI_INCLUDE_DIR (singular) only and
passes that to find_package_handle_standard_arguments, but later
tests and marks as advanced GSSAPI_INCLUDE_DIRS (plural), which doesn't
exist. GSSAPI_INCLUDE_DIR is what's used in src/CMakeLists.txt
This hasn't had a major effect, because GSSAPI_FOUND gets set by
find_package_handle_standard_args, so the if statement that tests
GSSAPI_INCLUDE_DIRS (and never succeeded) would have been a no-op
in any case, so remove it. Standardize on the singular version when
marking as advanced.
Signed-off-by: John Thacker <johnthacker@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The sftp API functions cannot interoperate properly with a
nonblocking ssh session.
Therefore code has been added in sftp_new() due to which the
function will return failure if the caller passes a non
blocking session without even trying to connect.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The macos images are not available for third-party contributors and
they prevent the CI to continue from the tests stage.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This is supported in clang, but only on x86_64 so we need to back down to the
architecture checks. Otherwise the checks pass with warning, but the build
itself fails with errors (-Werror).
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
All of the initial work was done by Simon. Jakub cleaned up the
formatting issues, resolved the padding of bignum to match specs
and be interoperable with OpenSSH (and few more minor details).
Closes: #194.
Signed-off-by: Simon Josefsson <simon@josefsson.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
A too low timeout caused spurious self-test failures in pkd_hello_i1.
Signed-off-by: Simon Josefsson <simon@josefsson.org>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
This implements stub for the compression functions and includes the
gzip.c in the compilation target uncoditionally, keeping the WITH_ZLIB
conditional compilation only in the gzip.c
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Coverity did not like the DEADCODE on 64b architecture.
After better look, the SFTP packet have read/write lengths in uint32 so
we really can not use the limits up to the uint64. Therefore we cap the
limits to uint32 while parsing and ignore any large value.
We then cap our reads/writes to this value, which is safe to cast to uint32
for wire format.
Thanks Coverity CID 1589435, CID 1589434, CID 1589432, CID 1589431
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
For some reason, the mingw64 builds were failing on these inputs quite reliably
as the passed value was interpretted as a value larger than UINT32_MAX.
This was not caught before because the value is casted from size_t to uint32_t
implicitly so the MSBs were not affecting the result.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The visual studio windows builds spit dozens of lines of warnings
on these.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
When the user frees the channel, they no longer expect any callbacks
to be triggered on it. When we delay the close before we receive
the remaining messages, we should not trigger the user callbacks
as it might be already freed.
I believe this is the random torture_session test failures and
errors we are getting from valgrind from time to time.
We keep the callbacks cleanup in the do_cleanup() in case the
calling application sets the callback after free for some reason.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
On mingw we are frequently getting a failure like this:
[ ERROR ] --- 451 is not within the range 1-450
This means the 50ms sleep did not manage to elapse the 50ms in the timeout
structure. Extending the range to 460 will give use more wiggle room if the
clock is not as it should be.
Related: #273
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
ssh_init calls ssh_crypto_init() which initializes the secure memory of
gcrypt. Those should actually be just called by the application once.
Lets do that.
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
In the meantime libgcrypt allocates 32k of secure memory, the minimum is
16k.
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The Visual Studio builds are completing in a manageable speed now.
Putting it back to the dependent chain of the CI to not cause any false
positive representation of the analysis stage (when the stage has all
skipped jobs, but the independent VS jobs succeed, the stage is shown as
success)
Reverting part of commit 91703202
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The previous code created private key curve25519 in OpenSSL, then exported
private key and during key generation, created a new OpenSSL private key object.
This is needless amount of copying potentially sensitive data back and forth and
this will not work when the private key would be backed with external OpenSSL
provider, such as pkcs11 provider or different crypto accelerator handling the
private key operations for us.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
This commit changes the way in which receiving sftp
responses is handled.
The old way polled/blocked on the channel before
checking the sftp response queue which could cause infinite
waiting by default if the required response is already present
in the response queue and no other sftp response is ever sent
by the server.
The new way checks the sftp response queue first for the
response before polling/blocking on the channel. This gets
rid of the potential infinite waiting bug.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
For the sake of reducing code repetition, this commit
adds a helper function to receive sftp response
messages. The function can operate in both blocking
and non-blocking modes.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This commit adds tests to check that the sftp aio API works
properly if the API user waits for responses of the read/write
requests in an order different from the sending order of the
requests.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Some SFTP servers (Cisco) do not implement the v3 protocol correctly and do not
send the mandatory part of the status message. This falls back to the v2
behavior when the error message and language tag are not provided.
Fixes: #272
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Fixed include order and formatting issues regarding the 80 char
column width limit
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The second argument to strftime() should be the size of the buffer
as per the manpage.
The previous code used size - 1 as the second argument. This commit modifies
that behaviour to use buffer size as the second argument of strftime().
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Windows supports localtime_s() instead of POSIX's localtime_r()
and the function prototype of localtime_s() is different as compared
to localtime_r().
This commit introduces ssh_localtime() (having same prototype as localtime_r())
for Windows which acts as a wrapper for localtime_s(), and defines localtime_r
as a macro which expands to ssh_localtime for Windows.
As a result, libssh can now use localtime_r() on Windows in the same manner
as localtime_r() can be used on POSIX systems.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Some SDK already defined __VA_NARG__, so without #ifndef __VA_NARG__ guard
we got a lot of "__VA_NARG__" redefined warnings.
Fix it by adding #ifndef __VA_NARG__ guard in include/libssh/priv.h.
Fixes: #279
Signed-off-by: Axel Lin <axel.lin@ingics.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Presumably this header is always imported with all the other necessary ones so
it doesn't usually make a difference, but generating Julia bindings from the
header by itself requires all the types to be defined (e.g. ssh_session,
ssh_channel, etc).
Signed-off-by: James Wrigley <james@puiterwijk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Thanks to Jakub Jelen for debugging and suggested fix. Fixes#273.
Signed-off-by: Simon Josefsson <simon@josefsson.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The global openssl configuration file automatically loads a pkcs11
provider, but it does it before we set up the token, which makes
the pkcs11 tests failing.
The workaround is to not load the global configuration, which is
delaying the loading of the pkcs11 provider to the time of first
use.
Consequently, this will require separate integration end-to-end
test that will verify the libssh works correctly with the pkcs11
provider loaded early.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Thanks to Jakub Jelen for debugging. Fixes#273.
Reproduce problem by changing the value to 1ms.
Signed-off-by: Simon Josefsson <simon@josefsson.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Commit 25a678190c introduced code coverage
collection. That also introduced a dependency to CXX language.
When cross-compiling libssh in an environment that doesn't have a C++ compiler
the following error is raised: "No CMAKE_CXX_COMPILER could be found.".
Since the C++ part is only needed for the coverage part, this commit only enable
that language dependency when actually needing it.
Signed-off-by: Thomas Perale <thomas.perale@mind.be>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Currently if a non-blocking `ssh_session` is passed most calls will fail because
they don't know how to handle `SSH_AGAIN`.
Signed-off-by: James Wrigley <james@puiterwijk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Renamed `process_unsupposed` to `process_unsupported`.
Signed-off-by: James Wrigley <james@puiterwijk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The "sane" default is now based on the man stty "sane" alias with addition of
utf8.
Fixes: #270
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The new OpenSSH rate limits the failed authentication attempts per source
address and drops connection when the amount is reached, which is happening
in our testsuite.
By whitelisting the IP address of the client on the socket wrapper,
this allows the tests to pass.
https://man.openbsd.org/sshd_config.5#PerSourcePenaltyExemptList
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
For some reason, both compress and decompress contexts were terminated
with both compress and decompress end functions (if the deflateEnd worked),
which was causing for some another unexplained reasons issues on i686
architecture when running the torture_packet unit test.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
... before checking the content.
This test was failing randomly when the read returned only partial buffer.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Casting int to bool might not always work as expected
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This is more portable than specifying a compiler flag explicitly.
Signed-off-by: James Wrigley <james@puiterwijk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Some architectures (esp32) might not have this API.
Fixes: #263
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This removes the code reported by the following coverity issue:
*** CID 1548867: Insecure data handling (INTEGER_OVERFLOW)
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Eshan Kelkar <eshankelkar@galorithm.com>
The coverity thinks the best_nlines could be 0 for logging at the end of the
function. It is obvious that the 0 is immediately incremented. Changing the code
to do this in one step to make it easier to understand for static analyzers.
** CID 1548873: Integer handling issues (INTEGER_OVERFLOW)
Thanks coverity
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Eshan Kelkar <eshankelkar@galorithm.com>
fix: implement gssapi logging according to docs
fix: remove redundant setting of session->gssapi to NULL
feat: add gssapi struct and functions to header file
refactor: initialize gssapi context once
fix: remove redundant ssh_gssapi_free
Signed-off-by: Gauravsingh Sisodia <xaerru@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
feat: tests set hostname for sshd, make GSSAPIStrictAcceptorCheck yes pass
feat: add GSSAPI_TESTING cmake option
feat: gssapi libssh server test
feat: make kdc setup and teardown functions
feat: add kinit, kadmin scripts to kdc setup function
feat: add some client gssapi auth tests
Signed-off-by: Gauravsingh Sisodia <xaerru@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
tests: modify proxyjump tests to check for ssh_jump_info_struct
tests: add proxyjump functionality test
feat: add SSH_OPTIONS_PROXYJUMP
tests: proxyjump, check authentication
fix: ssh_socket_connect_proxyjump add exit label to exit on error
feat: implement io forwarding using pthread
feat: proxyjump: use threading instead of forking
feat: proxyjump: cancel forwarding threads on ssh_disconnect
fix: proxyjump remove ProxyJump bool and put pthread ifdefs
feat: use ssh_event for io forwarding instead of threads
reformat: tests to use assert_int_not_equal
fix: link to pthread
refactor: make function to free proxy jump list
docs: add comment for proxy jump channel
feat: add env variable to enable libssh proxy jump
feat: open channel for proxyjump like OpenSSH
feat: add more tests for proxy jump
fix: use a global variable to close io forwarding, this prevents segfaults
fix: handle proxy list in thread without creating copy
Signed-off-by: Gauravsingh Sisodia <xaerru@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Eshan Kelkar <eshankelkar@galorithm.com>
.. to satisfy restricted environment or fuzzers
We are encountering weird issues in the oss-fuzz that the file disappears during
coverage build so I assume some corpus sneaked in, that contains some commands
that end up being executed as part of the coverage run causing it randomly
failing.
The solution I propose is to build fuzzers without ability to call arbitrary
commands on the filesystem (such as `rm -rf /`) as this is not the point the
fuzzers should be testing.
This is controlled by the WITH_EXEC CMake option (enabled by default).
https://github.com/google/oss-fuzz/issues/10136
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Eshan Kelkar <eshankelkar@galorithm.com>
Turns out it indirectly included err.h, which was needed for some other uses in
this file.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Eshan Kelkar <eshankelkar@galorithm.com>
The libgcrypt has a lot of reachable code so allowing it to fail
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The endif preprocessor directive was corresponding to an ifndef _WIN32,
a comment has been added which clarifies that.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
This commit also initializes the pointers in match_group() to NULL
in order to follow libssh coding guidelines.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
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>
This also adds mbedtls in the places where it was missing in documentation.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Previously the call to ssh_handle_packets() would be skipped if the buffer
wasn't empty. This meant that if ssh_channel_poll() was called on a non-blocking
channel with callbacks to handle incoming data, and the buffer already had some
data, the callbacks would never be called.
Signed-off-by: James Wrigley <james@puiterwijk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The datatype of the option value for the option SSH_OPTIONS_CONTROL_MASTER
should be int, this wasn't mentioned in the documentation. This commit
mentions that.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Text has been added to the documentation of ssh_options_set() and
ssh_bind_options_set() which explains what the third argument (value argument)
should be depending on the option value to set.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
For the data type of the third argument <value> corresponding to the
second argument <option type>, the documentation of ssh_options_set()
and ssh_bind_options_set() uses a scheme of (data_type *) in some
places whereas (data_type) in other places. Here data_type is the type
of the value which is to be set (it can be const char *, int, bool,
long, ssh_key etc)
This commit removes this inconsistency and uses the (data_type)
scheme everywhere.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The argument for RSA_MIN_SIZE ssh and sshbind option is of
(int *) type, and hence the caller can supply a pointer to a
location storing a negative value. The commit adds a check to
not allow minimum rsa key size to be set to a negative value.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
%u was being used for printing int type argument which is signed.
This commit changes the format specifier to %d.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
When the callback structure is allocated with malloc, some fields might be
uninitialized and therefore could cause undefined behavior or crashes.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
This way we will get errors as return code else we don't know if the
function failed (SSH_ERROR) or the exit_status is -1 which would
correspond to SSH_ERROR.
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This is what we get in the packet and is defined in RFC4254.
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This makes it easier to select it as a single test with:
`ctest -R torture_server_default*`
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This will only send the disconnect message and close the socket. We
should not free any memory here. This should be done by the server
implementation.
Pair-Programmed-With: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
This should prevent the long standing random failures of libgcrypt pipeline. I
was recently able to reproduce it only with dropbear, which sounds like choking
on the signature starting with bit 1, possibly interpretting it as a negative
value.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Add an error handler unittest for ssh_options_set with case SSH_OPTIONS_HOST when ssh_config_parse_uri returns error.
Signed-off-by: Wenjie Yang <yangw.ing@foxmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Add ssh_set_error_invalid in ssh_options_set with case SSH_OPTIONS_HOST after ssh_config_parse_uri returns error.
Signed-off-by: Wenjie Yang <yangw.ing@foxmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Legacy code in 'ssh_set_callbacks' will fallback to
'ssh_legacy_log_callback' (if the current log cb is
NULL) setting the user data to the current session.
However, if any other session is created afterwards,
it won't update the user data with the new session,
potentially leading to a use-after-free.
Fixes#238.
Signed-off-by: Diego Roux <diegoroux04@protonmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
and provide helpful warning message
CID 1533680: Memory - illegal accesses (OVERRUN)
Thanks coverity
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Since sftp_init() returns 0 on success, < 0 on error with ssh error set. This
change sets the appropriate ssh error when the SSH_FXP_VERSION packet cannot be
unpacked and sftp_init() return with -1.
Signed-off-by: Ajit Singh <ajeetsinghchahar2@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
fix: memory leak
fix: add defaults after parsing
fix: set defaults in ssh_bind_listen
tests: add test for checking default hostkey paths
remove: null check for hostkey paths, can't happen since we set defaults now
examples: ssh_server remove "no default keys", default hostkeys set in ssh_bind_listen
Signed-off-by: Gauravsingh Sisodia <xaerru@gmail.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Find the path to the library using cmake and enable this sort of logging only
with TORTURE_PKCS11 environment variable.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
The gcc should be able to select this automatically based on the presence of
-pthread is present on the commandline, but given that we link the tests static,
we do not have this?
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The OpenSSH as part of the new test torture_request_pty_modes attempts to chown
the pty to the faked user, which is obviously not permitted when the test does
not run as a root. But since all the permissions for SSH are faked, just
ignoring these requests should be safe enough giving expected results.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
When opening a PTY on the server, try to use the current TTY's settings
(i.e. based on STDIN). If that fails or STDIN isn't a TTY, use default
modes that avoid any character translation.
Don't rely on stdin to be a TTY (breaks CI). Instead, open a PTY and
temporarily use that as "fake" stdin.
Signed-off-by: Daniel Evers (daniel.evers@utimaco.com)
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Added the new function `ssh_channel_request_pty_size_modes` which allows
to pass additional encoded SSH terminal modes (see opcodes in RFC 4245).
Signed-off-by: Daniel Evers (daniel.evers@utimaco.com)
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Imported targets are highly preferred over the individual variables
for includes and libs because they will be used in a coherent way
and any spelling mistakes or unavailability won't go unnoticed.
Also it will prevent bugs like conan-io/conan-center-index#16900
or using mismatching header/libs combinations.
Signed-off-by: Gregor Jasny <gjasny@googlemail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
because if a fallback happens, the WITH_(GCRYPT|MBEDTLS) variables
do not match the selection, anymore. Also a silent fallback is pretty
bad if it is unnoticed.
Signed-off-by: Gregor Jasny <gjasny@googlemail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Socket paths have a length limit, and depending on the working directory of the
source code, these tests occasionally fail if the path is too long. Avoid this
by using a template string that is absolute and in /tmp, which should avoid the
socket path length issues.
This fixes building libssh with pkcs11 provider support in 'fedpkg mockbuild'.
Signed-off-by: Clemens Lang <cllang@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Fix read file with fgets and remove memory leaks
Remove use of ssh_pki_import_pubkey_file in ssh server and update max line size
Fix example server line no. and formatting
Fix check for leading whitespace in line
Reformat to avoid nesting
Remove setting sdata->authenticated to 0, the default is 0
Better error messages and handle case for fgets failing
Increment lineno at start
Signed-off-by: Gauravsingh Sisodia <xaerru@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Malicious code can be injected using the username with metacharacters,
therefore the username must be validated before using it with any %u.
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
A section has been added to explain the capping applied by the
sftp aio API. Also the example codes have been changed such that
they expect sftp_aio_begin_*() functions to return an ssize_t
indicating the number of bytes it requested the server to
read/write.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Following changes have been made:
1. The benchmark now expects sftp_aio_begin_write() to
return a ssize_t indicating an error (or) the number of
bytes for which it sent a write request.
2. If the user sets the chunk size > max limit for writing
via CLI, the benchmark does not use the set chunk size and
instead uses the max limit for writing as the chunk size
3. fprintf calls have been added to print the reason for
failure if the benchmark fails.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Following changes have been made :
1. The benchmark now expects sftp_aio_begin_read() to
return an ssize_t indicating an error (or) the number of
bytes for which it sent a read request.
2. If the user sets a chunk size > max limit for the reading
via CLI, the benchmark does not use the set chunk size and
instead uses the max limit for reading as the chunk size for
download.
3. fprintf calls have been introduced to print the reason
for the failure if the benchmark fails.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The tests have been changed such that the return value of
sftp_aio_begin_*() functions is expected to be a ssize_t
which indicates the number of bytes for which the function
sent a read/write request or error.
Tests for trying to read/write bytes more than the max limit
enforced by the API have also been added.
The negative tests for reading and writing have also been
seperated for the sake of clarity.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Test has been changed such that sftp_limits() is called
when the limits@openssh.com extension is supported as well
as when it is not supported.
Also, a simple negative test has been added for NULL
argument.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
In the sftp_init() call, the limits are stored in the sftp_sesssion.
If the limits@openssh.com extension is supported the limits are retrieved
from the server, else libssh uses the default limits.
The sftp library functions that require the limits can access them using
the sftp session.
The library user can call sftp_limits() to get a copy of the limits
stored in the sftp session. Since the limits were already retrieved
from the server during sftp_init(), this sftp_limits() call requires
no communication with the server.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This incorrectly stated that it would return an integer value instead of a
string.
Signed-off-by: James Wrigley <james@puiterwijk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The closing brace of the @addtogroup command was too low, causing some logging
functions to be added to the threads group.
Signed-off-by: James Wrigley <james@puiterwijk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <norbertpocs0@gmail.com>
The current documentation incorrectly states that it will return 0 on EOF, but
the function calls ssh_channel_poll() internally, which will return SSH_EOF,
which will then be returned by ssh_channel_read_nonblocking().
Signed-off-by: James Wrigley <james@puiterwijk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <norbertpocs0@gmail.com>
... and make coerity happy avoiding dead code
CID 1531320
CID 1531321
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <norbertpocs0@gmail.com>
This creates an XML file with information about each symbol, including the
anchors used in the URL. It's useful to have this to generate links to the
documentation from other documentation systems.
Signed-off-by: James Wrigley <james@puiterwijk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This prevents code injection.
The domain name syntax checker is based on RFC1035.
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The hostname can be a domain name or an ip address. The colon has to be
allowed because of IPv6 even it is prohibited in domain names.
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Using ssh_config_parse_uri can simplify the parsing of the host
parsing inside the function of ssh_options_set
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This also adds an fallback to OpenSSH file format in non-OpenSSL backends and
OpenSSH-compatible private key export for writing OpenSSH private keys.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The OpenSSL claims that these parameters are not mandatory and just speed up
calculations. But in reality, if they are missing, we can not export this key
into PEM files or if we export them, they are not readable/valid.
This was discussed in the following OpenSSL issue even with some proposed fix,
but it will take time before this will be implemented so in the meantime, we
back down to calculating the parameters manually as done in OpenSSH.
https://github.com/openssl/openssl/issues/21826
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
There are several tests that depended in the past on the fact that we can not
read public key from private encrypted keys. This is no longer the case for some
time as the OpenSSH file format has public key in plaintext.
This change just converts the same key into the PEM Format, which should still
be opaque for us and trigger code paths that enforce opening of the accompanied
public key file.
Converted using the following command:
$ ssh-keygen -m PEM -p -N secret -P secret -f tests/keys/id_rsa_protected
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
This avoids very-long test and false positives when using some
auto-pubkey authentication from picking up default keys, which are available in
bob's home directory when we want to test the certificate authentication.
The separate file is also needed because once we change to bob's UID, we can not
simply go back different UID and this sounds cleaner than setting up SSH_DIR to
different users ...
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
This involves reading the certificates from configuration files through options
and handling them similarly as the OpenSSH does when doing the auto pubkey
authentication, also in combination with agent or identities only.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
To not "pollute" projects with the compile-commands.json link if they include
libssh as a subproject (e.g. with add_subdirectory()), check if libssh is the
root project and only create the link in this case.
Signed-off-by: Sven Fischer <sven@leiderfischer.de>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Build binary dir contains the libssh_version.h file. By adding the binary dir to
the target include path, the include file can be found by projects which use
libssh as a sub-project by add_subdirectory().
Signed-off-by: Sven Fischer <sven@leiderfischer.de>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This commit removes the references of the old async sftp API from the
libssh sftp tutorial because the old async API is to be deprecated and
replaced by the sftp aio API.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
"Copying a file to the remote computer" is not an appropriate heading
for a subsection that describes how to open a remote file and write
"Hello World" to it.
That heading is not appropriate as the subsection does not show how
to copy a file from local to remote computer. Hence, this commit changes
that heading to a more suitable heading.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
benchmarks_async_sftp_aio_up() has been added in
tests/benchmarks/bench_sftp.c to obtain the performance
metrics of a upload using the low level async sftp aio
api.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
benchmarks_async_sftp_aio_down() has been added in
tests/benchmarks/bench_sftp.c to obtain the performance
metrics of a download using the low level async sftp aio
api.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
benchmark code present in tests/benchmarks/ directory
was linked with libssh dynamically due to which it
could use only the functions exposed in the public API
of libssh.
To be able to use those functions in the benchmark
code which are a part of libssh api but not a part of
the public api for libssh (examples of such functions
are ssh_list api functions), the benchmark code needs
to be linked statically to libssh.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This commit fixes connect_host() such that if
ssh_new() fails, connect_host() fails and provides
the reason for failure. Prior to this commit if
ssh_new() failed, connect_host() failed but did
not provide the reason for failure to connect to
the host.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
According to libssh coding conventions, function
return values must not be directly passed to if-
or while- conditions. This rule was not being followed
in connect_host(). A helper variable has been introduced
which stores the return code of the functions which
is then passed to the if- conditions.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
torture_sftp_aio.c has been added in tests/client/ directory.
It contains torture_sftp_aio_read(), torture_sftp_aio_write()
and torture_sftp_aio_negative().
torture_sftp_aio_read() tests sftp_aio_begin_read() and
sftp_aio_wait_read() to perform async reads.
torture_sftp_aio_write() tests sftp_aio_begin_write() and
sftp_aio_wait_write() to perform async writes.
torture_sftp_aio_negative() performs negative tests on the
sftp aio read/write API.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The existing sftp async read api has two problems :
1. sftp_async_read() assumes that the value of the third
parameter count is same as the number of bytes requested
to read in the corresponding call to sftp_async_read_begin().
But the documentation of sftp_async_read() allows the value of
count parameter to be more than that requested length. If value
of count parameter is more than that requested length then
sftp_async_read() updates the file->offset incorrectly which
leads to further read/writes occuring from incorrect offsets.
The problem here is that sftp_async_read() doesn't know about
the number of bytes requested to read specified in the call to
sftp_async_read_begin(), and it wrongly assumes the value
of its count parameter (which is actually the size of the buffer
to store the read data) to be the same as the number of bytes
requested to read.
2. sftp_async_read_begin() returns an uint32_t type value type
casted to int as a request identifier, whereas sftp_async_read()
expects an uint32_t type value as a request identifier. Due to this
the user has to typecast the identifier returned by sftp_async_read_begin()
from int to uint32_t and then pass it to sftp_async_read(). This
type casting is cumbersome for the user and hence the approach is
not user-friendly.
This commit solves the above two problems by introducing a new
sftp aio api.
The sftp_aio_begin_*() functions in the api send an i/o request to
the sftp server and provide the caller a dynamically allocated
structure storing information about the sent request. Information
like number of bytes requested for i/o, id of sent request etc is
stored in the structure.
That structure should be provided to the sftp_aio_wait_*() functions
in the api which wait for the response corresponding to the request whose
info is stored in the provided structure.
The libssh user is supposed to handle that structure through an
opaque type sftp_aio.
Since the structure stores the number of bytes requested for i/o,
sftp_aio_wait_*() knows about the number of bytes requested for i/o
(specified in the call to sftp_aio_begin_*()) and hence updates the
file->offset correctly solving problem #1 present in the existing
async api.
Since the structure provided by sftp_aio_begin_*() (containing the
request id) is supplied to sftp_aio_wait_*(), no casting of id's
needs to be done by the user solving problem #2 of the existing
async api.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Currently the sftp api code is limited to sftp.c, sftpserver.c
In future it can be required to add new sftp related APIs which
are present in their own separate source files instead of adding
their code to the already large sftp.c file.
Those new hypothetical or existing (in sftpserver.c) sftp API
functions present in the source files other than sftp.c will
need to call certain functions present in sftp.c which are not
provided in the public api as they are for internal use (by other
sftp related functions) only. Some of these sftp.c functions have
external linkage, some of them don't and cannot be currently accessed
outside sftp.c
This commit :
1. Moves such functions along with the functions they depend on
from sftp.c to a new file sftp_common.c, to seperate them out
from other sftp api functions.
2. Makes necessary changes to make required functions visible
outside sftp_common.c
3. Uses the header file sftp_priv.h for necessary declarations
(and not sftp.h) since these functions are not to be provided
in the public sftp api.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
ssh_channel_read() reads the data into the buffer, but doesn't
append a '\0' after it. When the buffer is asserted to be equal to
a string further in the test, the assertion could fail if the byte
after the data stored in the buffer doesn't contain '\0' (and it mayn't)
This commit appends a '\0' after the data read into the buffer before
comparing it with a string.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The curl does not do any (or enough) chunking when writing large files using the
sftp_write() function which causes some servers to choke [1]. The simplest
solution is to limit the SFTP packet size according the SFTP specification
recommendation which is 32768 B and not write more.
This means the function will not write the whole amount of data it was asked to
write and the calling applications are required to handle the return values
correctly.
More complicated solution would be to send several SFTP packet from the single
sftp_write() function by iterating over the all data passed.
The next improvement in the long term should be respecting the value reported by
the server in the limits@openssh.com extension, which specifies the maximum
packet size and reads/writes explicitly (if supported).
[1] https://github.com/curl/curl/pull/11804
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Some architectures (s390x) provide different poll events such as POLLHUP in case
the remote end closed the connection (and they keep reporting this forever).
This is an issue when the user provided callback registering this event as an
error and tries to send some reply (for example EOF) using
`ssh_channel_send_eof()` which will lead to infinite recursion and sefgaults.
This was not solved by the 30b5a2e33b because the
POLLHUP event is not provided by the poll in events bitfield, but only reported
by the poll in revents bit field thus we need to filter these events later on
when the poll is recursively.
Fixes#202
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The FD locking was modified in 30b5a2e33b but it
caused some weird issues on s390x in Debian tests, which were getting POLLHUP,
causing infinite recursion while the callback tried to close socket.
Previously, the lock blocked only the POLLIN events as we believed these were
the only events we could get recursively that could cause issues. But it looks
like more sane behavior will be blocking everything but POLLOUT to allow the
buffers to be flushed.
Fixes#202
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The window size controls how much data the peer can send before
we send back a message to to increase the window.
This changes the default window from 1.28MB to 2MiB. 2MiB matches
the OpenSSH default session size.
The code is also refactored to grow the windows on code paths
where data is consumed, and move the condition that checks
if the growing the window is needed into the grow method.
Signed-off-by: Tom Deseyn <tom.deseyn@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
We use string(APPEND) from version 3.4 for 5 years and CMake is deprecating
support for versions before 3.5 so bumping one more version.
Fixes: #209
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Aris Adamantiadis <aris@0xbadc0de.be>
The intention is that this releases memory allocated by the crypto
library, for functions like bignum_bn2hex() and bignum_bn2dec().
Consequently, ssh_gcry_bn2dec and ssh_mbedcry_bn2num should use
gcry_malloc() and mbedtls_calloc() respectively to allocate
memory since it will/should be released by ssh_crypto_free() so
that the internal APIs are consistent between crypto libraries.
Signed-off-by: Simon Josefsson <simon@josefsson.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
A call to write() may perform a short write on a local file.
To avoid short writes, ssh_writen() can be used.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
A call to read() may peform a short read from a local file even when
sufficient data is present in the file. ssh_readn() can be used
instead of read() to avoid such short reads.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
This aligns it with libgcrypt/OpenSSL backends which uses char*.
It also aligns mbedcrypto's bignum_bn2hex() to use an unsigned
cast just like OpenSSL backend.
Signed-off-by: Simon Josefsson <simon@josefsson.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
assume stderr by default and log only warning in case the data type is
non-standard.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Compatibility code for mapping open, read, write, close and unlink
to _open, _read, _write, _close and _unlink respectively on Windows
was repeated in a lot of .c files.
This commit adds that compatibility code to include/libssh/priv.h
and removes it from the .c files (while ensuring that those .c
files include priv.h) so that the compatibility code stays in one
place, can be maintained easily and can be added easily to another
source file by including priv.h in that file.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Originally reported by Wei Chong Tan <shellcurity at protonmail.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
... to better describe the function, which is only replying to the client with
our version and extensions.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
These can be replaced by user-provided functions when needed.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
there were unused structure members and some code formatted not following our
code guidelines.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
This allows the libssh to build with more memory-strict compliers.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
When one host has multiple addresses returned by `getaddrinfo` try not just
the first address. The scenario where the first address is wrong but the
second is good was failing, because the second address was never tried.
This applies to ipv6 as well as to ipv4 addresses.
As the implementation uses non-blocking sockets it may return EINPROGRESS
when error happened as well as just "non-blocking" statement. The socket
can not be queried for status code to determine the error if any, because it
requires calling blocking functions.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
If IPv6 address fail to connect IPv4 should be tried in non-blocking mode.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
When the server is reaches MaxAuthTries it sends disconnect immediately
which was not correctly handled in the libssh client and hanged.
Solves #11
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
It does not have a PATH_MAX compile-time limitation. Instead of using it we
can just allocate dynamically.
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The pkcs11 tests bring a lot of dependencies that are outside of our control
(openssl, engine_pkcs11, pkcs11_provider, p11-kit, softhsm, ...) and that might
always not handle the memory well.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
We used to have this in the old ctest. The code should be ready for this as we
already skip tracing openssh sshd (ca4fb9c6) and have workaround for openssl
issues (55252e4d), but it took me some time to figure out the secret command to
run tests under valgrind with cmake.
This adds also convenient custom target to run the memcheck manually.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
The "dynamically" loaded server is using allocated state and using something
else complicates proper cleanup.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
The address was tested twice so repurposed the needless check for the check for
hostkey, which is also mandatory
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Fixed a memory leak in pki_privkey_build_ecdsa. The BIGNUM bexp was
getting allocated, but not free'd. It gets stored by reference in
param_bld.
Signed-off-by: roytak <xjanot04@fit.vutbr.cz>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
strtoul returns 0 if no valid character was parsed, which
can unwantedly misconfigure the options.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The return value is changed by the call to pki_key_check_hash_compatible
causing the possibility of returning SSH_OK if memory allocation error
happens later in the function.
The assignment of SSH_ERROR if the verification fails is no longer needed,
because the value of the variable is already SSH_ERROR.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This was for some reason failing on CentOS 7 in 0.10 branch so bringing this to
the master too.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
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>
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>
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>
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>
torture_sftp_rename has been added which
tries to rename an existing file (positive
test case) and tries to rename a file that
does not exist (negative test case).
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Changes done in sftp_rename such that it will use
posix-rename@openssh.com extension if supported
and send a SSH_FXP_EXTENDED request. If the
extension is not supported a normal SSH_FXP_RENAME
request will be sent.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
For testing sftp_hardlink, torture_sftp_hardlink has been
introduced in tests/client.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
sftp_hardlink() has been introduced which when called
sends a SSH_FXP_EXTENDED request to server for creating
a hardlink.
Signed-off-by: Eshan Kelkar <eshankelkar@galorithm.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The client fuzzer can get stuck in poll call, when there is long connection
timeout and there are no usable message from the peer. Setting smaller user
timeout allows us spend more time productively fuzzing and exit early when there
is no message from peer.
Thanks oss-fuzz.
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56935
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Related to the accidental removal of compiler flags as reported in #185
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
Fixed memory leak in pki_publickey_to_blob when using an EC type of
hostkey.
Signed-off-by: roytak <xjanot04@stud.fit.vutbr.cz>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Removed it from the wanted methods list in the ssh_options_set function. Now users have to set the compression value to 'zlib' explicitly to enable it.
Updated unit tests to reflect removing zlib compression algo from the defaults compression algorithms.
Signed-off-by: Khalid Mamdouh <khalidmamdou7@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Calculate the coverage of the documentation so we now where we are at.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The choco server is somtimes ratelimited. Avoid running into issues
and use cmake already installed on the runner.
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The Coverity scan CID 1506418 found the null pointer dereferencing
Signed-off-by: Ahsen Kamal <itsahsenkamal@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The engine API in OpenSSL 3.0 is deprecated so we are in the progress of working
on a PKCS#11 provider for OpenSSL. This commit introduces a conditional build
with the pkcs11-provider support (instead of engines) with all the changes
required for the provider to work with existing code and tests.
The CI modification is only temporary before we will have the real package in
Fedora or somewhere to use.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
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>
Libssh defaults to QUIET or SSH_LOG_NONE regarding of loglevel. Have it
documented to not confuse the users.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
(Some) structures, typedefs and macros don't need to be included in the
documentation.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
- The nid is unused in the new context
- The variable `params` is defined locally in the function, fixing redefinition
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
and improve debug logs and error checking. Thanks Norbert for the hints.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
It turns out there is a way to get the uncompressed format from the low-level
API, which is not (yet?) deprecated so this removes all of the TODO's for ECDSA
keys and moves the EC_KEY structure in the high-level EVP_PKEY.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
... through bundled code with OpenSSL. These were needed with older OpenSSL
versions before 1.1.1.
After removal in 358ce46551 these were just static
ifdef so this will simplify the code.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
The FreeBSD builder complains like this:
/home/gitlab-runner/builds/jtxr_hfi/0/jjelen/libssh-mirror/src/token.c:438:15: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]
if ((list == NULL)) {
~~~~~^~~~~~~
/home/gitlab-runner/builds/jtxr_hfi/0/jjelen/libssh-mirror/src/token.c:438:15: note: remove extraneous parentheses around the comparison to silence this warning
if ((list == NULL)) {
~ ^ ~
/home/gitlab-runner/builds/jtxr_hfi/0/jjelen/libssh-mirror/src/token.c:438:15: note: use '=' to turn this equality comparison into an assignment
if ((list == NULL)) {
^~
=
1 warning generated.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
This avoids a lot of long and hard to read constants by replacing them with
dynamic snprintf()s and a bit or reformatting
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
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>
This prevents building the pkcs11-related functions and printing pkcs11-related
log messages when the libssh is built without PKCS#11 support.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
This adds the priv_wrapper options to skip the OpenSSH server sandbox, which
prevented in the past writing any debug information or coverage files causing
SIGSYS/crashes.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
This adjusts also usage of ARGP_LIBRARY to use ARGP_LIBRARIES which is defined
by the FindArgp module, unlike the former one in case it is provided by libc
directly.
Fixes: #167
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
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>
For code simplification and less ifdefs removing DSA and RSA
structures of the old openssl api and using the new EVP_PKEY
api to store the legacy keys.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Test that ssh_options_apply can be called multiple times without expanding
escape characters more than once. If the options are updated after calling
ssh_options_apply keep the last options.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Test if the ssh_options_apply is called on session before ssh_options_copy,
then `opts.identity` ssh_list will be copied
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Calling `ssh_options_apply` more times can result in an unwanted behaviour of
expanding the escape characters more times. Adding flags to check if the
expansion was already done on the current string variables.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The configuration of identities are first saved to `opts.identities_non_exp`,
then moved to `opts.identities` after calling ssh_options_apply and expanding
the identity strings. These tests are testing against the proper configuration
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Expanding a string twice could lead to unwanted behaviour.
This solution creates a ssh_list (`opts.identites_non_exp`) to store the strings
before expansion and by using ssh_apply it moves the string to the
`opts.identities`. This way the expanded strings are separated.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The openssl 3.0 support has introduced some memory leaks at key build as
OSSL_PARAM_BLD_push_BN duplicates the bignum and does not save the pointer
itself.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
src/init.c:118:9: warning: incompatible implicit declaration of built-in
function 'fprintf' [enabled by default]
fprintf(stderr, "Error in auto_init()\n");
^
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Norbert Pocs <npocs@redhat.com>
src/dh.c:824:5: warning: incompatible implicit declaration of built-in
function 'fprintf' [enabled by default]
fprintf(stderr, "%s\n", fingerprint);
^
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Adjust some subtle usage of argv string handling in the pkd
test options: rather than conditionally overwrite the two
mkdtemp strings with a newly-allocated buffer to be later
freed, keep the original const argv pointer around in its
own dedicated field.
See also these changes in the same area that were due to the
previous arrangement, which was a bit too subtle:
- 61ce3310b864802a101cb01ff103f0bc2da936e6
- e1a8b359c1
Addresses:
- https://gitlab.com/libssh/libssh-mirror/-/merge_requests/320#note_1173911211
Signed-off-by: Jon Simons <jon@jonsimons.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Use the new `-L` flag for the pkd tests so that they use a
unique temporary directory for scratch space while running.
Note the choice of `pkd_scratch_XXXXXX` in contrast to a
path living under `/tmp`: by using a relative path, one can
gather the full set of log artifacts from the GitLab CI jobs
in the event that there is a test failure. The logs contain
lots of information to help pinpoint what went wrong.
Resolves https://gitlab.com/libssh/libssh-mirror/-/issues/143.
Testing notes:
- In the GitLab CI jobs I can see the flag being used, and
can observe that I am able to gather the full set of
detailed `pkd` logs in the event of a legitimate test
failure.
Signed-off-by: Jon Simons <jon@jonsimons.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Teach `pkd` a new flag `-L, --temp-dir=<mkdtemp-template>` to enable
behavior whereby `pkd` creates a new temporary directory and uses it
for a workspace while running.
The original design of `pkd` assumed that it could freely use the
current working directory from wherever it happened to be invoked.
But, this could pose a problem when multiple `pkd` instances are run
in parallel from the same working directory, due to the usage of
various temporary files within that directory.
To avoid the problem of multiple `pkd` instances interfering with
each other, expose a `-L` flag for optionally specifying a `mkdtemp`
template string such that a temporary scratch space is used instead.
Testing notes:
- I ran handfuls of iterations locally using the new flag
and observed `pkd` is indeed using scratch space as desired.
Resolves https://gitlab.com/libssh/libssh-mirror/-/issues/143.
Signed-off-by: Jon Simons <jon@jonsimons.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Relax the `pthread_kill` result assertion in `pkd_stop` to tolerate
`ESRCH`, and guard against only `EINVAL` and `ENOTSUP`.
On macOS what can happen is that the `pthread_kill` returns `ESRCH` due
to the accept thread being already terminated. For that case, the
teardown path should proceed to `pthread_join`.
Testing notes:
- On my macOS setup I consistently encountered `ESRCH` in this
codepath, causing pkd tests to fail unnecessarily. With the
change, I observe the tests passing.
Signed-off-by: Jon Simons <jon@jonsimons.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Fix this warning generated by cmake 3.24.3 on macOS:
CMake Warning (dev) at /opt/homebrew/Cellar/cmake/3.24.3/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:438 (message):
The package name passed to `find_package_handle_standard_args` (ARGP) does
not match the name of the calling package (Argp). This can lead to
problems in calling code that expects `find_package` result variables
(e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
cmake/Modules/FindArgp.cmake:63 (find_package_handle_standard_args)
CMakeLists.txt:107 (find_package)
This warning is for project developers. Use -Wno-dev to suppress it.
Testing notes:
- With this change, the warning is gone on my macOS setup.
Signed-off-by: Jon Simons <jon@jonsimons.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
In e1a8b359c1 a missing `free` was
added to `pkd_cleanup_socket_wrapper` to free a string allocated
for the socket wrapper directory name.
Move that `free` such that it also runs in the error-out paths in
`pkd_cleanup_socket_wrapper`, to avoid a leak in those cases, too.
Signed-off-by: Jon Simons <jon@jonsimons.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
It should be possible to use features to add,remove,prioritize
algorithms in the algorithm list from the config file.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
These features allow for options Ciphers, HostKeyAlgorithms, KexAlgorithms and
MACs to append, remove and prepend to the default list of algorithms
respectively
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
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>
Functions `ssh_remove_all_matching` and `ssh_prefix_without_duplicates` were
added; a little test suite will suite them.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
"template" is a c++ keyword which will make the build fail.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
To make sure c++ name mangling works correctly c code should be noted "extern"
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
EVP_DigestSign and EVP_DigestVerify are implicitly included in new (>1.1.1)
openssl versions, no need to use the old functions.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
POLY1305 is implicitly included in new (>1.1.1) openssl version, no need
to check it explicitly.
CHACHA20 is implicitly included too, but it can be turned off at config
in openssl, so we still need to check it.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
X25519 is implicitly included in new (>1.1.1) openssl version, no need
to check it explicitly.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
ED25519 is implicitly included in new (>1.1.1) openssl version, no need
to check it explicitly.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Removing support for older openssl versions than 1.1.1 makes some functions
implicitly included; we do not have to check the availability of these
functions.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The c7s uses not longer supported openssl version which will make
the CI fail when we remove the supported of the old openssl versions.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
As openssl 1.1.0, 1.0.2, 1.0.1, 1.0.0 and 0.9.8 are no longer supported
let's remove them.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
As openssl 1.1.0, 1.0.2, 1.0.1, 1.0.0 and 0.9.8 are no longer supported
let's remove them.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
As openssl 1.1.0, 1.0.2, 1.0.1, 1.0.0 and 0.9.8 are no longer supported
let's remove them.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
libsofthsm detection is broken in i686 architecture. The approach is to export
the path found by cmake to `tests_config.h` and the script
setup-softhsm-tokens.sh gets that value through cli parameters.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The error was the following
/builds/libssh/libssh-mirror/examples/sshnetcat.c:241:18: error: a function
declaration without a prototype is deprecated in all versions of C
[-Werror,-Wstrict-prototypes]
void cleanup_pcap(){
^
void
and similar
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Let's check tests in fips mode with an up to date system too as we already
found some issues running the tests there.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
uint32_t should be formated by PRI?32
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Change-Id: I811cfd764010f9e8bb599b370155ac065ee1905c
When we decide we need to rekey, we enqueue any further packets until we've sent
our SSH2_MSG_NEWKEYS message, after which we dequeue these packets and send them
to the other side. This enqueueing is done based on ssh_packet_in_rekey checking
the session flags and whether DH handshake state is marked as finished.
However, the handshake state is not reset to DH_STATE_FINISHED until the other
side has sent us their new keys. This leaves a gap between sending our new keys
and receiving the other side's new keys where we would still decide to enqueue a
packet.
These enqueued packets will not be dequeued as we've already sent our new keys.
Once we've received the other side's new keys, we'll go back to a finished
handshake and we will send out our caller's new data, skipping however much data
we've enqueued.
Fix this by changing ssh_packet_in_rekey to return false once we've sent our new
keys.
Signed-off-by: Carlos Martín Nieto <carlosmn@github.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
These warning should be logging when something fatal happens and give
information on the error to the user.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
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>
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>
No info log will be printed out when Loglevel WARN is set, only errors
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Commit 8cf9c816 "Do not force GNU_SOURCE during build to fix#141" has broken support for CYGWIN.
The build hangs with this error message:
libssh-0.10.4/src/config.c: In function ‘local_parse_glob’:
libssh-0.10.4/src/config.c:252:25: error: ‘GLOB_TILDE’ undeclared (first use in this function)
252 | rt = glob(fileglob, GLOB_TILDE, NULL, &globbuf);
| ^~~~~~~~~~
I think that it would be better to re-add it, by using target_compile_definitions() rather than target_compile_options(), which is more appropriate in my opinion.
Signed-off-by: Carlo Bramini carlo_bramini@users.sourceforge.net
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
FIPS 140-3 puts big limitations on using TDEA and it is
already disabled in rhel9.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This fixes patch relocation of the output of pkgconfig by adding
missing keywords like prefix, exec_prefix, libdir and includedir.
The pkgconfig output changes are like following:
* Before:
$ pkg-config -libs libssh
-lssh
* After:
$ pkg-config -libs libssh
-LF:/msys64/ucrt64/lib -lssh
See https://people.freedesktop.org/~dbn/pkg-config-guide.html for
further documentation.
Signed-off-by: Biswapriyo Nath <nathbappai@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Missing escaping results in syntax errors in Zsh shell because of square
brackets getting interpreted as being a pattern for globbing.
Signed-off-by: Thomas Baag <libssh-git@spam.b2ag.de>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The key_type is only a letter, if we use and `int` and then cast it to
(const char *) we will end up with a 0 value on big endian.
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
We should not end up with an infinite loop here.
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
The test failed on Fedora Koji and openSUSE Build Service on i686 only. Probably
the rekey on the server needs longer here to collect enough entropy. So we need
to try harder before we stop :-)
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This wont create a new process but replace the shell.
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The ncat tool from nmap is available on all unix platforms. The nc
binary might link to ncat or something else. Settle on one we know
also the options can be used if needed.
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
error: ‘%u’ directive output may be truncated writing between 1 and 10
bytes into a region of size 6.
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
mingw does have this function, even though it appears to be deprecated.
So the symbol has to have a different name, or linking becomes
impossible.
Signed-off-by: Timo Rothenpieler <timo@rothenpieler.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The old version is missing the EVP_PKEY_up_ref(), which is needed to keep track
of the EVP_PKEY references.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
The added gitleaks configuration file uses 'tests/*' as the pattern of
paths allowed to contain private keys. This avoids false positives
during code scans caused by private keys used for testing.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
what we set in open funtion by the argument mode. The mode of the created file
is (mode & ~umask), So we set umask to typical default value(octal 022).
Signed-off-by: renmingshuai <renmingshuai@huawei.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
There are long-standing issues with fuzzing, which cause the send() not writing
all the provided bytes and causing the fuzzer driver to crash. This can be
simply solved by limiting the input size to reasonably large value.
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=21967
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This also allows testing mbedtls with the PKCS8 PEM files
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
How to use:
Install 'git-format-clang' which is part of the clang suite (Fedora:
git-clang-format, openSUSE: clang-tools).
Now do your changes and stage them with `git add`. Once they are staged
format the code using `git clang-format` before you commit.
Now the formatting changed can be viewed with `git diff` against the
staged changes.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The external ed25519 requires also the sha512 functions to work.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The curve25519 depends on ssh_get_random, which is normally built into libssh.
For the external override tests to build, we need to have them in separate
source file that can be included for this test.
For some reason, this did not happen on CI builds, but it did happen in koji
during RPM builds.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The KDF was changed in the new API, fetching the algorithm first
then creating the context using it.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The new API does not provide EVP_KDF_CTX_new_id function, insted
it works with EVP_KDF_CTX_new and fetching the algorithm.
Adding a check for both to make it work with the new API too.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
EVP_PKEY_dup can't be used with ENGINE generated keys and
the key can't be freed because it is passing the main key
from the struct.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Rawhide has openssl 3.0 support which can test the new changes.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
New openssl API, libmbedtls, libgcrypt use size_t for
HMAC len pameter.
New helper functions were added in libcrypto.c to avoid
code duplication. (the header pki.h is needed for this
reason)
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
- strerror_r for linux
- strerror_s for windows
Keep in mind that strerror_r has two versions:
- XSI
- GNU
see manpage for more information
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The tests are compiled without -D_GNU_SOURCE, therefore
the XSI version of strerror_r is used. Defining
_GNU_SOURCE in torture.h then including *.c gives error
because it is assuming GNU version of strerror_r in
the source file.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Fix#98 by adding 'ssh_session_set_disconnect_message' Whenever the ssh
session disconnects a "Bye Bye" message was set and displayed. Now the
peer has a choice to set a customised message to be sent after the
session is disconnected. The default "Bye Bye" will be set if this
function is not called or not called correctly. The testcases in
tests/server/torture_server can also demonstrate how this function
works.
Signed-off-by: Om Sheladia <omsheladia10@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This commit adds an `agent_socket` field to the session options
and connects the config parser to that option.
`SSH_OPTIONS_IDENTITY_AGENT` is added to allow applications to
set this option for themselves.
agent.c is updated to take the `agent_socket` value in preference
to the `SSH_AUTH_SOCK` environment variable.
Signed-off-by: Wez Furlong <wez@fb.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Windows has supported unix domain sockets for a couple of years
now; see this article for more information about that:
<https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/>
This commit allows libssh to consider using agent authentication
on Windows systems.
It is mostly removing `#ifndef _WIN32` that prevented the unix
domain socket code from being compiled in, and adjusting the use
of `read(2)` and `write(2)` to `recv(2)` and `send(2)`, as the former
functions are not compatible with sockets on Windows systems.
For mingw systems, afunix.h isn't available so we use the
technique as was used to resolve building with mingw as used
by the curl project in: https://github.com/curl/curl/pull/5170
Signed-off-by: Wez Furlong <wez@fb.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
According to the documentation the return value is the number of
processed bytes, so the returned value is never negative. We should not
use ssize_t in public headers as it isn't available on Windows! We only
have it defined in priv.h!
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Contaminated errno can happen before strtoul call, thereofore
cleaning it before the call.
The errno is not used for checking later in code if fail happens,
therefore cleaning it right after error.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
also remove anything mentioning limitation to SSHv2 as it is the only
protocol supported these days.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The cmocka captures all signals so I was not able to reproduce the code
path. But leaving the code in for future readers.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Added API function ssh_channel_open_forward_port that is the same as
ssh_channel_accept_forward with the addition to determine the
originator address and port
Signed-off-by: Tomas Holmqvist <tomhol@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Thanks to Harry Sintonen from WithSecure for pointing this out.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Thanks to Harry Sintonen from WithSecure for pointing this out.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Thanks to Harry Sintonen from WithSecure for pointing this out.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Thanks to Harry Sintonen from WithSecure for pointing this out.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The original key had 2018 bits, which falls short for current security
requirements
Steps I used:
$ ssh-keygen -f tests/keys/certauth/id_rsa -t rsa -b 2048 -C libssh_torture_auth -N ''
$ ssh-keygen -s tests/keys/user_ca -I torture_auth_carlos -n alice ./tests/keys/certauth/id_rsa.pub
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Remove config options: protocol, mac, cipher, compressionlevel,
gssapikeyexchange,
gssapirenewalforcesrekey,
gssapitrustdns,
rhostsrsaauthentication,
rsaauthentication,
useprivilegedport,
pubkeyacceptedtypes
since they are not supported by OpenSSH
Rename some config options:
hostbasedkeytypes, challengeresponseauthentication and pubkeyacceptedkeytypes
to hosbasedacceptedalgorithms, kdbinteractiveauthentication and pubkeyacceptedalgorithms
to be consistent with the OpenSSH manual https://man.openbsd.org/sshd_config.
Keep pubkeyacceptedkeytypes for backward compatibility.
Rename SOC_PUBKEYACCEPTEDTYPES to SOC_PUBKEYACCEPTEDKEYTYPES in config.h
Update unittests/torture_config.c and unittests/torture_options.c
Signed-off-by: Anh Minh Tran <anhminh@seznam.cz>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Use the POSIX strtok_r() or equivalent implementations to resolve.
Thanks to @wez1 for the early review.
Fixes#104
Signed-off-by: Gene Oden <goden@fb.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
FIPS_mode function is no longer supported in openssl version 3
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The libssh versions before this included files with relative path based
on the current working directory, which can result unexpected results
and is different from the OpenSSH include behavior. The manual page for
ssh_config lists:
> iles without absolute paths are assumed to be in ~/.ssh if included in
> a user configuration file or /etc/ssh if included from the system
> configuration file.
This is following the semantics as close as possible with a difference
that we do not use the hardcoded ~/.ssh for user configuration files,
but the path defined with SSH_OPTIONS_SSH_DIR, which is already used
to reference any other files in used home directory.
Fixes#93
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This fails if the gethostname() fails in a way that does not write the
buffer, but returns 0 as a success.
Fixes#106
Thanks oss-fuzz
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
This prevents the usage of uninitialized value on error in the known
hosts hasing code if invalid (empty) hostname is used.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
The adjacent question marks and asterisks can be simplified to single
wildcard so there is no need to excersise all the recursive pattern
matching.
These inputs were generated by oss-fuzz and probably caused also the
previously reported timeouts.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This is more than enough as it reflects to 16 asterisks in the match
string is more than enough. With larger values oss-fuzz was generating
long match strings with asterisks interleaved with normal characters,
which were timing out.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
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>
RFC 4252 §7 states that the public key algorithm in a
SSH_MSG_USERAUTH_PK_OK response is the public key algorithm name from
the request. When using RSA with SHA-2, this will be either
"rsa-sha2-256" or "rsa-sha2-512" as specified by RFC 8332 §3.2.
However, currently libssh emits the public key type instead, which is
"ssh-rsa". This is not in conformance with the RFCs, so let's fix this
by storing the signature type and emitting it in our response instead of
the public key when sending SSH_MSG_USERAUTH_PK_OK in the server.
Signed-off-by: brian m. carlson <bk2204@github.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Similarly as we already have for the client configuraiton file
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Do not compile tests which need SSHD_EXECUTABLE when
it is not available
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Calling some channel procedures on a freed channel is always resulting
in segmentation fault errors. The reason is that when a channel is
freed with 'ssh_channel_do_free' procedure, its 'session' field is set
to NULL; then when a channel procedure tries to access any field of
'channel->session' structure it is effectively dereferencing a NULL
pointer.
The change fixes that behavior by adding a check which ensures that a
channel state is not SSH_CHANNEL_FLAG_FREED_LOCAL before accessing its
parent session.
Also the test suite is updated to check for the fixed errors, and the
Doxygen documentation updated accordingly.
There was a bug introduced in b0fb7d15: 'ssh_channel_poll',
'ssh_channel_poll_timeout' and 'ssh_channel_get_exit_status' would
compare the channel state to the 'SSH_CHANNEL_FLAG_FREED_LOCAL'
constant to check if the channel is alive. But the procedures must
check the channel flags for the presence of
'SSH_CHANNEL_FLAG_FREED_LOCAL' bits instead. This change fixes the
bug.
Signed-off-by: Artyom V. Poptsov <poptsov.artyom@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
We do not use SHA1 as it is disabled in many systems
Verifies CVE-2021-3634
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
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>
and add LOG_SIZE macro to control the buffer size
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Change-Id: I3eaeea001fc531fdb55074fc3a9d140b27847c1f
since ssh_scp_deny_request is seldom called, let's
utilize malloc to reserve the precise size memory.
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Change-Id: I8e7a6d3153cff7691329b9487cd534a7f2887a35
to avoid allocate 4KB buffer from stack
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Change-Id: Id144ff764ee1ae98f87aee36793a9f0e4fce21b7
so the same code base demo both multi-process and multi-thread model
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Change-Id: I98554a99b7a31586be37abde7c357f81a05c3d6e
to avoid allocate 16KB buffer from stack and one memory copy
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Change-Id: Ib71cb5834b7810bf9791e13c58571e2b9fa5bca1
to avoid allocate 8KB buffer from stack
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Change-Id: Ifc198705cb8ecec6f0a609f84965382dc151693b
to reduce the stack size requirement
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Change-Id: I6a91250524786af3358b0fd0f05ba8e45f76d278
to match the implemntation in packet_crypt.c
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Change-Id: Ib76c3585f67dae22ed0f1dfc10dadcd03c762032
to avoid their prototype different from ssh_cipher_struct
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Change-Id: I6cba2d4fea131f13d028226023da692494caa87d
We need a new one. Disable till set up and registered
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
enable pass username and password from command line
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Change-Id: I6404b90a99253d3240f7a28827635b159ff6a574
and free sources at the end of program
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Change-Id: Ia6a51d52439722b46547449e85350b3193e5ba28
since not all POSIX platform support SIGWINCH signal
and remove the global variable chan
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Change-Id: I70217020c84b056270ed680008a1871383b5fc7b
This started failing CI on Fedora with new GCC
/builds/jjelen/libssh-mirror/src/external/ed25519.c:80:48: error: argument 1 of type 'unsigned char *' declared as a pointer [-Werror=array-parameter=]
80 | int crypto_sign_ed25519_keypair(unsigned char *pk,
| ~~~~~~~~~~~~~~~^~
In file included from /builds/jjelen/libssh-mirror/src/external/ed25519.c:15:
/builds/jjelen/libssh-mirror/include/libssh/ed25519.h:46:48: note: previously declared as an array 'uint8_t[32]' {aka 'unsigned char[32]'}
46 | int crypto_sign_ed25519_keypair(ed25519_pubkey pk, ed25519_privkey sk);
| ~~~~~~~~~~~~~~~^~
/builds/jjelen/libssh-mirror/src/external/ed25519.c:81:48: error: argument 2 of type 'unsigned char *' declared as a pointer [-Werror=array-parameter=]
81 | unsigned char *sk)
| ~~~~~~~~~~~~~~~^~
In file included from /builds/jjelen/libssh-mirror/src/external/ed25519.c:15:
/builds/jjelen/libssh-mirror/include/libssh/ed25519.h:46:68: note: previously declared as an array 'uint8_t[64]' {aka 'unsigned char[64]'}
46 | int crypto_sign_ed25519_keypair(ed25519_pubkey pk, ed25519_privkey sk);
| ~~~~~~~~~~~~~~~~^~
/builds/jjelen/libssh-mirror/src/external/ed25519.c:117:46: error: argument 5 of type 'const unsigned char *' declared as a pointer [-Werror=array-parameter=]
117 | const unsigned char *sk)
| ~~~~~~~~~~~~~~~~~~~~~^~
In file included from /builds/jjelen/libssh-mirror/src/external/ed25519.c:15:
/builds/jjelen/libssh-mirror/include/libssh/ed25519.h:61:27: note: previously declared as an array 'const uint8_t[64]' {aka 'const unsigned char[64]'}
61 | const ed25519_privkey sk);
| ~~~~~~~~~~~~~~~~~~~~~~^~
/builds/jjelen/libssh-mirror/src/external/ed25519.c:180:51: error: argument 5 of type 'const unsigned char *' declared as a pointer [-Werror=array-parameter=]
180 | const unsigned char *pk)
| ~~~~~~~~~~~~~~~~~~~~~^~
In file included from /builds/jjelen/libssh-mirror/src/external/ed25519.c:15:
/builds/jjelen/libssh-mirror/include/libssh/ed25519.h:76:26: note: previously declared as an array 'const uint8_t[32]' {aka 'const unsigned char[32]'}
76 | const ed25519_pubkey pk);
| ~~~~~~~~~~~~~~~~~~~~~^~
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
to avoid their prototype different from ssh_cipher_struct
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Change-Id: I6cba2d4fea131f13d028226023da692494caa87d
to match the implemntation in packet_crypt.c
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Change-Id: Ib76c3585f67dae22ed0f1dfc10dadcd03c762032
Currently "cmake -G Ninja" complains about "multiple rules generate
src/libssh_dev.map", because the target has the same name as the output
of the custom command.
Signed-off-by: DDoSolitary <DDoSolitary@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Due to the missing include, the compiler makes assumptions and leads to
a crash in ssh_mutex_lock() during runtime.
Signed-off-by: Chris Townsend <christopher.townsend@canonical.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Calling set_target_properties directly overrides previously set flags,
so replace them with target_compile_definitions and target_link_options.
Signed-off-by: DDoSolitary <DDoSolitary@gmail.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
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>
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>
The “SubmittingPatches” file is no more since commit
a76badf77a, but the READMEs were still
referencing it.
They now correctly point to “CONTRIBUTING.md”.
Signed-off-by: Paul Capron <paul@fragara.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
There were two identical entries for v0.3.1, next to each other.
Signed-off-by: Paul Capron <paul@fragara.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
In function pki_signature_from_blob(), the warning message in case of
an oversized RSA key was missing an ‘o’ (reading “to” instead of “too”).
While we are here, make this oversized message the same than the ones
found in pki_crypto.c & pki_mbedcrypto.c: put the expected size in it.
The message in case of an _under_sized key include the expected size,
so that’s more consistent in that regard too (and more informative!)
Signed-off-by: Paul Capron <paul@fragara.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The preprocessor guard was previously effectively inoperative;
it used to read “MAX_PACKAT_LEN” (note the ‘A’!), now is ‘E’.
Signed-off-by: Paul Capron <paul@fragara.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Avoid calling random() and use ssh_get_random() instead.
CID #1412376
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
In 906cc7e7e9 a memory leak was fixed but
a similar one is present here that needs a fix as well.
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Add a test case to verify that the server returns the correct signature
when it negotiated an RSA signature algorithm for the host key different
from the one it prefers (e.g. when the client prefers ssh-rsa over
rsa-sha2-256 and rsa-sha2-512).
Fixes T240
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The test checks if the client can handle the error returned by the
server when the maximum number of channel sessions is exceeded.
Fixes T239
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
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>
The test might fail if there is a local configuration file that changes
the location of the known_hosts file. The test should not be affected
by configuration files present in the testing environment.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
If the server properly closed the channel, the client should not return
error if it finds the channel closed.
Fixes T231
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Previously, the exec example would fail if it could not write the whole
read buffer to stdout. With this changes, the exec example will be able
to write parts of the buffer until the whole buffer is written.
This makes the exec example to run when the stdout buffer is small.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The test checks if a command appended to the file path is not executed.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Thanks to Ramin Farajpour Cami for spotting this.
Fixes T232
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
They do not build anymore and I believe most of their functionality is already
covered by new testst.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
ssh_pki_export_pubkey_blob() is incorrectly used to export ecdsa pubkeys from privkeys
when pubkeys are not imported into pkcs #11 tokens.
Signed-off-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
determines if public keys must be loaded in pkcs #11 tokens or not.
tests: Adds the load_public parameter in all files where torture_setup_tokens() was used.
Signed-off-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
private keys for ecdsa keys.
ssh_userauth_publickey() calls ssh_pki_export_pubkey_blob() and tries to export
the the public key from private key if public keys are not already imported
into pkcs #11 tokens.
Signed-off-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Set the cmake project verision to the autogenerated file to have a single
point to set the version. This will be included in the libssh.h file.
Pair-Programmed-With: Andreas Schneider <asn@cryptomilk.org>
Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
tests/fuzz/ssh_client_fuzzer.cpp:45:1: error: designator order for field ‘ssh_callbacks_struct::userdata’ does not match declaration order in ‘ssh_callbacks_struct’
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This option is unknown to older OpenSSH versions (for example CentOS 7
with OpenSSH 7.4) so we can not add it everywhere.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Add SSH_AGAIN as return value to ssh_handle_packets documentation.
Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The introduction of stages in gitlab-ci had quite a unfortune side
effect that is described in the documentation [1]. The whole artifacts
path (in our case obj/) is passed from one stage to another by default,
which is causing very odd behavior as the previous results are only
partially overwritten by the new cmake command and can even lead to
execution of tests that are not supposed to run in particular job.
[1] https://docs.gitlab.com/ee/ci/yaml/#dependencies
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
The OpenSSH 7.4 or 7.6 in Ubuntu and CentOS 7 does not support SHA2
RSA certificates and libssh automatically falls back to SHA1, which
is not allowed by default.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Previously, it would use only the default set, which makes some tests failing
including the DSA ones and disabled RSA with SHA1.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
EVP_CIPHER_CTX_init was replaced with _reset.
Removed EVP_CIPHER_CTX_cleanup. The successive _free call handles that.
Removed old SSLeay function usage.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Added extra functions. The next commit will switch to them.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
All subsecond timestamps are only in the packets if both the
SUBSECOND_TIMES flag and the timestamp flag, e.g. ATTR_ACCESSTIME
are set.
SUBSECOND_TIMES are not very common across server implementations
(e.g. openssh does not include it, nor does libssh's sftpserver
implementation), but this interpretation of the SFTP protocol draft
is used by WinSCP and lftp.
Fixes T219.
Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
If the library is not initialized, SSH_ERROR is returned and the error
message is set properly.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The introduced function returns whether the library is initialized or
not.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
When libssh is statically linked, it is necessary to explicitly call
ssh_init() before calling any other provided API. It is also necessary
to call ssh_finalize() before exiting to free allocated resources.
Fixes T222
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
fail_msg() is not expected to return. Mark the points after calling it
as unreachable to clarify this to the compiler.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Timeout will kill the server if it hangs.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Use the function which starts the test server under timeout. This way
timeout will kill the server if it hangs.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The added function runs the test server under timeout program to kill it
if it elapses the default timeout of 5 minutes.
An auxiliary function to create a libssh server configuration file was
also added.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Using the added option it is possible to set a path to a file in which
the server will write its PID.
This can be used later to kill the server.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Ignore the system-wide configuration when simply trying to reach the
host.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
When a key is rejected, free the allocated memory before returning.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Verify the error code returned by kill() in torture_terminate_process().
The error code is raised when killing the process failed.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The SFTP files wouldn't be closed during the rekey tests leading to
memory leak.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The temporary directory created in torture_sftp_session() wouldn't be
removed. This removes such directory in torture_sftp_close().
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
In torture_reload_sshd_server(), instead of trying to use SIGHUP to
reload the configuration file, kill the original process and create a
new one with the new configuration. With this change, both
torture_setup_sshd_server() and torture_reload_sshd_server() need to
start sshd, with the only difference in the configuration setup. The
shared code to start the sshd server was moved to a new introduced
internal function torture_start_sshd_server().
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This adds server-side support for the newly introduced OpenSSH
keytypes sk-ecdsa-sha2-nistp256@openssh.com and sk-ed25519@openssh.com
(including their corresponding certificates), which are backed
by U2F/FIDO2 tokens.
Change-Id: Ib73425c572601c3002be45974e6ea051f1d7efdc
Signed-off-by: Sebastian Kinne <skinne@google.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The obj directory is not created anymore when the git repository is
cloned. Create the directory during the build.
Also set the variable "ErrorActionPreference: STOP" to make the build to
fail if an error occurs.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The error was
src/options.c:971:13: warning: Value stored to 'u' is never read
# u = 0;
# ^ ~
src/options.c:971:13: note: Value stored to 'u' is never read
# u = 0;
# ^ ~
# 969| case SSH_OPTIONS_KBDINT_AUTH:
# 970| case SSH_OPTIONS_GSSAPI_AUTH:
# 971|-> u = 0;
# 972| if (value == NULL) {
# 973| ssh_set_error_invalid(session);
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
as nothing really sets these keys up and they are probably preserverd
from some previous test, which is really not a good testing strategy.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The static sshd directory wasn't matching the detected sshd and prevented the testcases to be run against local OpenSSH builts
Signed-off-by: Aris Adamantiadis <aris@0xbadc0de.be>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Imports private and public keys from the engine via PKCS11 URIs. Uses
the imported keys to authenticate to the ssh server.
Signed-off-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Libgcrypt has supported ChaCha20 and Poly1305 since 1.7.0 version and
provides fast assembler implementations.
v3:
- initialize pointers to NULL
- use 'bool' for chacha20_poly1305_keysched.initialized
- pass error codes from libgcrypt calls to variable
- add SSH_LOG on error paths
v2:
- use braces for one-line blocks
- use UNUSED_PARAM/UNUSED_VAR instead of cast to void
- use calloc instead of malloc+memset
Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
v3:
- add tests for malformed encrypted inputs
v2:
- use proper size key for chacha20poly1305
- make copy of cleartext for chacha20poly1305 test-case
- update chacha20_encrypted
Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Ensure that the `keylen` argument as provided to `EVP_PKEY_derive`
is initialized, otherwise depending on stack contents, the function
call may fail.
Fixes T205.
Signed-off-by: Jon Simons <jon@jonsimons.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Test if the file name is correctly escaped to avoid protocol message
injection.
Fixes T189
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
When pushing files or directories, encode the newlines contained in the
names as the string "\\n". This way the user cannot inject protocol
messages through the file name.
Fixes T189
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Given a string, the added function encodes existing newline characters
('\n') as the string "\\n" and puts into a given output buffer.
The output buffer must have at least 2 times the length of the input
string plus 1 for the terminating '\0'. In the worst case, each
character can be replaced by 2 characters.
Fixes T189
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This is a public struct and the version should be unsigned there.
Fixes T188
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Those are not marked as LIBSSH_API so not part of the public API and the
symbols aren't exported!
Fixes T188
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Single quote file paths to be used on commands to be executed on remote
shell.
Fixes T181
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The added function quote file names strings to be used in a shell.
Special cases are treated for the charactes '\'' and '!'.
Fixes T181
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Fixes T181
Previously, warnings received from the server were ignored. With this
change the warning message sent by the server will be logged.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
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>
In ssh_session_new(), initialize the state machines states explicitly
for better readability.
Fixes T194
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
In ssh_message_new(), initialize msg->auth_request.signature_state
explicitly for better readability.
Fixes T194
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
In ssh_channel_new(), initialize channel->state and
channel->request_state explicitly for better readability.
Fixes T194
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
When a new ssh_auth_auto_state_struct is allocated in
ssh_userauth_publickey_auto(), initialize the state explicitly for
better readability.
Fixes T194
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Use the enum type instead of int in the ssh_session_struct
Fixes T194
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Use the enum type instead of char in the ssh_auth_request struct
Fixes T194
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Generally, when the extension negotiation is enabled and client supports
SHA2 algorithms for RSA, they are supposed to be prioritized against the
old SHA1. If it is not (ssh-rsa is listed in front of rsa-sha2-* hostkey
algorithms during negotiation), the server wrongly tries to provide the
new typo of signature, ignoring the negotiated algirithm
This commit propagates the digest algorithm from negotiation to the actual
signature functions, which were previously responsible for decision
about the hash algorithm based just on the negotiated extensions.
Fixes T191
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This is only compiled for tests and fuzzers!
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
We will honor BUILD_SHARED_LIBS from cmake next.
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
This is already part of LIBSSH_REQUIRED_LIBRARIES
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
It is possible for OpenSSL to have the type EVP_PKEY_ED25519 in
openssl/evp.h, but not the single shot functions EVP_DigestSign() and
EVP_DigestVerify() which are required to generate Ed25519 signatures.
Only switch to use OpenSSL Ed25519 implementation when OpenSSL have all
required types and functions.
Fixes: T197
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The added example is an application which can generate keys of different
types using libssh.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Debian removed the cross compiling toolchain. So lets drop it.
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Do not parse global config file in sessions created by
torture_ssh_session().
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This removes the tests which uses external configuration files. This
makes the tests no not change behaviour depending on the environment
they are being executed.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
In pkd tests, avoid processing the global server configuration file.
This is to allow testing with algorithms not allowed in the global
server configuration.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Do not process system-wide configuration when reinitializing a session
during testing. This could lead to different settings set from the
expected one (which was initialized during test setup).
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Previously, the tests were sharing the same file path to create the
known_hosts file, which can create a race condition if the tests run in
parallel. Such file was deleted between tests.
By using different different files for each test, the risk of race
conditions is eliminated. Moreover, it makes unnecessary to destroy the
file between tests.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
These functions modify the provided pointer by advancing to the end of
if (point to the byte after the last written). This makes the pointer
invalid, making necessary to use a temporary variable.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The implementation does not work on Windows, where it still reports unsupported
configuration option. On windows, separate code invoking subprocess needs to be
implemented.
Fixes T169
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
This adds support for Ed25519 keys from files in PEM format when using
OpenSSL with Ed25519 support. The default encoding for the PEM file is
expected to be PKCS#8. Encrypted files are supported.
For the lack of an API, it is not possible to export keys in PEM format,
only in OpenSSH format.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
If supported, use OpenSSL X25519 implementation for the curve25519 key
exchange.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This removes unused function pki_signature_verify()
from pki_{crypto, mbedcrypto, gcrypt}. The function was also removed
from include/libssh/pki_priv.h. The function ssh_pki_signature_verify()
was changed to receive a const unsigned char *input.
All tests calling pki_signature_verify() were changed to call
ssh_pki_signature_verify() instead.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Use OpenSSL to generate and verify Ed25519 signatures, if supported.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This is a preparation to use the Ed25519 implementation from OpenSSL.
The function pki_ed25519_sig_to_blob() was renamed to
pki_ed25519_signature_to_blob() and pki_ed25519_sig_from_blob() was
renamed to pki_signature_from_ed25519_blob() to follow the naming for
other algorithms.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Generate and verify Ed25519 signatures along with the other signature
types.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Verify the Ed25519 signature in pki_verify_data_signature() along with
the other signature types.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Use the newer APIs EVP_DigestSign{Init}() and EVP_DigestVerify{Init}()
to generate and verify signatures instead of the older EVP_Sign{Init,
Update, Final} and EVP_Verify{Init, Update, Final} if supported.
Also use the single shot signature/verification if supported as all the
input is provided at once.
This is a preparation to use Ed25519 implementation from OpenSSL.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This is required to avoid csbuild scan failures when a commit removes
source files. The command prep is run only once before all the builds,
making csbuild to try to compile the old files using the configuration
files generated for the newest version.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
With the old token parser, the data was simply broken on the = sign even
if the uri was in quotes and ignored.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
> note that keywords are case-insensitive and arguments are case-sensitive
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
In torture_threads_pki_rsa, skip the test which generates 1024 bits RSA
key pair when in FIPS mode.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Previously, if the path to known_hosts file set through
SSH_OPTIONS_KNOWNHOSTS included missing directories,
ssh_session_update_known_hosts() would fail. The added test case checks
that this is not the case anymore.
The logic of checking if the directory is accessible before creating it
was replaced by creating the directory if opening the file failed. This
is to minimize the risk of TOCTOU race conditions.
Fixes: T166
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
If the given path includes missing directories, ssh_mkdirs() tries to
create them recursively.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The introduced internal function checks if the provided path is for an
existing directory which is accessible for writing.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
For example "%d/config%%1" is expanded to "~/.ssh/config%1".
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
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>
The introduced internal function is intended to be a replacement for the
deprecated function ssh_print_hexa().
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Try to find the OpenSSH private key header not only at the beginning of
the file. This makes the parser to skip comments and white spaces.
Fixes: T76
Fixes: T123
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This should stop detecting it as dead code.
CID 1402934
Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
LibreSSL does not support FIPS mode, check for FIPS_mode() in
ConfigureChecks.cmake.
Signed-off-by: Stefan Strogin <steils@gentoo.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
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>
The added internal function obtain a newly allocated string containing a
list of the signature types that can be generated by the keys present in
the known_hosts files, separated by commas.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Added a function to remove duplicates from lists. This function is used
in a new provided function to append lists removing duplicates.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Avoid trying to open the files if they are not accessible. This was
already treated as a non-error, but with this we save one function call.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Make sure known hosts check works when local known_hosts file is
unaccessible, but the host is present in global known_hosts file.
Remove double return value check in previous existing test.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The memory allocated for host_port can leak if the global knownhosts
file is unaccessible.
Found by address sanitizer build in CI.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Verify that the check process will not fail if the global known_hosts
file is not accessible and the local known_hosts file contain the host.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Previously, if the global known_hosts file (default:
/etc/ssh/ssh_known_hosts) was inaccessible, the check for known hosts
failed. This makes the check to fail if both files are inaccessible.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The internal function ssh_connect_host() is not used.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
In non-blocking mode, it is expected SSH_AGAIN to be returned many
times. Do not flood the log with error messages.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The tests were disabled because of failures in torture_auth. The server
tests are not enabled because the pkd tests are failing.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
When we are talking to old OpenSSH versions which does not support
rsa-sha2-{256,512}-cert-v01@openssh.com or SHA2 in certificates,
fallback to old supported values.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This was introduced during fixes to run pkd tests in FIPS mode.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The test checks if the server handles unknown global requests properly.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Added a test case where invalid global requests are sent to the server
which should reject them, but not stop working.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
When the session is a client session, reject tcpip-forward requests.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
When an unknown global request is received, consume the message to avoid
sending UNIMPLEMENTED later. Only report the failure if the request
wants a reply.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The OpenSSH in RHEL 8 in FIPS Mode outputs information about this on start
and it needs to be skipped for the version detection (and build) to pass:
$ ssh -V
FIPS mode initialized
OpenSSH_8.0p1, OpenSSL 1.1.1c FIPS 28 May 2019
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Add a description of the new feature/bug fix. Reference any relevant bugs.
## Checklist
* [ ] Commits have `Signed-off-by:` with name/author being identical to the commit author
* [ ] Code modified for feature
* [ ] Test suite updated with functionality tests
* [ ] Test suite updated with negative tests
* [ ] Documentation updated
* [ ] The project pipelines timeout is [extended](https://docs.gitlab.com/ee/ci/pipelines/settings.html#set-a-limit-for-how-long-jobs-can-run) at least to 2 hours.
## Reviewer's checklist:
* [ ] Any issues marked for closing are addressed
* [ ] There is a test suite reasonably covering new functionality or modifications
* [ ] Function naming, parameters, return values, types, etc., are consistent and according to [CONTRIBUTING.md](https://gitlab.com/libssh/libssh-mirror/-/blob/master/CONTRIBUTING.md)
* [ ] This feature/change has adequate documentation added
While many people have been academically taught that "goto"s are fundamentally
evil, they can greatly enhance readability and reduce memory leaks when used as
the single exit point from a function. But in no libssh world what so ever is a
goto outside of a function or block of code a good idea.
Good Examples:
int function foo(int y)
{
int *z = NULL;
int rc = 0;
if (y < 10) {
z = malloc(sizeof(int)*y);
if (z == NULL) {
rc = 1;
goto done;
}
}
print("Allocated %d elements.\n", y);
done:
if (z != NULL) {
free(z);
}
return rc;
}
### Initialize pointers
All pointer variables **MUST** be initialized to `NULL`. History has
demonstrated that uninitialized pointer variables have lead to various
bugs and security issues.
Pointers **MUST** be initialized even if the assignment directly follows
the declaration, like pointer2 in the example below, because the
instructions sequence may change over time.
Good Example:
char *pointer1 = NULL;
char *pointer2 = NULL;
pointer2 = some_func2();
...
pointer1 = some_func1();
### Typedefs
libssh tries to avoid `typedef struct { .. } x_t;` so we do always try to use
`struct x { .. };`. We know there are still such typedefs in the code, but for
new code, please don't do that anymore.
### Make use of helper variables
Please try to avoid passing function calls as function parameters in new code.
This makes the code much easier to read and it's also easier to use the "step"
command within gdb.
Good Example:
char *name;
name = get_some_name();
if (name == NULL) {
...
}
rc = some_function_my_name(name);
...
Bad Example:
rc = some_function_my_name(get_some_name());
...
Please try to avoid passing function return values to if- or while-conditions.
The reason for this is better handling of code under a debugger.
Good example:
x = malloc(sizeof(short) * 10);
if (x == NULL) {
fprintf(stderr, "Unable to alloc memory!\n");
}
Bad example:
if ((x = malloc(sizeof(short)*10)) == NULL ) {
fprintf(stderr, "Unable to alloc memory!\n");
}
There are exceptions to this rule. One example is walking a data structure in
an iterator style:
while ((opt = poptGetNextOpt(pc)) != -1) {
... do something with opt ...
}
But in general, please try to avoid this pattern.
### Control-Flow changing macros
Macros like `STATUS_NOT_OK_RETURN` that change control flow (return/goto/etc)
from within the macro are considered bad, because they look like function calls
that never change control flow. Please do not introduce them.
### Switch/case indentation
The `case` should not be indented to avoid wasting too much horizontal space.
When the case block contains local variables that need to be wrapped in braces,
they should not be indented again either.
Good example:
switch (x) {
case 0:
do_stuff();
break;
case 1: {
int y;
do_stuff();
break;
}
default:
do_other_stuff();
break;
}
Bad example:
switch (x) {
case 0:
do_stuff();
break;
case 1:
{
int y;
do_stuff();
break;
}
default:
do_other_stuff();
break;
}
## ABI Versioning and Symbol Management
To maintain [ABI](https://en.wikipedia.org/wiki/Application_binary_interface) stability
and ensure backward compatibility, libssh uses **symbol versioning** to track and manage
exported functions and variables. This allows libssh to introduce new symbols or modify
existing functions in an ABI-compatible way.
When introducing a new symbol:
1. Use the `LIBSSH_API` macro to mark the symbol as part of the public API.
2. If you have [abimap](https://github.com/ansasaki/abimap) installed, the new symbols are
automatically generated in the `src/libssh_dev.map` file in the **build** directory and used automatically for building the updated library. But, depending on the version of `abimap` under use, you may face linker errors like: `unable to find version dependency LIBSSH_4_9_0`. In this case, you need to manually replace the existing `src/libssh.map` file with the generated `libssh_dev.map` file to update the symbol versioning.
3. If you do not have abimap installed, the modified/added symbols must manually be added to the
`src/libssh.map` file. The symbols must be added in the following format (assuming that 4_10_0 is the latest released version):
```
LIBSSH_AFTER_4_10_0
{
global:
new_function;
new_variable;
} LIBSSH_4_10_0;
```
4. After following either of the above steps, the library can be successfully built and
tested without any linker errors.
5. When submitting the patch, make sure that any new symbols have been added to `libssh.map` as described in step 3, so that the new additions may not be excluded from the next release due to human error.
Also, to maintain ABI compatibility, existing symbols must not be removed. Instead, they can
be marked as deprecated using the `LIBSSH_DEPRECATED` macro. This allows the symbol to be
removed in a future release without breaking the ABI.
option(GSSAPI_TESTING"Build with GSSAPI tests; requires krb5-server,krb5-libs and krb5-workstation"OFF)
option(WITH_BENCHMARKS"Build benchmarks tools; enables unit testing and client tests"OFF)
option(WITH_EXAMPLES"Build examples"ON)
option(WITH_NACL"Build with libnacl (curve25519)"ON)
option(WITH_SYMBOL_VERSIONING"Build with symbol versioning"ON)
option(WITH_ABI_BREAK"Allow ABI break"OFF)
option(WITH_GEX"Enable DH Group exchange mechanisms"ON)
option(FUZZ_TESTING"Build with fuzzer for the server"OFF)
option(WITH_INSECURE_NONE"Enable insecure none cipher and MAC algorithms (not suitable for production!)"OFF)
option(WITH_EXEC"Enable libssh to execute arbitrary commands from configuration files or options (match exec, proxy commands and OpenSSH-based proxy-jumps)."ON)
option(FUZZ_TESTING"Build with fuzzer for the server and client (automatically enables none cipher!)"OFF)
option(PICKY_DEVELOPER"Build with picky developer flags"OFF)
option(WITH_HERMETIC_USR"Build with support for hermetic /usr/"OFF)
_ssh_log(SSH_LOG_PROTOCOL,"=== my_channel_eof_function","Got EOF on channel %d:%d. Shuting down write on socket (fd = %d).",channel->local_channel,channel->remote_channel,*event_fd_data->p_fd);
_ssh_log(SSH_LOG_PROTOCOL,
"=== my_channel_eof_function",
"Got EOF on channel. Shutting down write on socket (fd = %d).",
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
