Also:
- allow to override `AR` and `ARFLAGS`.
- The extra `src` subdir in the target directory is no longer, to
simplify things.
- gone the dynamically generated `objects.mk`. Now replaced with some
tricky logic to do that inline.
- add necessary `LIBS` for WinCNG. (untested)
Lightly tested via clang-cl.
- fix shellcheck warnings:
- use quotes
- use `$()`
- use `printf` (instead of calling perl).
- indent.
- copy/adapt header comment from curl to `maketgz`.
This results in better job names (now including CPU), avoiding the
complex exception rules, and fine-tuning the order and variation of
these tests.
Enable `LIBSSH2DEBUG` for two of the existing jobs.
- add MSVS 2022 WinCNG builds for x64 and ARM64,
replacing MSVS 2013 WinCNG builds for x64 and x86.
- add MSVS 2022 OpenSSL builds for x64.
- fix a compiler warning uncovered by the new ARM64 build:
```
tests\openssh_fixture.c(393,17): warning C4477: 'fprintf' : format string '%d' requires an argument of type 'int', but variadic argument 1 has type 'libssh2_socket_t'
tests\openssh_fixture.c(393,17): message : consider using '%lld' in the format string
tests\openssh_fixture.c(393,17): message : consider using '%Id' in the format string
tests\openssh_fixture.c(393,17): message : consider using '%I64d' in the format string
```
- echo the actual CMake command-line.
- cmake: echo the DLL filenames found by the OpenSSL DLL-finder
heuristics.
- cmake: delete `libcrypto.dll` and `libssl.dll` names from the above
logic.
I've added these in 19884e5055. That
resulted in CMake picking up a rogue `libcrypto.dll` (with no
`libssl.dll` pair) from `C:\Windows\System32\` on the
`Visual Studio 2022` image, breaking tests.
Turns out, OpenSSL v1.0.2 uses the "EAY" names, but let's not re-add
those either, because CMake mis-picks those up from
`C:/OpenSSL-Win64/bin/`, even while pointing `OPENSSL_ROOT_DIR` to a
v1.1.1 installation.
- cmake: set `NO_DEFAULT_PATH` for OpenSSL DLL lookup to avoid picking
up all kinds of wrong DLLs. CMake considers not the first, but the
_last_ hit the valid one. This happened to be
`C:/Program Files/Meson/lib*-1_1.dll` when using the
`Visual Studio 2022` image.
Ref: https://cmake.org/cmake/help/latest/command/find_file.html
- cmake: leave two commented debug lines that will be useful next time
the DLL detection lookup goes wrong.
Ref: https://cmake.org/cmake/help/latest/variable/CMAKE_FIND_DEBUG_MODE.html
- on error, also dump `CMakeFiles/CMakeConfigureLog.yaml` if it exists
(requires CMake 3.26 and newer)
Make our CMake config more self-documenting by introducing variables
for the shared and static lib target names. Without this, it might be
non-trivial to find out which line is referring to a target name vs
libname, export name or other occurrences of `libssh2`.
This allows to rename back the shared lib target name to the value used
before 4e2580628d:
`libssh2_shared` -> `libssh2`, if necessary for compatibility. Notice:
before that patch, `libssh2` name referred to either the static or
shared lib, depending on build settings.
Update from:
5fec927374/scripts/checksrc.pl
- suppress these new checks:
- EQUALSNULL: 320 warnings
- NOTEQUALSZERO: 142 warnings
- TYPEDEFSTRUCT: 16 warnings
We can enabled them in the future.
- fix all other new ones.
- also fix whitespace in two `NMakefile` files.
`--parallel 2` did not seem to make builds faster. Neither did 4 or 6.
Delete this option from both GHA and AppVeyor jobs.
On AppVeyor, with VS, it uses MSBuild under the hood where apparently
`--parallel` doesn't do much [1]. The suggested MSBuild-specific option
`/p:CL_MPcount=2` did not improve build times either.
CMake spends significant time (comparable to building the project
itself) on feature detection, it'd be nice to execute those in parallel,
but I found not such CMake option.
[1] https://discourse.cmake.org/t/parallel-does-not-really-enable-parallel-compiles-with-msbuild/964
Partial revert of 7a039d9a7a
Null-cipher and null-MAC are security footguns we want to avoid.
Existing option names to toggle these were ambiguous and gave room for
misinterpretation. Some projects may have had these options enabled by
accident.
This patch aims to make it more difficult to enable them, and making
sure that existing methods require an update to stay enabled.
- delete CMake/autotools settings to enable the "none" cipher and MAC.
- rename existing C macros that can enable them.
To use them, pass them as custom `CPPFLAGS` to the build.
- enable them only if `LIBSSH2DEBUG` is also enabled.
Best would be to delete them, though they may have some use while
developing libssh2 itself, or debugging.
libssh2 supports an "old" style KEX message
`SSH2_MSG_KEX_DH_GEX_REQUEST_OLD`, as an off-by-default build option.
OpenSSH deprecated/disabled this feature in v6.9 (2015-07-01):
https://www.openssh.com/releasenotes.html#6.9
This patch deletes this obsolete feature from libssh2, with no option
to enable it.
Added to libssh2 in: cf8ca63ea0 (2004-12-31)
RFC: https://datatracker.ietf.org/doc/html/rfc4419 (2006-03)
`#undef snprintf` before redefining it, when `HAVE_SNPRINTF` is not
defined, even though `snprintf` is available and it should have been.
Possibly with 3rd party builds.
Downside is that cases of missing `HAVE_SNPRINTF` are less trivially
detected at compile-time.
Silence alignment warnings in WinCNG, by reworking the code.
Also add two unrelated casts to avoid gcc compiler warnings
in surrounding code.
`increases required alignment from 1 to 4 [-Wcast-align]`
`increases required alignment from 1 to 8 [-Wcast-align]`
See warning details in the PR's individual commits.
Reviewed-by: Marc Hörsken in <https://github.com/libssh2/libssh2/pull/846#pullrequestreview-1350253621>
Cherry-picked from #846Closes#880
Apply type changes to avoid casts and warnings. In most cases this
means changing to a larger type, usually `size_t` or `ssize_t`.
Change signedness in a few places.
Also introduce new variables to avoid reusing them for multiple
purposes, to avoid casts and warnings.
- add FIXME for public `libssh2_sftp_readdir_ex()` return type.
- fix `_libssh2_mbedtls_rsa_sha2_verify()` to verify if `sig_len`
is large enough.
- fix `_libssh2_dh_key_pair()` in `wincng.c` to return error if
`group_order` input is negative.
Maybe we should also reject zero?
- bump `_libssh2_random()` size type `int` -> `size_t`. Add checks
for WinCNG and OpenSSL to return error if requested more than they
support (`ULONG_MAX`, `INT_MAX` respectively).
- change `_libssh2_ntohu32()` return value `unsigned int` -> `uint32_t`.
- fix `_libssh2_mbedtls_bignum_random()` to check for a negative `top`
input.
- size down `_libssh2_wincng_key_sha_verify()` `hashlen` to match
Windows'.
- fix `session_disconnect()` to limit length of `lang_len`
(to 256 bytes).
- fix bad syntax in an `assert()`.
- add a few `const` to casts.
- `while(1)` -> `for(;;)`.
- add casts that didn't fit into #876.
- update `docs/HACKING-CRYPTO` with new sizes.
May need review for OS400QC3: /cc @monnerat @jonrumsey
See warning details in the PR's individual commits.
Cherry-picked from #846Closes#879
Most of the changes aim to silence warnings by adding casts.
An assortment of other issues, mainly compiler warnings, resolved:
- unreachable code fixed by using `goto` in
`publickey_response_success()` in `publickey.c`.
- potentially uninitialized variable in `sftp_open()`.
- MSVS-specific bogus warnings with `nid_type` in `kex.c`.
- check result of `kex_session_ecdh_curve_type()`.
- add missing function declarations.
- type changes to fit values without casts:
- `cmd_len` in `scp_recv()` and `scp_send()`: `int` -> `size_t`
- `Blowfish_expandstate()`, `Blowfish_expand0state()` loop counters:
`uint16_t` -> `int`
- `RECV_SEND_ALL()`: `int` -> `ssize_t`
- `shell_quotearg()` -> `unsigned` -> `size_t`
- `sig_len` in `_libssh2_mbedtls_rsa_sha2_sign()`:
`unsigned` -> `size_t`
- `prefs_len` in `libssh2_session_method_pref()`: `int` -> `size_t`
- `firstsec` in `_libssh2_debug_low()`: `int` -> `long`
- `method_len` in `libssh2_session_method_pref()`: `int` -> `size_t`
- simplify `_libssh2_ntohu64()`.
- fix `LIBSSH2_INT64_T_FORMAT` for MinGW.
- fix gcc warning by not using a bit field for
`burn_optimistic_kexinit`.
- fix unused variable warning in `_libssh2_cipher_crypt()` in
`libgcrypt.c`.
- fix unused variables with `HAVE_DISABLED_NONBLOCKING`.
- avoid const stripping with `BIO_new_mem_buf()` and OpenSSL 1.0.2 and
newer.
- add a missing const in `wincng.h`.
- FIXME added for public:
- `libssh2_channel_window_read_ex()` `read_avail` argument type.
- `libssh2_base64_decode()` `datalen` argument type.
- fix possible overflow in `sftp_read()`.
Ref: 4552c73cd5
- formatting in `wincng.h`.
See warning details in the PR's individual commits.
Cherry-picked from #846Closes#876
In a recent CMake update I left the original CMake EXPORTS macro
unchanged (`libssh2_EXPORTS`) for compatibility.
However, that macro was also recently added [1] and not present in an
official release yet, so we might as well just use the new native one
instead (`libssh2_shared_EXPORTS`), defined by CMake automatically.
This way we don't need to define the old macro manually.
CMake forms this macro from the lib's internal name as defined in
`add_library()` by appending `_EXPORTS`. That target name changed from
`libssh2` to `libssh2_shared` after introducing dual shared + static
builds in the recent update.
If we're here, add a new, stable, build-tool agnostic macro with the
same effect, for non-CMake use: `LIBSSH2_EXPORTS`
[1] 1f0fe7443a (2021-10-26)
Follow-up to 4e2580628d
Before this patch, cmake did a single compilation pass when we enabled
both shared and static lib targets. This saves build time (esp. with
MinGW targets and cross-compiling), but has the disadvantage that static
libs built this way must have PIC enabled (offering slightly less
performance) and `dllexport` enabled also, which means that executables
linking the static libssh2 lib export its public symbols.
To avoid these downsides, this patch separates the two passes and
creates a non-PIC, non-`dllexport` static lib, even when also building
the shared lib.
- limit static-only build to a single platform (x64).
- skip running ctest for the static-only build.
- use MSVS 2013 for static-only builds. It's faster.
- run static-only test before WinCNG ones. Otherwise it's often skipped
due to WinCNG failures (#804).
With CMake builds supporting static-shared libssh2 builds in a single
pass, we no longer need to run static and shared jobs separately. For
the same effect it's enough to run builds with both shared and static
builds enabled. Halving CI jobs.
We add an extra run to test the CMake config-path without shared builds
enabled.
This allows to add useful jobs, e.g. MSVS 2022 or ZLIB-enabled builds
for Windows, valgrind builds or other useful stuff, without stretching
CI run times further.
Ref: #863
- `BUILD_SHARED_LIBS=ON` no longer disables building static lib.
When set, we build the static lib with PIC enabled.
For shared lib only, set `BUILD_STATIC_LIBS=OFF`. For static lib
without PIC, leave this option disabled.
- new setting: `BUILD_STATIC_LIBS`. `ON` by default.
Force-enabled when building examples or tests (we build those in
static mode always.)
- fix to exclude Windows Resource from the static lib.
- fix to not overwrite static lib with shared implib on Windows
platforms using identical suffix for them (MSVS). By using
`libssh2_imp<.ext>` implib filename.
- add support for `STATIC_LIB_SUFFIX` setting to set an optional suffix
(e.g. `_static`) for the static lib. (experimental, not documented).
Overrides the above when set.
- fix to set `dllexport` when building shared lib.
- set `TrackFileAccess=false` for MSVS.
For faster builds, shorter verbose logs.
- tests: new test linking against shared libssh2: `test_warmup_shared`
- tests: simplify 'runner' lib by merging 3 libs into a single one.
- tests: drop hack from `test_keyboard_interactive_auth_info_request`
build.
We no longer need to compile `src/misc.c` because we always link
libssh2 statically.
- tests: limit `FIXTURE_WORKDIR=` to the `runner` target.
TL;DR: Default behavior unchanged: static (no-PIC), no shared.
Enabling shared unchanged, but now also builds a static (PIC)
lib by default.
Based-on: b60dca8b64#547 by berney on github
Fixes: #547Fixes: #675Closes: #863
Fix or silence all C compiler warnings discovered with (or without)
`PICKY_COMPILER=ON` (in CMake). This means all warnings showing up in
CI (gcc, clang, MSVS 2013/2015), in local tests on macOS (clang 14) and
Windows cross-builds using gcc (12) and llvm/clang (14/15).
Also fix the expression `nread -= nread` in `sftp_RW_nonblock.c`.
Cherry-picked from: #846Closes#861
libssh2 built with OpenSSL and without its `EVP_aes_128_ctr()`, aka
`HAVE_EVP_AES_128_CTR`, option are working incorrectly. This option
wasn't always auto-detected by autotools up until recently (#811).
Non-cmake, non-autotools build methods never enabled it automatically.
OpenSSL supports this options since at least v1.0.2, which is already
EOLed and considered obsolete. OpenSSL forks (LibreSSL, BoringSSL)
supported it all along.
In this patch we enable this option unconditionally, now requiring
OpenSSL supporting this function, or one of its forks.
Also modernize OpenSSL lib references to what 1.0.2 and newer versions
have been using.
Fixes#739
- cmake: fix compiler warnings in `CheckNonblockingSocketSupport`.
detection functions.
Without this, these detections fail when `ENABLE_WERROR=ON`.
- cmake: disable ENABLE_WERROR for MSVC during symbol checks in `src`.
CMake's built-in symbol check function `check_symbol_exists()`
generate warnings with MSVC. With warnings considered errors, these
detections fail permanently. Our workaround is to disable
warnings-as-errors while running these checks.
```
CheckSymbolExists.c(8): warning C4054: 'type cast': from function pointer '__int64 (__cdecl *)(const char *,char **,int)' to data pointer 'int *'
in `return ((int*)(&strtoll))[argc];`
```
Ref: https://ci.appveyor.com/project/libssh2org/libssh2/builds/46537222/job/4vg4yg333mu2lg9b
- example: replace `strcasecmp()` with C89 `strcmp()`.
To avoid using CMake symbol checks in `example`.
Another option is to duplicate the `check_symbol_exists()` workaround
from `src`, but I figure it's not worth the complexity. We use
`strcasecmp()` solely to check optional command-line options for
example programs, and those are fine as lower-case.
Without this, these detections fail when `ENABLE_WERROR=ON`.
- also delete `__function__` detection/use in `example`.
To avoid the complexity for the sake of using it at a single place in
of the example's error branch. Replace that use with a literal name of
the function.
- cmake: also use `CMakePushCheckState` functions instead of manual
save/restore.
Closes#857
- cmake: extend workaround for linking a test with shared libssh2.
One of the tests uses internal libssh2 functions, and with CMake it
compiles `src/misc.c` directly for this. `misc.c` references bcrypt /
blowfish code. This needs a workaround for build configs where libssh2
doesn't export these.
Before this patch, we enabled this workaround for MSVC.
In the patch we extend this to all Windows. There is no CI test for
this, but gcc and llvm/clang + mingw64 builds also need it. This may
well apply to other configurations (it should, as shared libs are not
supposed to export internal functions), so also make it easy to enable
it at a single point.
[ autotools builds force-link this one test against static libssh2. ]
- make `misc.c` not depend on bcrypt.
By moving out our `bcrypt_pbkdf()` wrapper into `bcrypt_pbkdf.c`
itself.
This allows to compile `misc.c` into tests without pulling in bcrypt /
blowfish functions, and simplify the above workaround.
Source code uses `HAVE_BCRYPT_PBKDF`, a leftover from original bcrypt
source. We never define this inside libssh2. Defining it breaks the
build, and this patch doesn't change that.
- make `bcrypt_pbkdf()` static.
While here, make the low-level `bcrypt_pbkdf()` function static to
avoid namespace pollution.
Closes#855
- add timeout to SSH connection wait loop in AppVeyor test prep.
(2 minutes)
- switch to per-step timeout for GitHub CI cmake/ctest runs.
(10 minutes)
ctest timeout (of 450 seconds) didn't seem to make any difference.
- `ctest` shows a the default timeout '10000000' (turns out to be
in seconds), cause infinite waits e.g. in case the necessary server
worker is not available.
CMake CI tests take approx:
- GitHub / Linux : 125 seconds
- AppVeyor / Windows: 300 seconds
New timeouts are: 450 and 900 seconds respectively.
- set timeouts for style-check, fuzz, Linux and Windows GitHub CI
jobs to avoid hanging forever.
Also:
- move `choco install` to before_test to make builds start faster
in `appveyor.yml`.
- fix some yamllint `ON`/`OFF`-confusion issue by quoting these
values in `appveyor.yml`.
- fix indentation in `appveyor.yml`.
- convert to GitHub workflows to LF line-ending.
Ref: https://github.com/libssh2/libssh2/pull/655#issuecomment-1472853493Closes#851
