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

runtests: fix test key format for libssh2 WinCNG (and others) #16781

Closed
wants to merge 11 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Mar 21, 2025

SFTP/SCP tests were failing in CI with WinCNG libssh2 since we first
added such job. With curl: (67) Authentication failure.

The reason is that the default ssh-keygen RSA private key format
changed to OpenSSH (RFC4716) in 2018. libssh2 does not support this
format with some of its crypto backends.

Fix it by generating keys explicitly in PEM format as necessary via
the -m option. This format is universally recognized for RSA keys.

2018-08-24: https://www.openssh.com/txt/release-7.8: OpenSSH format becomes default
2010-08-23: https://www.openssh.com/txt/release-5.6: -m option first supported

This fixed the auth issue, just to reveal a known flakiness issue in
libssh2 + WinCNG, causing:

curl: (2) Failure establishing ssh session: -8, Unable to exchange encryption keys

Ref: https://github.com/curl/curl/actions/runs/14000494428/job/39205633258?pr=16781#step:15:1796
Tracked here: libssh2/libssh2#804
Mitigated in libssh2 tests by retrying them.

Due to this, keep ignoring these test results.

Also:

  • add an env to customize key format: CURL_TEST_SSH_KEY_FORMAT
  • display the generated format in the log.
  • GHA/linux: document the wolfSSH error code causing it to fail tests:
    curl: (79) wolfssh SFTP connect error -1051 / WS_MATCH_KEY_ALGO_E / cannot match key algo with peer
    

Follow-up to 4911e7a #16735
Follow-up to 0ec72c1 #16672
Follow-up to e53523f #14859
Follow-up to e26cbe2 #13979

@vszakats vszakats marked this pull request as draft March 21, 2025 10:49
@github-actions github-actions bot added tests CI Continuous Integration labels Mar 21, 2025
@vszakats
Copy link
Member Author

vszakats commented Mar 21, 2025

Confirmed that the SSH auth failures also happen with the MSYS2 libssh2-wincng package, not just with vcpkg's libssh2[core]. Therefore pointing to a libssh2 WinCNG backend issue.

https://github.com/curl/curl/actions/runs/13990239834/job/39172667787
https://github.com/curl/curl/actions/runs/13990239834/job/39172668063

@testclutch

This comment was marked as outdated.

@vszakats
Copy link
Member Author

Ok, the solution is clear.

@vszakats vszakats changed the title [TEST] libssh2 wincng runtests: fix test key format for libssh2 WinCNG (and more) Mar 21, 2025
@vszakats
Copy link
Member Author

vszakats commented Mar 21, 2025

The reason for the failing libssh2 WinCNG tests was that the default ssh-keygen private key format is incompatible with some libssh2 crypto backends, for example WinCNG. In fact it only supports it with OpenSSL (& fork) and wolfSSL. And not with libgcrypt, mbedTLS and WinCNG, though I haven't retested the other two.

https://github.com/libssh2/libssh2/blob/6746b789704cbadbefbf4488522399bb9726fabe/tests/test_auth_pubkey_ok_rsa_openssh.c#L10-L11

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats
@vszakats
Copy link
Member Author

vszakats commented Mar 21, 2025

All this effort, just to discover that with that fixed, the tests are flaky now
because of a known libssh2 + WinCNG issue, which we also spent a lot
of time on, without success.

If you have an insight on why this happens and how to fix it, let us know.

Ref: libssh2/libssh2#804

@vszakats vszakats marked this pull request as ready for review March 21, 2025 22:01

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats
@vszakats vszakats changed the title runtests: fix test key format for libssh2 WinCNG (and more) runtests: fix test key format for libssh2 WinCNG (and others) Mar 21, 2025

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats
@vszakats vszakats closed this in 89f306a Mar 23, 2025
@vszakats vszakats deleted the libssh2-wincng branch March 23, 2025 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration SCP/SFTP tests
Development

Successfully merging this pull request may close these issues.

None yet

2 participants