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

Added a patch to address CVE-2023-39325. #11931

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 152 additions & 0 deletions SPECS/multus/CVE-2023-39325.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
From 84b30b3380727ea94e05c438ab695ea24e38fb0c Mon Sep 17 00:00:00 2001
From: Damien Neil <[email protected]>
Date: Fri, 6 Oct 2023 09:51:19 -0700
Subject: [PATCH] http2: limit maximum handler goroutines to
MaxConcurrentStreams

When the peer opens a new stream while we have MaxConcurrentStreams
handler goroutines running, defer starting a handler until one
of the existing handlers exits.

Fixes golang/go#63417
Fixes CVE-2023-39325

Change-Id: If0531e177b125700f3e24c5ebd24b1023098fa6d
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2045854
TryBot-Result: Security TryBots <[email protected]>
Reviewed-by: Ian Cottrell <[email protected]>
Reviewed-by: Tatiana Bradley <[email protected]>
Run-TryBot: Damien Neil <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/net/+/534215
Reviewed-by: Michael Pratt <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: Damien Neil <[email protected]>

Modified to apply to vendored code by: Daniel McIlvaney <[email protected]>
- Adjusted paths
- Removed reference to server_test.go
---
.../vendor/golang.org/x/net/http2/server.go | 66 ++++++++++++++++++-
1 file changed, 64 insertions(+), 2 deletions(-)

diff --git a/vendor/golang.org/x/net/http2/server.go b/vendor/golang.org/x/net/http2/server.go
index 8cb14f3..6000140 100644
--- a/vendor/golang.org/x/net/http2/server.go
+++ b/vendor/golang.org/x/net/http2/server.go
@@ -581,9 +581,11 @@ type serverConn struct {
advMaxStreams uint32 // our SETTINGS_MAX_CONCURRENT_STREAMS advertised the client
curClientStreams uint32 // number of open streams initiated by the client
curPushedStreams uint32 // number of open streams initiated by server push
+ curHandlers uint32 // number of running handler goroutines
maxClientStreamID uint32 // max ever seen from client (odd), or 0 if there have been no client requests
maxPushPromiseID uint32 // ID of the last push promise (even), or 0 if there have been no pushes
streams map[uint32]*stream
+ unstartedHandlers []unstartedHandler
initialStreamSendWindowSize int32
maxFrameSize int32
peerMaxHeaderListSize uint32 // zero means unknown (default)
@@ -981,6 +983,8 @@ func (sc *serverConn) serve() {
return
case gracefulShutdownMsg:
sc.startGracefulShutdownInternal()
+ case handlerDoneMsg:
+ sc.handlerDone()
default:
panic("unknown timer")
}
@@ -1028,6 +1032,7 @@ var (
idleTimerMsg = new(serverMessage)
shutdownTimerMsg = new(serverMessage)
gracefulShutdownMsg = new(serverMessage)
+ handlerDoneMsg = new(serverMessage)
)

func (sc *serverConn) onSettingsTimer() { sc.sendServeMsg(settingsTimerMsg) }
@@ -2022,8 +2027,7 @@ func (sc *serverConn) processHeaders(f *MetaHeadersFrame) error {
}
}

- go sc.runHandler(rw, req, handler)
- return nil
+ return sc.scheduleHandler(id, rw, req, handler)
}

func (sc *serverConn) upgradeRequest(req *http.Request) {
@@ -2043,6 +2047,10 @@ func (sc *serverConn) upgradeRequest(req *http.Request) {
sc.conn.SetReadDeadline(time.Time{})
}

+ // This is the first request on the connection,
+ // so start the handler directly rather than going
+ // through scheduleHandler.
+ sc.curHandlers++
go sc.runHandler(rw, req, sc.handler.ServeHTTP)
}

@@ -2283,8 +2291,62 @@ func (sc *serverConn) newResponseWriter(st *stream, req *http.Request) *response
return &responseWriter{rws: rws}
}

+type unstartedHandler struct {
+ streamID uint32
+ rw *responseWriter
+ req *http.Request
+ handler func(http.ResponseWriter, *http.Request)
+}
+
+// scheduleHandler starts a handler goroutine,
+// or schedules one to start as soon as an existing handler finishes.
+func (sc *serverConn) scheduleHandler(streamID uint32, rw *responseWriter, req *http.Request, handler func(http.ResponseWriter, *http.Request)) error {
+ sc.serveG.check()
+ maxHandlers := sc.advMaxStreams
+ if sc.curHandlers < maxHandlers {
+ sc.curHandlers++
+ go sc.runHandler(rw, req, handler)
+ return nil
+ }
+ if len(sc.unstartedHandlers) > int(4*sc.advMaxStreams) {
+ return sc.countError("too_many_early_resets", ConnectionError(ErrCodeEnhanceYourCalm))
+ }
+ sc.unstartedHandlers = append(sc.unstartedHandlers, unstartedHandler{
+ streamID: streamID,
+ rw: rw,
+ req: req,
+ handler: handler,
+ })
+ return nil
+}
+
+func (sc *serverConn) handlerDone() {
+ sc.serveG.check()
+ sc.curHandlers--
+ i := 0
+ maxHandlers := sc.advMaxStreams
+ for ; i < len(sc.unstartedHandlers); i++ {
+ u := sc.unstartedHandlers[i]
+ if sc.streams[u.streamID] == nil {
+ // This stream was reset before its goroutine had a chance to start.
+ continue
+ }
+ if sc.curHandlers >= maxHandlers {
+ break
+ }
+ sc.curHandlers++
+ go sc.runHandler(u.rw, u.req, u.handler)
+ sc.unstartedHandlers[i] = unstartedHandler{} // don't retain references
+ }
+ sc.unstartedHandlers = sc.unstartedHandlers[i:]
+ if len(sc.unstartedHandlers) == 0 {
+ sc.unstartedHandlers = nil
+ }
+}
+
// Run on its own goroutine.
func (sc *serverConn) runHandler(rw *responseWriter, req *http.Request, handler func(http.ResponseWriter, *http.Request)) {
+ defer sc.sendServeMsg(handlerDoneMsg)
didPanic := true
defer func() {
rw.rws.stream.cancelCtx()
--
2.33.8
7 changes: 6 additions & 1 deletion SPECS/multus/multus.spec
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
Summary: CNI plugin providing multiple interfaces in containers
Name: multus
Version: 4.0.2
Release: 3%{?dist}
Release: 4%{?dist}
License: ASL 2.0
Vendor: Microsoft Corporation
Distribution: Azure Linux
Expand All @@ -30,6 +30,7 @@ Source0: https://github.com/k8snetworkplumbingwg/multus-cni/archive/refs/
Patch0: CVE-2023-3978.patch
Patch1: CVE-2023-44487.patch
Patch2: CVE-2023-45288.patch
Patch3: CVE-2023-39325.patch
BuildRequires: golang
BuildRequires: golang-packaging

Expand Down Expand Up @@ -72,6 +73,10 @@ install -D -m0644 deployments/multus-daemonset-crio.yml %{buildroot}%{_datadir}/
%{_datarootdir}/k8s-yaml/multus/multus.yaml

%changelog
* Wed Jan 15 2025 Lanze Liu <[email protected]> - 4.0.2-4
- Added a patch to address CVE-2023-39325.
- Corrected previous assumption that CVE-2023-39325 is a subset of CVE-2023-44487.

* Fri Nov 22 2024 Xiaohong Deng <[email protected]> - 4.0.2-3
- Add patches to resolve CVE-2023-39325, CVE-2023-44487 and CVE-2023-45288.
- CVE-2023-39325 is a subset of CVE-2023-44487 and the patches are combined.
Expand Down
Loading