Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/update libssh2 1.11.0 #4

Open
wants to merge 425 commits into
base: feature/move-to-native-tipi-deps
Choose a base branch
from

Conversation

pysco68
Copy link

@pysco68 pysco68 commented Aug 24, 2023

Updating to the latest release of libssh2 to fix an issue with libgit2 not being able to push to remotes with more recent versions of openssh running.

vszakats and others added 30 commits March 28, 2023 17:47
Also skip detecting these and `memset_s()` for Windows targets in CMake,
to save detection time. On Windows we always use `SecureZeroMemory()`.
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 4e25806:
`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.
Last related commit happened 15 years ago.
NetWare had it last release in 2009.

All links referenced from the make file are inaccessible.
```
src/session.c:675:52: warning: implicit conversion loses integer precision: 'long' to '__darwin_suseconds_t' (aka 'int') [-Wshorten-64-to-32]
        tv.tv_usec = (ms_to_next - tv.tv_sec*1000) * 1000;
                   ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
```
- 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 19884e5. 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)
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.
- fix shellcheck warnings:
  - use quotes
  - use `$()`
- use `printf` (instead of calling perl).
- indent.
- copy/adapt header comment from curl to `maketgz`.
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.
- `HAVE_MEMSET_S` missing since
  0309229 (2018-08-02)

- `HAVE_EXPLICIT_BZERO` and `HAVE_EXPLICIT_MEMSET` missing since
  0000568 (2023-03-28)
Before this patch CMake did crypto-backend detection in both
`src/CMakefiles.txt` and `tests/CMakefiles.txt`.

Merge them and move it to the root `CMakefiles.txt`.

While here, also add zlib for OpenSSL. Necessary when using OpenSSL
builds with zlib enabled.

Closes libssh2#905
Before this patch CMake did feature detections in three files:
`src/CMakefiles.txt`, `examples/CMakefiles.txt` and
`tests/CMakefiles.txt`.

Merge and move them to the root `CMakefiles.txt`.

After this patch we end up with a single `src/libssh2_config.h`. This
brings CMake in sync with autotools builds, which already worked with
a single config header.

This also prevents mistakes where feature detection went out of sync
between `src` & `tests` (see ae90a35).
`tests` do compile sources from `src` directly, so these should always
be in sync.

It also allows to better integrate hand-crafted, platform-specific
config headers into the builds, like the one currently residing in
the `win32` directory (and also in `vms` and `os400`). Subject to an
upcoming PR.

Also fix a warning revealed after this patch made CMake correctly
enable `HAVE_GETTIMEOFDAY` for `example` programs.

Closes libssh2#906
Whether to build the `x11` example or not was decided by each build
tool. CMake didn't build it even on supported platforms. GNUMakefile
used a specific blocklist for it, while autotools enabled it based on
feature-detection.

Migrate the enabler logic to an #ifdef in source and build `x11`
unconditionally with all build tools.

On unsupported platforms (=Windows) this program now displays a short
message stating that fact.

Also:

- fix `x11.c` warnings uncovered after CMake started building it.

- use `libssh2_socket_t` type for portability in `x11.c` too.

- use detected header guards in `x11.c`.

- delete a duplicate reference to `-lws2_32` from `win32/GNUmakefile`
  while there.

Closes libssh2#909
- enable `HAVE_LONGLONG` for MinGW and MSVC versions supporting it.

  Necessary for `GNUmakefile`/`NMakefile` builds to create the same
  binaries as CMake/autotools ones do.

- enable `HAVE_STDLIB_H`. It has been universally available on
  Windows for a long time.

  Fixes these clang-cl warnings:
  ```
  src\wincng.c(444,5) :  warning: implicit declaration of function 'free' is invalid in C99 [-Wimplicit-function-declaration]
      free(buf);
      ^
  src\wincng.c(491,20) :  warning: implicitly declaring library function 'malloc' with type 'void *(unsigned long long)' [-Wimplicit-function-declaration]
      pbHashObject = malloc(dwHashObject);
                     ^
  src\wincng.c(491,20) :  note: include the header <stdlib.h> or explicitly provide a declaration for 'malloc'
  src\wincng.c(2106,14) :  warning: implicitly declaring library function 'realloc' with type 'void *(void *, unsigned long long)' [-Wimplicit-function-declaration]
      bignum = realloc(bn->bignum, length);
               ^
  src\wincng.c(2106,14) :  note: include the header <stdlib.h> or explicitly provide a declaration for 'realloc'
  3 warnings generated.
  ```
It was used once in `src/libssh2_priv.h`, but without any effect.
The header included `ws2tcpip.h` twice, once guarded by
`HAVE_WS2TCPIP_H` and another time by `HAVE_WINSOCK2_H`.

Dedupe these to not use `HAVE_WS2TCPIP_H`. Then delete detection
of this feature from all build methods.

TODO: Replace `HAVE_WINSOCK2_H` with `_WIN32`/`WIN32`.
- `-lws2_32` is necessary when building examples.

- drop a temporary variable.

Follow-up to d245c66
Also check for wolfSSL before mbedTLS to match CMake.
With 20-ish extra lines, make this Makefile support all GCC-like
toolchains.

The temporary directory becomes `<triplet>-{release|debug}` from
the former `{release|debug}`.

Also change the lib directory name in the `dist` package from
`win32` to `lib`, to match other packages and build tools.
Also more consistent. Refer to DLL/SO/shared as 'dyn'.

Also add comment on how to find customizable environment variables.
In the previous commit 9694871,
the commit message should read `win32/GNUmakefile: ` instead of
`libssh2-gnumake.sh: `. Sorry for the mixup.
From `<triplet>-{release|debug}` to `{release|debug}-<triplet>`.

Follow-up to 68fd02f
- replace `OPENSSLINC` and `OPENSSLLIB` with `OPENSSL_PATH`.
  Assume `include` and `lib` subdirs for headers and libs.

- replace `WITH_ZLIB`, `ZLIBINC` and `ZLIBLIB` with `ZLIB_PATH`.
  Assume `include` and `lib` subdirs for header and lib.

- make WinCNG the default if `WITH_OPENSSL` is not set.
vszakats and others added 30 commits May 5, 2023 19:52
`_libssh2_store_str()` and `_libssh2_store_bignum2_bytes()` accept
inputs of `size_t` max, store the size as 32-bit unsigned integer, then
store the complete input buffer.

With inputs larger than `UINT_MAX` this means the stored size is smaller
than the data that follows it.

This patch truncates the stored data to the stored size, and now returns
a boolean with false if the stored length differs from the requested
one. Also add `assert()`s for this condition.

This is still not a correct fix, as we now dump consistent, but still
truncated data which is not what the caller wants. In future steps we'll
need to update all callers that might pass large data to this function
to check the return value and handle an error, or make sure to not call
this function with more than UINT_MAX bytes of data.

Ref: c3bcdd8 (2010-04-17)
Ref: ed439a2 (2022-09-29)

Closes libssh2#1025
Before this patch libssh2 used a variety of solutions to pass the source
directory to tests: `FIXTURE_WORKDIR` build-time macro (cmake),
`FIXTURE_WORKDIR` envvar (unused), setting `srcdir` manually
(autotools), setting current directory (cmake), and also `builddir`
envvar (autotools) for passing current working dir to `mansyntax.sh`.

This patch reduces this to using existing `srcdir` with autotools and
setting it ourselves in CMake. This was mostly enabled by this recent
patch: 4c9ed51

Details:

- cmake: replace baked-in `FIXTURE_WORKDIR` macro with env.

  Added in 54bef4c libssh2#198 (2018-03-21)

- rename `FIXTURE_WORKDIR` to `srcdir`, to match autotools.

- cmake: add missing `srcdir` for algo and sshd tests.

- session_fixture: stop `chdir()`-ing, rely on prefixing with `srcdir`.

  Changing current directory should be unnecessary after
    4c9ed51 libssh2#801 (2023-02-24),
  that prefixes referenced input filenames with the `srcdir` envvar.

  The `srcdir` envvar was already exported by autotools, and now we're
  also setting it from CMake.

- cmake: stop setting `WORKING_DIRECTORY`, rely on `srcdir` env.

  `WORKING_DIRECTORY` is no longer necessary, after passing `srcdir` to
  all tests, so they can find our source tree and keys/etc in it
  regardless of the current directory.

  Also this past commit hints that `WORKING_DIRECTORY` wasn't always
  working for this purpose as expected:
    "tests: Xcode doesn't obey CMake's test working directory"
  Ref: libssh2@10a5cbf

- autotools: delete explicit `srcdir` for test env.

  Added in 13f8add (2015-07-02)

  automake documents `srcdir` as exported to the test environment:
  https://github.com/autotools-mirror/automake/blob/c04c4e8856e3c933239959ce18e16599fcc04a8b/doc/automake.texi#L9302-L9304
  https://www.gnu.org/software/automake/manual/html_node/Scripts_002dbased-Testsuites.html
  It's mentioned in the docs back in 1997 and got a regression test in
  2012. We can safely assume it to be available without setting it
  ourselves.

- autotools: delete explicit `builddir`.

  Added in 13f8add (2015-07-02)

  It seems this wasn't necessary to make the above fix work, and
  `mansyntax.sh` is able to figure out the build workdir by reading
  `$PWD`. Our out-of-tree and `make distcheck` CI builds also work
  without it.

  Let us know if there is a scenario we're missing and needs this.

Closes libssh2#1032
"Unity" (aka "jumbo", aka "amalgamation" builds concatenate source files
before compiling. It has these benefits for example: faster builds,
improved code optimization, cleaner code. Let's support and test this.

- enable unity builds for some existing CI builds to test this build
  scenario.
- tune `UNITY_BUILD_BATCH_SIZE` size.
- disable unity build for example and test programs (they use one source
  each already).

You can enable it by passing `-DCMAKE_UNITY_BUILD=ON` to cmake.
Supported by CMake 3.16 and newer.

Ref: https://cmake.org/cmake/help/latest/prop_tgt/UNITY_BUILD.html

Closes libssh2#1034
Shared libs improve example/tests build times. For "unity"
builds the overhead of building shared lib is negligible, so
this even reduced the overall build-time.

Follow-up to 3d64a3f
Follow-up to d93ccf4

Tests:
https://github.com/libssh2/libssh2/actions/runs/4906586658: unity builds enabled
https://github.com/libssh2/libssh2/actions/runs/4906925743: unity builds enabled + parallel msys2 builds
https://github.com/libssh2/libssh2/actions/runs/4906777629: unity + shared lib (this commit)
https://github.com/libssh2/libssh2/actions/runs/4906927190: unity + shared lib (this commit) + parallel msys2 builds

Consider making shared libs enabled by default also in CMake, to sync it with autotools?

Closes libssh2#1035
Before this patch, this test returned success even when one of its tests
failed. Fix it by returning 1 in case any of the tests fails.

This issue masked a CMake build bug with logging enabled. Subject to an
upcoming patch.

Cherry-picked from libssh2#1037
Required for tests using libssh2 internals. These are the ones
requiring the libssh2 _static_ lib.

Before this patch, `src` and `tests` declared the `session` structure
differently, due to extra struct members added with the `LIBSSH2DEBUG`
macro set. But, the macro was only set for `src` when using CMake. At
runtime this caused struct members to be at different offsets between
lib and test code, resulting in the test failures below.

Due to another bug in the affected test, these failures did not reflect
in the exit code, which always returned success, so this went unnoticed
for a good while. Fixed in: 84d31d0

```
     Start  5: test_auth_keyboard_info_request
