Skip to content
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

[release/1.6] update to go1.20.10, test go1.21.3 #9264

Merged
merged 2 commits into from Nov 9, 2023

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Oct 18, 2023


[release/1.6] update to go1.20.9, test go1.21.2

go1.20.9 (released 2023-10-05) includes one security fixes to the cmd/go package,
as well as bug fixes to the go command and the linker. See the Go 1.20.9
milestone on our issue tracker for details:

https://github.com/golang/go/issues?q=milestone%3AGo1.20.9+label%3ACherryPickApproved

full diff: golang/go@go1.20.8...go1.20.9

From the security mailing:

[security] Go 1.21.2 and Go 1.20.9 are released

Hello gophers,

We have just released Go versions 1.21.2 and 1.20.9, minor point releases.

These minor releases include 1 security fixes following the security policy:

  • cmd/go: line directives allows arbitrary execution during build

    "//line" directives can be used to bypass the restrictions on "//go:cgo_"
    directives, allowing blocked linker and compiler flags to be passed during
    compliation. This can result in unexpected execution of arbitrary code when
    running "go build". The line directive requires the absolute path of the file in
    which the directive lives, which makes exploting this issue significantly more
    complex.

    This is CVE-2023-39323 and Go issue https://go.dev/issue/63211.


[release/1.6] update to go1.20.10, test go1.21.3

go1.20.10 (released 2023-10-10) includes a security fix to the net/http package.
See the Go 1.20.10 milestone on our issue tracker for details:

https://github.com/golang/go/issues?q=milestone%3AGo1.20.10+label%3ACherryPickApproved

full diff: golang/go@go1.20.9...go1.20.10

From the security mailing:

[security] Go 1.21.3 and Go 1.20.10 are released

Hello gophers,

We have just released Go versions 1.21.3 and 1.20.10, minor point releases.

These minor releases include 1 security fixes following the security policy:

  • net/http: rapid stream resets can cause excessive work

    A malicious HTTP/2 client which rapidly creates requests and
    immediately resets them can cause excessive server resource consumption.
    While the total number of requests is bounded to the
    http2.Server.MaxConcurrentStreams setting, resetting an in-progress
    request allows the attacker to create a new request while the existing
    one is still executing.

    HTTP/2 servers now bound the number of simultaneously executing
    handler goroutines to the stream concurrency limit. New requests
    arriving when at the limit (which can only happen after the client
    has reset an existing, in-flight request) will be queued until a
    handler exits. If the request queue grows too large, the server
    will terminate the connection.

    This issue is also fixed in golang.org/x/net/http2 v0.17.0,
    for users manually configuring HTTP/2.

    The default stream concurrency limit is 250 streams (requests)
    per HTTP/2 connection. This value may be adjusted using the
    golang.org/x/net/http2 package; see the Server.MaxConcurrentStreams
    setting and the ConfigureServer function.

    This is CVE-2023-39325 and Go issue https://go.dev/issue/63417.
    This is also tracked by CVE-2023-44487.

@k8s-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@thaJeztah

This comment was marked as resolved.

@thaJeztah
Copy link
Member Author

Test was also failing / flaky on #9210 (comment)

@jiechen0826
Copy link

Hi @thaJeztah, just checking in to see when this PR will be reviewed and merged. Hoping to take up this change to address the CVE GHSA-4374-p667-p6c8.

Thanks!

@thaJeztah
Copy link
Member Author

oh! forgot I still had this one in draft; I just rebased, but saw that there's some failures that could indicate that the 1.6 branch is missing some patches in testing / CI that are needed to fix compatibility with the latest Go versions.

I rebased the branch to get a fresh run of CI

@thaJeztah thaJeztah marked this pull request as ready for review October 24, 2023 11:06
@henry118
Copy link
Member

henry118 commented Nov 6, 2023

@thaJeztah The CI Vagrant failure was fixed by #9332. Rebasing this PR would probably fix the CI failures. TestIssue9103 is a flaky test though, hopefully the CI can be unblocked by multiple runs :)

@thaJeztah
Copy link
Member Author

Rebased 🤞

@thaJeztah
Copy link
Member Author

Failures; first one looks to be flaky in this branch;

=== FAIL: . TestIssue9103/should_be_stopped_status_if_init_has_been_killed (3.11s)
    container_linux_test.go:1575: 
        	Error Trace:	/home/runner/work/containerd/containerd/integration/client/container_linux_test.go:1575
        	Error:      	Not equal: 
        	            	expected: "created"
        	            	actual  : "stopped"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,2 +1,2 @@
        	            	-(containerd.ProcessStatus) (len=7) "created"
        	            	+(containerd.ProcessStatus) (len=7) "stopped"
        	            	 
        	Test:       	TestIssue9103/should_be_stopped_status_if_init_has_been_killed
    --- FAIL: TestIssue9103/should_be_stopped_status_if_init_has_been_killed (3.11s)
