Ref:
https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html
(2023-11-29)
Enable new warnings:
- replace `-Wno-sign-conversion` with `-Wsign-conversion`.
Fix them in example, tests and wincng. There remain about 360 of these
warnings in `src`. Add a TODO item for those and disable `-Werror` for
this particular warning.
- enable `-Wformat=2` for clang (in both cmake and autotools).
- enable `__attribute__((format))` for `_libssh2_debug()`,
`_libssh2_snprintf()` and in tests for `run_command()`.
`LIBSSH2_PRINTF()` copied from `CURL_TEMP_PRINTF()` in curl.
- enable `-Wimplicit-fallthrough`.
- enable `-Wtrampolines`.
Fix them:
- src: replace obsolete fall-through-comments with
`__attribute__((fallthrough))`.
- wincng: fix `-Wsign-conversion` warnings.
- tests: fix `-Wsign-conversion` warnings.
- example: fix `-Wsign-conversion` warnings.
- src: fix `-Wformat` issues in trace calls.
Also, where necessary fix `int` and `unsigned char` casts to
`unsigned int` and adjust printf format strings. These were not
causing compiler warnings.
Cast large types to `long` to avoid dealing with printf masks for
`size_t` and other C99 types. Existing code often used `int` for this.
I'll update them to `long` in an upcoming commit.
- tests: fix `-Wformat` warning.
- silence `-Wformat-nonliteral` warnings.
- mbedtls: silence `-Wsign-conversion`/`-Warith-conversion`
in external header.
Closes#1257
It works with gcc without the libtool warnings seen with clang
on Windows in 96682bd5e1#1227.
Sync usage of of this macro with CMake and
autotools + clang + non-Windows. Making it enabled everywhere except
autotools + clang + Windows due to the libtool stub issue.
Follow-up to 7ecc309cd1#1224Closes#1262
Then sync this warning option with curl.
Seems like a false positive and/or couldn't figure how to fix it, so silence:
```
example/ssh2.c:227:38: error: '%s' directive output may be truncated writing likely 1 or more bytes into a region of size 0 [-Werror=format-truncation=]
227 | snprintf(fn1, fn1sz, "%s/%s", h, pubkey);
| ^~
example/ssh2.c:227:34: note: assuming directive output of 1 byte
227 | snprintf(fn1, fn1sz, "%s/%s", h, pubkey);
| ^~~~~~~
example/ssh2.c:227:13: note: 'snprintf' output 3 or more bytes (assuming 4) into a destination of size 2
227 | snprintf(fn1, fn1sz, "%s/%s", h, pubkey);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
example/ssh2.c:228:38: error: '%s' directive output may be truncated writing likely 1 or more bytes into a region of size 0 [-Werror=format-truncation=]
228 | snprintf(fn2, fn2sz, "%s/%s", h, privkey);
| ^~
example/ssh2.c:228:34: note: assuming directive output of 1 byte
228 | snprintf(fn2, fn2sz, "%s/%s", h, privkey);
| ^~~~~~~
example/ssh2.c:228:13: note: 'snprintf' output 3 or more bytes (assuming 4) into a destination of size 2
228 | snprintf(fn2, fn2sz, "%s/%s", h, privkey);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
Ref: https://github.com/libssh2/libssh2/actions/runs/7055480458/job/19205970397#step:10:98
Fix:
```
tests/openssh_fixture.c:116:38: error: ' 2>&1' directive output may be truncated writing 5 bytes into a region of size between 1 and 1024 [-Werror=format-truncation=]
tests/openssh_fixture.c:116:11: note: 'snprintf' output between 6 and 1029 bytes into a destination of size 1024
```
Ref: https://github.com/libssh2/libssh2/actions/runs/7055480458/job/19205969221#step:10:51
Tested via #1257
Move warning option suppression to `src/mbedtls.h` to surround the actual
external header #includes that need it.
Follow-up to ecec68a2c1#1226
Follow-up to 7ecc309cd1#1224
Tested via #1257
- use OpenSSL 3 API when available for HMAC.
This fixes building with OpenSSL 3 `no-deprecated` builds.
- ensure we support pure OpenSSL 3 API by adding a CI job using
OpenSSL 3 custom-built with `no-deprecated`.
Follow-up to b0ab005fe7#1207Fixes#1235Closes#1243
- switch to an alternate GitHub action. This one seems (more) actively
maintained, and runs faster:
https://github.com/cross-platform-actions/action
- use clang instead of gcc. clang is already present in the base
install, saving install time and bandwidth.
- stop installing `openssl-quictls` and use the OpenSSL (v1.1) from
the base system.
(I'm suspecting that quictls before this patch wasn't detected by
the build.)
https://wiki.freebsd.org/OpenSSL
Cherry-picked from #1250
Underscored macros are reserved for the compiler / standard lib / etc.
Stop using them in user code.
We used them as header guards in `src` and in `__FILESIZE` in `example`.
Closes#1248
- fix missing `DSA` type when building for OpenSSL 3 `no-deprecated`.
- fix fallouts after fixing the above by switching away from `DSA`
with OpenSSL 3.
Follow-up to b0ab005fe7#1207Closes#1244
If the `$USER` variable is empty, fall back to using `$LOGNAME` to
retrieve the logged-in username.
In POSIX, `$LOGNAME` is a mandatory variable, while `$USER` isn't, and
on some systems it may not be set. Without this value, tests were unable
to provide the correct username when logging into the SSH server running
under the active user's session.
Reported-by: Nicolas Mora
Suggested-by: Nicolas Mora
Ref: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1056348Fixes#1240Closes#1241
Sync up `libssh2.h` Windows detection with the libssh2 source code.
`libssh2.h` was using `WIN32` and `LIBSSH2_WIN32` for Windows detection,
next to the official `_WIN32`. After this patch it only uses `_WIN32`
for this. Also, make it stop defining `LIBSSH2_WIN32`.
There is a slight chance these break compatibility with Windows
compilers that fail to define `_WIN32`. I'm not aware of any obsolete
or modern compiler affected, but in case there is one, one possible
solution is to define this macro manually.
Closes#1238
Fixes:
```
src/openssl.c:650:5: error: use of undeclared identifier 'EC_KEY'
EC_KEY *ec_key = EC_KEY_new_by_curve_name(curve);
^
src/openssl.c:650:13: error: use of undeclared identifier 'ec_key'
EC_KEY *ec_key = EC_KEY_new_by_curve_name(curve);
^
src/openssl.c:650:22: error: implicit declaration of function 'EC_KEY_new_by_curve_name' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
EC_KEY *ec_key = EC_KEY_new_by_curve_name(curve);
^
src/openssl.c:650:22: note: did you mean 'EC_GROUP_new_by_curve_name'?
./quictls/_a64-mac-sys/usr/include/openssl/ec.h:483:11: note: 'EC_GROUP_new_by_curve_name' declared here
EC_GROUP *EC_GROUP_new_by_curve_name(int nid);
^
In file included from ./_a64-mac-sys-bld/src/CMakeFiles/libssh2_static.dir/Unity/unity_0_c.c:19:
In file included from src/crypto.c:10:
src/openssl.c:652:8: error: use of undeclared identifier 'ec_key'
if(ec_key) {
^
```
Ref: https://github.com/curl/curl-for-win/actions/runs/6950001225/job/18909297867#step:3:4341
Follow-up to b0ab005fe7#1207
Bug #1235Closes#1236
- fix parsing dotless (major-only) gcc versions.
Follow-up to 00a3b88c51#1187
- sync gcc detection variable names with curl.
- fix Apple clang version detection for releases between
'Apple LLVM version 7.3.0' and 'Apple LLVM version 10.0.1' where the
version was under-detected as 3.7 llvm/clang equivalent.
- fix Apple clang version detection for 'Apple clang version 11.0.0'
and newer where the Apple clang version was detected, instead of its
llvm/clang equivalent.
- revert to show `clang` instead of `Apple clang`, because we follow it
with an llvm/clang version number. (Apple-ness still visible in raw
version.)
Used this collection for Apple clang / llvm/clang translation and test
inputs: https://gist.github.com/yamaya/2924292Closes#1232
Seen on Windows with clang64, in libtool-generated stub code for
examples and tests.
The error didn't break the CI job for some reason.
msys2 (autotools, clang64, clang-x86_64:
```
[...]
2023-11-17T20:14:17.8639574Z ./.libs/lt-test_read.c:91:10: error: macro is not used [-Werror,-Wunused-macros]
[...]
2023-11-17T20:14:39.8729255Z ./.libs/lt-sftp_write_nonblock.c:91:10: error: macro is not used [-Werror,-Wunused-macros]
[...]
```
Ref: https://github.com/libssh2/libssh2/actions/runs/6908585056/job/18798193405?pr=1226#step:8:474
Follow-up to 7ecc309cd1#1224Closes#1227
Disable these warnings specifically for the mbedTLS public headers
and leave it on for the the rest of the code. This also fixes this
issue for autotools. Previous solution was globally disabling this
warning for the whole code when using mbedTLS and only with CMake.
Follow-up to 7ecc309cd1#1224Closes#1226
Enable more picky compiler warnings. I've found these options in the
nghttp3 project when implementing the CMake quick picky warning
functionality for it.
Fix issues found along the way:
- wincng, mbedtls: delete duplicate function declarations.
Most of this was due to re-#defining crypto functions to
crypto-backend specific implementations These redefines also remapped
the declarations in `crypto.h`, making the backend-specific
declarations duplicates.
This patch deletes the backend-specific declarations.
- wincng mapped two crypto functions to the same local function.
Also causing double declarations.
Fix this by adding two disctinct wrappers and moving
the common function to a static one.
- delete unreachable `break;` statements.
- kex: disable macros when unused.
- agent: disable unused constants.
- mbedtls: disable double declaration warnings because public mbedTLS
headers trigger it. (with function `psa_set_key_domain_parameters`)
- crypto.h: formatting.
Ref: a70edb08e9/cmake/PickyWarningsC.cmakeCloses#1224
- sync clang warning version limits with CMake.
- make `WARN=` vs. `CURL_ADD_COMPILER_WARNINGS()` consistent with curl
and between clang and gcc (`WARN=` is for `no-` options in general).
Closes#1221
- cmake, autotools: sync picky gcc warnings with curl.
- cmake, autotools: add `-Wold-style-definition` for clang too.
- cmake, autotools: add comment for `-Wformat-truncation=1`.
- cmake: more precise version info for old clang options.
Closes#1219
We haven't been using the FreeBSD version. Also it turns out,
the single version supported is 13.2 at the moment:
https://github.com/vmactions/freebsd-vm/tree/main/conf
Stop trying to set the version and instead rely on the action
providing the latest supported one automatically.
Follow-up to a7d2a573be
Also:
- add more details to the FreeBSD job description.
- bump mbedtls version while here.
Closes#1217
Also extend our integration test double inclusion. It will still not
catch this case, because that requires
`cmake_minimum_required(VERSION 3.18)` or higher.
Fixes:
```
CMake Error at .../lib/cmake/libssh2/libssh2-config.cmake:8 (add_library):
add_library cannot create ALIAS target "libssh2::libssh2" because another
target with the same name already exists.
Call Stack (most recent call first):
CMakeLists.txt:24 (find_package)
CMake Error at .../lib/cmake/libssh2/libssh2-config.cmake:13 (add_library):
add_library cannot create ALIAS target "Libssh2::libssh2" because another
target with the same name already exists.
Call Stack (most recent call first):
CMakeLists.txt:24 (find_package)
```
Test to reproduce:
```cmake
cmake_minimum_required(VERSION 3.18) # must be 3.18 or higher
project(test)
find_package(libssh2 CONFIG)
find_package(libssh2 CONFIG) # fails
add_executable(test main.c)
target_link_libraries(test libssh2::libssh2)
```
Ref: https://cmake.org/cmake/help/latest/release/3.18.html#other-changes
Ref: https://cmake.org/cmake/help/v3.18/policy/CMP0107.html
Assisted-by: Kai Pastor
Assisted-by: Harry Mallon
Ref: https://github.com/curl/curl/pull/11913Closes#1216
```
PendingDeprecationWarning:
Copyright and licensing information for 'tests/openssh_server/Dockerfile'
has been found in both 'tests/openssh_server/Dockerfile' and in the DEP5
file located at '.reuse/dep5'. The information for these two sources has
been aggregated. In the future this behaviour will change, and you will
need to explicitly enable aggregation. [...]
```
Ref: https://github.com/libssh2/libssh2/actions/runs/6789274955/job/18456085964#step:4:4
We recommend using CMake instead. Especially in unity mode, it's faster
and probably more familiar for most. It's also easily portable.
(`Makefile.mk` was also portable, but in practice only usable for
Windows. Other platforms required a manual config header.)
Also:
- migrate `LIBSSH2_NO_*` option CI tests to CMake.
- make MSYS2 CMake builds verbose to show compilation options.
Closes#1204
- os400: delete unused `HAVE_STDINT_H`.
- fuzz: delete redundant `stdint.h` use.
`inttypes.h` is already included via `testinput.h`.
- docs/TODO: adjust type in planned function.
Closes#1212
