diff --git a/internal/envconfig/envconfig.go b/internal/envconfig/envconfig.go index 51add6c0295a..e3adb91d4c4b 100644 --- a/internal/envconfig/envconfig.go +++ b/internal/envconfig/envconfig.go @@ -103,6 +103,22 @@ var ( // environment variable "GRPC_GO_EXPERIMENTAL_XDS_RESOURCE_PANIC_RECOVERY" // to "false". XDSRecoverPanicInResourceParsing = boolFromEnv("GRPC_GO_EXPERIMENTAL_XDS_RESOURCE_PANIC_RECOVERY", true) + + // DisableStrictPathChecking indicates whether strict path checking is + // disabled. This feature can be disabled by setting the environment + // variable GRPC_GO_EXPERIMENTAL_DISABLE_STRICT_PATH_CHECKING to "true". + // + // When strict path checking is enabled, gRPC will reject requests with + // paths that do not conform to the gRPC over HTTP/2 specification found at + // https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md. + // + // When disabled, gRPC will allow paths that do not contain a leading slash. + // Enabling strict path checking is recommended for security reasons, as it + // prevents potential path traversal vulnerabilities. + // + // A future release will remove this environment variable, enabling strict + // path checking behavior unconditionally. + DisableStrictPathChecking = boolFromEnv("GRPC_GO_EXPERIMENTAL_DISABLE_STRICT_PATH_CHECKING", false) ) func boolFromEnv(envVar string, def bool) bool { diff --git a/server.go b/server.go index 9e8295c160e2..5229adf71174 100644 --- a/server.go +++ b/server.go @@ -42,6 +42,7 @@ import ( "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/binarylog" "google.golang.org/grpc/internal/channelz" + "google.golang.org/grpc/internal/envconfig" "google.golang.org/grpc/internal/grpcsync" "google.golang.org/grpc/internal/grpcutil" istats "google.golang.org/grpc/internal/stats" @@ -149,6 +150,8 @@ type Server struct { serverWorkerChannel chan func() serverWorkerChannelClose func() + + strictPathCheckingLogEmitted atomic.Bool } type serverOptions struct { @@ -1763,6 +1766,24 @@ func (s *Server) processStreamingRPC(ctx context.Context, stream *transport.Serv return ss.s.WriteStatus(statusOK) } +func (s *Server) handleMalformedMethodName(stream *transport.ServerStream, ti *traceInfo) { + if ti != nil { + ti.tr.LazyLog(&fmtStringer{"Malformed method name %q", []any{stream.Method()}}, true) + ti.tr.SetError() + } + errDesc := fmt.Sprintf("malformed method name: %q", stream.Method()) + if err := stream.WriteStatus(status.New(codes.Unimplemented, errDesc)); err != nil { + if ti != nil { + ti.tr.LazyLog(&fmtStringer{"%v", []any{err}}, true) + ti.tr.SetError() + } + channelz.Warningf(logger, s.channelz, "grpc: Server.handleStream failed to write status: %v", err) + } + if ti != nil { + ti.tr.Finish() + } +} + func (s *Server) handleStream(t transport.ServerTransport, stream *transport.ServerStream) { ctx := stream.Context() ctx = contextWithServer(ctx, s) @@ -1783,26 +1804,30 @@ func (s *Server) handleStream(t transport.ServerTransport, stream *transport.Ser } sm := stream.Method() - if sm != "" && sm[0] == '/' { + if sm == "" { + s.handleMalformedMethodName(stream, ti) + return + } + if sm[0] != '/' { + // TODO(easwars): Add a link to the CVE in the below log messages once + // published. + if envconfig.DisableStrictPathChecking { + if old := s.strictPathCheckingLogEmitted.Swap(true); !old { + 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) + } + } else { + if old := s.strictPathCheckingLogEmitted.Swap(true); !old { + 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) + } + s.handleMalformedMethodName(stream, ti) + return + } + } else { sm = sm[1:] } pos := strings.LastIndex(sm, "/") if pos == -1 { - if ti != nil { - ti.tr.LazyLog(&fmtStringer{"Malformed method name %q", []any{sm}}, true) - ti.tr.SetError() - } - errDesc := fmt.Sprintf("malformed method name: %q", stream.Method()) - if err := stream.WriteStatus(status.New(codes.Unimplemented, errDesc)); err != nil { - if ti != nil { - ti.tr.LazyLog(&fmtStringer{"%v", []any{err}}, true) - ti.tr.SetError() - } - channelz.Warningf(logger, s.channelz, "grpc: Server.handleStream failed to write status: %v", err) - } - if ti != nil { - ti.tr.Finish() - } + s.handleMalformedMethodName(stream, ti) return } service := sm[:pos] diff --git a/test/malformed_method_test.go b/test/malformed_method_test.go new file mode 100644 index 000000000000..00e391a25958 --- /dev/null +++ b/test/malformed_method_test.go @@ -0,0 +1,177 @@ +/* + * + * Copyright 2026 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package test + +import ( + "bytes" + "context" + "net" + "testing" + + "golang.org/x/net/http2" + "golang.org/x/net/http2/hpack" + "google.golang.org/grpc/internal/envconfig" + "google.golang.org/grpc/internal/stubserver" + "google.golang.org/grpc/internal/testutils" + + testpb "google.golang.org/grpc/interop/grpc_testing" +) + +// TestMalformedMethodPath tests that the server responds with Unimplemented +// when the method path is malformed. This verifies that the server does not +// route requests with a malformed method path to the application handler. +func (s) TestMalformedMethodPath(t *testing.T) { + tests := []struct { + name string + path string + envVar bool + wantStatus string // string representation of codes.Code + }{ + { + name: "missing_leading_slash_disableStrictPathChecking_false", + path: "grpc.testing.TestService/UnaryCall", + wantStatus: "12", // Unimplemented + }, + { + name: "empty_path_disableStrictPathChecking_false", + path: "", + wantStatus: "12", // Unimplemented + }, + { + name: "just_slash_disableStrictPathChecking_false", + path: "/", + wantStatus: "12", // Unimplemented + }, + { + name: "missing_leading_slash_disableStrictPathChecking_true", + path: "grpc.testing.TestService/UnaryCall", + envVar: true, + wantStatus: "0", // OK + }, + { + name: "empty_path_disableStrictPathChecking_true", + path: "", + envVar: true, + wantStatus: "12", // Unimplemented + }, + { + name: "just_slash_disableStrictPathChecking_true", + path: "/", + envVar: true, + wantStatus: "12", // Unimplemented + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + + testutils.SetEnvConfig(t, &envconfig.DisableStrictPathChecking, tc.envVar) + + ss := &stubserver.StubServer{ + UnaryCallF: func(context.Context, *testpb.SimpleRequest) (*testpb.SimpleResponse, error) { + return &testpb.SimpleResponse{Payload: &testpb.Payload{Body: []byte("pwned")}}, nil + }, + } + if err := ss.Start(nil); err != nil { + t.Fatalf("Error starting endpoint server: %v", err) + } + defer ss.Stop() + + // Open a raw TCP connection to the server and speak HTTP/2 directly. + tcpConn, err := net.Dial("tcp", ss.Address) + if err != nil { + t.Fatalf("Failed to dial tcp: %v", err) + } + defer tcpConn.Close() + + // Write the HTTP/2 connection preface and the initial settings frame. + if _, err := tcpConn.Write([]byte("PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n")); err != nil { + t.Fatalf("Failed to write preface: %v", err) + } + framer := http2.NewFramer(tcpConn, tcpConn) + if err := framer.WriteSettings(); err != nil { + t.Fatalf("Failed to write settings: %v", err) + } + + // Encode and write the HEADERS frame. + var headerBuf bytes.Buffer + enc := hpack.NewEncoder(&headerBuf) + writeHeader := func(name, value string) { + enc.WriteField(hpack.HeaderField{Name: name, Value: value}) + } + writeHeader(":method", "POST") + writeHeader(":scheme", "http") + writeHeader(":authority", ss.Address) + writeHeader(":path", tc.path) + writeHeader("content-type", "application/grpc") + writeHeader("te", "trailers") + if err := framer.WriteHeaders(http2.HeadersFrameParam{ + StreamID: 1, + BlockFragment: headerBuf.Bytes(), + EndStream: false, + EndHeaders: true, + }); err != nil { + t.Fatalf("Failed to write headers: %v", err) + } + + // Send a small gRPC-encoded data frame (0 length). + if err := framer.WriteData(1, true, []byte{0, 0, 0, 0, 0}); err != nil { + t.Fatalf("Failed to write data: %v", err) + } + + // Read responses and look for grpc-status. + gotStatus := "" + dec := hpack.NewDecoder(4096, func(f hpack.HeaderField) { + if f.Name == "grpc-status" { + gotStatus = f.Value + } + }) + done := make(chan struct{}) + go func() { + defer close(done) + for { + frame, err := framer.ReadFrame() + if err != nil { + return + } + if headers, ok := frame.(*http2.HeadersFrame); ok { + if _, err := dec.Write(headers.HeaderBlockFragment()); err != nil { + return + } + if headers.StreamEnded() { + return + } + } + } + }() + + select { + case <-done: + case <-ctx.Done(): + t.Fatalf("Timed out waiting for response") + } + + if gotStatus != tc.wantStatus { + t.Errorf("Got grpc-status %v, want %v", gotStatus, tc.wantStatus) + } + }) + } +}