mirror of
https://github.com/argoproj/argo-cd
synced 2026-04-21 17:07:16 +00:00
fix: mitigation of grpc-go CVE-2026-33186 for release-3.2 (#26983)
Signed-off-by: Eugene Doudine <eugene.doudine@octopus.com>
This commit is contained in:
dudinea
GitHub
parent
65378e6d14
commit
5fca1ce7d8
4 changed files with 187 additions and 0 deletions
|
|
@ -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(),
|
||||
|
|
|
|||
|
|
@ -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())
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
})
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue