Skip to content

Set OPENSSL_ENABLE_SHA1_SIGNATURES=1 in SendToHelix.proj #5809

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

akoeplinger
Copy link
Member

Instead of hardcoding this in the Helix image, see dotnet/dotnet-buildtools-prereqs-docker#1391

@akoeplinger akoeplinger marked this pull request as ready for review July 14, 2025 16:19
@akoeplinger akoeplinger requested a review from mconnew July 14, 2025 16:33
@mconnew
Copy link
Member

mconnew commented Jul 14, 2025

Test failure looks to have just been a test infrastructure blip and nothing to do with the tests themselves.

While I have no problem with this being in our pipeline setup, I am slightly concerned about there being zero need for this in the dotnet/runtime side. I fully understand that we shouldn't choose to use SHA1 for hashing, but we absolutely should be having tests to make sure validation of things like xml dsig signatures correctly validate (which is the consumption side of SHA1, not the generation side). Sometimes specs outside of our control require support for SHA1 and we should validate those scenarios (which is why WCF has a need for this). Also SHA1 isn't considered broken when using in an HMAC, only when used as a raw hashing function so there are still some legitimate (although pre-emptively discouraged) uses for SHA1.

@akoeplinger
Copy link
Member Author

akoeplinger commented Jul 14, 2025

Tests have been adjusted on dotnet/runtime in these environments, e.g. dotnet/runtime#70821.

If it only affects a couple specific tests maybe something similar would work for WCF instead of the env variable?

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.

2 participants