The version 0.4.0 fixed the issues of multi-digit version numbers
which we hit with releaseing libssh ABI version 4_10 with last
release.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
When we use empty configuration file, some stuff go south in c10s
and for example fips mode detection does not work anymore.
Providing minimal configuration file avoids the issues of loading
the provider too early, while keeping fips mode activation working
and tests happy.
It also configures the pkcs11-provider to assume the token provides
FIPS approved crypto so the tests can work.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The maximal lenght of unix domain socket path is 108 characters. When
the build directory (and UID wrapper home directories) are too deep
in the filesystem, OpenSSH will fail to create the socket file,
which is failing this test.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
without explicitly setting the algorithms, they might be set by
some other configuration file, for example crypto policies pulled
from `/etc/libssh/libssh_server.config` during RPM build.
Log also the generated configuration file and change the other case
to use standard logging mechanism instead of fprintf.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This linking worked only in CI and local builds, but not during
the build in RPM as it fails on missing symbols that were defined
only in the main library. This is solved as with the other digest
dependencies in external crypto by removing the intermediate
function. We are already linking the md_*.o objects.
The error was like this
sh: symbol lookup error: /path/libssh/libssh-0.12.0-build/libssh-0.12.0/redhat-linux-build/lib/libsntrup761_override.so: undefined symbol: crypto_hash_sha512
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Coverity thought that the best_nlines could underflow, but the best_size is
initialized to 0 before calling this function so its moot. Adjusting the code
to be better understandable to static analyzers by initializing the variable
inside of the function.
Thanks coverity!
CID 1548873
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
... if underlying functions read or write more than expected.
This should never happen, but static analysis tools are inventive.
Thanks coverity!
CID 1548868
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The `first` is intialized to -1 and if we reach this without setting this, we
needlessly call close(-1). It should be no-op, but better be safe.
Thanks coverity!
CID 1644001
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Checking the session->gssapi is resulting in the very same results, but this
approach is more direct and makes static analysis tools more happy.
Thanks coverity!
CID 1644000
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
... file, even if it was already set before. The options
level handles what was already set.
The proxyJump implementation sets the username from the proxyjump, which
is setting it to NULL, effectively writing the current username to the
new session, which was not possible to override due to the following check.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
We want to make sure it suceeds because it could fail if
the client tries to send a hostbound public key authentication
request.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
If there is no host key (e.g., because we are doing
gssapi-keyex with "null" host key algorithm), it does not
make sense to use host bound authentication.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
All gssapi-keyex tests have to be disabled in Centos Stream 8
because the KEX is not allowed in FIPS. In Centos Stream 9,
only tests against OpenSSH have to be disabled because
OpenSSH only enables gssapi-keyex since Centos Stream 10.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
And setup a KDC server before pinging the server so we
can connect.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Do not try to verify mic if gssapi-keyex was not performed,
and fix a memory leak of the mic on error.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
These are not strictly necessary because we always check
that we performed GSSAPI KEX, but they won't hurt us.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Reject all GSSAPI-related messages when compiled
without GSSAPI support.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
In particular, gss-nistp256-sha256-* and
gss-curve25519-sha256-*.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
With packet filtering now implemented for type 65,
the current test packet would be rejected, resulting
in failed tests.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Both gcry_mpi_copy and gcry_mpi_set take a pointer to
const gcry_mpi, which const_bignum is not.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This is necessary to reuse the function
in gss-curve25519-sha256-* KEX.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Make it use the one-shot API of hash functions,
and remove the FIPS restriction for OpenSSL 3.5+
where we can fetch the MD5 implementation from
a non-FIPS provider to use for non-crypto purposes.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Which can be used for non-cryptographic purposes
even in FIPS mode.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
fix: skip gssapi tests in fips mode
fix: skip gssapi_key_exchange_null test on ubuntu and tumbleweed
fix: return early when rc != 0 to show error
tests: replace int asserts by ssh return code asserts
fix: add fatal error when hostkeys are not found and gssapi kex is not enabled
ci: add comment linking gssapi null kex bug in ubuntu and tumbleweed
fix: don't specify hostkeys in config instead of deleting files
tests: assert kex method was null
refactor: remove redundant include
refactor: better error message
fix: check null before accessing in gssapi.c
fix: allow setting no hostkeys
Signed-off-by: Gauravsingh Sisodia <xaerru@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
feat: add negative auth client tests, and more key exchange server tests
feat: add function for checkinf if GSSAPI key exchange was performed
Signed-off-by: Gauravsingh Sisodia <xaerru@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
feat: add generic functions for importing name and initializing ctx
feat: add suffix to gsskex algs dynamically
feat: move gssapi key exchange to another file
feat: add gssapi key exchange for server
refactor: remove unnecessary fields in gssapi struct
refactor: add some documentation and improve logging
fix: remove gss_dh callbacks
feat: add a check to see if GSSAPI is configured correctly
fix: memory leaks
feat: add client side "gssapi-keyex" auth
feat: add gssapi_key_exchange_algs for server
fix: some memory issues
feat: add gssapi kex options to config
feat: add check to see if GSSAPI key exchange was performed
feat: add more tests for gssapi key exchange
fix: add valgrind supp
Signed-off-by: Gauravsingh Sisodia <xaerru@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
When the first key object is a certificate object, this match will
fall through to the generic key comparison that is unable to handle
the ed25519 keys and fails.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
* allow parsing of AddressFamily in config and cli
* supports options "any", "inet" and "inet6"
* introduce SSH_OPTIONS_ADDRESS_FAMILY
Signed-off-by: Samir Benmendil <me@rmz.io>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
The shared secret is derived as bignum, and draft-ietf-sshm-mlkem-hybrid-kex
mandates that it is converted to a fixed-size byte array. Not doing this
would lead to incompatibilities with other implementations when the derived
shared secret happens to start with zero bytes.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Systems define O_NONBLOCK & O_NDELAY as the same thing. POSIX however
only defines O_NONBLOCK. Rename the current define to be portable.
Signed-off-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Use the openssl constants algorithm names instead of string
representations. They should not change, but it's clearer to have it
this way.
Signed-off-by: Norbert Pocs <norbertpocs0@gmail.com>
Signed-off-by: Norbert Pocs <norbertp@openssl.org>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
... and prevent memory leak of host_port on memory allocation failure.
Thanks Xiaoke Wang for the report!
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This builds on top of a9c8f94. The pure ML-KEM
code is now separated from the hybrid parts,
with the hybrid implementation generalized to
support NIST curves.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
This adds a new internal API function (ssh_ecdh_init),
similar to how it's done in curve25519 implementation.
The new function can be used in hybrid key exchange
constructions.
Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
The session struct now contains an ssh_pki_ctx struct as its member to allow for passing user configured pki options across many functions.
The ssh_options_set API has been extended to allow users to set this member.
Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Eshan Kelkar <eshankelkar@galorithm.com>
A new generic struct is introduced which contains the various configuration options that can be used by pki operations.
API functions have been provided to configure all the options.
Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Eshan Kelkar <eshankelkar@galorithm.com>
Add some common helper functions that can be used by any developer
writing callbacks for interacting with FIDO2/U2F devices.
Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Eshan Kelkar <eshankelkar@galorithm.com>
Declare ssh_sk_callbacks_struct so that the users can define custom functions as callbacks for interacting with FIDO2/U2F devices.
Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Eshan Kelkar <eshankelkar@galorithm.com>
The sk_api.h file added is a copy of the sk-api.h file in openSSH, including only the struct and constant definitions.
This has been done to ensure compatibility with any security key middleware developed for openSSH.
Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Eshan Kelkar <eshankelkar@galorithm.com>
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>
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
To avoid confusion, indentations have to be 4 spaces. Do not use tabs!. When
wrapping parameters for function calls, align the parameter list with the first
@@ -478,6 +524,76 @@ 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(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)
style="color:#000000;clip-rule:nonzero;display:inline;overflow:visible;visibility:visible;opacity:1;isolation:auto;mix-blend-mode:normal;color-interpolation:sRGB;color-interpolation-filters:linearRGB;solid-color:#000000;solid-opacity:1;fill:#4d4d4d;fill-opacity:1;fill-rule:nonzero;stroke:none;stroke-width:0.26458332;stroke-linecap:butt;stroke-linejoin:miter;stroke-miterlimit:4;stroke-dasharray:none;stroke-dashoffset:0;stroke-opacity:1;paint-order:stroke fill markers;color-rendering:auto;image-rendering:auto;shape-rendering:auto;text-rendering:auto;enable-background:accumulate"
d="m 0.52916667,292.2374 -0.26458334,0.52925 v 3.43958 H 4.7625001 v -3.43958 H 2.38125 L 2.1166667,292.2374 Z"
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.
