Skip to content

Commit 79fe0a7

Browse files
CBL-Mariner-Botdmcilvaney
CBL-Mariner-Bot
and
dmcilvaney
authored
[AUTO-CHERRYPICK] Fix coredns CVE-2023-44487 by patching vendor tar - branch main (#7628)
Co-authored-by: Daniel McIlvaney <damcilva@microsoft.com>
1 parent fc0b1c0 commit 79fe0a7

3 files changed

Lines changed: 173 additions & 3 deletions

File tree

‎SPECS/coredns/CVE-2023-44487.patch‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
From b225e7ca6dde1ef5a5ae5ce922861bda011cfabd 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+
32+
vendor/golang.org/x/net/http2/server.go | 66 ++++++++++++++++++++++++-
33+
vendor/golang.org/x/net/http2/server_test.go | 113 +++++++++++++++++++++++++++++++++++++++++++
34+
2 files changed, 177 insertions(+), 2 deletions(-)
35+
36+
diff --git a/vendor/golang.org/x/net/http2/server.go b/vendor/golang.org/x/net/http2/server.go
37+
index de60fa88f..02c88b6b3 100644
38+
--- a/vendor/golang.org/x/net/http2/server.go
39+
+++ b/vendor/golang.org/x/net/http2/server.go
40+
@@ -581,9 +581,11 @@ type serverConn struct {
41+
advMaxStreams uint32 // our SETTINGS_MAX_CONCURRENT_STREAMS advertised the client
42+
curClientStreams uint32 // number of open streams initiated by the client
43+
curPushedStreams uint32 // number of open streams initiated by server push
44+
+ curHandlers uint32 // number of running handler goroutines
45+
maxClientStreamID uint32 // max ever seen from client (odd), or 0 if there have been no client requests
46+
maxPushPromiseID uint32 // ID of the last push promise (even), or 0 if there have been no pushes
47+
streams map[uint32]*stream
48+
+ unstartedHandlers []unstartedHandler
49+
initialStreamSendWindowSize int32
50+
maxFrameSize int32
51+
peerMaxHeaderListSize uint32 // zero means unknown (default)
52+
@@ -981,6 +983,8 @@ func (sc *serverConn) serve() {
53+
return
54+
case gracefulShutdownMsg:
55+
sc.startGracefulShutdownInternal()
56+
+ case handlerDoneMsg:
57+
+ sc.handlerDone()
58+
default:
59+
panic("unknown timer")
60+
}
61+
@@ -1020,6 +1024,7 @@ var (
62+
idleTimerMsg = new(serverMessage)
63+
shutdownTimerMsg = new(serverMessage)
64+
gracefulShutdownMsg = new(serverMessage)
65+
+ handlerDoneMsg = new(serverMessage)
66+
)
67+
68+
func (sc *serverConn) onSettingsTimer() { sc.sendServeMsg(settingsTimerMsg) }
69+
@@ -2017,8 +2022,7 @@ func (sc *serverConn) processHeaders(f *MetaHeadersFrame) error {
70+
st.readDeadline = time.AfterFunc(sc.hs.ReadTimeout, st.onReadTimeout)
71+
}
72+
73+
- go sc.runHandler(rw, req, handler)
74+
- return nil
75+
+ return sc.scheduleHandler(id, rw, req, handler)
76+
}
77+
78+
func (sc *serverConn) upgradeRequest(req *http.Request) {
79+
@@ -2038,6 +2042,10 @@ func (sc *serverConn) upgradeRequest(req *http.Request) {
80+
sc.conn.SetReadDeadline(time.Time{})
81+
}
82+
83+
+ // This is the first request on the connection,
84+
+ // so start the handler directly rather than going
85+
+ // through scheduleHandler.
86+
+ sc.curHandlers++
87+
go sc.runHandler(rw, req, sc.handler.ServeHTTP)
88+
}
89+
90+
@@ -2278,8 +2286,62 @@ func (sc *serverConn) newResponseWriter(st *stream, req *http.Request) *response
91+
return &responseWriter{rws: rws}
92+
}
93+
94+
+type unstartedHandler struct {
95+
+ streamID uint32
96+
+ rw *responseWriter
97+
+ req *http.Request
98+
+ handler func(http.ResponseWriter, *http.Request)
99+
+}
100+
+
101+
+// scheduleHandler starts a handler goroutine,
102+
+// or schedules one to start as soon as an existing handler finishes.
103+
+func (sc *serverConn) scheduleHandler(streamID uint32, rw *responseWriter, req *http.Request, handler func(http.ResponseWriter, *http.Request)) error {
104+
+ sc.serveG.check()
105+
+ maxHandlers := sc.advMaxStreams
106+
+ if sc.curHandlers < maxHandlers {
107+
+ sc.curHandlers++
108+
+ go sc.runHandler(rw, req, handler)
109+
+ return nil
110+
+ }
111+
+ if len(sc.unstartedHandlers) > int(4*sc.advMaxStreams) {
112+
+ return sc.countError("too_many_early_resets", ConnectionError(ErrCodeEnhanceYourCalm))
113+
+ }
114+
+ sc.unstartedHandlers = append(sc.unstartedHandlers, unstartedHandler{
115+
+ streamID: streamID,
116+
+ rw: rw,
117+
+ req: req,
118+
+ handler: handler,
119+
+ })
120+
+ return nil
121+
+}
122+
+
123+
+func (sc *serverConn) handlerDone() {
124+
+ sc.serveG.check()
125+
+ sc.curHandlers--
126+
+ i := 0
127+
+ maxHandlers := sc.advMaxStreams
128+
+ for ; i < len(sc.unstartedHandlers); i++ {
129+
+ u := sc.unstartedHandlers[i]
130+
+ if sc.streams[u.streamID] == nil {
131+
+ // This stream was reset before its goroutine had a chance to start.
132+
+ continue
133+
+ }
134+
+ if sc.curHandlers >= maxHandlers {
135+
+ break
136+
+ }
137+
+ sc.curHandlers++
138+
+ go sc.runHandler(u.rw, u.req, u.handler)
139+
+ sc.unstartedHandlers[i] = unstartedHandler{} // don't retain references
140+
+ }
141+
+ sc.unstartedHandlers = sc.unstartedHandlers[i:]
142+
+ if len(sc.unstartedHandlers) == 0 {
143+
+ sc.unstartedHandlers = nil
144+
+ }
145+
+}
146+
+
147+
// Run on its own goroutine.
148+
func (sc *serverConn) runHandler(rw *responseWriter, req *http.Request, handler func(http.ResponseWriter, *http.Request)) {
149+
+ defer sc.sendServeMsg(handlerDoneMsg)
150+
didPanic := true
151+
defer func() {
152+
rw.rws.stream.cancelCtx()

‎SPECS/coredns/coredns.signatures.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@
33
"coredns-1.11.1.tar.gz": "4e1cde1759d1705baa9375127eb405cd2f5031f9152947bb958a51fee5898d8c",
44
"coredns-1.11.1-vendor.tar.gz": "f6713fb6bdb6da88bab4c93a53317907991fa2e304cc2f224bfee88df6c26846"
55
}
6-
}
6+
}

‎SPECS/coredns/coredns.spec‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
Summary: Fast and flexible DNS server
44
Name: coredns
55
Version: 1.11.1
6-
Release: 1%{?dist}
6+
Release: 2%{?dist}
77
License: Apache License 2.0
88
Vendor: Microsoft Corporation
99
Distribution: Mariner
@@ -32,22 +32,37 @@ Source0: %{name}-%{version}.tar.gz
3232
Source1: %{name}-%{version}-vendor.tar.gz
3333
Patch0: makefile-buildoption-commitnb.patch
3434

35+
# Patch for old x/net/http2 vendored code, apply after vendored code is extracted.
36+
Patch1000: CVE-2023-44487.patch
37+
3538
BuildRequires: golang >= 1.12
3639

3740
%description
3841
CoreDNS is a fast and flexible DNS server.
3942

4043
%prep
41-
%autosetup -p1
44+
%autosetup -N
45+
%autopatch -p1 -M 999
4246

4347
%build
4448
# create vendor folder from the vendor tarball and set vendor mode
4549
tar -xf %{SOURCE1} --no-same-owner
50+
patch -p1 < %{PATCH1000}
4651
export BUILDOPTS="-mod=vendor -v"
4752
# set commit number that correspond to the github tag for that version
4853
export GITCOMMIT="ae2bbc29be1aaae0b3ded5d188968a6c97bb3144"
4954
make
5055

56+
%check
57+
# From go.test.yml
58+
go install github.com/fatih/faillint@latest && \
59+
(cd request && go test -v -race ./...) && \
60+
(cd core && go test -v -race ./...) && \
61+
(cd coremain && go test -v -race ./...) && \
62+
(cd plugin && go test -v -race ./...) && \
63+
(cd test && go test -v -race ./...) && \
64+
./coredns -version
65+
5166
%install
5267
install -m 755 -d %{buildroot}%{_bindir}
5368
install -p -m 755 -t %{buildroot}%{_bindir} %{name}
@@ -58,6 +73,9 @@ install -p -m 755 -t %{buildroot}%{_bindir} %{name}
5873
%{_bindir}/%{name}
5974

6075
%changelog
76+
* Mon Jan 29 2024 Daniel McIlvaney <damcilva@microsoft.com> - 1.11.1-2
77+
- Address CVE-2023-44487 by patching vendored golang.org/x/net
78+
6179
* Tue Oct 18 2023 Nicolas Guibourge <nicolasg@microsoft.com> - 1.11.1-1
6280
- Upgrade to 1.11.1 to match version required by kubernetes
6381

0 commit comments

Comments
 (0)