diff --git a/server/server.go b/server/server.go index 31321c9a45..83ec912fe6 100644 --- a/server/server.go +++ b/server/server.go @@ -948,6 +948,8 @@ func (server *ArgoCDServer) newGRPCServer(prometheusRegistry *prometheus.Registr // NOTE: notice we do not configure the gRPC server here with TLS (e.g. grpc.Creds(creds)) // This is because TLS handshaking occurs in cmux handling sOpts = append(sOpts, grpc.ChainStreamInterceptor( + // for mitigation of grpc-go CVE-2026-33186, see https://github.com/argoproj/argo-cd/issues/26932 + grpc_util.InvalidMethodNameErrorStreamServerInterceptor(), logging.StreamServerInterceptor(grpc_util.InterceptorLogger(server.log)), serverMetrics.StreamServerInterceptor(), grpc_auth.StreamServerInterceptor(server.Authenticate), @@ -960,6 +962,8 @@ func (server *ArgoCDServer) newGRPCServer(prometheusRegistry *prometheus.Registr recovery.StreamServerInterceptor(recovery.WithRecoveryHandler(grpc_util.LoggerRecoveryHandler(server.log))), )) sOpts = append(sOpts, grpc.ChainUnaryInterceptor( + // for mitigation of grpc-go CVE-2026-33186, see https://github.com/argoproj/argo-cd/issues/26932 + grpc_util.InvalidMethodNameErrorUnaryServerInterceptor(), bug21955WorkaroundInterceptor, logging.UnaryServerInterceptor(grpc_util.InterceptorLogger(server.log)), serverMetrics.UnaryServerInterceptor(), diff --git a/server/server_test.go b/server/server_test.go index bc11f0f71c..63f484e824 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -18,6 +18,7 @@ import ( log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "google.golang.org/grpc" "google.golang.org/grpc/metadata" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -29,6 +30,7 @@ import ( "github.com/argoproj/argo-cd/v3/common" "github.com/argoproj/argo-cd/v3/pkg/apiclient" + "github.com/argoproj/argo-cd/v3/pkg/apiclient/project" "github.com/argoproj/argo-cd/v3/pkg/apiclient/session" "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1" apps "github.com/argoproj/argo-cd/v3/pkg/client/clientset/versioned/fake" @@ -40,6 +42,10 @@ import ( "github.com/argoproj/argo-cd/v3/util/cache" appstatecache "github.com/argoproj/argo-cd/v3/util/cache/appstate" "github.com/argoproj/argo-cd/v3/util/oidc" + + "google.golang.org/grpc/credentials/insecure" + + grpc_util "github.com/argoproj/argo-cd/v3/util/grpc" "github.com/argoproj/argo-cd/v3/util/rbac" settings_util "github.com/argoproj/argo-cd/v3/util/settings" testutil "github.com/argoproj/argo-cd/v3/util/test" @@ -1710,3 +1716,101 @@ func Test_StaticAssetsDir_no_symlink_traversal(t *testing.T) { resp = w.Result() assert.Equal(t, http.StatusOK, resp.StatusCode, "should have been able to access the normal file") } + +// test mitigation for grpc-go CVE-2026-33186, see https://github.com/argoproj/argo-cd/issues/26932 +func TestGrpcInvalidMethodNameCVEFix(t *testing.T) { + timeout := 10 * time.Second + listenHost := "localhost" + listenPort, err := test.GetFreePort() + require.NoError(t, err) + serverAddr := fmt.Sprintf("%s:%d", listenHost, listenPort) + redis, redisCloser := test.NewInMemoryRedis() + defer redisCloser() + argoCDOpts := ArgoCDServerOpts{ + DisableAuth: true, + Insecure: true, + ListenPort: listenPort, + ListenHost: listenHost, + Namespace: test.FakeArgoCDNamespace, + KubeClientset: fake.NewSimpleClientset(test.NewFakeConfigMap(), test.NewFakeSecret()), + AppClientset: apps.NewSimpleClientset(), + RepoClientset: &mocks.Clientset{RepoServerServiceClient: &mocks.RepoServerServiceClient{}}, + RedisClient: redis, + } + runCtx, runCancel := context.WithTimeout(t.Context(), timeout) + defer runCancel() + argocd := NewServer(runCtx, argoCDOpts, ApplicationSetOpts{}) + assert.NotNil(t, argocd) + listeners, err := argocd.Listen() + require.NoError(t, err) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + argocd.Init(ctx) + + wg := gosync.WaitGroup{} + wg.Add(1) + go func() { + defer wg.Done() + argocd.Run(ctx, listeners) + }() + + err = test.WaitForPortListen(serverAddr, timeout) + require.NoError(t, err) + + var dialOpts []grpc.DialOption + creds := insecure.NewCredentials() + conn, err := grpc_util.BlockingNewClient(ctx, "tcp", serverAddr, creds, dialOpts...) + require.NoError(t, err) + defer conn.Close() + + projectGetOut := new(v1alpha1.AppProject) + projectGetIn := &project.ProjectQuery{Name: "default"} + invalidunaryServiceName := "project.ProjectService/Get" + invalidStreamingMethodName := "application.ApplicationService/GetManifestsWithFiles" + + streamDesc := &grpc.StreamDesc{ + StreamName: "dummy_stream", + ClientStreams: true, + ServerStreams: false, + } + + t.Run("unary method with invalid name", func(t *testing.T) { + err = conn.Invoke(ctx, invalidunaryServiceName, projectGetIn, projectGetOut) + // it should fail with the "malformed method name" error message from interceptor, + // but it does not, because unary methods do not seem to be vulnerable because of + // the way their handler code is autogenerated: if there are interceptors + // it implicitly sanitizes the service name before calling the actual handler, + require.NoError(t, err) + }) + t.Run("unary method with valid name", func(t *testing.T) { + err = conn.Invoke(ctx, "/"+invalidunaryServiceName, projectGetIn, projectGetOut) + require.NoError(t, err) + }) + t.Run("streaming method with invalid name", func(t *testing.T) { + stream, err := conn.NewStream(ctx, streamDesc, invalidStreamingMethodName) + require.NoError(t, err) + err = stream.CloseSend() + require.NoError(t, err) + var resp any + err = stream.RecvMsg(&resp) + // ensure we get error method from interceptor + require.ErrorContains(t, err, "code = InvalidArgument desc = malformed method name: \""+invalidStreamingMethodName+"\"") + }) + t.Run("streaming method with valid name", func(t *testing.T) { + stream, err := conn.NewStream(ctx, streamDesc, "/"+invalidStreamingMethodName) + require.NoError(t, err) + err = stream.CloseSend() + require.NoError(t, err) + var resp any + err = stream.RecvMsg(&resp) + // ensure we get the expected error from the actual logic of the method + require.ErrorContains(t, err, "code = Unknown desc = error getting query: failed to receive header: EOF") + }) + argocd.stopCh <- syscall.SIGINT + wg.Wait() + + err = argocd.healthCheck(&http.Request{URL: &url.URL{Path: "/healthz", RawQuery: "full=true"}}) + require.Error(t, err, "API Server is terminating and unable to serve requests.") + assert.True(t, argocd.terminateRequested.Load()) + assert.False(t, argocd.available.Load()) +} diff --git a/util/grpc/errors.go b/util/grpc/errors.go index bf4230cb98..15d7311f8c 100644 --- a/util/grpc/errors.go +++ b/util/grpc/errors.go @@ -3,6 +3,8 @@ package grpc import ( "context" "errors" + "fmt" + "strings" giterr "github.com/go-git/go-git/v5/plumbing/transport" "google.golang.org/grpc" @@ -132,3 +134,25 @@ func ErrorCodeK8sStreamServerInterceptor() grpc.StreamServerInterceptor { return kubeErrToGRPC(err) } } + +// InvalidMethodNameErrorUnaryServerInterceptor is for mitigation of grpc-go CVE-2026-33186 +// see discussion in https://github.com/argoproj/argo-cd/issues/26932 +func InvalidMethodNameErrorUnaryServerInterceptor() grpc.UnaryServerInterceptor { + return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp any, err error) { + if !strings.HasPrefix(info.FullMethod, "/") { + return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("malformed method name: %q", info.FullMethod)) + } + return handler(ctx, req) + } +} + +// InvalidMethodNameErrorStreamServerInterceptor is for mitigation of grpc-go CVE-2026-33186 +// see discussion in https://github.com/argoproj/argo-cd/issues/26932 +func InvalidMethodNameErrorStreamServerInterceptor() grpc.StreamServerInterceptor { + return func(srv any, ss grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error { + if !strings.HasPrefix(info.FullMethod, "/") { + return status.Error(codes.InvalidArgument, fmt.Sprintf("malformed method name: %q", info.FullMethod)) + } + return handler(srv, ss) + } +} diff --git a/util/grpc/errors_test.go b/util/grpc/errors_test.go index 5e6bfd7b69..d16b178727 100644 --- a/util/grpc/errors_test.go +++ b/util/grpc/errors_test.go @@ -1,10 +1,12 @@ package grpc import ( + "context" "errors" "fmt" "testing" + "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -153,3 +155,56 @@ func Test_kubeErrToGRPC(t *testing.T) { }) } } + +func checkGrpcError(t *testing.T, err error, msg string) { + t.Helper() + require.Error(t, err) + s, ok := status.FromError(err) + assert.True(t, ok) + assert.Equal(t, codes.InvalidArgument, s.Code()) + assert.ErrorContains(t, err, msg) +} + +func TestInvalidMethodNameErrorUnaryServerInterceptor(t *testing.T) { + interceptor := InvalidMethodNameErrorUnaryServerInterceptor() + handler := func(_ context.Context, _ any) (any, error) { + return nil, nil + } + t.Run("Test invalid method name", func(t *testing.T) { + info := &grpc.UnaryServerInfo{FullMethod: "foo"} + _, err := interceptor(t.Context(), nil, info, handler) + checkGrpcError(t, err, "malformed method name: \"foo\"") + }) + t.Run("Test empty method name", func(t *testing.T) { + info := &grpc.UnaryServerInfo{FullMethod: ""} + _, err := interceptor(t.Context(), nil, info, handler) + checkGrpcError(t, err, "malformed method name: \"\"") + }) + t.Run("Test valid method name", func(t *testing.T) { + info := &grpc.UnaryServerInfo{FullMethod: "/foo"} + _, err := interceptor(t.Context(), nil, info, handler) + assert.NoError(t, err) + }) +} + +func TestInvalidMethodNameErrorStreamServerInterceptor(t *testing.T) { + interceptor := InvalidMethodNameErrorStreamServerInterceptor() + handler := func(_ any, _ grpc.ServerStream) error { + return nil + } + t.Run("Test invalid method name", func(t *testing.T) { + info := &grpc.StreamServerInfo{FullMethod: "foo"} + err := interceptor(t.Context(), nil, info, handler) + checkGrpcError(t, err, "malformed method name: \"foo\"") + }) + t.Run("Test empty method name", func(t *testing.T) { + info := &grpc.StreamServerInfo{FullMethod: ""} + err := interceptor(t.Context(), nil, info, handler) + checkGrpcError(t, err, "malformed method name: \"\"") + }) + t.Run("Test valid method name", func(t *testing.T) { + info := &grpc.StreamServerInfo{FullMethod: "/foo"} + err := interceptor(nil, nil, info, handler) + assert.NoError(t, err) + }) +}