By micromanaging the project dependency and its inclusion into the test
project. It feels like an awkward construct, but perhaps better than
nothing.
It's also fragile because it's a static build with no assistance from
the external project (curl in this case). Mitigated in test by disabling
all dependencies and some features.
Since there is no special core cmake logic to be tested here, in CI
the test is tested really. To keep CI jobs at minimum, only add 3 of
them, taking 26s in total. (All 6 would take 203s.)
Follow-up to 77df767784#1589Closes#1637
Also:
- merge CI check and shellcheck jobs into a single one.
To share the same shellcheck version and less overhead.
- use `set -eu` in more scripts.
- make sure CI scripts run from any cwd.
To make it easy to run them on local machine.
- minor tidy-ups.
Closes#1618
It was an exercise to run old cmake versions in CI and in the test suite.
It also revealed that 3.7.2 2017-01-13 is too old to consume libssh2 via
`find_package()` due to:
```
CMake Error at bld-libssh2/_pkg/lib/cmake/libssh2/libssh2-config.cmake:35 (add_library):
add_library cannot create ALIAS target "libssh2::libssh2" because target
"libssh2::libssh2_shared" is IMPORTED.
Call Stack (most recent call first):
CMakeLists.txt:27 (find_package)
```
The mitigation for this issue requires 3.11.
Also:
- rename a few existing envs to use the `TEST_` prefix.
- make the `find_package` test provider stage verbose.
Closes#1591
- cmake: sync `-ftree-vrp` behavior with autotools.
- build: enable `-Wjump-misses-init` for GCC 4.5+.
Credits-to: Marcel Raad
- packet: fix `-Wjump-misses-init` warnings.
```
src/packet.c: In function ‘_libssh2_packet_add’:
src/packet.c:671:9: error: jump skips variable initialization [-Werror=jump-misses-init]
src/packet.c:920:31: note: ‘want_reply’ declared here
src/packet.c:671:9: error: jump skips variable initialization [-Werror=jump-misses-init]
src/packet.c:919:26: note: ‘len’ declared here
src/packet.c:669:9: error: jump skips variable initialization [-Werror=jump-misses-init]
src/packet.c:1121:31: note: ‘want_reply’ declared here
src/packet.c:669:9: error: jump skips variable initialization [-Werror=jump-misses-init]
src/packet.c:1120:26: note: ‘len’ declared here
src/packet.c:669:9: error: jump skips variable initialization [-Werror=jump-misses-init]
src/packet.c:1119:26: note: ‘channel’ declared here
```
- build: enable gcc-12/13+, clang-10+ picky warnings
- acinclude.m4: sync formatting/comments with curl.
- autotools: fix `-Wtrampolines` picky warning for gcc 4.x versions.
Follow-up to 854cfa8292#1524
- cmake: enable `-Wall` for MSVC when `PICKY_COMPILER=ON`.
- MSVC: fix `-Wall` warnings.
Seen on VS2015. Not seen on VS2022. Unknown for other versions.
```
tests\test_simple.c(60): warning C4777: 'fprintf' : format string '%d' requires an argument of type 'int', but variadic argument 1 has type 'std::size_t'
tests\test_simple.c(60): warning C4777: 'fprintf' : format string '%.*s' requires an argument of type 'int', but variadic argument 2 has type 'std::size_t'
```
- mbedtls: stop silencing warnings in 3rd-party header.
Follow-up to a3aa6b4ca8#1525
- cmake: stop deleting `-W<n>` from `CMAKE_C_FLAGS` (MSVC)
1. `CMAKE_C_FLAGS` may apply to other projects, and deleting/altering it
may be unexpected.
2. We pass `-W4`/`-Wall` internally now, which do override custom
`-W<n>` options as tested with VS2008 and newer VS generators.
Closes#1588
- ci/GHA: add cmake integration tests for Windows.
- ci/GHA: test `add_subdirectory` with Libgcrypt.
- make them run faster with prefill, unity, Ninja, omitting curl tool.
- add support for any build configuration.
- add old-cmake support with auto-detection.
- auto-detect Ninja.
- run consumer test apps to see if they work.
Also show the cryptography backend.
- add support for Windows.
- make it more verbose.
- re-add `ExternalProject` cmake consumer test. It's broken.
- tidy up terminology.
Cherry-picked from #1581Closes#1589
- kex: drop unused assigment.
- knownhost: error when salt is NULL.
- mbedtls: avoid unnecessary inline assigments, that were ignored for
the second block and replaceable with a `ret = 0` initialization for
the first one.
- mbedtls: fix ignoring an API failure and ending up calling
`mbedtls_rsa_check_privkey()` unconditionally.
- misc: initialize datalen on error in `_libssh2_base64_decode()`.
- openssl: drop unused assigments.
- openssl: fix unused static function.
- packet: avoid NULL deref.
- packet: avoid NULL in `memcpy` src.
- publickey: optimize struct layout to avoid padding.
- sftp: replace ignored `rc` error assigment with `_libssh2_error()` call.
- transport: fix potential NULL ptr dereferences.
- transport: silence uninitialized value warnings.
- userauth: drop unused assigment.
- userauth: possible use of unitialized pointer.
- userauth: replace `rewind()` with `fseek()`.
`rewind()` returns an error condition in `errno`. `errno` is
problematic and reduces portability. Use `fseek()` to avoid it.
- userauth: replace potential NULL deref by returning error from
`sign_frommemory()`. Possible false positive. `rc` should be set
upstream if the callback is NULL.
- userauth: replace potential NULL deref by returning error from
`sign_fromfile()`. clang-tidy did not warn about this one, but
let's match `sign_frommemory()` anyway.
- wincng: fix potentially unused macros.
- wincng: make sure bignum is not NULL before use.
tests:
- openssh_fixture: drop unused assignment.
- session_fixture: exit if `username` not set, to avoid `strlen(NULL)`.
- session_fixture: replace `rewind()` with `fseek()`.
`rewind()` returns an error condition in `errno`. `errno` is
problematic and reduces portability. Use `fseek()` to avoid it.
- test_read: exit if `username` not set, to avoid `strlen(NULL)`.
examples:
- scp_write_nonblock: fix file handle leak.
- sftp_write_nonblock: file handle leak on error.
- sftp_write_sliding: file handle leak on error.
- ssh2_agent_forwarding: fix unused error codes.
Details in the subcommits under the PR.
Thanks-to: Michael Buckley
Thanks-to: Will Cosgrove
Closes#1561
To run test program via `wine`:
```shell
export LIBSSH2_TEST_EXE_RUNNER=wine
```
It prefixes commands with the specified runner. For systems where this
isn't automatic or supported, e.g. macOS.
Closes#1562
- move dependency properties (libs, libdirs, C flags, header dirs,
pkg-config module names) from global lists to imported target
`INTERFACE` properties. Rework FInd modules to return their results
like this and update the libssh2 build process to use it. It makes
Find modules re-usable from the cmake-config script by libssh2
consumers, to integrate with libssh2 dependencies.
- define libssh2 dependencies as "imported targets" by the name:
`libssh2::<depname>`, e.g. `libssh2::libgcrypt`.
- cmake-config: add fall-back logic for CMake without
CMP0099 (v3.17 2020-03-20) to set lib directories.
- generate `libssh2.pc` based on imported target properties (instead of
global lists).
- add target property dump debug function.
- ci/GHA: also test cmake integration on macOS.
Follow-up to 96d7f404e7#1534Closes#1535
CMake:
- Find*: set `<modulename>_FOUND` for compatibility when found via
`pkg-config`. E.g. `MbedTLS_FOUND`.
`find_package_handle_standard_args()` sets both `<MODULENAME>_FOUND`
and `<Modulename>_FOUND` when detecting the dependency. Some CMake
code relies on this and 3rd-party code may rely on it too. Make sure
to set the latter variant when detecting the dependency via
`pkg-config`, where we don't call
`find_package_handle_standard_args()`.
CMake sets these variable to `TRUE` (not `ON` or `1`). Replicate this
for compatibility.
- libssh2-config.cmake: inherit default `LIBSSH2_USE_PKGCONFIG`.
Follow-up to a3aa6b4ca8#1525
- document variables consumed by `libssh2-config.cmake.in`.
- `libssh2-config.cmake`: fix to link to non-OpenSSL crypto backends.
This is most likely not how this is supposed to be done, but better
than failing.
What's the canonical way to do this, and how OpenSSL and zlib does it
is yet to be figured out.
- use `ZLIB::ZLIB` to reference zlib.
- use `IN ITEMS` where missed.
- harmonize variable dump output formats.
CMake `find_package` integration tests:
- extend to all crypto backends (was: OpenSSL).
- show libssh2 variables set by `find_package()`.
- stop building examples and tests for the consumed package.
For performance.
- enable zlib, for coverage.
- be verbose when building the test targets.
ci/GHA:
- add packaged mbedTLS (2.x) build to Linux matrix.
- alphasort some tests.
Follow-up to d9c2e550ca#1460
Follow-up to 82b09f9b3a#1322Closes#1534
- show platform flags (via curl).
- add `LIBSSH2_USE_PKGCONFIG` option to control whether to use
`pkg-config` to find dependencies.
- set `.pc` names withing the Find modules.
- add `mbedcrypto` to `libssh2.pc` only when detected via `pkg-config`.
Workaround for older mbedtls versions and non-CMake mbedTLS builds
(as of mbedTLS 3.6.2) that don't emit an `mbedcrypto.pc` file.
- set header paths relative to the project root (tidy-up).
- use `-isystem` for crypto backend and zlib header paths.
To match autotools.
- sync header path order with autotools.
- rename local variables to underscore-lowercase.
- minor tidy-ups.
Cherry-picked from #1484Closes#1525
- cmake: add support to build ossfuzz.
Enable with `-DBUILD_OSSFUZZ=ON`.
Also supports `-DLIB_FUZZING_ENGINE=` like autotools does.
- check for `__clang__` when suppressing warnings in source. Necessary
for clang-cl, which set `__clang__`, but doesn't set `__GNU__`.
- cmake: optimize out 4 picky warning option detections with gcc.
- cmake: bring `-pedantic-error`, `-Wall` use closer to curl's.
- cmake: set `-Wno-language-extension-token` for clang-cl.
- cmake: escape only the necessary `-W` options for clang-cl.
- cmake: apply picky warnings to C++.
- cmake: replace `unset(VAR)` with `set(VAR "")` for init.
- cmake: prefer dash-style MSVC options.
- cmake: simplify `MATCHES` expression.
- cmake: formatting/whitespace.
- ci/GHA: bump `actions/upload-artifact` to v4
Closes#1524
For no reason it broke when trying to silence a CMake deprecation
warning in #1510. Then when tested locally, it did not work either with
or without the patch in #1510.
I'm not sure, but existing implementation may have worked by accident
by re-using leftovers from the preceding two integration tests.
After spending a days trying to fix this, I declare defeat. If such
amount of time of testing, reading documentation, blog posts, variable
traces, logs, bug reports is not enough to make this work, or even
to understand how this should work, this seems like a lost cause.
CMake makes it impossible to cleanly query the properties of a target,
which would be essential for debugging. There are rough workarounds
with years of iteration, and those still don't work to this day:
https://stackoverflow.com/questions/32183975/how-to-print-all-the-properties-of-a-target-in-cmake
Copy-pasting an incantation from a blog post that made this work:
https://inhzus.io/posts/2023-12-01-cmake-external-project/
almost made it work, except that it had a workaround for a 10-year old
pending bug, another workaround for Ninja which required CMake 3.29,
with settings hard-wired, and explicitly configured in weird ways. But,
it still missed to pass the libssh2 library to the test target and
failed to link.
Then tried to pass the libssh2 lib the "usual" way via:
```
target_link_libraries(test PRIVATE libssh2)
```
That also did not work because CMake decided that the external libssh2
target is of "UTILITY" type, and errored with:
```
CMake Error at CMakeLists.txt:39 (target_link_libraries):
Target "libssh2" of type UTILITY may not be linked into another target.
One may link only to INTERFACE, OBJECT, STATIC or SHARED libraries, or to
executables with the ENABLE_EXPORTS property set.
```
This type property is read-only, and documentation has no mention of it,
or how to set it whatsoever:
https://cmake.org/cmake/help/latest/module/ExternalProject.html
libssh2's `docs/INSTALL_CMAKE.md` mentions ExternalProject as a way to
use libssh2. Added there with the initial CMake commit. We should
probably delete it from there.
This consumption method has a single mention in public issues:
https://github.com/libssh2/libssh2/issues/1116Closes#1522
Building 3 tests require static libssh2 lib. Some may prefer not to
create the static lib, yet prefer to build all tests, including those
3 that require it.
Detect such intent by looking for an explicit `BUILD_TESTING=ON` and
`BUILD_STATIC_LIBS=OFF`, then build the static lib anyway but without
installing it.
Reported-by: Eli Schwartz
Fixes#1450Closes#1469
Also:
- add `LIBSSH2_DSA_ENABLE` to enable it explicitly.
- test the above option in CI.
- say 'deprecated' in docs and public header.
- disable DSA in the CI server config.
(OpenSSH 9.8 no longer builds with it by default)
https://www.openssh.com/txt/release-9.8
Patch-by: Jose Quaresma
- disable more DSA code when not enabled.
Fixes#1433Closes#1435
Replace hard-coded crypto backends and rely on `LIBSSH2_GCM` macro
to decide whether to run AES-GCM tests.
Without this, build attempted to run AES-GCM tests (and failed)
for crypto backends that have conditional support for this feature, e.g.
wolfSSL without the necessary features built-in
(as in before Homewbrew wolfssl 5.7.0_1, or OpenSSL v1.1.0 and older).
This patch is part of a series of fixes to make wolfSSL AES-GCM support
work together with libssh2.
Cherry-picked from #1407Closes#1410
Use `AM_CFLAGS` to pass custom, per-target C flags. This replaces using
`CFLAGS` which triggered this warning when running `autoreconf -fi`:
```
tests/Makefile.am:8: warning: 'CFLAGS' is a user variable, you should not override it;
tests/Makefile.am:8: use 'AM_CFLAGS' instead
```
(Only for `tests`, even though `example` and `src` also used this
method. The warning is also missing from curl, that also uses
`CFLAGS`.)
Follow-up to 3ec53f3ea2#1286Closes#1378
Replicating OpenSSH's behavior to handle RSA certificate authentication
differently based on the remote server version.
1. For OpenSSH versions >= 7.8, ascertain server's support for RSA Cert
types by checking if the certificate's signature type is present in
the `server-sig-algs`.
2. For OpenSSH versions < 7.8, Set the "SSH_BUG_SIGTYPE" flag when the
RSA key in question is a certificate to ignore `server-sig-algs` and
only offer ssh-rsa signature algorithm for RSA certs.
This arises from the fact that OpenSSH versions up to 7.7 accept
RSA-SHA2 keys but not RSA-SHA2 certificate types. Although OpenSSH <=7.7
includes RSA-SHA2 keys in the `server-sig-algs`, versions <=7.7 do not
actually support RSA certs. Therefore, server sending RSA-SHA2 keys in
`server-sig-algs` should not be interpreted as indicating support for
RSA-SHA2 certs. So, `server-sig-algs` are ignored when the RSA key in
question is a cert, and the remote server version is 7.7 or below.
Relevant sections of the OpenSSH source code:
<https://github.com/openssh/openssh-portable/blob/V_8_9_P1/sshconnect2.c#L1191-L1197>
<https://github.com/openssh/openssh-portable/blob/master/compat.c#L43>
Assisted-by: Will Cosgrove
Reviewed-by: Viktor Szakats
Also markup a vararg function as such.
In functions marked up as vararg functions, there is no need to suppress
`-Wformat-nonliteral` warnings. It's done automatically by the compiler.
Closes#1342
