(PE-36344) Disable curl's ntlm if OpenSSL excludes it #702
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Locally: |
Here is what your code changes would affect:Project
|
|
Oooooo, thank you @joshcooper ! I didn't intend to change curl for 7.x runtimes.... |
|
I will see if I can update this to conditionally disable ntlm if |
joshcooper
reviewed
Jul 18, 2023
OpenSSL 3.0 by default disables some legacy algorithms that are required for ntlm. We previously kept them enabled for Bolt's WinRM transport but in b85bf0f made the inclusion of these legacy algorithms optional so that the agent and company no longer have to include insecure crypto on account of Bolt. However, removing the NTLM algorithms from OpenSSL caused downstream failures in the compilation of curl, which this patch addresses by disabling curl's support for NTLM.
justinstoller
force-pushed
the
disable-ntlm-in-curl
branch
from
50a42ab
to
ab2de7b
Compare
joshcooper
approved these changes
Jul 18, 2023
|
|
Weird, the result I pasted above is from the invocation that Josh gave me in the previous PR so it includes those openssl changes as well. And the pdk-runtime only seems to have those openssl changes... I ran the command again with the correct invocation for just this PR and it only showed changes for agent-runtime-main. BUT, since pdk-runtime changed in the last PR I expected it to change here as well. I'm currently building that locally. |
|
There's a bug in the component_diff command when using markdown, as it doesn't put a newline between |
|
Thank you |
joshcooper
approved these changes
Jul 18, 2023
|
Okay, well pdk-runtime builds just fine so I guess my concerns were unfounded. |
|
Since this was only known to fail in the agent runtime builds, it is the only changed project, and we got an approval from phoenix I'm not going to wait for other reviews to merge. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
OpenSSL 3.0 by default disables some legacy algorithms that are required for ntlm. We previously kept them enabled for Bolt's WinRM transport but in b85bf0f made the inclusion of these legacy algorithms optional so that the agent and company no longer have to include insecure crypto on account of Bolt.
However, removing the NTLM algorithms from OpenSSL caused downstream failures in the compilation of curl, which this patch addresses by disabling curl's support for NTLM.