From 90f8c4378f4a00d5cf5a927e4fc367edd25c3ff2 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 1 Oct 2026 23:12:23 +0800 Subject: [PATCH] s3: restrict admin gRPC to local callers when no signing key (#11530) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * s3: restrict admin gRPC to local callers when no signing key The S3 gateway's gRPC port (default 0.0.0.0:19000, always on) serves the IAM cache and internal lifecycle admin services. checkAdminAuth was a no-op when jwt.filer_signing.key was unset, so any reachable host could PutIdentity an admin identity and take over the bucket data. Without a shared key callers cannot be distinguished, so admin RPCs are now limited to unix-socket, loopback, and the server's own interface addresses. Remote filer-to-S3 propagation and lifecycle workers must set jwt.filer_signing.key; the Bearer-token path is unchanged. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: fail closed on nil guard and refresh local addresses per call Review feedback: a nil filerGuard bypassed all checks — treat it like a missing key and require a local peer. The own-address set was cached forever, so interfaces added later were rejected; enumerate per call instead since admin RPCs are rare. Nil ctx is denied rather than panics. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: read the signing key once and bound interface enumeration Review feedback: reading SigningKey twice could straddle a SIGHUP reload — an old nonempty key skipped the local-peer check while the new empty key verified the token. And enumerating interfaces per no-key call is wasteful for co-located workers dialing the announced address; cache the address set for 30s so new interfaces still become usable promptly. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: enumerate interface addresses per no-key admin call A cached address set keeps trusting an IP after it is removed from the host and reassigned to another machine — that host would then hold unauthenticated admin access for the cache TTL. Per-call enumeration only runs for non-loopback TCP peers on the no-key path, which is low-volume admin traffic, so the freshness is worth the syscall. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- weed/s3api/s3api_internal_lifecycle_test.go | 24 ++++-- weed/s3api/s3api_server_grpc.go | 58 +++++++++++-- weed/s3api/s3api_server_grpc_test.go | 92 +++++++++++++++++++-- 3 files changed, 158 insertions(+), 16 deletions(-) diff --git a/weed/s3api/s3api_internal_lifecycle_test.go b/weed/s3api/s3api_internal_lifecycle_test.go index b2a518c22..54d44a49e 100644 --- a/weed/s3api/s3api_internal_lifecycle_test.go +++ b/weed/s3api/s3api_internal_lifecycle_test.go @@ -3,6 +3,7 @@ package s3api import ( "bytes" "context" + "net" "testing" "github.com/prometheus/client_golang/prometheus/testutil" @@ -12,6 +13,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/security" stats_collect "github.com/seaweedfs/seaweedfs/weed/stats" "google.golang.org/grpc/codes" + "google.golang.org/grpc/peer" "google.golang.org/grpc/status" ) @@ -115,7 +117,7 @@ func TestIdentityMatches_AllFieldsCompared(t *testing.T) { func TestLifecycleDelete_RejectsEmptyRequest(t *testing.T) { s := &S3ApiServer{} - resp, err := s.LifecycleDelete(nil, &s3_lifecycle_pb.LifecycleDeleteRequest{}) + resp, err := s.LifecycleDelete(s3LocalPeerCtx(), &s3_lifecycle_pb.LifecycleDeleteRequest{}) if err != nil { t.Fatalf("unexpected gRPC error: %v", err) } @@ -138,7 +140,7 @@ func TestLifecycleAbortMPU_RejectsTraversalUploadIDs(t *testing.T) { } for _, path := range cases { t.Run(path, func(t *testing.T) { - resp, err := s.LifecycleDelete(nil, &s3_lifecycle_pb.LifecycleDeleteRequest{ + resp, err := s.LifecycleDelete(s3LocalPeerCtx(), &s3_lifecycle_pb.LifecycleDeleteRequest{ Bucket: "bk", ObjectPath: path, ActionKind: s3_lifecycle_pb.ActionKind_ABORT_MPU, @@ -353,11 +355,23 @@ func TestLifecycleDelete_RequiresAdminAuth(t *testing.T) { } } -func TestLifecycleDelete_NoSigningKey_Allowed(t *testing.T) { +func TestLifecycleDelete_NoSigningKey_LocalPeerOnly(t *testing.T) { s := newTestS3IamCacheServer(t, "") - resp, err := s.LifecycleDelete(context.Background(), &s3_lifecycle_pb.LifecycleDeleteRequest{}) + _, err := s.LifecycleDelete(context.Background(), &s3_lifecycle_pb.LifecycleDeleteRequest{}) + if got, want := status.Code(err), codes.Unauthenticated; got != want { + t.Fatalf("LifecycleDelete with no peer info: got code %v, want %v (err=%v)", got, want, err) + } + remoteCtx := peer.NewContext(context.Background(), + &peer.Peer{Addr: &net.TCPAddr{IP: net.ParseIP("203.0.113.7"), Port: 11111}}) + _, err = s.LifecycleDelete(remoteCtx, &s3_lifecycle_pb.LifecycleDeleteRequest{}) + if got, want := status.Code(err), codes.Unauthenticated; got != want { + t.Fatalf("LifecycleDelete from remote peer: got code %v, want %v (err=%v)", got, want, err) + } + localCtx := peer.NewContext(context.Background(), + &peer.Peer{Addr: &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 11111}}) + resp, err := s.LifecycleDelete(localCtx, &s3_lifecycle_pb.LifecycleDeleteRequest{}) if err != nil { - t.Fatalf("LifecycleDelete without signing key: unexpected error %v", err) + t.Fatalf("LifecycleDelete from loopback without signing key: unexpected error %v", err) } if resp == nil || resp.Outcome != s3_lifecycle_pb.LifecycleDeleteOutcome_BLOCKED { t.Fatalf("expected BLOCKED for empty request without signing key, got %v", resp) diff --git a/weed/s3api/s3api_server_grpc.go b/weed/s3api/s3api_server_grpc.go index 61aecf679..059bb14ba 100644 --- a/weed/s3api/s3api_server_grpc.go +++ b/weed/s3api/s3api_server_grpc.go @@ -2,6 +2,7 @@ package s3api import ( "context" + "net" "strings" "github.com/seaweedfs/seaweedfs/weed/glog" @@ -9,6 +10,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/security" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" + "google.golang.org/grpc/peer" "google.golang.org/grpc/status" ) @@ -19,15 +21,16 @@ import ( // checkAdminAuth verifies the caller presented a Bearer token signed by the // filer write-signing key (jwt.filer_signing.key). It mirrors the filer's // IamGrpcServer.checkAdminAuth so the same operator knob that locks down the -// filer IAM gRPC service also locks down this cache. With no key configured the -// check is a no-op, matching the rest of SeaweedFS's gRPC surface. +// filer IAM gRPC service also locks down this cache. With no key configured +// remote callers cannot be told apart, so only local clients (unix socket, +// loopback, or the server's own addresses) are allowed. func (s3a *S3ApiServer) checkAdminAuth(ctx context.Context) error { - if s3a.filerGuard == nil { - return nil + var signingKey security.SigningKey + if s3a.filerGuard != nil { + signingKey = s3a.filerGuard.SigningKey() } - signingKey := s3a.filerGuard.SigningKey() if len(signingKey) == 0 { - return nil + return checkLocalPeer(ctx) } md, ok := metadata.FromIncomingContext(ctx) if !ok { @@ -52,6 +55,49 @@ func (s3a *S3ApiServer) checkAdminAuth(ctx context.Context) error { return nil } +func checkLocalPeer(ctx context.Context) error { + if ctx == nil { + return status.Error(codes.Unauthenticated, "admin gRPC calls require jwt.filer_signing.key or a local client") + } + pr, ok := peer.FromContext(ctx) + if !ok { + return status.Error(codes.Unauthenticated, "admin gRPC calls require jwt.filer_signing.key or a local client") + } + if _, isUnix := pr.Addr.(*net.UnixAddr); isUnix { + return nil + } + var ip net.IP + if tcpAddr, ok := pr.Addr.(*net.TCPAddr); ok { + ip = tcpAddr.IP + } else if h, _, err := net.SplitHostPort(pr.Addr.String()); err == nil { + ip = net.ParseIP(h) + } else { + ip = net.ParseIP(pr.Addr.String()) + } + if ip != nil && (ip.IsLoopback() || isLocalAddress(ip)) { + return nil + } + glog.V(1).Infof("rejected unauthenticated admin gRPC call from %s: no jwt.filer_signing.key configured", pr.Addr) + return status.Error(codes.Unauthenticated, "admin gRPC calls require jwt.filer_signing.key or a local client") +} + +// isLocalAddress enumerates interfaces per call: only reachable for +// non-loopback TCP peers on the no-key admin path, which is low-volume — +// and a cached set would keep trusting an address after it is removed from +// the host and reassigned. +func isLocalAddress(ip net.IP) bool { + addrs, err := net.InterfaceAddrs() + if err != nil { + return false + } + for _, a := range addrs { + if ipNet, ok := a.(*net.IPNet); ok && ipNet.IP.Equal(ip) { + return true + } + } + return false +} + func (s3a *S3ApiServer) PutIdentity(ctx context.Context, req *iam_pb.PutIdentityRequest) (*iam_pb.PutIdentityResponse, error) { if err := s3a.checkAdminAuth(ctx); err != nil { return nil, err diff --git a/weed/s3api/s3api_server_grpc_test.go b/weed/s3api/s3api_server_grpc_test.go index f40971ee4..1b3cf8901 100644 --- a/weed/s3api/s3api_server_grpc_test.go +++ b/weed/s3api/s3api_server_grpc_test.go @@ -2,6 +2,7 @@ package s3api import ( "context" + "net" "testing" "time" @@ -10,6 +11,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/security" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" + "google.golang.org/grpc/peer" "google.golang.org/grpc/status" ) @@ -27,6 +29,11 @@ func s3IamCacheBearerCtx(token string) context.Context { return metadata.NewIncomingContext(context.Background(), md) } +func s3LocalPeerCtx() context.Context { + return peer.NewContext(context.Background(), + &peer.Peer{Addr: &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 1}}) +} + func TestS3IamCache_NoMetadata_Unauthenticated(t *testing.T) { s := newTestS3IamCacheServer(t, testS3IamCacheSigningKey) _, err := s.PutIdentity(context.Background(), &iam_pb.PutIdentityRequest{ @@ -133,22 +140,97 @@ func TestS3IamCache_ValidToken_WritesIdentity(t *testing.T) { } } -func TestS3IamCache_NoSigningKey_Unauthenticated_Allowed(t *testing.T) { +// Without a signing key remote callers cannot be authenticated, so admin +// RPCs are restricted to local clients: unix socket, loopback, or the +// server's own interface addresses. Remote peers and missing peer info are +// denied. +func TestS3IamCache_NoSigningKey_RemotePeer_Unauthenticated(t *testing.T) { s := newTestS3IamCacheServer(t, "") _, err := s.PutIdentity(context.Background(), &iam_pb.PutIdentityRequest{ - Identity: &iam_pb.Identity{Name: "pushed", Actions: []string{"Read"}}, + Identity: &iam_pb.Identity{Name: "pwn", Actions: []string{"Admin"}}, }) - if err != nil { - t.Fatalf("PutIdentity without key: unexpected error %v", err) + if got, want := status.Code(err), codes.Unauthenticated; got != want { + t.Fatalf("PutIdentity with no peer info: got code %v, want %v (err=%v)", got, want, err) + } + remoteCtx := peer.NewContext(context.Background(), + &peer.Peer{Addr: &net.TCPAddr{IP: net.ParseIP("203.0.113.7"), Port: 11111}}) + _, err = s.PutIdentity(remoteCtx, &iam_pb.PutIdentityRequest{ + Identity: &iam_pb.Identity{Name: "pwn", Actions: []string{"Admin"}}, + }) + if got, want := status.Code(err), codes.Unauthenticated; got != want { + t.Fatalf("PutIdentity from remote peer without key: got code %v, want %v (err=%v)", got, want, err) + } + + unguarded := &S3ApiServer{} + if _, err := unguarded.PutIdentity(remoteCtx, &iam_pb.PutIdentityRequest{ + Identity: &iam_pb.Identity{Name: "pwn", Actions: []string{"Admin"}}, + }); status.Code(err) != codes.Unauthenticated { + t.Fatalf("PutIdentity from remote peer with nil guard: got code %v, want Unauthenticated", status.Code(err)) + } +} + +func TestS3IamCache_NoSigningKey_LocalPeer_Allowed(t *testing.T) { + s := newTestS3IamCacheServer(t, "") + for name, addr := range map[string]net.Addr{ + "unix": &net.UnixAddr{Name: "/tmp/x.sock", Net: "unix"}, + "loopback": &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 11111}, + "loopbackV6": &net.TCPAddr{IP: net.ParseIP("::1"), Port: 11111}, + } { + ctx := peer.NewContext(context.Background(), &peer.Peer{Addr: addr}) + if _, err := s.PutIdentity(ctx, &iam_pb.PutIdentityRequest{ + Identity: &iam_pb.Identity{Name: "pushed-" + name, Actions: []string{"Read"}}, + }); err != nil { + t.Fatalf("PutIdentity from %s peer without key: unexpected error %v", name, err) + } + } + if own := firstLocalTCPAddr(t); own != nil { + ctx := peer.NewContext(context.Background(), &peer.Peer{Addr: own}) + if _, err := s.PutIdentity(ctx, &iam_pb.PutIdentityRequest{ + Identity: &iam_pb.Identity{Name: "pushed-own-ip", Actions: []string{"Read"}}, + }); err != nil { + t.Fatalf("PutIdentity from own interface address: unexpected error %v", err) + } } good := security.GenJwtForFilerAdmin(security.SigningKey(testS3IamCacheSigningKey), 60) - if _, err := s.PutIdentity(s3IamCacheBearerCtx(string(good)), &iam_pb.PutIdentityRequest{ + loopbackCtx := peer.NewContext(context.Background(), + &peer.Peer{Addr: &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 11111}}) + ctx := metadata.NewIncomingContext(loopbackCtx, metadata.New(map[string]string{"authorization": "Bearer " + string(good)})) + if _, err := s.PutIdentity(ctx, &iam_pb.PutIdentityRequest{ Identity: &iam_pb.Identity{Name: "pushed2", Actions: []string{"Read"}}, }); err != nil { t.Fatalf("PutIdentity with stray token but no server key: unexpected error %v", err) } } +// With a signing key only a valid Bearer token is accepted; being local does +// not bypass it. +func TestS3IamCache_WithKey_LocalPeerStillNeedsToken(t *testing.T) { + s := newTestS3IamCacheServer(t, testS3IamCacheSigningKey) + ctx := peer.NewContext(context.Background(), + &peer.Peer{Addr: &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 11111}}) + _, err := s.PutIdentity(ctx, &iam_pb.PutIdentityRequest{ + Identity: &iam_pb.Identity{Name: "pwn", Actions: []string{"Admin"}}, + }) + if got, want := status.Code(err), codes.Unauthenticated; got != want { + t.Fatalf("PutIdentity from loopback without token: got code %v, want %v (err=%v)", got, want, err) + } +} + +func firstLocalTCPAddr(t *testing.T) *net.TCPAddr { + t.Helper() + addrs, err := net.InterfaceAddrs() + if err != nil { + t.Logf("interface addresses unavailable, skipping own-address case: %v", err) + return nil + } + for _, a := range addrs { + if ipNet, ok := a.(*net.IPNet); ok && !ipNet.IP.IsLoopback() { + return &net.TCPAddr{IP: ipNet.IP, Port: 11111} + } + } + return nil +} + func TestS3IamCache_RemoveIdentity_RequiresAuth(t *testing.T) { s := newTestS3IamCacheServer(t, testS3IamCacheSigningKey) _, err := s.RemoveIdentity(context.Background(), &iam_pb.RemoveIdentityRequest{Username: "anyone"})