Skip to content

Commit f3363e0

Browse files
committed
http2: handle server errors after sending GOAWAY
The HTTP/2 server uses serverConn.goAwayCode to track whether a connection has encountered a fatal error. If an error is encountered after sending a ErrCodeNo GOAWAY, upgrade goAwayCode to reflect the error status of the connection. Fixes an issue where a server connection could hang forever waiting for a clean shutdown that was preempted by a subsequent fatal error. Fixes CVE-2022-27664 For golang/go#53977 Change-Id: I165b81ab53176c77a68c42976030499d57bb05d3 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1413887 Reviewed-by: Damien Neil <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/net/+/428735 Run-TryBot: Damien Neil <[email protected]> Reviewed-by: Carlos Amedee <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent 83b083e commit f3363e0

File tree

2 files changed

+46
-0
lines changed

2 files changed

+46
-0
lines changed

http2/server.go

+3
Original file line numberDiff line numberDiff line change
@@ -1371,6 +1371,9 @@ func (sc *serverConn) startGracefulShutdownInternal() {
13711371
func (sc *serverConn) goAway(code ErrCode) {
13721372
sc.serveG.check()
13731373
if sc.inGoAway {
1374+
if sc.goAwayCode == ErrCodeNo {
1375+
sc.goAwayCode = code
1376+
}
13741377
return
13751378
}
13761379
sc.inGoAway = true

http2/server_test.go

+43
Original file line numberDiff line numberDiff line change
@@ -4366,3 +4366,46 @@ func TestServerSendsEarlyHints(t *testing.T) {
43664366
}
43674367
})
43684368
}
4369+
4370+
func TestProtocolErrorAfterGoAway(t *testing.T) {
4371+
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
4372+
io.Copy(io.Discard, r.Body)
4373+
})
4374+
defer st.Close()
4375+
4376+
st.greet()
4377+
content := "some content"
4378+
st.writeHeaders(HeadersFrameParam{
4379+
StreamID: 1,
4380+
BlockFragment: st.encodeHeader(
4381+
":method", "POST",
4382+
"content-length", strconv.Itoa(len(content)),
4383+
),
4384+
EndStream: false,
4385+
EndHeaders: true,
4386+
})
4387+
st.writeData(1, false, []byte(content[:5]))
4388+
4389+
_, err := st.readFrame()
4390+
if err != nil {
4391+
st.t.Fatal(err)
4392+
}
4393+
4394+
// Send a GOAWAY with ErrCodeNo, followed by a bogus window update.
4395+
// The server should close the connection.
4396+
if err := st.fr.WriteGoAway(1, ErrCodeNo, nil); err != nil {
4397+
t.Fatal(err)
4398+
}
4399+
if err := st.fr.WriteWindowUpdate(0, 1<<31-1); err != nil {
4400+
t.Fatal(err)
4401+
}
4402+
4403+
for {
4404+
if _, err := st.readFrame(); err != nil {
4405+
if err != io.EOF {
4406+
t.Errorf("unexpected readFrame error: %v", err)
4407+
}
4408+
break
4409+
}
4410+
}
4411+
}

0 commit comments

Comments
 (0)