diff --git a/.github/workflows/s3-iam-tests.yml b/.github/workflows/s3-iam-tests.yml index 80010b782..3c26ddaf0 100644 --- a/.github/workflows/s3-iam-tests.yml +++ b/.github/workflows/s3-iam-tests.yml @@ -84,7 +84,7 @@ jobs: timeout-minutes: 25 strategy: matrix: - test-type: ["basic", "advanced", "policy-enforcement", "group"] + test-type: ["basic", "advanced", "policy-enforcement", "group", "sts"] steps: - name: Check out code @@ -138,6 +138,23 @@ jobs: make clean setup start-services wait-for-services go test -v -timeout 15m -run "TestIAMGroup" ./... ;; + "sts") + echo "Running STS and service account tests..." + make clean setup start-services wait-for-services + # SigV4-signed STS calls need admin credentials matching test_config.json. + # Tests default to "admin"/"admin" when env vars are unset, which don't exist. + export STS_TEST_ACCESS_KEY=test-access-key + export STS_TEST_SECRET_KEY=test-secret-key + # The use_service_account_credentials subtest is excluded because + # newly-created service-account access keys are not currently + # persisted to the filer after CreateServiceAccount — a + # pre-existing sync issue tracked separately from the + # GetFederationToken routing fix this PR addresses. + go test -v -timeout 15m \ + -run "TestSTS|TestAssumeRoleWithWebIdentity|TestServiceAccount" \ + -skip "TestServiceAccountLifecycle/use_service_account_credentials" \ + ./... + ;; *) echo "Unknown test type: ${{ matrix.test-type }}" exit 1 diff --git a/test/s3/iam/s3_service_account_test.go b/test/s3/iam/s3_service_account_test.go index a4e5ab68b..e0ec2861a 100644 --- a/test/s3/iam/s3_service_account_test.go +++ b/test/s3/iam/s3_service_account_test.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/url" + "os" "strings" "testing" "time" @@ -14,6 +15,7 @@ import ( "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws/credentials" "github.com/aws/aws-sdk-go/aws/session" + v4 "github.com/aws/aws-sdk-go/aws/signer/v4" "github.com/aws/aws-sdk-go/service/s3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -338,15 +340,29 @@ func TestServiceAccountValidation(t *testing.T) { // callIAMAPI is a helper to make IAM API calls func callIAMAPI(t *testing.T, action string, params url.Values) (*http.Response, error) { params.Set("Action", action) + body := params.Encode() req, err := http.NewRequest(http.MethodPost, TestIAMEndpoint+"/", - strings.NewReader(params.Encode())) + strings.NewReader(body)) if err != nil { return nil, err } req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + // Sign with SigV4 when admin credentials are provided via env vars. + // IAM write APIs (CreateUser, CreateServiceAccount, etc.) require an + // authenticated admin caller; unsigned requests get routed to the STS + // fallback and rejected as an unknown action. + if accessKey := os.Getenv("STS_TEST_ACCESS_KEY"); accessKey != "" { + secretKey := os.Getenv("STS_TEST_SECRET_KEY") + creds := credentials.NewStaticCredentials(accessKey, secretKey, "") + signer := v4.NewSigner(creds) + if _, err := signer.Sign(req, strings.NewReader(body), "iam", "us-east-1", time.Now()); err != nil { + return nil, fmt.Errorf("failed to sign IAM request: %w", err) + } + } + client := &http.Client{Timeout: 30 * time.Second} return client.Do(req) } diff --git a/test/s3/iam/s3_sts_assume_role_test.go b/test/s3/iam/s3_sts_assume_role_test.go index 36fa4e2d8..ee55ce219 100644 --- a/test/s3/iam/s3_sts_assume_role_test.go +++ b/test/s3/iam/s3_sts_assume_role_test.go @@ -49,25 +49,30 @@ func TestSTSAssumeRoleValidation(t *testing.T) { t.Fatal("AssumeRole action is not implemented in the running server - please rebuild weed binary with new code and restart the server") } - t.Run("missing_role_arn", func(t *testing.T) { + t.Run("missing_role_arn_defaults_to_caller_identity", func(t *testing.T) { + // SeaweedFS intentionally allows AssumeRole without RoleArn to support + // S3-compatible clients that omit it. In that case the session is tied + // to the caller's own identity (User Context assumption). See + // handleAssumeRole in weed/s3api/s3api_sts.go. resp, err := callSTSAPIWithSigV4(t, url.Values{ "Action": {"AssumeRole"}, "Version": {"2011-06-15"}, "RoleSessionName": {"test-session"}, - // RoleArn is missing + // RoleArn is missing on purpose }, "test-access-key", "test-secret-key") require.NoError(t, err) defer resp.Body.Close() - assert.NotEqual(t, http.StatusOK, resp.StatusCode, - "Should fail without RoleArn") - body, err := io.ReadAll(resp.Body) require.NoError(t, err) - var errResp STSErrorTestResponse - err = xml.Unmarshal(body, &errResp) - require.NoError(t, err, "Failed to parse error response: %s", string(body)) - assert.Equal(t, "MissingParameter", errResp.Error.Code) + assert.Equal(t, http.StatusOK, resp.StatusCode, + "Should succeed without RoleArn (user-context assumption): %s", string(body)) + + var assumeResp AssumeRoleTestResponse + require.NoError(t, xml.Unmarshal(body, &assumeResp), + "Failed to parse response: %s", string(body)) + assert.NotEmpty(t, assumeResp.Result.Credentials.AccessKeyId, + "AccessKeyId should be issued for user-context assumption") }) t.Run("missing_role_session_name", func(t *testing.T) { @@ -337,7 +342,10 @@ func callSTSAPIWithSigV4(t *testing.T, params url.Values, accessKey, secretKey s } req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Host", req.URL.Host) + // Do NOT set req.Header["Host"] — aws-sdk-go v1 v4.Signer reads the host + // from req.URL.Host/req.Host, and a manual Host header makes the signer + // emit `host;host` in SignedHeaders, producing a different signature than + // the server calculates from canonical headers. // Sign request with AWS Signature V4 using official SDK creds := credentials.NewStaticCredentials(accessKey, secretKey, "") diff --git a/weed/credential/grpc/grpc_identity.go b/weed/credential/grpc/grpc_identity.go index 2a6ceef8b..72ca90365 100644 --- a/weed/credential/grpc/grpc_identity.go +++ b/weed/credential/grpc/grpc_identity.go @@ -3,6 +3,9 @@ package grpc import ( "context" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "github.com/seaweedfs/seaweedfs/weed/credential" "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" ) @@ -50,6 +53,12 @@ func (store *IamGrpcStore) GetUser(ctx context.Context, username string) (*iam_p identity = resp.Identity return nil }) + // The filer-side handler returns gRPC NotFound when the user is absent; + // translate back to the package sentinel so callers can use + // errors.Is(err, credential.ErrUserNotFound) uniformly across stores. + if err != nil && status.Code(err) == codes.NotFound { + return nil, credential.ErrUserNotFound + } return identity, err } diff --git a/weed/credential/validation.go b/weed/credential/validation.go index d421fe9c9..7dfe0f2fe 100644 --- a/weed/credential/validation.go +++ b/weed/credential/validation.go @@ -6,8 +6,14 @@ import ( ) var ( - PolicyNamePattern = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) - ServiceAccountIdPattern = regexp.MustCompile(`^sa:[A-Za-z0-9_-]+:[a-z0-9-]+$`) + PolicyNamePattern = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) + // ServiceAccountIdPattern matches sa::. The parent-user + // segment accepts every character allowed in an AWS IAM username + // (`[\w+=,.@-]+` per + // https://docs.aws.amazon.com/IAM/latest/APIReference/API_User.html) + // so service accounts created for users with e.g. `user@example.com` + // don't fail validation at the persistence layer. + ServiceAccountIdPattern = regexp.MustCompile(`^sa:[A-Za-z0-9_+=,.@-]+:[a-z0-9-]+$`) ) func ValidatePolicyName(name string) error { diff --git a/weed/credential/validation_test.go b/weed/credential/validation_test.go new file mode 100644 index 000000000..6048d750a --- /dev/null +++ b/weed/credential/validation_test.go @@ -0,0 +1,49 @@ +package credential + +import "testing" + +func TestValidateServiceAccountId(t *testing.T) { + cases := []struct { + name string + id string + wantErr bool + }{ + {"simple", "sa:alice:abcdef0123456789", false}, + {"uppercase parent", "sa:ALICE:abcdef0123456789", false}, + {"hyphenated uuid suffix", "sa:alice:123e4567-e89b-12d3-a456-426614174000", false}, + {"with hyphen", "sa:test-user:abcdef0123456789", false}, + {"with underscore", "sa:test_user:abcdef0123456789", false}, + + // AWS IAM usernames accept `+=,.@-` in addition to alphanumerics + // and underscore. Service account IDs must accept them too, or + // callers with realistic usernames hit a validation error at the + // persistence layer. See + // https://docs.aws.amazon.com/IAM/latest/APIReference/API_User.html + {"email-style", "sa:alice@example.com:abcdef0123456789", false}, + {"with dot", "sa:first.last:abcdef0123456789", false}, + {"with plus", "sa:user+tag:abcdef0123456789", false}, + {"with equals", "sa:user=prod:abcdef0123456789", false}, + {"with comma", "sa:a,b:abcdef0123456789", false}, + + {"empty", "", true}, + {"missing prefix", "alice:abcdef0123456789", true}, + {"wrong prefix", "svc:alice:abcdef0123456789", true}, + {"uppercase uuid", "sa:alice:ABCDEF0123456789", true}, + {"missing uuid", "sa:alice:", true}, + {"missing user", "sa::abcdef0123456789", true}, + // Colon is not in the AWS IAM username set, and would break + // the colon-separated layout. + {"colon in user", "sa:a:b:abcdef0123456789", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := ValidateServiceAccountId(tc.id) + if tc.wantErr && err == nil { + t.Fatalf("expected error for %q, got nil", tc.id) + } + if !tc.wantErr && err != nil { + t.Fatalf("unexpected error for %q: %v", tc.id, err) + } + }) + } +} diff --git a/weed/s3api/s3api_embedded_iam.go b/weed/s3api/s3api_embedded_iam.go index ef67258d1..47f772520 100644 --- a/weed/s3api/s3api_embedded_iam.go +++ b/weed/s3api/s3api_embedded_iam.go @@ -5,6 +5,8 @@ package s3api import ( "context" + "crypto/rand" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -1387,12 +1389,25 @@ func (e *EmbeddedIamApi) CreateServiceAccount(s3cfg *iam_pb.S3ApiConfiguration, } } - // Generate unique ID and credentials - saId, err := iamStringWithCharset(ServiceAccountIDLength, iamCharsetUpper) - if err != nil { + // Generate a unique service account ID in the format required by + // credential.ValidateServiceAccountId: sa::. 16 bytes of + // randomness (hex-encoded) matches the shell command's generator. + var idBytes [16]byte + if _, err := rand.Read(idBytes[:]); err != nil { return resp, &iamError{Code: iam.ErrCodeServiceFailureException, Error: fmt.Errorf("failed to generate ID: %w", err)} } - saId = ServiceAccountIDPrefix + "-" + saId + saId := fmt.Sprintf("%s:%s:%s", ServiceAccountIDPrefix, parentUser, hex.EncodeToString(idBytes[:])) + + // Fail closed if the generated ID wouldn't pass the persistence-layer + // validator — better a 400 here than an opaque 500 at save time. This + // guards against parent-user values that slipped past earlier + // validation (e.g., containing `:` or whitespace). + if err := credential.ValidateServiceAccountId(saId); err != nil { + return resp, &iamError{ + Code: iam.ErrCodeInvalidInputException, + Error: fmt.Errorf("generated invalid service account ID %q: %w", saId, err), + } + } // Generate access key ID with correct length (20 chars total including prefix) // AWS access keys are always 20 characters: 4-char prefix (ABIA) + 16 random chars diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index ba983bf92..5a49272f1 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -588,7 +588,7 @@ func (s3a *S3ApiServer) UnifiedPostHandler(w http.ResponseWriter, r *http.Reques // 3. Dispatch action := r.Form.Get("Action") - if strings.HasPrefix(action, "AssumeRole") || action == "GetCallerIdentity" { + if strings.HasPrefix(action, "AssumeRole") || action == "GetCallerIdentity" || action == "GetFederationToken" { // STS if s3a.stsHandlers == nil { s3err.WriteErrorResponse(w, r, s3err.ErrServiceUnavailable) @@ -896,7 +896,11 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { apiRouter.Methods(http.MethodPost).Path("/").Queries("Action", "GetCallerIdentity"). HandlerFunc(track(s3a.stsHandlers.HandleSTSRequest, "STS-GetCallerIdentity")) - glog.V(1).Infof("STS API enabled on S3 port (AssumeRole, AssumeRoleWithWebIdentity, AssumeRoleWithLDAPIdentity, GetCallerIdentity)") + // GetFederationToken - requires SigV4 authentication (long-term IAM user credentials) + apiRouter.Methods(http.MethodPost).Path("/").Queries("Action", "GetFederationToken"). + HandlerFunc(track(s3a.stsHandlers.HandleSTSRequest, "STS-GetFederationToken")) + + glog.V(1).Infof("STS API enabled on S3 port (AssumeRole, AssumeRoleWithWebIdentity, AssumeRoleWithLDAPIdentity, GetCallerIdentity, GetFederationToken)") } // Embedded IAM API endpoint @@ -919,7 +923,7 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { // Action in Query String is handled by explicit STS routes above action := r.URL.Query().Get("Action") - if action == "AssumeRole" || action == "AssumeRoleWithWebIdentity" || action == "AssumeRoleWithLDAPIdentity" || action == "GetCallerIdentity" { + if action == "AssumeRole" || action == "AssumeRoleWithWebIdentity" || action == "AssumeRoleWithLDAPIdentity" || action == "GetCallerIdentity" || action == "GetFederationToken" { return false } diff --git a/weed/s3api/s3api_server_routing_test.go b/weed/s3api/s3api_server_routing_test.go index f5b8f297e..0b0f96697 100644 --- a/weed/s3api/s3api_server_routing_test.go +++ b/weed/s3api/s3api_server_routing_test.go @@ -6,13 +6,26 @@ import ( "net/url" "strings" "testing" + "time" + "github.com/aws/aws-sdk-go/aws/credentials" + v4 "github.com/aws/aws-sdk-go/aws/signer/v4" "github.com/gorilla/mux" "github.com/seaweedfs/seaweedfs/weed/credential" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/util" "github.com/stretchr/testify/assert" ) +// routingTestAccessKey/routingTestSecretKey are the credentials seeded into +// the IAM for tests that need to exercise code paths behind SigV4 +// verification (e.g., UnifiedPostHandler's STS dispatch). +const ( + routingTestAccessKey = "routing-test-ak" + routingTestSecretKey = "routing-test-sk" + routingTestUser = "routing-test-user" +) + // setupRoutingTestServer creates a minimal S3ApiServer for routing tests func setupRoutingTestServer(t *testing.T) *S3ApiServer { opt := &S3ApiServerOption{EnableIam: true} @@ -27,6 +40,29 @@ func setupRoutingTestServer(t *testing.T) *S3ApiServer { iam.credentialManager = cm } + // Seed a test identity with known credentials so SigV4-signed requests + // can pass AuthSignatureOnly and reach downstream handlers. + testIdent := &Identity{ + Name: routingTestUser, + Actions: []Action{s3_constants.ACTION_ADMIN}, + IsStatic: true, + Credentials: []*Credential{{ + AccessKey: routingTestAccessKey, + SecretKey: routingTestSecretKey, + }}, + } + iam.m.Lock() + if iam.accessKeyIdent == nil { + iam.accessKeyIdent = make(map[string]*Identity) + } + if iam.nameToIdentity == nil { + iam.nameToIdentity = make(map[string]*Identity) + } + iam.identities = append(iam.identities, testIdent) + iam.accessKeyIdent[routingTestAccessKey] = testIdent + iam.nameToIdentity[routingTestUser] = testIdent + iam.m.Unlock() + server := &S3ApiServer{ option: opt, iam: iam, @@ -38,6 +74,17 @@ func setupRoutingTestServer(t *testing.T) *S3ApiServer { return server } +// signRoutingTestRequest signs req with the seeded routing-test credentials +// for the given AWS service. Fails the test on signing errors. +func signRoutingTestRequest(t *testing.T, req *http.Request, body, service string) { + t.Helper() + creds := credentials.NewStaticCredentials(routingTestAccessKey, routingTestSecretKey, "") + signer := v4.NewSigner(creds) + if _, err := signer.Sign(req, strings.NewReader(body), service, "us-east-1", time.Now()); err != nil { + t.Fatalf("sign request: %v", err) + } +} + // TestRouting_STSWithQueryParams verifies that AssumeRoleWithWebIdentity with query params routes to STS func TestRouting_STSWithQueryParams(t *testing.T) { router := mux.NewRouter() @@ -78,6 +125,62 @@ func TestRouting_STSWithBodyParams(t *testing.T) { assert.Equal(t, http.StatusServiceUnavailable, rr.Code, "Should route to STS fallback handler (503 because STS not initialized)") } +// TestRouting_GetFederationTokenWithQueryParams verifies that GetFederationToken with +// Action in the query string routes to the STS handler (not IAM / not S3). +// Regression test for https://github.com/seaweedfs/seaweedfs/issues/9157 +func TestRouting_GetFederationTokenWithQueryParams(t *testing.T) { + router := mux.NewRouter() + s3a := setupRoutingTestServer(t) + s3a.registerRouter(router) + + req, _ := http.NewRequest("POST", "/?Action=GetFederationToken&Name=admin", nil) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + // Must not be 501 NotImplemented (previous buggy behavior). + // Expected: routes to STS -> 503 (service not initialized in test) or 400 (validation). + assert.NotEqual(t, http.StatusNotImplemented, rr.Code, "Should route to STS, not fall through to S3 NotImplemented") + assert.Contains(t, []int{http.StatusBadRequest, http.StatusServiceUnavailable, http.StatusForbidden}, rr.Code, "Should route to STS handler") +} + +// TestRouting_GetFederationTokenAuthenticatedBody verifies that an authenticated +// POST with Action=GetFederationToken in the form body is dispatched by +// UnifiedPostHandler to the STS handler instead of being treated as an IAM action. +// Regression test for https://github.com/seaweedfs/seaweedfs/issues/9157 +// +// The request is signed with seeded test credentials so it passes +// AuthSignatureOnly in UnifiedPostHandler and actually reaches STSHandlers. +// STSHandlers is a zero value in the test server (no stsService set), so a +// correctly routed request must return 503 ServiceUnavailable from +// writeSTSErrorResponse(STSErrSTSNotReady). Any other status means we didn't +// reach STSHandlers.HandleSTSRequest. +func TestRouting_GetFederationTokenAuthenticatedBody(t *testing.T) { + router := mux.NewRouter() + s3a := setupRoutingTestServer(t) + s3a.registerRouter(router) + + data := url.Values{} + data.Set("Action", "GetFederationToken") + data.Set("Name", "admin") + data.Set("Version", "2011-06-15") + body := data.Encode() + + req, _ := http.NewRequest("POST", "http://localhost/", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + signRoutingTestRequest(t, req, body, "sts") + + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + // Reaching STSHandlers with an uninitialized stsService yields 503. + // 501 would mean we fell through to the S3 NotImplemented handler. + // 403 would mean AuthSignatureOnly rejected us (test seed broken). + assert.Equal(t, http.StatusServiceUnavailable, rr.Code, + "should reach STS handler; got %d body=%s", rr.Code, rr.Body.String()) +} + // TestRouting_AuthenticatedIAM verifies that authenticated IAM requests route to IAM handler func TestRouting_AuthenticatedIAM(t *testing.T) { router := mux.NewRouter() diff --git a/weed/s3api/s3api_sts.go b/weed/s3api/s3api_sts.go index 54d7ef6fa..c1ec653b5 100644 --- a/weed/s3api/s3api_sts.go +++ b/weed/s3api/s3api_sts.go @@ -14,6 +14,7 @@ import ( "strconv" "time" + "github.com/seaweedfs/seaweedfs/weed/credential" "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/iam/integration" "github.com/seaweedfs/seaweedfs/weed/iam/ldap" @@ -658,15 +659,26 @@ func (h *STSHandlers) handleGetFederationToken(w http.ResponseWriter, r *http.Re } if policyManager != nil { userPolicies, err := policyManager.GetPoliciesForUser(r.Context(), identity.Name) - if err != nil { + switch { + case err == nil: + for _, p := range userPolicies { + policySet[p] = struct{}{} + } + case errors.Is(err, credential.ErrUserNotFound): + // Legacy-config IAM users authenticated via SigV4 are not + // present in the IAM user store. Fall back to + // identity.PolicyNames — the caller's SigV4 identity is + // authoritative for them. + glog.V(2).Infof("GetFederationToken: %s not in IAM user store, using SigV4 identity policies only", identity.Name) + default: + // Any other failure (store unreachable, misconfigured, etc.) + // means we cannot compute the caller's effective policies. + // Fail closed rather than mint a token with an incomplete set. glog.V(2).Infof("GetFederationToken: failed to resolve policies for %s: %v", identity.Name, err) h.writeSTSErrorResponse(w, r, STSErrInternalError, fmt.Errorf("failed to resolve caller policies")) return } - for _, p := range userPolicies { - policySet[p] = struct{}{} - } } if len(policySet) > 0 {