-
Notifications
You must be signed in to change notification settings - Fork 88
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
(PA-6962) update curl to address CVE-2024-8096 #921
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the context for adding in
--without-libpsl
? Is this required for every platform we're building on? Does the Curl 7.88.1 package support building with that flag?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while building, curl 8.10.1
libpsl
was expected to be installed which was never the case for prior versions. One was was to addlibpsl
was to either install it as a package while building or bypass using--without-libpsl
.I checked with the flag and there were no failures or dependency as such for puppet while building.
I checked building for
curl-7.88.1
with--without-libpsl
flag and the build was successful. As earlier also there were no such dependency hence it passed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amitkarsale we went over this PR in standup and have some suggestions.
Did a bit of research, there is a good article by the developer of Curl on PSL and their use in Curl: https://daniel.haxx.se/blog/2024/01/10/psl-in-curl/
The short of this is PSL support was added a bit ago in the 8.x stream of Curl, previous it used to just warn and looks like in the recent past it started to fail the configure action if the required libraries were not found.
Reading the blog post I think short term it really doesn't change anything to include the flag as is, but we should for sure ticket and make this a known issue.
So if you could make a small change to this PR, pull out the
--without-libpsl
from the commit with the Curl bump and make that its own commit with its own message.Then would you mind filing a Jira ticket to cover the work of adding libpsl support to puppet-runtime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've created a new ticket to address the libpsl installation. Maybe post release I can take it to unblock the release.