=== FAIL: . TestContainerStartWithAbsRuntimePath (0.83s)
    container_test.go:260: failed to start shim: start failed: : fork/exec /tmp/containerd-shim-runc-v2: text file busy: unknown

gave CI another kick

@henry118
Copy link
Member

henry118 commented Nov 8, 2023

TestIssue9103 seems consistently fails in this branch. I'm also getting 100% local repro. I was able to fix it by adding a short pause as below.

@fuweid does the following patch make sense to you?
@samuelkarp what's your plan with #9337?

diff --git a/integration/client/container_linux_test.go b/integration/client/container_linux_test.go
index 20d93c0f0..d35f41a40 100644
--- a/integration/client/container_linux_test.go
+++ b/integration/client/container_linux_test.go
@@ -1567,6 +1567,7 @@ func TestIssue9103(t *testing.T) {
                        task, err := container.NewTask(cctx, empty())
                        ccancel()
                        require.NoError(t, err)
+                       time.Sleep(3 * time.Second)

                        defer task.Delete(ctx, WithProcessKill)

@samuelkarp
Copy link
Member

@henry118 I haven't been able to come back to this for a few days (and don't expect to tomorrow either). I think @fuweid's suggestion of calling task.Wait makes more sense to me than inserting a sleep.

@fuweid
Copy link
Member

fuweid commented Nov 8, 2023

I can carry the pull request today, if you don't mind 😂

@samuelkarp
Copy link
Member

@fuweid Please do so!

@fuweid
Copy link
Member

fuweid commented Nov 9, 2023

@thaJeztah please rebase and I think the flaky case is gone. Thanks!

go1.20.9 (released 2023-10-05) includes one security fixes to the cmd/go package,
as well as bug fixes to the go command and the linker. See the Go 1.20.9
milestone on our issue tracker for details:

https://github.com/golang/go/issues?q=milestone%3AGo1.20.9+label%3ACherryPickApproved

full diff: golang/go@go1.20.8...go1.20.9

From the security mailing:

[security] Go 1.21.2 and Go 1.20.9 are released

Hello gophers,

We have just released Go versions 1.21.2 and 1.20.9, minor point releases.

These minor releases include 1 security fixes following the security policy:

- cmd/go: line directives allows arbitrary execution during build

  "//line" directives can be used to bypass the restrictions on "//go:cgo_"
  directives, allowing blocked linker and compiler flags to be passed during
  compliation. This can result in unexpected execution of arbitrary code when
  running "go build". The line directive requires the absolute path of the file in
  which the directive lives, which makes exploting this issue significantly more
  complex.

  This is CVE-2023-39323 and Go issue https://go.dev/issue/63211.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
go1.20.10 (released 2023-10-10) includes a security fix to the net/http package.
See the Go 1.20.10 milestone on our issue tracker for details:

https://github.com/golang/go/issues?q=milestone%3AGo1.20.10+label%3ACherryPickApproved

full diff: golang/go@go1.20.9...go1.20.10

From the security mailing:

[security] Go 1.21.3 and Go 1.20.10 are released

Hello gophers,

We have just released Go versions 1.21.3 and 1.20.10, minor point releases.

These minor releases include 1 security fixes following the security policy:

- net/http: rapid stream resets can cause excessive work

  A malicious HTTP/2 client which rapidly creates requests and
  immediately resets them can cause excessive server resource consumption.
  While the total number of requests is bounded to the
  http2.Server.MaxConcurrentStreams setting, resetting an in-progress
  request allows the attacker to create a new request while the existing
  one is still executing.

  HTTP/2 servers now bound the number of simultaneously executing
  handler goroutines to the stream concurrency limit. New requests
  arriving when at the limit (which can only happen after the client
  has reset an existing, in-flight request) will be queued until a
  handler exits. If the request queue grows too large, the server
  will terminate the connection.

  This issue is also fixed in golang.org/x/net/http2 v0.17.0,
  for users manually configuring HTTP/2.

  The default stream concurrency limit is 250 streams (requests)
  per HTTP/2 connection. This value may be adjusted using the
  golang.org/x/net/http2 package; see the Server.MaxConcurrentStreams
  setting and the ConfigureServer function.

  This is CVE-2023-39325 and Go issue https://go.dev/issue/63417.
  This is also tracked by CVE-2023-44487.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

Ah! Saw the PR fly by (thank you!). Rebased 👍

@thaJeztah
Copy link
Member Author

All green now! 🎉

@estesp estesp merged commit f08be49 into containerd:release/1.6 Nov 9, 2023
38 checks passed
@thaJeztah thaJeztah deleted the 1.6_update_golang_1.20.10 branch November 9, 2023 21:10
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.

None yet

7 participants