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

Jump to bottom

Disable msys2 path interpretation in Cirrus CI #8084

Closed
jay opened this issue Dec 2, 2021 · 3 comments
Closed

Disable msys2 path interpretation in Cirrus CI #8084

jay opened this issue Dec 2, 2021 · 3 comments
Assignees
@mback2k
Labels
CI Continuous Integration Windows Windows-specific

Comments

@jay
Copy link
Member

jay commented Dec 2, 2021 •
edited

I did this

This test failure from Cirrus CI shows test3021 failing:

curl: (60) Denied establishing ssh session: mismatch sha256 fingerprint. Remote ��z�i/XEa"��P�R��h�]���wl�q���)� is not equal to C:/msys64/Ad6smkvWEVhIs/nUM1S2fOjaL9djxb6d2zkcbQUgik

The error message is supposed to show two sha256 fingerprints in base64 format. The gibberish came from a printing mistake and I landed 3467e89 to fix it. The reason the match failed was not because of that but because one of the keys is C:/msys64/Ad6smkvWEVhIs/nUM1S2fOjaL9djxb6d2zkcbQUgik, note the erroneous C:/msys64/. There should be just a base64 string there.

Tracing it back I don't see anything unusual.

curl/lib/vssh/libssh2.c

Line 623 in 9e560d1

const char *pubkey_sha256 = data->set.str[STRING_SSH_HOST_PUBLIC_KEY_SHA256];

curl/tests/data/test3021

Line 27 in 9e560d1

--hostpubsha256 %SSHSRVSHA256 --key curl_client_key --pubkey curl_client_key.pub -u %USER: sftp://%HOSTIP:%SSHPORT%SSH_PWD/log/file%TESTNUMBER.txt

curl/tests/runtests.pl

Lines 2291 to 2294 in 9e560d1

my $hstpubsha256f = "curl_host_rsa_key.pub_sha256";
if(!open(PUBSHA256FILE, "<", $hstpubsha256f) ||
(read(PUBSHA256FILE, $SSHSRVSHA256, 48) == 0) ||
!close(PUBSHA256FILE))

curl/tests/sshserver.pl

Lines 402 to 404 in 9e560d1

open(PUBSHA256FILE, ">$hstpubsha256f");
print PUBSHA256FILE sha256_base64(decode_base64($rsahostkey[1]));
close(PUBSHA256FILE);

I expected the following

My guess is this has something to do with msys2 path interpretation. Maybe the base64 string had a leading / and so msys2 converted that to C:/msys64/. There is an msys2 environment variable MSYS2_ARG_CONV_EXCL that can be set to stop msys2 from tampering with arguments. That is already done in appveyor configs, for example:

curl/appveyor.yml

Line 145 in 9e560d1

MSYS2_ARG_CONV_EXCL: "/*"

I think that should be done for Cirrus CI as well.
@jay jay added Windows Windows-specific CI Continuous Integration labels Dec 2, 2021
@MarcelRaad
Copy link
Member

MarcelRaad commented Dec 3, 2021 •
edited

I added that for the MSYS-based CMake builds because of

curl/src/CMakeLists.txt

Line 48 in 82d3340

COMMAND ${CMAKE_COMMAND} -E echo "/* built-in manual is disabled, blank function */" > tool_hugehelp.c
. If it's needed for a test, it should be set by the test itself to also work outside of our CI system. See e.g. #2765.

mback2k added a commit to mback2k/curl that referenced this issue Dec 13, 2021
@mback2k
tests/data/test302[12]: fix MSYS2 path conversion of hostpubsha256
d330017 
Fixes curl#8084
@mback2k mback2k mentioned this issue Dec 13, 2021
Closed
@jay
Copy link
Member Author

jay commented Jan 24, 2022

Unfortunately this is still an issue, but much less so. For example this test 3021 CI failure:

2022-01-23T11:16:54.9314783Z  11:16:54.265727 == Info: SSH SHA256 public key: +C:/msys64/EYG2YDzDGm6yiwepEMSuExgRRMoTi8Di1UN3kixZw
2022-01-23T11:16:54.9317248Z  11:16:54.265745 == Info: SSH SHA256 fingerprint: +/EYG2YDzDGm6yiwepEMSuExgRRMoTi8Di1UN3kixZw=
2022-01-23T11:16:54.9319293Z  11:16:54.265754 == Info: Denied establishing ssh session: mismatch sha256 fingerprint. Remote +/EYG2YDzDGm6yiwepEMSuExgRRMoTi8Di1UN3kixZw= is not equal to +C:/msys64/EYG2YDzDGm6yiwepEMSuExgRRMoTi8Di1UN3kixZw

MSYS2 porting doc says:

Setting MSYS2_ARG_CONV_EXCL=* prevents any path transformation.

Would that work as expected?

@jay jay reopened this Jan 24, 2022
@mback2k
Copy link
Member

mback2k commented Jan 24, 2022

Yes, please give it a try for those tests.

jay added a commit to jay/curl that referenced this issue Jan 25, 2022
@jay
test3021: disable all msys2 path transformation
7ef1193 
- Disable all MSYS2 path transformation in test3021 and test3022.

Prior to this change path transformation in those tests were disabled
only for arguments that start with forward slashes. However arguments
that are in base64 contain forward slashes at any position and caused
unwanted translations.

== Info: Denied establishing ssh session: mismatch sha256 fingerprint.
Remote +/EYG2YDzDGm6yiwepEMSuExgRRMoTi8Di1UN3kixZw= is not equal to
+C:/msys64/EYG2YDzDGm6yiwepEMSuExgRRMoTi8Di1UN3kixZw

In the above example an argument containing a base64 sha256 fingerprint
was passed to curl after MSYS2 translated +/ into +C:/msys64/, and then
the fingerprint didn't match what was expected.

Ref: https://www.msys2.org/wiki/Porting/

Fixes curl#8084
Closes #xxxx
jay added a commit to jay/curl that referenced this issue Jan 25, 2022
@jay
test3021: disable all msys2 path transformation
68ff9dd 
- Disable all MSYS2 path transformation in test3021 and test3022.

Prior to this change path transformation in those tests was disabled
only for arguments that start with forward slashes. However arguments
that are in base64 contain forward slashes at any position and caused
unwanted translations.

== Info: Denied establishing ssh session: mismatch sha256 fingerprint.
Remote +/EYG2YDzDGm6yiwepEMSuExgRRMoTi8Di1UN3kixZw= is not equal to
+C:/msys64/EYG2YDzDGm6yiwepEMSuExgRRMoTi8Di1UN3kixZw

In the above example an argument containing a base64 sha256 fingerprint
was passed to curl after MSYS2 translated +/ into +C:/msys64/, and then
the fingerprint didn't match what was expected.

Ref: https://www.msys2.org/wiki/Porting/

Fixes curl#8084
Closes #xxxx
@jay jay mentioned this issue Jan 25, 2022
Closed
@jay jay closed this as completed in 9b8ed6b Jan 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mback2k mback2k

Labels
CI Continuous Integration Windows Windows-specific
Milestone
No milestone
3 participants
@mback2k @jay @MarcelRaad