[...]
5: Test case 1 passed
5: Test case 2 passed
5: Test case 3: expected return code to be 0 got -1
5: Test case 4: expected last error code to be "-6" got "-38"
5: Test case 5: expected last error code to be "-6" got "-38"
5: Test case 6: expected last error code to be "-6" got "-38"
5: Test case 7: expected last error message to be "Unable to decode keyboard-interactive number of keyboard prompts" got "userauth keyboard data buffer too small to get l
5: Test case 8: expected last error code to be "-41" got "-38"
5: Test case 9: expected return code to be 0 got -1
5: Test case 10: expected return code to be 0 got -1
5: Test case 11: expected last error code to be "-6" got "-38"
5: Test case 12: expected last error message to be "Unable to decode user auth keyboard prompt echo" got "userauth keyboard data buffer too small to get length"
5: Test case 13: expected return code to be 0 got -1
5: Test case 14: expected return code to be 0 got -1
5: Test case 15: expected last error code to be "-6" got "-38"
5: Test case 16: expected last error code to be "-6" got "-38"
5: Test case 17: expected last error code to be "-6" got "-38"
5: Test case 18: expected last error code to be "-6" got "-38"
```
Ref: https://ci.appveyor.com/project/libssh2org/libssh2/builds/46925869/job/i9uasceu3coss0i2#L440
Ref: https://ci.appveyor.com/project/libssh2org/libssh2/builds/46983040/job/c3vag25c26a77lyr#L485

Cherry-picked from libssh2#1037
Closes libssh2#1037
Before this patch, the CMake build did not allow to disable static
libssh2 library while also building tests.

This patch removes this constraint, and makes this combination possible.
In this case the 3 (at the moment) tests that require a static libssh2
library, are skipped from the build and test runs.

Cherry-picked from libssh2#1036
This build combination did not have a CI test before.

Cherry-picked from libssh2#1036
`DYN=1` means to build examples/tests against the shared libssh2.

Before this patch this was broken for building tests. This patch skips
building tests that require the static libssh2 library, so the build now
succeeds.

Also move the list of tests that require static lib from
`CMakeLists.txt` to `Makefile.inc`, so that we can reuse it in
`Makefile.mk`.

Couldn't find a way to also reuse it in `Makefile.am`. Move the
`Makefile.am` specific definitions close to the shared list, to make it
easier to keep them synced.

Cherry-picked from libssh2#1036
Allow resolving the import/static library name collision also by setting
a custom _import_ library name suffix.

Follow-up to 4e25806

Cherry-picked from libssh2#1036
The collision issue affects (typically) MSVC, when building both shared
and static libssh2 in one go.

Ref: https://stackoverflow.com/questions/2140129/what-is-proper-naming-convention-for-msvc-dlls-static-libraries-and-import-libr

Initially we handled this by appending the `_imp` suffix to the import
library filename. This is how curl tackles this, but on a second look,
this solution seem to be accidental and has no widespread use.

It seems more widely accepted to use the '_static' suffix for the static
library. This patch implements this.

(MinGW, Cygwin and unixy platforms are not affected by this issue.)

Follow-up to 4e25806

Cherry-picked from libssh2#1036
This brings default behaviour in sync with autotools, which builds both
lib flavours by default.

(Notice that on Windows, autotools includes the Windows Resource in the
static library, when building both at the same time. CMake doesn't have
this issue.)

Enabling both lib flavours has a side-effect when using non-MinGW
toolchains (e.g. MSVC): to resolve the filename conflict between import
and static libraries, we add a suffix to the static lib, naming it
`libssh2_static.lib`. This can break dependent builds relying on
`libssh2.lib` for linking the static libssh2.

Workarounds:

- disable either shared or static libssh2 via
  `-DBUILD_STATIC_LIBS=OFF` or
  `-DBUILD_SHARED_LIBS=OFF`. This results in a libssh2 library (either
  static or shared) without a prefix: `libssh2.lib`.

- set a custom static library suffix via:
  `-DSTATIC_LIB_SUFFIX=_my_static`. Resulting in
  `libssh2_my_static.lib`, and import library
  `libssh2.lib`.

- set a custom import library suffix via:
  `-DIMPORT_LIB_SUFFIX=_my_implib`. Resulting in
  `libssh2_my_implib.lib` import library, and static library
  `libssh2.lib`.

- customize the default static/import library suffix (incl. extension)
  via
  `-DCMAKE_STATIC_LIBRARY_SUFFIX=_my_static_suffix.lib` or
  `-DCMAKE_IMPORT_LIBRARY_SUFFIX=_my_import_suffix.lib`.

Cherry-picked from libssh2#1036
Both autotools and cmake build both shared and static lib by default.

Ref: 896154b

Delete configuration enabling these explicitly in CI jobs.

Cherry-picked from libssh2#1036
Closes libssh2#1036
Also update two labels to match the rest of the source.

checksrc update credit: Emanuele Torre @emanuele6

Ref: curl/curl#11134

Closes libssh2#1042
`list(PREPEND)` requires CMake v3.15, our minimum is v3.1. `APPEND`
should work fine for headers anyway.

Also fix a wrongly placed comment.

Ref: https://cmake.org/cmake/help/latest/command/list.html#prepend

Regression from 1e3319a167d2f32d295603167486e9e88af9bb4e

Closes libssh2#1043
- add `LIBSSH2_NO_ED25519` build-time option to force-disable ED25519
  support. Useful to replicate crypto-backend builds without ED25519,
  such as wolfSSL.

- openssl: fix unused variable and function warnings with all supported
  `LIBSSH2_NO_*` options enabled.

- mbedtls: fix misplaced `#endif` leaving out the required internal
  public function `libssh2_supported_key_sign_algorithms()`.

- mbedtls: add missing prototype for two internal public functions.

- delete a redundant block.

All `NO` options:
```shell
CPPFLAGS='
-DLIBSSH2_NO_MD5 -DLIBSSH2_NO_HMAC_RIPEMD -DLIBSSH2_NO_DSA
-DLIBSSH2_NO_RSA -DLIBSSH2_NO_RSA_SHA1
-DLIBSSH2_NO_ECDSA -DLIBSSH2_NO_ED25519 -DLIBSSH2_NO_AES_CTR
-DLIBSSH2_NO_BLOWFISH -DLIBSSH2_NO_RC4 -DLIBSSH2_NO_CAST
-DLIBSSH2_NO_3DES'
```

Closes libssh2#1044
Exclude wolfSSL builds from tests. All fail:

```
2/43 Test  #2: test_aa_warmup ............................***Failed    5.59 sec
libssh2_session_handshake failed (-44): Unable to ask for ssh-userauth service
```
Ref: https://github.com/libssh2/libssh2/actions/runs/5085775952/jobs/9139583212#step:12:942 (with logging)
Ref: https://github.com/libssh2/libssh2/actions/runs/5085586301/jobs/9139192562#step:12:225

wolfSSL version:
```
Get:1 http://azure.archive.ubuntu.com/ubuntu jammy/universe amd64 libwolfssl32 amd64 5.2.0-2 [818 kB]
Get:2 http://azure.archive.ubuntu.com/ubuntu jammy/universe amd64 libwolfssl-dev amd64 5.2.0-2 [1194 kB]
```

