In `kex_method_diffie_hellman_group_exchange_sha256_key_exchange`,
`p` and `g` are later initialized with `_libssh2_bn_from_bin`, so they
should be initially created using `_libssh2_bn_init_from_bin` rather
than `_libssh2_bn_init`, as is done in
`kex_method_diffie_hellman_group_exchange_sha1_key_exchange`.
Fixing memory leaks when using the libgcrypt backend.
Follow-up to 09c5e59933
Ref: https://web.archive.org/web/trac.libssh2.org/ticket/168Closes#1599
- Reworking the `Libs.private` collector logic for INTERFACE targets,
broke the original lib order. Fix it by going back a single loop
to retain order.
Follow-up to df0563a857#1535
- Implement the above with one change: move implicit CMake libs
to the end of the list (was: the beginning).
I expect these to be libs that any custom libs may depend on,
like system libs, C runtime, compiler runtime lib.
Follow-up to c87f129630#1466Closes#1623
`LIBSSH2_PC_LIBS_PRIVATE` ends up in `Libs.private` in `libssh2.pc`.
The order and duplication may be significant for linkers that rely on
strict lib order and unable to resolve symbols without it. Such linker
is binutils `ld`. De-duplication can break it.
As of now there is no purposeful duplication in libs in libssh2, thus
the de-duplication most likely did not affect actual builds.
It was originally introduced to avoid a repeat `-lz` (with
a zlib-enabled OpenSSL or wolfSSL build.) To keep this feature, this
patch makes sure to only delete duplicates that are next to each other.
Follow-up to 6464301820#1131Closes#1621
The `libssh2.pc` generator logic automatically adds `-lws2_32` while
parsing `LIBSSH2_LIBS`, which contains this lib already. Then discard
the duplicate.
This patch introduces a change in the position of `ws2_32` within
the lib list advertised via `libssh2.pc` for static builds.
This order might in cases by significant, but:
- libssh2 no longer links against `libssl`, which was the library
also referencing `ws2_32` and breaking picky binutils `ld` linker
when not passed in strict dependency order.
Ref: c84745e34e#1128
- since switching to INTERFACE targets, cmake messes up the lib order
anyway, adding `OpenSSL:Crypto` last, instead of `ws2_32`. This did
not seem to cause an issue so far.
Ref: df0563a857#1535
For these reasons it seems unlikely this position change could break
Windows OpenSSL static gcc/ld builds relying on `libssh2.pc` for their
lib list. It least no more than it was before this patch.
Turns out these theories don't stand in practice and the order is broken
possibly by introducing INTERFACE targets, with or without this patch.
`libcrypto` (tested with LibreSSL) is in fact depending on `ws2_32`, and
the `bcrypt` dependency is also causing breakage with picky binutils ld.
I may try addressing it in a separate PR.
Ref: 33b6d5f89d#827
Ref: 31fb8860db#811Closes#1619
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
Also:
- fix old yamllint warnings.
- make quoting more consistent.
- drop stray step name: "checkout".
- merge REUSE workflow into main CI one.
Closes#1614
It looks like the incorrect length is used to copy the public key method
into the session in `libssh2_agent_sign()` and while the public key type
at the start of a identity blob is often identical to the public key
method it might not always be such as when method is `rsa-sha2-256` for
`ssh-rsa` keys.
Closes#1603
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
Also to sync up with similar `./configure` feature via
`CURL_ADD_COMPILER_WARNINGS()`.
Example: `-DCMAKE_C_FLAGS=-Wno-xor-used-as-pow`
It may be useful as a workaround if a specific build combination hits
a picky warning within libssh2's source code. If such happens, we do
appreciate a report to fix it in libssh2 itself.
Closes#1590
- 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
- fix `add_subdirectory` builds for old CMake versions.
- libssh2-config.cmake: fix to set CMP0099 for CMake 3.17+ only.
- libssh2-config.cmake: generalize code to support any number of deps.
(mainly to sync with curl.)
- libssh2-config.cmake: bind dependencies to the static libssh2 only.
Follow-up to a0d8529b08#1571
Follow-up to df0563a857#1535Closes#1581
Fixes:
```
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 but not globally visible.
Call Stack (most recent call first):
CMakeLists.txt:27 (find_package)
CMake Error at bld-libssh2/_pkg/lib/cmake/libssh2/libssh2-config.cmake:40 (add_library):
add_library cannot create ALIAS target "Libssh2::libssh2" because target
"libssh2::libssh2_shared" is imported but not globally visible.
Call Stack (most recent call first):
CMakeLists.txt:27 (find_package)
```
tests/cmake reproducer (requires #1581):
```shell
export CMAKE_CONSUMER=/path/to/CMake-3.12.0/bin/cmake
./test.sh find_package
```
I don't understand what this error says, why it happens in certain CMake
versions, and why a workaround is necessary for what seems like
a standard export/consume configuration. This patch is based on internet
suggestions and other projects ending up with this workaround.
Ref: https://github.com/curl/curl/pull/16973
Ref: https://github.com/curl/curl/pull/17140
Cherry-picked from #1581Closes#1587
Tidy up the way we handle options not to be passed to feature checks,
and make sure to show them in the configure log.
Also:
- drop redundant and obsolete compiler check.
- drop redundant parenthesis.
Closes#1576
- replace `CMAKE_C_FLAGS*` and `CMAKE_CXX_FLAGS` with `COMPILE_OPTIONS`.
- replace `CMAKE_SHARED_LINKER_FLAGS_DEBUG` with
`LINK_OPTIONS`/`LINK_FLAGS`.
- make it explicit to pass these C flags to feature checks.
- enable `-pedantic-errors` picky option for GCC with CMake <3.23.
- drop redundant condition when stripping existing MSVC `/Wn` options.
CMake passes `CMAKE_C_FLAGS` to targets, feature checks and raw
`try_compile()` calls. With `COMPILE_OPTIONS`, this is limited to
targets, and we must explicitly pass them to feature checks. This
makes the build logic clearer, and offers more control. It also
reduces log noise by omitting these options from linker commands,
and from `CMAKE_C_FLAGS` dumps in feature checks.
Closes#1575
- drop `VERSION` target property for cmake <3.19 compatibility
```
CMake Error at CMake/Find*.cmake:90 (set_target_properties):
INTERFACE_LIBRARY targets may only have whitelisted properties.
The property "VERSION" is not allowed.
```
- move custom target property to the `INTERFACE_` namespace
for cmake <3.19 compatibility. (To avoid same error as above)
- fix forwarding multiple `CFLAGS`, when detected via `pkg-config`.
- restore support for `-framework` and raw libs when processing
the internal lib list for generating `libssh2.pc`. For good measure,
at the moment libssh2 doesn't depend on a Framework.
- limit `libssh2_dumptargetprops()` to cmake 3.19+. It doesn't work with
older versions.
Issues found while applying this change to curl. They did not surface in
libssh2 CI.
Follow-up to df0563a857#1535Closes#1571
- 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
Instead build all crypto backend sources always, and exclude inactive
ones with guards.
To play better with code checkers and compilers that are blind to
included C sources, e.g. clang with certain compiler warnings and
clang-tidy.
We continue to include `blowfish.c`.
Follow-up to 4f0f4bff5a#941
Cherry-picked from #1561
To play better with code checkers and compilers that are blind to
included C sources, e.g. clang with certain compiler warnings and
clang-tidy.
Follow-up to 4f0f4bff5a#941
Cherry-picked from #1561
