-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Update to Go 1.18, drop older Go version support, and update modules and dependencies #4963
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
Conversation
|
Ah, I see the |
|
I'll take a further look at this later, but I suspect we may have to go the route proposed in #4903 and just get rid of vendored Go modules entirely. UPDATE: I actually don't think this would resolve the incompatibility; it would just make it slightly easier to update modules. Sigh. |
Update the vendored golang.org/x/crypto and golang.org/x/text
modules to the latest versions, which in turn updates the vendored
copy of the golang.org/x/net and golang.org/x/sys modules.
Updating these modules' entries in vendor/modules.txt and go.{mod,sum}
means we will not be flagged by security scanners regarding either
CVE-2021-38561 or CVE-2022-27191, neither of which should actually
affect Git LFS.
The Git LFS client should not be affected by CVE-2021-38561 as it
pertains the Go x/text/language package and specifically the BCP 47
tag functions, which Git LFS does not use.
The Git LFS client should not be affected by CVE-2022-27191 as it
pertains to the Go x/crypto/ssh package and specifically a crash
vulnerability in the SSH server functions, which Git LFS does not use.
The specific commands run to perform this update were:
go get golang.org/x/crypto@latest &&
go get golang.org/x/text@latest &&
go mod tidy && go mod vendor
c2922a8 to
c175fdb
Compare
|
On taking a closer look, I don't see a way around the incompatibility of the newer Dropping our vendored modules would not help, as I first thought, since we'd still specify an Specifically, per the Go language announcement, we need version That version of At least, that's as far as I've gotten today; I hope that's all correct. (One other data point is that Go only supports two major releases at a time, so anything older than 1.17 is unsupported.) Footnotes |
|
I think in that case, we may need to just drop support for versions older than 1.17. I don't think we have much of choice, to be honest. |
That's fine with me; my only question would be whether there were specific reasons to continue supporting older versions (e.g., building in older Linux distributions rather than using a pre-compiled binary?) I also note that the same announcement mentions that version
I don't know if the new pure SSH transfer adaptor, for instance, would be affected by that. |
Yeah, we try to make things a little easier on older distros when possible. In this case, I don't think it's possible, though.
Nope, we use OpenSSH regardless. |
Yeah, that makes sense, especially since I just noted that we don't vendor the |
As our official policy is to support the latest version of Go, we upgrade our CI and release workflows to version 1.18, which was recently released. We also drop support for versions older than 1.17 as they are not supported by upstream Go, and because the mitigations for CVE-2022-27191 are only available in Go 1.17 and later. Specifically, the updates to the x/crypto/ssh package (which Git LFS does not use, so this security issue does not strictly affect Git LFS) from version v0.0.0-20220314234659-1baeb1ce4c0b depend on changes in the x/net package which are not available in any versions of that package prior to v0.0.0-20211112202133-69e39bad7dc2. See for reference: https://groups.google.com/g/golang-announce/c/-cp44ypCT5s
After upgrading to Go version 1.18 the "go get" command no longer builds or installs packages by default; it only manages dependencies in go.mod and go.sum. We therefore switch to using "go install" to build and install the goimports and goversioninfo. We then also no longer need to reset our working tree because "go install" does not modify go.mod or go.sum.
x/crypto and x/text Go modules and dependenciesCorrect a typo from commit 1bd8630 in this PR and use "go install" when building goversioninfo in our release workflow. h/t bk2204 on PR review
As our official policy is to support the latest version of Go, we upgrade our CI and release workflows to version 1.18, which was recently released.
We also drop support for versions older than 1.17 as they are not supported by upstream Go, and because the mitigations for CVE-2022-27191 are only available in Go 1.17 and later; see below for details.
After upgrading to Go version 1.18 the
go getcommand no longer builds or installs packages by default; it only manages dependencies ingo.modandgo.sum. We therefore switch to usinggo installto build and installgoimportsandgoversioninfo. We then also no longer need to reset our working tree becausego installdoes not modifygo.modorgo.sum.We update the vendored
golang.org/x/cryptoandgolang.org/x/textmodules to the latest versions, which in turn updates the vendored copy of thegolang.org/x/netandgolang.org/x/sysmodules.Updating these modules' entries in
vendor/modules.txtandgo.{mod,sum}means we will not be flagged by security scanners regarding either CVE-2021-38561 or CVE-2022-27191, neither of which should actually affect Git LFS.The Git LFS client should not be affected by CVE-2021-38561 as it pertains the Go
x/text/languagepackage and specifically the BCP 47 tag functions, which Git LFS does not use.The Git LFS client should not be affected by CVE-2022-27191 as it pertains to the Go
x/crypto/sshpackage and specifically a crash vulnerability in the SSH server functions, which Git LFS does not use.The specific commands run to perform this update were:
(As an aside, this is another example of why PR #4903 is a nice idea!)
Per the Go language announcement, we need version
v0.0.0-20220314234659-1baeb1ce4c0bor higher ofgolang.org/x/cryptoto include the fixes for CVE-2022-27191 (which, again, we don't technically require as we don't use thex/crypto/sshpackage). That version ofgolang.org/x/crypto[depends] (https://github.com/golang/crypto/blob/1baeb1ce4c0b006eff0f294c47cb7617598dfb3d/go.mod#L6) ongolang.org/x/[email protected], which includes the backwards-compatibility breaking change from golang/net@540bb53 that introduces the use of theos.ErrDeadlineExceededwhile addressing golang/go#48830./cc @ChriFo as reporter in #4961.