From e3db95e0c1dacf3494c995efa65a7ffa9e7cbffc Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 30 Dec 2025 16:34:05 -0800 Subject: [PATCH] Fix: Route unauthenticated specific STS requests to STS handler correctly (#7920) * Fix STS Access Denied for AssumeRoleWithWebIdentity (Issue #7917) * Fix logging regression: ensure IAM status is logged even if STS is enabled * Address PR feedback: fix duplicate log, clarify comments, add comprehensive routing tests * Add edge case test: authenticated STS action routes to IAM (auth precedence) --- weed/s3api/s3api_server.go | 28 +++- weed/s3api/s3api_server_routing_test.go | 195 ++++++++++++++++++++++++ 2 files changed, 216 insertions(+), 7 deletions(-) create mode 100644 weed/s3api/s3api_server_routing_test.go diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index 5917b5195..530a8af4b 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -618,9 +618,8 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { // STS API endpoint for AssumeRoleWithWebIdentity // POST /?Action=AssumeRoleWithWebIdentity&WebIdentityToken=... - // This endpoint is unauthenticated - the JWT token in the request is the authentication - // IMPORTANT: Register this BEFORE the general IAM route to prevent interception if s3a.stsHandlers != nil { + // 1. Explicit query param match (highest priority) apiRouter.Methods(http.MethodPost).Path("/").Queries("Action", "AssumeRoleWithWebIdentity"). HandlerFunc(track(s3a.stsHandlers.HandleSTSRequest, "STS")) glog.V(0).Infof("STS API enabled on S3 port (AssumeRoleWithWebIdentity)") @@ -628,15 +627,30 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { // Embedded IAM API endpoint // POST / (without specific query parameters) - // This must be before ListBuckets since IAM uses POST and ListBuckets uses GET - // Uses AuthIam for granular permission checking: - // - Self-service operations (own access keys) don't require admin - // - Operations on other users require admin privileges + // Uses AuthIam for granular permission checking if s3a.embeddedIam != nil { - apiRouter.Methods(http.MethodPost).Path("/").HandlerFunc(track(s3a.embeddedIam.AuthIam(s3a.cb.Limit(s3a.embeddedIam.DoActions, ACTION_WRITE)), "IAM")) + // 2. Authenticated IAM requests + // Only match if the request appears to be authenticated (AWS Signature) + // This prevents unauthenticated STS requests (like AssumeRoleWithWebIdentity in body) + // from being captured by the IAM handler which would reject them. + iamMatcher := func(r *http.Request, rm *mux.RouteMatch) bool { + return getRequestAuthType(r) != authTypeAnonymous + } + + apiRouter.Methods(http.MethodPost).Path("/").MatcherFunc(iamMatcher). + HandlerFunc(track(s3a.embeddedIam.AuthIam(s3a.cb.Limit(s3a.embeddedIam.DoActions, ACTION_WRITE)), "IAM")) glog.V(0).Infof("Embedded IAM API enabled on S3 port") } + // 3. Fallback STS handler (lowest priority) + // Catches unauthenticated POST / requests that didn't match specific query params. + // This primarily handles AssumeRoleWithWebIdentity where parameters are in the POST body. + if s3a.stsHandlers != nil { + glog.V(1).Infof("Registering fallback STS handler for unauthenticated POST requests") + apiRouter.Methods(http.MethodPost).Path("/"). + HandlerFunc(track(s3a.stsHandlers.HandleSTSRequest, "STS-Fallback")) + } + // ListBuckets apiRouter.Methods(http.MethodGet).Path("/").HandlerFunc(track(s3a.iam.Auth(s3a.ListBucketsHandler, ACTION_LIST), "LIST")) diff --git a/weed/s3api/s3api_server_routing_test.go b/weed/s3api/s3api_server_routing_test.go new file mode 100644 index 000000000..5aed24d39 --- /dev/null +++ b/weed/s3api/s3api_server_routing_test.go @@ -0,0 +1,195 @@ +package s3api + +import ( + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/gorilla/mux" + "github.com/seaweedfs/seaweedfs/weed/credential" + "github.com/seaweedfs/seaweedfs/weed/util" + "github.com/stretchr/testify/assert" +) + +// setupRoutingTestServer creates a minimal S3ApiServer for routing tests +func setupRoutingTestServer(t *testing.T) *S3ApiServer { + opt := &S3ApiServerOption{EnableIam: true} + iam := NewIdentityAccessManagementWithStore(opt, "memory") + iam.isAuthEnabled = true + + if iam.credentialManager == nil { + cm, err := credential.NewCredentialManager("memory", util.GetViper(), "") + if err != nil { + t.Fatalf("Failed to create credential manager: %v", err) + } + iam.credentialManager = cm + } + + server := &S3ApiServer{ + option: opt, + iam: iam, + credentialManager: iam.credentialManager, + embeddedIam: NewEmbeddedIamApi(iam.credentialManager, iam), + stsHandlers: &STSHandlers{}, + } + + return server +} + +// TestRouting_STSWithQueryParams verifies that AssumeRoleWithWebIdentity with query params routes to STS +func TestRouting_STSWithQueryParams(t *testing.T) { + router := mux.NewRouter() + s3a := setupRoutingTestServer(t) + s3a.registerRouter(router) + + // Create request with Action in query params (no auth header) + req, _ := http.NewRequest("POST", "/?Action=AssumeRoleWithWebIdentity&WebIdentityToken=test-token&RoleArn=arn:aws:iam::123:role/test&RoleSessionName=test-session", nil) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + // Should route to STS handler -> 503 (service not initialized) or 400 (validation error) + assert.Contains(t, []int{http.StatusBadRequest, http.StatusServiceUnavailable}, rr.Code, "Should route to STS handler") +} + +// TestRouting_STSWithBodyParams verifies that AssumeRoleWithWebIdentity with body params routes to STS fallback +func TestRouting_STSWithBodyParams(t *testing.T) { + router := mux.NewRouter() + s3a := setupRoutingTestServer(t) + s3a.registerRouter(router) + + // Create request with Action in POST body (no auth header) + data := url.Values{} + data.Set("Action", "AssumeRoleWithWebIdentity") + data.Set("WebIdentityToken", "test-token") + data.Set("RoleArn", "arn:aws:iam::123:role/test") + data.Set("RoleSessionName", "test-session") + + req, _ := http.NewRequest("POST", "/", strings.NewReader(data.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + // Should route to STS fallback handler -> 503 (service not initialized in test) + assert.Equal(t, http.StatusServiceUnavailable, rr.Code, "Should route to STS fallback handler (503 because STS not initialized)") +} + +// TestRouting_AuthenticatedIAM verifies that authenticated IAM requests route to IAM handler +func TestRouting_AuthenticatedIAM(t *testing.T) { + router := mux.NewRouter() + s3a := setupRoutingTestServer(t) + s3a.registerRouter(router) + + // Create IAM request with Authorization header + data := url.Values{} + data.Set("Action", "CreateUser") + data.Set("UserName", "testuser") + + req, _ := http.NewRequest("POST", "/", strings.NewReader(data.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=AKIA.../...") + + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + // Should route to IAM handler -> 400/403 (invalid signature) + // NOT 503 (which would indicate STS handler) + assert.NotEqual(t, http.StatusServiceUnavailable, rr.Code, "Should NOT route to STS handler") + assert.Contains(t, []int{http.StatusBadRequest, http.StatusForbidden}, rr.Code, "Should route to IAM handler (400/403 due to invalid signature)") +} + +// TestRouting_IAMMatcherLogic verifies the iamMatcher correctly distinguishes auth types +func TestRouting_IAMMatcherLogic(t *testing.T) { + tests := []struct { + name string + authHeader string + queryParams string + expectsIAM bool + description string + }{ + { + name: "No auth - anonymous", + authHeader: "", + queryParams: "", + expectsIAM: false, + description: "Request with no auth should NOT match IAM", + }, + { + name: "AWS4 signature", + authHeader: "AWS4-HMAC-SHA256 Credential=AKIA.../...", + queryParams: "", + expectsIAM: true, + description: "Request with AWS4 signature should match IAM", + }, + { + name: "AWS2 signature", + authHeader: "AWS AKIA...:signature", + queryParams: "", + expectsIAM: true, + description: "Request with AWS2 signature should match IAM", + }, + { + name: "Presigned V4", + authHeader: "", + queryParams: "?X-Amz-Credential=AKIA...", + expectsIAM: true, + description: "Request with presigned V4 params should match IAM", + }, + { + name: "Presigned V2", + authHeader: "", + queryParams: "?AWSAccessKeyId=AKIA...", + expectsIAM: true, + description: "Request with presigned V2 params should match IAM", + }, + { + name: "AWS4 signature with STS action in body", + authHeader: "AWS4-HMAC-SHA256 Credential=AKIA.../...", + queryParams: "", + expectsIAM: true, + description: "Authenticated STS action should still route to IAM (auth takes precedence)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + router := mux.NewRouter() + s3a := setupRoutingTestServer(t) + s3a.registerRouter(router) + + data := url.Values{} + // For the authenticated STS action test, set the STS action + // For other tests, don't set Action to avoid STS validation errors + if tt.name == "AWS4 signature with STS action in body" { + data.Set("Action", "AssumeRoleWithWebIdentity") + data.Set("WebIdentityToken", "test-token") + data.Set("RoleArn", "arn:aws:iam::123:role/test") + data.Set("RoleSessionName", "test-session") + } + + req, _ := http.NewRequest("POST", "/"+tt.queryParams, strings.NewReader(data.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + if tt.authHeader != "" { + req.Header.Set("Authorization", tt.authHeader) + } + + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + if tt.expectsIAM { + // Should route to IAM (400/403 for invalid sig) + // NOT 400 from STS (which would be missing Action parameter) + // We distinguish by checking it's NOT a generic 400 with empty body + assert.NotEqual(t, http.StatusServiceUnavailable, rr.Code, tt.description) + } else { + // Should route to STS fallback + // Can be 503 (service not initialized) or 400 (missing/invalid Action parameter) + assert.Contains(t, []int{http.StatusBadRequest, http.StatusServiceUnavailable}, rr.Code, tt.description) + } + }) + } +}