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

chore(deps): golang.org/x/net #319

Merged
merged 1 commit into from
Nov 9, 2023
Merged

chore(deps): golang.org/x/net #319

merged 1 commit into from
Nov 9, 2023

Conversation

tenthirtyam
Copy link
Collaborator

Summary

Address the following CVEs for golang.org/x/net:

CVE-2023-39325

HTTP/2 rapid reset can cause excessive work in net/http

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 by 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.

With the fix applied, HTTP/2 servers now bound the number of simultaneously executing handler goroutines to the stream concurrency limit (MaxConcurrentStreams). 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 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.

CVE-2023-44487

swift-nio-http2 is vulnerable to a denial-of-service vulnerability in which a malicious client can create and then reset a large number of HTTP/2 streams in a short period of time. This causes swift-nio-http2 to commit to a large amount of expensive work which it then throws away, including creating entirely new Channels to serve the traffic. This can easily overwhelm an EventLoop and prevent it from making forward progress.

swift-nio-http2 1.28 contains a remediation for this issue that applies reset counter using a sliding window. This constrains the number of stream resets that may occur in a given window of time. Clients violating this limit will have their connections torn down. This allows clients to continue to cancel streams for legitimate reasons, while constraining malicious actors.

CVE-2023-3978

Text nodes not in the HTML namespace are incorrectly literally rendered, causing text which should be escaped to not be. This could lead to an XSS attack.

Changes

➜ go get -u golang.org/x/net
go: upgraded golang.org/x/crypto v0.1.0 => v0.15.0 go: upgraded golang.org/x/net v0.8.0 => v0.18.0
go: upgraded golang.org/x/sys v0.6.0 => v0.14.0
go: upgraded golang.org/x/term v0.6.0 => v0.14.0
go: upgraded golang.org/x/text v0.8.0 => v0.14.0

Reference

Closes https://github.com/hashicorp/packer-plugin-vsphere/security/dependabot/19
Closes https://github.com/hashicorp/packer-plugin-vsphere/security/dependabot/20
Closes https://github.com/hashicorp/packer-plugin-vsphere/security/dependabot/18

Address the following CVEs:

- CVE-2023-39325
- CVE-2023-44487
- CVE-2023-3978

Signed-off-by: Ryan Johnson <[email protected]>
@tenthirtyam tenthirtyam self-assigned this Nov 9, 2023
@tenthirtyam tenthirtyam requested a review from a team as a code owner November 9, 2023 03:22
@tenthirtyam tenthirtyam marked this pull request as draft November 9, 2023 03:27
@tenthirtyam
Copy link
Collaborator Author

Hi @dilyar85 and/or @cyclexuxu 👋🏻 -

Could you take a look at the CI failure for this dependency update? It's failing with the following:

2023/11/09 03:26:14 ui: Successfully obtained the source VM IP: 1.2.3.4
2023/11/09 03:26:14 ui: Getting source VM ingress IP from the VMService object
2023/11/09 03:26:14 ui: VMService object's ingress IP is empty, continue checking...
2023/11/09 03:26:14 Retryable error: VMService object's ingress IP is empty, continue checking...
2023/11/09 03:26:19 ui: Successfully retrieved the source VM ingress IP: 5.6.7.8
2023/11/09 03:26:19 ui: Source VM is now ready in Supervisor cluster
--- FAIL: TestWatchSource_Run (5.01s)
    utils_test.go:66: Expected output "Successfully retrieved the source VM ingress IP: 5.6.7.8" but got "VMService object's ingress IP is empty, continue checking..."
FAIL
FAIL	github.com/hashicorp/packer-plugin-vsphere/builder/vsphere/supervisor	6.217s
ok  	github.com/hashicorp/packer-plugin-vsphere/post-processor/vsphere	0.071s
ok  	github.com/hashicorp/packer-plugin-vsphere/post-processor/vsphere-template	0.088s
FAIL
Error: Process completed with exit code 1.

@tenthirtyam
Copy link
Collaborator Author

Could you take a look at the CI failure for this dependency update? It's failing with the following:

It might not be a real issue since 1 / 3 passed. Passed on Windows but failed on macOS and Linux tests. I just restarted them for another try.

@tenthirtyam tenthirtyam marked this pull request as ready for review November 9, 2023 03:40
Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bumping this module to address the open CVEs.

@nywilken nywilken merged commit 05dd647 into main Nov 9, 2023
12 checks passed
@nywilken nywilken deleted the chore(deps)/cve-2023-39325 branch November 9, 2023 09:40
@nywilken
Copy link
Contributor

nywilken commented Nov 9, 2023

Could you take a look at the CI failure for this dependency update? It's failing with the following:

It might not be a real issue since 1 / 3 passed. Passed on Windows but failed on macOS and Linux tests. I just restarted them for another try.

So I think this occurs when the retry.Config.Run for getting the ingressIP retries at least once, which causes "VMService object's ingress IP is empty, continue checking..." to logged once. The call to checkOutputLines then reads the logged lines and fails. It seems like a bit of a timing issue. I was able to reproduce once but that was it.

@dilyar85
Copy link
Contributor

dilyar85 commented Nov 9, 2023

Could you take a look at the CI failure for this dependency update? It's failing with the following:

It might not be a real issue since 1 / 3 passed. Passed on Windows but failed on macOS and Linux tests. I just restarted them for another try.

So I think this occurs when the retry.Config.Run for getting the ingressIP retries at least once, which causes "VMService object's ingress IP is empty, continue checking..." to logged once. The call to checkOutputLines then reads the logged lines and fails. It seems like a bit of a timing issue. I was able to reproduce once but that was it.

Yes. So in the test code, we update the VM IP and ingressIP sequentially. However, the latter is retrieved from the retry.Config instead of using a watch mechanism. This caused the test to fail due to a timing issue. I have created PR #321 to address this properly.

@tenthirtyam
Copy link
Collaborator Author

Thanks, Sai!

@hashicorp hashicorp locked and limited conversation to collaborators Jun 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants