diff --git a/weed/s3api/s3api_server_grpc.go b/weed/s3api/s3api_server_grpc.go index 3613d993de..123a5fc2f3 100644 --- a/weed/s3api/s3api_server_grpc.go +++ b/weed/s3api/s3api_server_grpc.go @@ -2,13 +2,11 @@ package s3api import ( "context" - "strings" "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" "github.com/seaweedfs/seaweedfs/weed/security" "google.golang.org/grpc/codes" - "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" ) @@ -30,27 +28,7 @@ func (s3a *S3ApiServer) checkAdminAuth(ctx context.Context) error { if len(signingKey) == 0 { return checkLocalPeer(ctx) } - md, ok := metadata.FromIncomingContext(ctx) - if !ok { - return status.Error(codes.Unauthenticated, "missing metadata") - } - authHeaders := md.Get("authorization") - if len(authHeaders) == 0 { - return status.Error(codes.Unauthenticated, "missing authorization metadata") - } - raw := strings.TrimSpace(authHeaders[0]) - parts := strings.Fields(raw) - if len(parts) != 2 || !strings.EqualFold(parts[0], "Bearer") || parts[1] == "" { - return status.Error(codes.Unauthenticated, "authorization header must use Bearer scheme") - } - parsed, err := security.DecodeJwt(signingKey, security.EncodedJwt(parts[1]), &security.SeaweedFilerAdminClaims{}) - if err != nil || parsed == nil || !parsed.Valid { - return status.Error(codes.Unauthenticated, "invalid admin token") - } - if claims, ok := parsed.Claims.(*security.SeaweedFilerAdminClaims); !ok || claims.SessionId != "" { - return status.Error(codes.Unauthenticated, "invalid admin token") - } - return nil + return security.CheckGrpcAdminToken(ctx, signingKey) } func checkLocalPeer(ctx context.Context) error { diff --git a/weed/s3api/s3api_server_grpc_test.go b/weed/s3api/s3api_server_grpc_test.go index 1b3cf89018..10113cad2f 100644 --- a/weed/s3api/s3api_server_grpc_test.go +++ b/weed/s3api/s3api_server_grpc_test.go @@ -140,6 +140,27 @@ func TestS3IamCache_ValidToken_WritesIdentity(t *testing.T) { } } +func TestS3IamCache_ValidTokenFollowingStaleToken_Succeeds(t *testing.T) { + s := newTestS3IamCacheServer(t, testS3IamCacheSigningKey) + bad := security.GenJwtForFilerAdmin(security.SigningKey("a-different-key"), 60) + good := security.GenJwtForFilerAdmin(security.SigningKey(testS3IamCacheSigningKey), 60) + multiCtx := metadata.NewIncomingContext(context.Background(), metadata.MD{ + "authorization": []string{ + security.BearerPrefix + string(bad), + security.BearerPrefix + string(good), + }, + }) + _, err := s.PutIdentity(multiCtx, &iam_pb.PutIdentityRequest{ + Identity: &iam_pb.Identity{ + Name: "pushed_multi", + Actions: []string{"Admin"}, + }, + }) + if err != nil { + t.Fatalf("PutIdentity with valid token following stale token: unexpected error %v", err) + } +} + // 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 diff --git a/weed/security/grpc_peer.go b/weed/security/grpc_peer.go index 9a2844067d..268d09edcf 100644 --- a/weed/security/grpc_peer.go +++ b/weed/security/grpc_peer.go @@ -25,6 +25,18 @@ func CheckGrpcAdminPeer(ctx context.Context, signingKey SigningKey) error { if len(signingKey) == 0 { return CheckLocalGrpcPeer(ctx) } + return CheckGrpcAdminToken(ctx, signingKey) +} + +// maxGrpcAdminAuthValues bounds the JWT parses one request can trigger; a +// caller sends its own token plus at most one forwarded from its context. +const maxGrpcAdminAuthValues = 4 + +// CheckGrpcAdminToken accepts the call when one of the first +// maxGrpcAdminAuthValues authorization values is a Bearer admin token signed +// by signingKey, so a fresh token still counts when the caller's context also +// forwards an older one. +func CheckGrpcAdminToken(ctx context.Context, signingKey SigningKey) error { md, ok := metadata.FromIncomingContext(ctx) if !ok { return status.Error(codes.Unauthenticated, "missing metadata") @@ -33,15 +45,21 @@ func CheckGrpcAdminPeer(ctx context.Context, signingKey SigningKey) error { if len(authHeaders) == 0 { return status.Error(codes.Unauthenticated, "missing authorization metadata") } - raw := strings.TrimSpace(authHeaders[0]) - parts := strings.Fields(raw) - if len(parts) != 2 || !strings.EqualFold(parts[0], "Bearer") || parts[1] == "" { - return status.Error(codes.Unauthenticated, "authorization header must use Bearer scheme") + if len(authHeaders) > maxGrpcAdminAuthValues { + authHeaders = authHeaders[:maxGrpcAdminAuthValues] } - if !IsFilerAdminToken(signingKey, EncodedJwt(parts[1])) { - return status.Error(codes.Unauthenticated, "invalid admin token") + err := status.Error(codes.Unauthenticated, "authorization header must use Bearer scheme") + for _, raw := range authHeaders { + parts := strings.Fields(raw) + if len(parts) != 2 || !strings.EqualFold(parts[0], "Bearer") || parts[1] == "" { + continue + } + if IsFilerAdminToken(signingKey, EncodedJwt(parts[1])) { + return nil + } + err = status.Error(codes.Unauthenticated, "invalid admin token") } - return nil + return err } // IsFilerAdminToken reports whether token is an admin token signed by diff --git a/weed/security/grpc_peer_test.go b/weed/security/grpc_peer_test.go index 8a6127d072..c67383c478 100644 --- a/weed/security/grpc_peer_test.go +++ b/weed/security/grpc_peer_test.go @@ -5,8 +5,11 @@ import ( "crypto/tls" "crypto/x509" "net" + "slices" "testing" + "time" + jwt "github.com/golang-jwt/jwt/v5" "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials" "google.golang.org/grpc/metadata" @@ -66,3 +69,50 @@ func TestGrpcAdminPeerCredentials(t *testing.T) { t.Fatalf("expected one authorization entry, got %v", md) } } + +func TestCheckGrpcAdminToken(t *testing.T) { + key := SigningKey("test-signing-key-0123456789abcdef") + fresh := "Bearer " + string(GenJwtForFilerAdmin(key, 60)) + stale := "Bearer " + string(GenJwtForFilerAdmin(SigningKey("another-key"), 60)) + expired, signErr := jwt.NewWithClaims(jwt.SigningMethodHS256, SeaweedFilerAdminClaims{ + RegisteredClaims: jwt.RegisteredClaims{ExpiresAt: jwt.NewNumericDate(time.Now().Add(-time.Hour))}, + }).SignedString([]byte(key)) + if signErr != nil { + t.Fatalf("sign expired token: %v", signErr) + } + session, signErr := jwt.NewWithClaims(jwt.SigningMethodHS256, SeaweedFilerAdminClaims{ + SessionId: "session", + RegisteredClaims: jwt.RegisteredClaims{ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour))}, + }).SignedString([]byte(key)) + if signErr != nil { + t.Fatalf("sign session token: %v", signErr) + } + + for name, tc := range map[string]struct { + values []string + wantErr string + }{ + "stale then fresh": {[]string{stale, fresh}, ""}, + "fresh then garbage": {[]string{fresh, "garbage"}, ""}, + "expired then fresh": {[]string{"Bearer " + expired, fresh}, ""}, + "only expired": {[]string{"Bearer " + expired}, "invalid admin token"}, + "only stale": {[]string{stale}, "invalid admin token"}, + "same-key session": {[]string{"Bearer " + session}, "invalid admin token"}, + "basic then stale": {[]string{"Basic dXNlcjpwYXNz", stale}, "invalid admin token"}, + "no bearer value": {[]string{"Basic dXNlcjpwYXNz", "garbage"}, "authorization header must use Bearer scheme"}, + "lowercase scheme ok": {[]string{"bearer " + fresh[len("Bearer "):]}, ""}, + "fresh past the cap": {append(slices.Repeat([]string{stale}, maxGrpcAdminAuthValues), fresh), "invalid admin token"}, + } { + ctx := metadata.NewIncomingContext(context.Background(), metadata.MD{"authorization": tc.values}) + err := CheckGrpcAdminToken(ctx, key) + if tc.wantErr == "" { + if err != nil { + t.Errorf("%s: unexpected error %v", name, err) + } + continue + } + if status.Code(err) != codes.Unauthenticated || status.Convert(err).Message() != tc.wantErr { + t.Errorf("%s: got %v, want Unauthenticated %q", name, err, tc.wantErr) + } + } +} diff --git a/weed/server/filer_server_handlers_iam_grpc.go b/weed/server/filer_server_handlers_iam_grpc.go index 508dcd2c3b..d92543c2fe 100644 --- a/weed/server/filer_server_handlers_iam_grpc.go +++ b/weed/server/filer_server_handlers_iam_grpc.go @@ -3,7 +3,6 @@ package weed_server import ( "context" "encoding/json" - "strings" "github.com/seaweedfs/seaweedfs/weed/credential" "github.com/seaweedfs/seaweedfs/weed/glog" @@ -12,7 +11,6 @@ import ( "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" "github.com/seaweedfs/seaweedfs/weed/security" "google.golang.org/grpc/codes" - "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" ) @@ -48,30 +46,7 @@ func (s *IamGrpcServer) checkAdminAuth(ctx context.Context) error { if len(s.adminSigningKey) == 0 { return nil } - md, ok := metadata.FromIncomingContext(ctx) - if !ok { - return status.Error(codes.Unauthenticated, "missing metadata") - } - authHeaders := md.Get("authorization") - if len(authHeaders) == 0 { - return status.Error(codes.Unauthenticated, "missing authorization metadata") - } - // RFC 6750 ยง2.1: the "Bearer" auth-scheme name is case-insensitive. - // Tolerate surrounding whitespace and any spacing between scheme and token. - raw := strings.TrimSpace(authHeaders[0]) - parts := strings.Fields(raw) - if len(parts) != 2 || !strings.EqualFold(parts[0], "Bearer") || parts[1] == "" { - return status.Error(codes.Unauthenticated, "authorization header must use Bearer scheme") - } - token := parts[1] - parsed, err := security.DecodeJwt(s.adminSigningKey, security.EncodedJwt(token), &security.SeaweedFilerAdminClaims{}) - if err != nil || parsed == nil || !parsed.Valid { - return status.Error(codes.Unauthenticated, "invalid admin token") - } - if claims, ok := parsed.Claims.(*security.SeaweedFilerAdminClaims); !ok || claims.SessionId != "" { - return status.Error(codes.Unauthenticated, "invalid admin token") - } - return nil + return security.CheckGrpcAdminToken(ctx, s.adminSigningKey) } ////////////////////////////////////////////////// diff --git a/weed/server/filer_server_handlers_iam_grpc_test.go b/weed/server/filer_server_handlers_iam_grpc_test.go index b6964c58bf..af5397da7d 100644 --- a/weed/server/filer_server_handlers_iam_grpc_test.go +++ b/weed/server/filer_server_handlers_iam_grpc_test.go @@ -117,6 +117,31 @@ func TestIamGrpc_ValidToken_ReachesHandler(t *testing.T) { } } +func TestIamGrpc_ValidTokenAfterStaleToken_ReachesHandler(t *testing.T) { + s := newTestIamGrpcServer(t) + stale := security.GenJwtForFilerAdmin(security.SigningKey("a-different-key"), 60) + good := security.GenJwtForFilerAdmin(security.SigningKey(testIamSigningKey), 60) + ctx := metadata.NewIncomingContext(context.Background(), metadata.MD{ + "authorization": []string{"Bearer " + string(stale), "Bearer " + string(good)}, + }) + if _, err := s.ListUsers(ctx, &iam_pb.ListUsersRequest{}); err != nil { + t.Fatalf("ListUsers with a valid token after a stale one: unexpected error %v", err) + } +} + +func TestIamGrpc_AllInvalidTokens_Unauthenticated(t *testing.T) { + s := newTestIamGrpcServer(t) + stale := security.GenJwtForFilerAdmin(security.SigningKey("a-different-key"), 60) + other := security.GenJwtForFilerAdmin(security.SigningKey("another-different-key"), 60) + ctx := metadata.NewIncomingContext(context.Background(), metadata.MD{ + "authorization": []string{"Bearer " + string(stale), "Bearer " + string(other)}, + }) + _, err := s.ListUsers(ctx, &iam_pb.ListUsersRequest{}) + if got, want := status.Code(err), codes.Unauthenticated; got != want { + t.Fatalf("ListUsers with two invalid tokens: got code %v, want %v (err=%v)", got, want, err) + } +} + func TestIamGrpc_NoSigningKey_Unauthenticated_Allowed(t *testing.T) { // Auth is opt-in: when the server is built without a signing key, every // RPC is accepted regardless of (or in the absence of) metadata so the