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

[2.0] prometheus address CVE-2023-39325 CVE-2023-45288 #10954

Merged
merged 3 commits into from
Nov 7, 2024
Merged
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/prometheus/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
86 changes: 86 additions & 0 deletions SPECS/prometheus/CVE-2023-45288.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
From 87bba52321835fa92f7c91be1b8eef89a93d2506 Mon Sep 17 00:00:00 2001
From: Damien Neil <[email protected]>
Date: Wed, 10 Jan 2024 13:41:39 -0800
Subject: [PATCH] http2: close connections when receiving too many headers

Maintaining HPACK state requires that we parse and process
all HEADERS and CONTINUATION frames on a connection.
When a request's headers exceed MaxHeaderBytes, we don't
allocate memory to store the excess headers but we do
parse them. This permits an attacker to cause an HTTP/2
endpoint to read arbitrary amounts of data, all associated
with a request which is going to be rejected.

Set a limit on the amount of excess header frames we
will process before closing a connection.

Thanks to Bartek Nowotarski for reporting this issue.

Fixes CVE-2023-45288
Fixes golang/go#65051

Change-Id: I15df097268df13bb5a9e9d3a5c04a8a141d850f6
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2130527
Reviewed-by: Roland Shoemaker <[email protected]>
Reviewed-by: Tatiana Bradley <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/net/+/576155
Reviewed-by: Dmitri Shuralyov <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
---
vendor/golang.org/x/net/http2/frame.go | 31 ++++++++++++++++++++++++++
1 file changed, 31 insertions(+)

diff --git a/vendor/golang.org/x/net/http2/frame.go b/vendor/golang.org/x/net/http2/frame.go
index c1f6b90..175c154 100644
--- a/vendor/golang.org/x/net/http2/frame.go
+++ b/vendor/golang.org/x/net/http2/frame.go
@@ -1565,6 +1565,7 @@ func (fr *Framer) readMetaFrame(hf *HeadersFrame) (*MetaHeadersFrame, error) {
if size > remainSize {
hdec.SetEmitEnabled(false)
mh.Truncated = true
+ remainSize = 0
return
}
remainSize -= size
@@ -1577,6 +1578,36 @@ func (fr *Framer) readMetaFrame(hf *HeadersFrame) (*MetaHeadersFrame, error) {
var hc headersOrContinuation = hf
for {
frag := hc.HeaderBlockFragment()
+
+ // Avoid parsing large amounts of headers that we will then discard.
+ // If the sender exceeds the max header list size by too much,
+ // skip parsing the fragment and close the connection.
+ //
+ // "Too much" is either any CONTINUATION frame after we've already
+ // exceeded the max header list size (in which case remainSize is 0),
+ // or a frame whose encoded size is more than twice the remaining
+ // header list bytes we're willing to accept.
+ if int64(len(frag)) > int64(2*remainSize) {
+ if VerboseLogs {
+ log.Printf("http2: header list too large")
+ }
+ // It would be nice to send a RST_STREAM before sending the GOAWAY,
+ // but the struture of the server's frame writer makes this difficult.
+ return nil, ConnectionError(ErrCodeProtocol)
+ }
+
+ // Also close the connection after any CONTINUATION frame following an
+ // invalid header, since we stop tracking the size of the headers after
+ // an invalid one.
+ if invalid != nil {
+ if VerboseLogs {
+ log.Printf("http2: invalid header: %v", invalid)
+ }
+ // It would be nice to send a RST_STREAM before sending the GOAWAY,
+ // but the struture of the server's frame writer makes this difficult.
+ return nil, ConnectionError(ErrCodeProtocol)
+ }
+
if _, err := hdec.Write(frag); err != nil {
return nil, ConnectionError(ErrCodeCompression)
}
--
2.44.0

10 changes: 8 additions & 2 deletions SPECS/prometheus/prometheus.spec
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
Summary: Prometheus monitoring system and time series database
Name: prometheus
Version: 2.37.9
Release: 1%{?dist}
Release: 2%{?dist}
License: Apache-2.0
Vendor: Microsoft Corporation
Distribution: Mariner
Expand All @@ -20,6 +20,8 @@ Source6: promu-%{promu_version}.tar.gz
Patch0: 02-Default_settings.patch
Patch1: CVE-2024-6104.patch
Patch2: CVE-2024-24786.patch
Patch3: CVE-2023-39325.patch
Patch4: CVE-2023-45288.patch
BuildRequires: golang
BuildRequires: nodejs
BuildRequires: systemd-rpm-macros
Expand Down Expand Up @@ -133,7 +135,11 @@ fi
%doc README.md RELEASE.md documentation

%changelog
* Tue Oct 08 2024 Bhagyashri Pathak <[email protected]> - 2.37.0-16
* Wed Nov 06 2024 Nicolas Guibourge <[email protected]> - 2.37.9-2
- Patch for CVE-2023-39325 CVE-2023-45288
- Fix previous changelog version number

* Tue Oct 08 2024 Bhagyashri Pathak <[email protected]> - 2.37.9-1
- Bump version to patch CVE-2022-41717
- Patch for CVE-2024-24786

Expand Down
Loading