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

wolfSSL: libssh2_session_handshake failed: -12/-44 DECRYPT/ENCRYPT #1020

Closed
vszakats opened this issue May 2, 2023 · 12 comments
Closed

wolfSSL: libssh2_session_handshake failed: -12/-44 DECRYPT/ENCRYPT #1020

vszakats opened this issue May 2, 2023 · 12 comments
Labels

Comments

@vszakats
Copy link
Member

vszakats commented May 2, 2023

Describe the bug
When running tests/test_ssh2, either of these errors happen:

This is the first packet with encryption.

To Reproduce
Run tests/test_ssh2 against an sshd server.

Expected behavior
Same result as with OpenSSL/LibreSSL backends:
https://github.com/libssh2/libssh2/actions/runs/4847366472/jobs/8637553403#step:7:72
https://github.com/libssh2/libssh2/actions/runs/4847366472/jobs/8637553480#step:7:72
https://github.com/libssh2/libssh2/actions/runs/4851558817/jobs/8645523936#step:7:71

Version:

  • OS and version: macOS (possibly others)
  • libssh2 version: 1.10.1-DEV
  • crypto backend: wolfSSL v5.6.0
@vszakats vszakats changed the title wolfSSL: libssh2_session_handshake failed -12 and -44 wolfSSL: libssh2_session_handshake failed: -12 and -44 May 2, 2023
@vszakats vszakats changed the title wolfSSL: libssh2_session_handshake failed: -12 and -44 wolfSSL: libssh2_session_handshake failed: -12/-44 DECRYPT/ENCRYPT May 2, 2023
@vszakats
Copy link
Member Author

vszakats commented May 2, 2023

Reverting the AES-GCM patch #797 / 3c953c0 fixes it.

The issue might be inside openssl.c / _libssh2_cipher_crypt(). Possibly with the interpretation of the return value of EVP_Cipher(), or EVP_CIPHER_CTX_ctrl().

@dfandrich Can you take a look?

The issue happens with both AES-GCM disabled for wolfSSL (as in master) or enabled.

UPDATE: Now enabled in master: df513c0.

@vszakats vszakats added the bug label May 2, 2023
vszakats added a commit that referenced this issue May 3, 2023
Follow-up to 3c953c0 #797

There is pending issue with wolfSSL, where encryption/decryption is not
working (both with and without this patch). Ref: #1020

Cherry-picked from #1017
@dfandrich
Copy link
Contributor

I'll take a look.

@vszakats vszakats pinned this issue May 9, 2023
@migueldeicaza
Copy link
Contributor

migueldeicaza commented May 9, 2023

I also have one user getting this error with OpenSSL, the trace looks like this:

https://gist.github.com/migueldeicaza/da684d07f91716ddefafe6ef298c3d64

@migueldeicaza
Copy link
Contributor

Update: reverting locally the patches for 0048f30 and 3c953c0 did not solve the problem.

Here is one additional data point: this only happens on an M1 Mac connecting to an M1 Mac, or a CentOS machine. It does not happen from an Intel Mac to those systems, and does not happen with iPhone/iPad on device or simulator.

@migueldeicaza
Copy link
Contributor

In case someone else runs into this: the issue on the M1 Mac was not related to the AES-GCM patch, that's why it did not make a difference. For some reason the version of OpenSSL I was using on Mac on ARM did not include the LIBSSH2_EC_CURVE_NISTP256 (NID_X9_62_prime256v1) curve, so the call to _libssh2_ecsa_curve_name_with_octal_new was failing which in turn made curve25519_sha256 fail.

Mind you, this code was not being triggered with the currently public version of libssh2

@vszakats vszakats unpinned this issue May 24, 2023
@kareem-wolfssl
Copy link

Hi @vszakats ,

I was able to reproduce the failures you're seeing here. Some failures are being caused by our AES-GCM streaming support not being enabled, you can enable this support while building wolfSSL by passing --enable-aesgcm-stream to ./configure. I will add this to --enable-libssh2 once I've confirmed it's working as expected.
I am still working on debugging the remaining test failures.

I would strongly recommend you manually build wolfSSL with --enable-libssh2 --enable-aesgcm-stream for your tests. The test appears to be using the Brew package of wolfSSL, which does not enable DES3, DSA or AES-CTR: https://github.com/Homebrew/homebrew-core/blob/7fd778f54e75e1f9f85f41486094a7182225d839/Formula/wolfssl.rb These algorithms are enabled by --enable-libssh2, and if they aren't enabled they will cause additional test failures.

vszakats added a commit to curl/curl-for-win that referenced this issue May 25, 2023
TODO to enable it once supported also in CMake.

Ref: libssh2/libssh2#1020 (comment)
vszakats added a commit to vszakats/libssh2 that referenced this issue May 25, 2023
@vszakats
Copy link
Member Author

vszakats commented May 25, 2023

@kareem-wolfssl, thanks for jumping in!

Opened a PR to detect AES-GCM via WOLFSSL_AESGCM_STREAM (instead of HAVE_AESGCM): #1045
Does this look correct to you?

I think it'd be useful for most users if the Homebrew package came enabled with everything necessary for AES-GCM. At the moment it lists --enable-aesgcm twice, perhaps one of those was to meant to enable stream support? (as for DSA and DES3, I think the they are OK as disabled by default, libssh2 will adapt.)

Building wolfSSL from source in CI requires constant maintenance (bumping on each upgrade). Also makes CI builds much slower and complex. I'd like to avoid this and rely on package defaults. This is also what users will encounter on this platform by default.

This particular issue is also present with AES-GCM disabled, so keeping it open.

@kareem-wolfssl
Copy link

Happy to help!

It is correct, but to be safe I'd recommend checking both. I left a comment in the review.

wolfSSL does not control any distributions' packages, you will have to check with the Homebrew project on modifying their wolfSSL package.

You are correct that with the way libssh2 is set up, your other tests should still pass while skipping DSA/DES3, but this isn't ideal as it isn't giving full test coverage. In general, we expect you to be using --enable-libssh2 when building wolfSSL against libssh2, and it is possible we may add more features under this flag in the future, like AES-GCM streaming. We expect users to do this as well.
Building wolfSSL is really just pulling our current latest release, ./configure it with --enable-libssh2, make and make install. It should safe to have this automatically use our latest release, and you are welcome to open a bug upstream if this ever fails after a new release from us.

However, if you aren't able to do this I understand. We aim to automatically test all of our open source integrations to catch issues like this. I will see if we currently have a libssh2 test and if not, look into setting one up.

@kareem-wolfssl
Copy link

kareem-wolfssl commented May 25, 2023

I have found another issue, wolfSSL does not like this call in openssl.c: ret = EVP_Cipher(*ctx, NULL, NULL, 0);
We don't support NULL src or dest and just return BAD_FUNC_ARG. Looks like we need to call EVP_CipherFinal instead here for wolfSSL, I am working on determining the right way to do that.

@vszakats
Copy link
Member Author

vszakats commented May 25, 2023

In theory easy, but in practice every sub-build is a source of issues, maintenance, error cases and flakyness. Starting from how to extract the current latest version. In Homebrew, It's a one-time effort with wider benefits – seems more practical.

I might give it a try adding wolfSSL Linux builds, though it will be an older version. [edit: added in 801aebc]

DES3/DSA is tested with other crypto-backends, which seems enough for libssh2 purposes.

Very off topic: I'd be fantastic to have wolfSSL's ed25519 support exposed via its OpenSSL compatibility interface. That way we could enable it with libssh2.

vszakats added a commit that referenced this issue May 25, 2023
@kareem-wolfssl
Copy link

Understood. I will still look into getting this added to our tests, to hopefully avoid situations like this in the future.

Looking at the libssh2 code, it looks like you just need ED25519 supported in the EVP APIs, as well as a couple of EVP APIs we don't currently support like EVP_PKEY_new_raw_public_key. We should be able to add this. I will log a feature request for this.

@vszakats
Copy link
Member Author

vszakats commented May 26, 2023

@kareem-wolfssl This is great news, thanks for that!

Meanwhile, I've added (801aebc) Linux builds with wolfSSL to our CI. Version v5.2.0. This platform has the ability to run our dockerized runtime tests. All of them fail, so for now I've disabled them. They can be enabled (incl. logging) for testing with this patch:

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index b42ae5af..3eddc7cf 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -57,6 +57,9 @@ jobs:
           [ "${{ matrix.crypto_backend }}" == 'wolfSSL' ] && pkg='libwolfssl-dev'
           [ "${{ matrix.crypto_backend }}" == 'Libgcrypt' ] && pkg='libgcrypt-dev'
           sudo apt-get install -y "${pkg}"
+          if [ "${{ matrix.crypto_backend }}" == 'wolfSSL' ]; then
+            echo 'TOOLCHAIN_OPTION=-DENABLE_DEBUG_LOGGING=ON' >> $GITHUB_ENV
+          fi
 
       - name: 'install mbedTLS from source'
         if: ${{ matrix.crypto_backend == 'mbedTLS' }}
@@ -100,9 +103,10 @@ jobs:
         if: ${{ matrix.build == 'cmake' }}
         run: cmake --build bld --parallel 3 --target package
       - name: 'cmake tests'
-        if: ${{ matrix.build == 'cmake' && matrix.crypto_backend != 'wolfSSL' }}
+        if: ${{ matrix.build == 'cmake' }}
         timeout-minutes: 10
         run: |
+          export FIXTURE_TRACE_ALL_CONNECT=1
           export OPENSSH_SERVER_IMAGE=ghcr.io/libssh2/ci_tests_openssh_server:$(git rev-parse --short=20 HEAD:tests/openssh_server)
           cd bld && ctest -VV --output-on-failure

@vszakats vszakats pinned this issue May 28, 2023
@vszakats vszakats unpinned this issue Jun 21, 2023
lampmanyao pushed a commit to lampmanyao/libssh2 that referenced this issue Jul 16, 2023
@vszakats vszakats closed this as not planned Won't fix, can't repro, duplicate, stale Jul 19, 2023
vszakats added a commit to curl/curl-for-win that referenced this issue Jul 26, 2023
It remains difficult to figure out which features are enabled exactly?
do we still need --enable-curl? does it include H3 features? and what
else is needed for libssh2?

Also latest libssh2 1.11.0 is broken [0] with wolfSSL, so this is all
theoretical at this point.

[0] libssh2/libssh2#1020
@vszakats vszakats reopened this Dec 26, 2023
@vszakats vszakats closed this as completed Apr 2, 2024
vszakats added a commit to curl/curl-for-win that referenced this issue Apr 3, 2024
remove support for:

- gsasl: because it requires autotools.

- libidn2: because it requires autotools.

- wolfssl: because its cmake support is lagging far behind autotools and
  not getting features/updates. Its autotools is complicated to tune for
  curl/libssh2.
  wolfssl support in libssh2 has been broken for a year with not much
  interest or any fix in sight.
    libssh2/libssh2#1020
  wolfssl-based CI tests are kept disabled to make libssh2 builds pass.

- wolfssh: because it requires autotools and wolfssl.

- autotools build method for: curl, libssh2, libressl.
  curl's CMake support is now first class (with few PRs pending).
  libssh2 got its CMake overhauled and on par with autotools.
  Both support unity builds. LibreSSL already had pretty good CMake
  support and with some kinks fixed recently, it's now working without
  any issue.
  Also, maintaining multiple parallel build methods is a major PITA and
  time sink in the long run.

- `CW_DEV_CROSSMAKE_REPRO` dev/debug build option.
  I used this to sync up cmake/autotools/gnumake builds in the past few
  years. This process was completed with success, allowing to drop
  gnumake completely and making autotools redundant. From time to time
  it's useful to compare builds made with different build tools, but the
  benefits aren't high enough to justify maintaining autotools as
  a build tool here just for that.

Autotools has been broken for certain configs in curl-for-win since
autumn, after introducing Linux MUSL builds. After many weeks of trying,
fixing it seems impossible. Possibly because of libtool. Besides this
specific issue, autotools turned out to be inflexible, slow,
unnecessarily complex, buggy, opaque, with practically unreadable
source code, also difficult to edit, and with no clear "best practices"
to follow.

Autotools support also seems to be coming historically with external
runnable code bundled into source tarballs, making reproducibility
difficult, and sneaking in backdoors easy. See CVE-2024-3094.

Autotools is also slow even compared to CMake. It doesn't support
single-pass builds for shared/static libs, and has other limitations
which appear historical and without any hope/desire to ever change.
Windows support is also pretty much accidental, and by being based on
arcane not-even-POSIX shell/utilities, it's not natively supporting it
anyway and never will.

This also means that curl-for-win builds will not attempt to support
dependencies that require autotools. But, this also means that this may
give way for supporting new build tools in the future.

Closes #69
@vszakats vszakats reopened this May 24, 2024
vszakats added a commit to vszakats/libssh2 that referenced this issue Jun 21, 2024
wolfSSL 5.7.0 fixes a bug that affects libssh2#1020 and libssh2#1299.

Closes #xxxx
vszakats added a commit to vszakats/libssh2 that referenced this issue Jun 21, 2024
wolfSSL 5.7.0 fixes this bug that affects libssh2#1020 and libssh2#1299:
wolfSSL/wolfssl#7143

`-DWOLFSSL_OPENSSLALL=ON` necessary for `wolfSSL_FIPS_mode()`

Closes #xxxx
vszakats added a commit that referenced this issue Jun 21, 2024
After this patch it's possible to run tests with wolfSSL 5.7.0.

wolfSSL 5.7.0 fixes this bug that affects open issues #1020 and #1299:
wolfSSL/wolfssl#7143

`-DWOLFSSL_OPENSSLALL=ON` is necessary for `wolfSSL_FIPS_mode()`

Closes #1408
vszakats added a commit to vszakats/libssh2 that referenced this issue Jun 24, 2024
Also re-enable tests in wolfSSL Linux CI jobs. That wolfSSL is v5.2.0,
which is broken with AES-GCM enabled.

This patch is part of a series of fixes to make wolfSSL AES-GCM support
work together with libssh2.

Ref: libssh2#1020
Ref: libssh2#1299
Cherry-picked from libssh2#1407
Closes libssh2#1411
vszakats added a commit to vszakats/libssh2 that referenced this issue Jun 24, 2024
Also re-enable tests in wolfSSL Linux CI jobs. That wolfSSL is v5.2.0,
which is broken with AES-GCM enabled.

This patch is part of a series of fixes to make wolfSSL AES-GCM support
work together with libssh2.

Ref: libssh2#1020
Ref: libssh2#1299
Cherry-picked from libssh2#1407
Closes libssh2#1411
vszakats added a commit that referenced this issue Jun 24, 2024
Earlier versions crash while running tests.

This patch is part of a series of fixes to make wolfSSL AES-GCM support
work together with libssh2.

Possibly related is this wolfSSL bugfix patch, released in v5.4.0:
wolfSSL/wolfssl#5205
wolfSSL/wolfssl@fb3c611
"Fix another AES-GCM EVP control command issue"

Ref: #1020
Ref: #1299
Cherry-picked from #1407
Closes #1411
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants