Implement picky warnings with clang in autotools. Extend picky gcc
warnings, sync them between build tools and compilers and greatly
speed up detection in CMake.
- autotools: enable clang compiler warnings with `--enable-debug`.
- autotools: enable more gcc compiler warnings with `--enable-debug`.
- autotools/cmake: sync compiler warning options between gcc and clang.
- sync compiler warning options between autotools and cmake.
- cmake: reduce option-checks to speed up the detection phase.
Bring them down to 3 (from 35). Leaving some checks to keep the
CMake logic alive and for an easy way to add new options.
clang 3.0 (2011-11-29) and gcc 2.95 (1999-07-31) now required.
- autotools logic copied from curl, with these differences:
- delete `-Wimplicit-fallthrough=4` due to a false positive.
- reduce `-Wformat-truncation=2` to `1` due to a false positive.
- simplify MinGW detection for `-Wno-pedantic-ms-format`.
- cmake: show enabled picky compiler options (like autotools).
- cmake: do compile `tests/simple.c` and `tests/ssh2.c`.
- fix new compiler warnings.
- `tests/CMakeLists.txt`: fix indentation.
Original source of autotools logic:
- a8fbdb461c/acinclude.m4
- a8fbdb461c/m4/curl-compilers.m4
Notice that the autotools implementation considers Apple clang as
legacy clang 3.7. CMake detection works more accurately, at the same
time more error-prone and difficult to update due to the sparsely
documented nature of Apple clang option evolution.
Closes#952
After recent build changes, 3rd party build that took the list of
C source to compile them as-is, stopped working as expected, due to
`blowfish.c` and crypto-backend C sources no longer expected to compile
separately but via `bcrypt_pbkdf.c` and `crypto.c`, respectively.
This patch ensures that compiling these files directly result in an
empty object instead of redundant code and duplicated symbols.
Also:
- add a compile-time error if none of the supported crypto backends
are enabled.
- fix `libssh2_crypto_engine()` for wolfSSL and os400qc3.
Rearrange code to avoid a hard-to-find copy of crypto-backend
selection guards.
Follow-up to 4f0f4bff5a
Follow-up to ff3c774e03Closes#951
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.
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
- convert `_libssh2_explicit_zero()` to macro. This allows inlining
where supported (e.g. `SecureZeroMemory()`).
- replace `SecureZeroMemory()` (in `wincng.c`) and
`LIBSSH2_CLEAR_MEMORY`-guarded `memset()` (in `os400qc3.c`) with
`_libssh2_explicit_zero()` macro.
- delete `LIBSSH2_CLEAR_MEMORY` guards, which enables secure-zeroing
universally.
- add `LIBSSH2_NO_CLEAR_MEMORY` option to disable secure-zeroing.
- while here, delete double/triple inclusion of `misc.h`.
`libssh2_priv.h` included it already.
Closes#810
- in `hostkey.c` check the result of `libssh2_sha256_init()` and
`libssh2_sha512_init()` calls. This avoid the warning that we're
ignoring the return values.
- fix code using `int` (or `SOCKET`) for sockets. Use libssh2's
dedicated `libssh2_socket_t` and `LIBSSH2_INVALID_SOCKET` instead.
- fix compiler warnings due to `STATUS_*` macro redefinitions between
`ntstatus.h` / `winnt.h`. Solve it by manually defining the single
`STATUS` value we need from `ntstatus.h` and stop including the whole
header.
Fixes#733
- improve Windows UWP/WinRT builds by detecting it with code copied
from the curl project. Then excluding problematic libssh2 parts
according to PR by Dmitry Kostjučenko.
Fixes#734
- always use `SecureZeroMemory()` on Windows.
We can tweak this if not found or not inlined by a C compiler which
we otherwise support. Same if it causes issues with UWP apps.
Ref: https://learn.microsoft.com/en-us/previous-versions/windows/desktop/legacy/aa366877(v=vs.85)
Ref: https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-rtlsecurezeromemory
- always enable `LIBSSH2_CLEAR_MEMORY` on Windows. CMake and
curl-for-win builds already did that. Delete `SecureZeroMemory()`
detection from autotools' WinCNG backend logic, that this
setting used to depend on.
TODO: Enable it for all platforms in a separate PR.
TODO: For clearing buffers in WinCNG, call `_libssh2_explicit_zero()`,
insead of a local function or explicit `SecureZeroMemory()`.
- Makefile.inc: move `os400qc3.h` to `HEADERS`. This fixes
compilation on non-unixy platforms. Recent regression.
- `libssh2.rc`: replace copyright with plain ASCII, as in curl.
Ref: curl/curl@1ca62bb
Ref: curl/curl#7765
Ref: curl/curl#7776
- CMake fixes and improvements:
- enable warnings with llvm/clang.
- enable more comprehensive warnings with gcc and llvm/clang.
Logic copied from curl:
233810bb5f/CMakeLists.txt (L131-L148)
- fix `Policy CMP0080` CMake warning by deleting that reference.
- add `ENABLE_WERROR` (default: `OFF`) option. Ported from curl.
- add `PICKY_COMPILER` (default: `ON`) option, as known from curl.
It controls both the newly added picky warnings for llvm/clang and
gcc, and also the pre-existing ones for MSVC.
- `win32/GNUmakefile` fixes and improvements:
- delete `_AMD64_` and add missing `-m64` for x64 builds under test.
- add support for `ARCH=custom`.
It disables hardcoded Intel 64-bit and Intel 32-bit options,
allowing ARM64 builds.
- add support for `LIBSSH2_RCFLAG_EXTRAS`.
To pass custom options to windres, e.g. in ARM64 builds.
- add support for `LIBSSH2_RC`. To override `windres`.
- delete support for Metrowerks C. Last released in 2004.
- `win32/libssh2_config.h`: delete unnecessary socket #includes
`src/libssh2_priv.h` includes `winsock2.h` and `ws2tcpip.h` further
down the line, triggered by `HAVE_WINSOCK2_H`.
`mswsock.h` does not seem to be necessary anymore.
Double-including these (before `windows.h`) caused compiler failures
when building against BoringSSL and warnings with LibreSSL. We could
work this around by passing `-DNOCRYPT`. Deleting the duplicates
fixes these issues.
Timeline:
2013: c910cd382d deleted `mswsock.h` from `src/libssh2_priv.h`
2008: 8c43bc52b1 added `winsock2.h` and `ws2tcpip.h` to `src/libssh2_priv.h`
2005: dc4bb1af96 added the now deleted #includes
- delete or replace `LIBSSH2_WIN32` with `WIN32`.
- replace hand-rolled `HAVE_WINDOWS_H` macro with `WIN32`. Also delete
its detections/definitions.
- delete unused `LIBSSH2_DARWIN` macro.
- delete unused `writev()` Windows implementation
There is no reference to `writev()` since 2007-02-02, commit
9d55db6501.
- fix a bunch of MSVC / llvm/clang / gcc compiler warnings:
- `warning C4100: '...': unreferenced formal parameter`
- using value of undefined PP macro `LIBSSH2DEBUG`
- missing void from function definition
- `if()` block missing in non-debug builds
- unreferenced variable in non-debug builds
- `warning: must specify at least one argument for '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments]`
in `_libssh2_debug()`
- `warning C4295: 'ciphertext' : array is too small to include a terminating null character`
- `warning C4706: assignment within conditional expression`
- `warning C4996: 'inet_addr': Use inet_pton() or InetPton() instead or
define _WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings`
By suppressning it. Would be best to use inet_pton() as suggested.
On Windows this needs Vista though.
- `warning C4152: nonstandard extension, function/data pointer conversion in expression`
(silenced locally)
- `warning C4068: unknown pragma`
Ref: https://ci.appveyor.com/project/libssh2org/libssh2/builds/46354480/job/j7d0m34qgq8rag5wCloses#808
Make libssh2 compile cleanly with mbedTLS 3.x and later.
This patch makes use of `MBEDTLS_PRIVATE()`, which is not the
recommended, future-proof way to access mbedTLS data structures. This
method may break with a minor upgrade, according to the authors. This
is also the method used by libcurl.
Also:
- Fix a potentially uninitialized variable in
`libssh2_mbedtls_rsa_sha2_sign()`. This happened in an error path,
resulting in an unnecessary mbedTLS API call, with an uninitialized
`md_type`.
- Bump mbedTLS version used in CI tests to 3.2.1.
Fixes#751
Notes:
* Host Key RSA 256/512 support #536
* Client side key hash upgrading for RFC 8332
* Support for server-sig-algs, ext-info-c server messages
* Customizing preferred server-sig-algs via the preference LIBSSH2_METHOD_SIGN_ALGO
Credit: Anders Borum, Will Cosgrove
Files:
mbedtls.c, mbedtls.h, .travis.yml
Notes:
This PR adds support for ECDSA for both key exchange and host key algorithms.
The following elliptic curves are supported:
256-bit curve defined by FIPS 186-4 and SEC1
384-bit curve defined by FIPS 186-4 and SEC1
521-bit curve defined by FIPS 186-4 and SEC1
Credit:
Sebastián Katzer
Use checksrc.pl from the curl project, with (for now)
suppressed long line warnings and indentation set to
4 spaces. Fixes are whitespace for the most part.
Warning count went down from 2704 to 12.
Also fix codespell typos, two non-ANSI C89 comments
and a stray tab in include/libssh2.h.
Ref: https://github.com/libssh2/libssh2/pull/235
Not all backends feature the low level API needed to compute a Diffie-Hellman
secret, but some of them directly implement Diffie-Hellman support with opaque
private data. The later approach is now generalized and backends are
responsible for all Diffie Hellman computations.
As a side effect, procedures/macros _libssh2_bn_rand and _libssh2_bn_mod_exp
are no longer needed outside the backends.
