Skip to content

Commit 72186f1

Browse files
easwarseaswars
authored
grpc: enforce strict path checking for incoming requests on the server (#8981)
RELEASE NOTES: * server: fix an authorization bypass where malformed :path headers (missing the leading slash) could bypass path-based restricted "deny" rules in interceptors like `grpc/authz`. Any request with a non-canonical path is now immediately rejected with an `Unimplemented` error.
1 parent 97ca352 commit 72186f1

19 files changed

+540
-326
lines changed

‎internal/envconfig/envconfig.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,22 @@ var (
8888
// feature can be disabled by setting the environment variable
8989
// GRPC_EXPERIMENTAL_PF_WEIGHTED_SHUFFLING to "false".
9090
PickFirstWeightedShuffling = boolFromEnv("GRPC_EXPERIMENTAL_PF_WEIGHTED_SHUFFLING", true)
91+
92+
// DisableStrictPathChecking indicates whether strict path checking is
93+
// disabled. This feature can be disabled by setting the environment
94+
// variable GRPC_GO_EXPERIMENTAL_DISABLE_STRICT_PATH_CHECKING to "true".
95+
//
96+
// When strict path checking is enabled, gRPC will reject requests with
97+
// paths that do not conform to the gRPC over HTTP/2 specification found at
98+
// https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md.
99+
//
100+
// When disabled, gRPC will allow paths that do not contain a leading slash.
101+
// Enabling strict path checking is recommended for security reasons, as it
102+
// prevents potential path traversal vulnerabilities.
103+
//
104+
// A future release will remove this environment variable, enabling strict
105+
// path checking behavior unconditionally.
106+
DisableStrictPathChecking = boolFromEnv("GRPC_GO_EXPERIMENTAL_DISABLE_STRICT_PATH_CHECKING", false)
91107
)
92108

93109
func boolFromEnv(envVar string, def bool) bool {

‎server.go‎

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import (
4242
"google.golang.org/grpc/internal"
4343
"google.golang.org/grpc/internal/binarylog"
4444
"google.golang.org/grpc/internal/channelz"
45+
"google.golang.org/grpc/internal/envconfig"
4546
"google.golang.org/grpc/internal/grpcsync"
4647
"google.golang.org/grpc/internal/grpcutil"
4748
istats "google.golang.org/grpc/internal/stats"
@@ -149,6 +150,8 @@ type Server struct {
149150

150151
serverWorkerChannel chan func()
151152
serverWorkerChannelClose func()
153+
154+
strictPathCheckingLogEmitted atomic.Bool
152155
}
153156

154157
type serverOptions struct {
@@ -1762,6 +1765,24 @@ func (s *Server) processStreamingRPC(ctx context.Context, stream *transport.Serv
17621765
return ss.s.WriteStatus(statusOK)
17631766
}
17641767

1768+
func (s *Server) handleMalformedMethodName(stream *transport.ServerStream, ti *traceInfo) {
1769+
if ti != nil {
1770+
ti.tr.LazyLog(&fmtStringer{"Malformed method name %q", []any{stream.Method()}}, true)
1771+
ti.tr.SetError()
1772+
}
1773+
errDesc := fmt.Sprintf("malformed method name: %q", stream.Method())
1774+
if err := stream.WriteStatus(status.New(codes.Unimplemented, errDesc)); err != nil {
1775+
if ti != nil {
1776+
ti.tr.LazyLog(&fmtStringer{"%v", []any{err}}, true)
1777+
ti.tr.SetError()
1778+
}
1779+
channelz.Warningf(logger, s.channelz, "grpc: Server.handleStream failed to write status: %v", err)
1780+
}
1781+
if ti != nil {
1782+
ti.tr.Finish()
1783+
}
1784+
}
1785+
17651786
func (s *Server) handleStream(t transport.ServerTransport, stream *transport.ServerStream) {
17661787
ctx := stream.Context()
17671788
ctx = contextWithServer(ctx, s)
@@ -1782,26 +1803,30 @@ func (s *Server) handleStream(t transport.ServerTransport, stream *transport.Ser
17821803
}
17831804

17841805
sm := stream.Method()
1785-
if sm != "" && sm[0] == '/' {
1806+
if sm == "" {
1807+
s.handleMalformedMethodName(stream, ti)
1808+
return
1809+
}
1810+
if sm[0] != '/' {
1811+
// TODO(easwars): Add a link to the CVE in the below log messages once
1812+
// published.
1813+
if envconfig.DisableStrictPathChecking {
1814+
if old := s.strictPathCheckingLogEmitted.Swap(true); !old {
1815+
channelz.Warningf(logger, s.channelz, "grpc: Server.handleStream received malformed method name %q. Allowing it because the environment variable GRPC_GO_EXPERIMENTAL_DISABLE_STRICT_PATH_CHECKING is set to true, but this option will be removed in a future release.", sm)
1816+
}
1817+
} else {
1818+
if old := s.strictPathCheckingLogEmitted.Swap(true); !old {
1819+
channelz.Warningf(logger, s.channelz, "grpc: Server.handleStream rejected malformed method name %q. To temporarily allow such requests, set the environment variable GRPC_GO_EXPERIMENTAL_DISABLE_STRICT_PATH_CHECKING to true. Note that this is not recommended as it may allow requests to bypass security policies.", sm)
1820+
}
1821+
s.handleMalformedMethodName(stream, ti)
1822+
return
1823+
}
1824+
} else {
17861825
sm = sm[1:]
17871826
}
17881827
pos := strings.LastIndex(sm, "/")
17891828
if pos == -1 {
1790-
if ti != nil {
1791-
ti.tr.LazyLog(&fmtStringer{"Malformed method name %q", []any{sm}}, true)
1792-
ti.tr.SetError()
1793-
}
1794-
errDesc := fmt.Sprintf("malformed method name: %q", stream.Method())
1795-
if err := stream.WriteStatus(status.New(codes.Unimplemented, errDesc)); err != nil {
1796-
if ti != nil {
1797-
ti.tr.LazyLog(&fmtStringer{"%v", []any{err}}, true)
1798-
ti.tr.SetError()
1799-
}
1800-
channelz.Warningf(logger, s.channelz, "grpc: Server.handleStream failed to write status: %v", err)
1801-
}
1802-
if ti != nil {
1803-
ti.tr.Finish()
1804-
}
1829+
s.handleMalformedMethodName(stream, ti)
18051830
return
18061831
}
18071832
service := sm[:pos]

‎test/malformed_method_test.go‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
/*
2+
*
3+
* Copyright 2026 gRPC authors.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package test
20+
21+
import (
22+
"bytes"
23+
"context"
24+
"net"
25+
"testing"
26+
27+
"golang.org/x/net/http2"
28+
"golang.org/x/net/http2/hpack"
29+
"google.golang.org/grpc/internal/envconfig"
30+
"google.golang.org/grpc/internal/stubserver"
31+
"google.golang.org/grpc/internal/testutils"
32+
33+
testpb "google.golang.org/grpc/interop/grpc_testing"
34+
)
35+
36+
// TestMalformedMethodPath tests that the server responds with Unimplemented
37+
// when the method path is malformed. This verifies that the server does not
38+
// route requests with a malformed method path to the application handler.
39+
func (s) TestMalformedMethodPath(t *testing.T) {
40+
tests := []struct {
41+
name string
42+
path string
43+
envVar bool
44+
wantStatus string // string representation of codes.Code
45+
}{
46+
{
47+
name: "missing_leading_slash_disableStrictPathChecking_false",
48+
path: "grpc.testing.TestService/UnaryCall",
49+
wantStatus: "12", // Unimplemented
50+
},
51+
{
52+
name: "empty_path_disableStrictPathChecking_false",
53+
path: "",
54+
wantStatus: "12", // Unimplemented
55+
},
56+
{
57+
name: "just_slash_disableStrictPathChecking_false",
58+
path: "/",
59+
wantStatus: "12", // Unimplemented
60+
},
61+
{
62+
name: "missing_leading_slash_disableStrictPathChecking_true",
63+
path: "grpc.testing.TestService/UnaryCall",
64+
envVar: true,
65+
wantStatus: "0", // OK
66+
},
67+
{
68+
name: "empty_path_disableStrictPathChecking_true",
69+
path: "",
70+
envVar: true,
71+
wantStatus: "12", // Unimplemented
72+
},
73+
{
74+
name: "just_slash_disableStrictPathChecking_true",
75+
path: "/",
76+
envVar: true,
77+
wantStatus: "12", // Unimplemented
78+
},
79+
}
80+
81+
for _, tc := range tests {
82+
t.Run(tc.name, func(t *testing.T) {
83+
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
84+
defer cancel()
85+
86+
testutils.SetEnvConfig(t, &envconfig.DisableStrictPathChecking, tc.envVar)
87+
88+
ss := &stubserver.StubServer{
89+
UnaryCallF: func(context.Context, *testpb.SimpleRequest) (*testpb.SimpleResponse, error) {
90+
return &testpb.SimpleResponse{Payload: &testpb.Payload{Body: []byte("pwned")}}, nil
91+
},
92+
}
93+
if err := ss.Start(nil); err != nil {
94+
t.Fatalf("Error starting endpoint server: %v", err)
95+
}
96+
defer ss.Stop()
97+
98+
// Open a raw TCP connection to the server and speak HTTP/2 directly.
99+
tcpConn, err := net.Dial("tcp", ss.Address)
100+
if err != nil {
101+
t.Fatalf("Failed to dial tcp: %v", err)
102+
}
103+
defer tcpConn.Close()
104+
105+
// Write the HTTP/2 connection preface and the initial settings frame.
106+
if _, err := tcpConn.Write([]byte("PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n")); err != nil {
107+
t.Fatalf("Failed to write preface: %v", err)
108+
}
109+
framer := http2.NewFramer(tcpConn, tcpConn)
110+
if err := framer.WriteSettings(); err != nil {
111+
t.Fatalf("Failed to write settings: %v", err)
112+
}
113+
114+
// Encode and write the HEADERS frame.
115+
var headerBuf bytes.Buffer
116+
enc := hpack.NewEncoder(&headerBuf)
117+
writeHeader := func(name, value string) {
118+
enc.WriteField(hpack.HeaderField{Name: name, Value: value})
119+
}
120+
writeHeader(":method", "POST")
121+
writeHeader(":scheme", "http")
122+
writeHeader(":authority", ss.Address)
123+
writeHeader(":path", tc.path)
124+
writeHeader("content-type", "application/grpc")
125+
writeHeader("te", "trailers")
126+
if err := framer.WriteHeaders(http2.HeadersFrameParam{
127+
StreamID: 1,
128+
BlockFragment: headerBuf.Bytes(),
129+
EndStream: false,
130+
EndHeaders: true,
131+
}); err != nil {
132+
t.Fatalf("Failed to write headers: %v", err)
133+
}
134+
135+
// Send a small gRPC-encoded data frame (0 length).
136+
if err := framer.WriteData(1, true, []byte{0, 0, 0, 0, 0}); err != nil {
137+
t.Fatalf("Failed to write data: %v", err)
138+
}
139+
140+
// Read responses and look for grpc-status.
141+
gotStatus := ""
142+
dec := hpack.NewDecoder(4096, func(f hpack.HeaderField) {
143+
if f.Name == "grpc-status" {
144+
gotStatus = f.Value
145+
}
146+
})
147+
done := make(chan struct{})
148+
go func() {
149+
defer close(done)
150+
for {
151+
frame, err := framer.ReadFrame()
152+
if err != nil {
153+
return
154+
}
155+
if headers, ok := frame.(*http2.HeadersFrame); ok {
156+
if _, err := dec.Write(headers.HeaderBlockFragment()); err != nil {
157+
return
158+
}
159+
if headers.StreamEnded() {
160+
return
161+
}
162+
}
163+
}
164+
}()
165+
166+
select {
167+
case <-done:
168+
case <-ctx.Done():
169+
t.Fatalf("Timed out waiting for response")
170+
}
171+
172+
if gotStatus != tc.wantStatus {
173+
t.Errorf("Got grpc-status %v, want %v", gotStatus, tc.wantStatus)
174+
}
175+
})
176+
}
177+
}

‎testdata/spiffe_end2end/README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
All of the following files in this directory except `server_spiffebundle.json`
22
and `client_spiffebundle.json` are generated with the `generate.sh` and
3-
`generate_intermediate.sh` script in this directory.
3+
`intermediate_gen.sh` script in this directory.
44

55
These comprise a root trust certificate authority (CA) that signs two
66
certificates - `client_spiffe.pem` and `server_spiffe.pem`. These are valid
@@ -28,7 +28,7 @@ certificate by getting the RSA key from the cert and extracting the value. This
2828
can be done in golang with the following codeblock:
2929

3030
```
31-
func(GetBase64ModulusFromPublicKey(key *rsa.PublicKey) string {
31+
func GetBase64ModulusFromPublicKey(key *rsa.PublicKey) string {
3232
return base64.RawURLEncoding.EncodeToString(key.N.Bytes())
3333
}
3434

0 commit comments

Comments
 (0)