Skip to content

Commit 0a527a2

Browse files
CBL-Mariner-Botcorvus-callidus
CBL-Mariner-Bot
and
corvus-callidus
authored
[AUTO-CHERRYPICK] Fix CVE-2023-44487 for moby-containerd-cc - branch 3.0-dev (#13009)
Co-authored-by: corvus-callidus <108946721+corvus-callidus@users.noreply.github.com>
1 parent 5fd49d6 commit 0a527a2

3 files changed

Lines changed: 622 additions & 139 deletions

File tree

‎SPECS/moby-containerd-cc/CVE-2023-39325.patch‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
From 84b30b3380727ea94e05c438ab695ea24e38fb0c Mon Sep 17 00:00:00 2001
2+
From: Damien Neil <dneil@google.com>
3+
Date: Fri, 6 Oct 2023 09:51:19 -0700
4+
Subject: [PATCH] http2: limit maximum handler goroutines to
5+
MaxConcurrentStreams
6+
7+
When the peer opens a new stream while we have MaxConcurrentStreams
8+
handler goroutines running, defer starting a handler until one
9+
of the existing handlers exits.
10+
11+
Fixes golang/go#63417
12+
Fixes CVE-2023-39325
13+
14+
Change-Id: If0531e177b125700f3e24c5ebd24b1023098fa6d
15+
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2045854
16+
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
17+
Reviewed-by: Ian Cottrell <iancottrell@google.com>
18+
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
19+
Run-TryBot: Damien Neil <dneil@google.com>
20+
Reviewed-on: https://go-review.googlesource.com/c/net/+/534215
21+
Reviewed-by: Michael Pratt <mpratt@google.com>
22+
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
23+
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
24+
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
25+
Reviewed-by: Damien Neil <dneil@google.com>
26+
27+
Modified to apply to vendored code by: Daniel McIlvaney <damcilva@microsoft.com>
28+
- Adjusted paths
29+
- Removed reference to server_test.go
30+
---
31+
.../vendor/golang.org/x/net/http2/server.go | 66 ++++++++++++++++++-
32+
1 file changed, 64 insertions(+), 2 deletions(-)
33+
34+
diff --git a/vendor/golang.org/x/net/http2/server.go b/vendor/golang.org/x/net/http2/server.go
35+
index 8cb14f3..6000140 100644
36+
--- a/vendor/golang.org/x/net/http2/server.go
37+
+++ b/vendor/golang.org/x/net/http2/server.go
38+
@@ -581,9 +581,11 @@ type serverConn struct {
39+
advMaxStreams uint32 // our SETTINGS_MAX_CONCURRENT_STREAMS advertised the client
40+
curClientStreams uint32 // number of open streams initiated by the client
41+
curPushedStreams uint32 // number of open streams initiated by server push
42+
+ curHandlers uint32 // number of running handler goroutines
43+
maxClientStreamID uint32 // max ever seen from client (odd), or 0 if there have been no client requests
44+
maxPushPromiseID uint32 // ID of the last push promise (even), or 0 if there have been no pushes
45+
streams map[uint32]*stream
46+
+ unstartedHandlers []unstartedHandler
47+
initialStreamSendWindowSize int32
48+
maxFrameSize int32
49+
peerMaxHeaderListSize uint32 // zero means unknown (default)
50+
@@ -981,6 +983,8 @@ func (sc *serverConn) serve() {
51+
return
52+
case gracefulShutdownMsg:
53+
sc.startGracefulShutdownInternal()
54+
+ case handlerDoneMsg:
55+
+ sc.handlerDone()
56+
default:
57+
panic("unknown timer")
58+
}
59+
@@ -1028,6 +1032,7 @@ var (
60+
idleTimerMsg = new(serverMessage)
61+
shutdownTimerMsg = new(serverMessage)
62+
gracefulShutdownMsg = new(serverMessage)
63+
+ handlerDoneMsg = new(serverMessage)
64+
)
65+
66+
func (sc *serverConn) onSettingsTimer() { sc.sendServeMsg(settingsTimerMsg) }
67+
@@ -2022,8 +2027,7 @@ func (sc *serverConn) processHeaders(f *MetaHeadersFrame) error {
68+
}
69+
}
70+
71+
- go sc.runHandler(rw, req, handler)
72+
- return nil
73+
+ return sc.scheduleHandler(id, rw, req, handler)
74+
}
75+
76+
func (sc *serverConn) upgradeRequest(req *http.Request) {
77+
@@ -2043,6 +2047,10 @@ func (sc *serverConn) upgradeRequest(req *http.Request) {
78+
sc.conn.SetReadDeadline(time.Time{})
79+
}
80+
81+
+ // This is the first request on the connection,
82+
+ // so start the handler directly rather than going
83+
+ // through scheduleHandler.
84+
+ sc.curHandlers++
85+
go sc.runHandler(rw, req, sc.handler.ServeHTTP)
86+
}
87+
88+
@@ -2283,8 +2291,62 @@ func (sc *serverConn) newResponseWriter(st *stream, req *http.Request) *response
89+
return &responseWriter{rws: rws}
90+
}
91+
92+
+type unstartedHandler struct {
93+
+ streamID uint32
94+
+ rw *responseWriter
95+
+ req *http.Request
96+
+ handler func(http.ResponseWriter, *http.Request)
97+
+}
98+
+
99+
+// scheduleHandler starts a handler goroutine,
100+
+// or schedules one to start as soon as an existing handler finishes.
101+
+func (sc *serverConn) scheduleHandler(streamID uint32, rw *responseWriter, req *http.Request, handler func(http.ResponseWriter, *http.Request)) error {
102+
+ sc.serveG.check()
103+
+ maxHandlers := sc.advMaxStreams
104+
+ if sc.curHandlers < maxHandlers {
105+
+ sc.curHandlers++
106+
+ go sc.runHandler(rw, req, handler)
107+
+ return nil
108+
+ }
109+
+ if len(sc.unstartedHandlers) > int(4*sc.advMaxStreams) {
110+
+ return sc.countError("too_many_early_resets", ConnectionError(ErrCodeEnhanceYourCalm))
111+
+ }
112+
+ sc.unstartedHandlers = append(sc.unstartedHandlers, unstartedHandler{
113+
+ streamID: streamID,
114+
+ rw: rw,
115+
+ req: req,
116+
+ handler: handler,
117+
+ })
118+
+ return nil
119+
+}
120+
+
121+
+func (sc *serverConn) handlerDone() {
122+
+ sc.serveG.check()
123+
+ sc.curHandlers--
124+
+ i := 0
125+
+ maxHandlers := sc.advMaxStreams
126+
+ for ; i < len(sc.unstartedHandlers); i++ {
127+
+ u := sc.unstartedHandlers[i]
128+
+ if sc.streams[u.streamID] == nil {
129+
+ // This stream was reset before its goroutine had a chance to start.
130+
+ continue
131+
+ }
132+
+ if sc.curHandlers >= maxHandlers {
133+
+ break
134+
+ }
135+
+ sc.curHandlers++
136+
+ go sc.runHandler(u.rw, u.req, u.handler)
137+
+ sc.unstartedHandlers[i] = unstartedHandler{} // don't retain references
138+
+ }
139+
+ sc.unstartedHandlers = sc.unstartedHandlers[i:]
140+
+ if len(sc.unstartedHandlers) == 0 {
141+
+ sc.unstartedHandlers = nil
142+
+ }
143+
+}
144+
+
145+
// Run on its own goroutine.
146+
func (sc *serverConn) runHandler(rw *responseWriter, req *http.Request, handler func(http.ResponseWriter, *http.Request)) {
147+
+ defer sc.sendServeMsg(handlerDoneMsg)
148+
didPanic := true
149+
defer func() {
150+
rw.rws.stream.cancelCtx()
151+
--
152+
2.33.8

0 commit comments

Comments
 (0)