Cherry-picked from libssh2#1046
Closes libssh2#1046
* Libssh2 1.11 release notes, copyright
1.11.0

# -----BEGIN PGP SIGNATURE-----
#
# iQFDBAABCgAtFiEEJ+3q8i86vOtQ25oSXMkI/bceEsIFAmR2HTsPHGRhbmllbEBo
# YXh4LnNlAAoJEFzJCP23HhLCnfQH+gJf/M2QO2K68DF3pr/9mUQqDfNeBVspYPYf
# thFTky6NvgIUMhX6Uz5EjKHt6Sn3bap+Dr6RZXveiaCUpz3Qh739giu6/zGZD95l
# l/OWBl+ytaw6VNkPeVfu/B3Qm5gGKdWa91wCtjW4ZDh126nGeHJfVy1lQOl7S1Wc
# hp3lC5F024IZ6/JBjC44cXVcrU7owgdIx8pAi1xohQmm152IsgBTFih4gMTh7pat
# Py+MK2Wu19+i2fuW1+w9un9LIcHwmmbwf6Qb6z+cfdx2yFvFb3ECMZ7RULdJlVFT
# BQcpFGLcCkETjjheM3uH4iAhHe4GEmKfkS/v6gDMN6Km/e+F+1I=
# =bDOR
# -----END PGP SIGNATURE-----
# gpg: Signature made Tue May 30 17:58:51 2023
# gpg:                using RSA key 27EDEAF22F3ABCEB50DB9A125CC908FDB71E12C2
# gpg:                issuer "[email protected]"
# gpg: Can't check signature: No public key

# Conflicts:
#	.gitignore
#	src/CMakeLists.txt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.