-
Notifications
You must be signed in to change notification settings - Fork 462
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
upgrate go to 1.20.7 and remove hardcoded crypto version #599
Conversation
// if coredns starts using >= v0.14.0 this pinned version can be removed | ||
github.com/apache/thrift => github.com/apache/thrift v0.14.0 | ||
|
||
// pinned latest version for vulnerability fixes, upgrade if there are newer versions | ||
golang.org/x/crypto => golang.org/x/crypto v0.1.0 |
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.
This is a good cleanup!
Just to ensure -- did you verify:
- This builds correctly.
golang.org/x/crypto
is not used at all or the version >= v0.1.0 is used? Not to regress the Upgraded to the latest golang crypto package for vuln fixes #552
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.
- yes
- From what I understand if makefile has 1.20.7 version hardcoded then this version of crypto library will be used. DIMS?
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.
My expectation, although the golang version that used to build the binary is updated, the packages version will be used from go.sum.
So I'd expect with these PR changes, the binary would not contain golang.org/x/crypto
at all.
If that's fine, could you run go version -m
? We'd ensure that the dependencies are correct and merge the PR.
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.
verified, there is no dependency on crypto library
/ok-to-test |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dpasiukevich, kl52752 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
fixes for Vulnerabilities: CVE-2023-29409, CVE-2023-39533, CVE-2023-29406