diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 9e0a0493e..32c7b044a 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -68,6 +68,10 @@ type IdentityAccessManagement struct { // Bucket policy engine for evaluating bucket policies policyEngine *BucketPolicyEngine + // Cached policy engine for IAM policy fallback evaluation. + // Keyed by policy name, kept in sync by PutPolicy/DeletePolicy. + iamPolicyEngine *policy_engine.PolicyEngine + // background polling stopChan chan struct{} shutdownOnce sync.Once @@ -659,6 +663,7 @@ func (iam *IdentityAccessManagement) ReplaceS3ApiConfiguration(config *iam_pb.S3 iam.nameToIdentity = nameToIdentity iam.accessKeyIdent = accessKeyIdent iam.policies = policies + iam.rebuildIAMPolicyEngineLocked() // Re-add environment-based identities that were preserved for _, envIdent := range envIdentities { @@ -915,6 +920,7 @@ func (iam *IdentityAccessManagement) MergeS3ApiConfiguration(config *iam_pb.S3Ap iam.nameToIdentity = nameToIdentity iam.accessKeyIdent = accessKeyIdent iam.policies = policies + iam.rebuildIAMPolicyEngineLocked() // Update authentication state based on whether identities exist // Once enabled, keep it enabled (one-way toggle) authJustEnabled := iam.updateAuthenticationState(len(identities)) @@ -1661,11 +1667,20 @@ func determineIAMAuthPath(sessionToken, principal, principalArn string) iamAuthP // evaluateIAMPolicies evaluates attached IAM policies for a user identity. // Returns true if any matching statement explicitly allows the action. +// Uses the cached iamPolicyEngine to avoid re-parsing policy JSON on every request. func (iam *IdentityAccessManagement) evaluateIAMPolicies(r *http.Request, identity *Identity, action Action, bucket, object string) bool { if identity == nil || len(identity.PolicyNames) == 0 { return false } + iam.m.RLock() + engine := iam.iamPolicyEngine + iam.m.RUnlock() + + if engine == nil { + return false + } + resource := buildResourceARN(bucket, object) principal := buildPrincipalARN(identity, r) s3Action := ResolveS3Action(r, string(action), bucket, object) @@ -1676,16 +1691,6 @@ func (iam *IdentityAccessManagement) evaluateIAMPolicies(r *http.Request, identi } for _, policyName := range identity.PolicyNames { - policy, err := iam.GetPolicy(policyName) - if err != nil { - continue - } - - engine := policy_engine.NewPolicyEngine() - if err := engine.SetBucketPolicy(policyName, policy.Content); err != nil { - continue - } - result := engine.EvaluatePolicy(policyName, &policy_engine.PolicyEvaluationArgs{ Action: s3Action, Resource: resource, @@ -1808,6 +1813,12 @@ func (iam *IdentityAccessManagement) PutPolicy(name string, content string) erro iam.policies = make(map[string]*iam_pb.Policy) } iam.policies[name] = &iam_pb.Policy{Name: name, Content: content} + iam.ensureIAMPolicyEngine() + // Remove old entry first so that a parse failure doesn't leave a stale allow. + _ = iam.iamPolicyEngine.DeleteBucketPolicy(name) + if err := iam.iamPolicyEngine.SetBucketPolicy(name, content); err != nil { + glog.Warningf("IAM policy %q is stored but could not be compiled for cache: %v", name, err) + } return nil } @@ -1826,9 +1837,36 @@ func (iam *IdentityAccessManagement) DeletePolicy(name string) error { iam.m.Lock() defer iam.m.Unlock() delete(iam.policies, name) + if iam.iamPolicyEngine != nil { + _ = iam.iamPolicyEngine.DeleteBucketPolicy(name) + } return nil } +// ensureIAMPolicyEngine lazily initializes the shared IAM policy engine. +// Must be called with iam.m held. +func (iam *IdentityAccessManagement) ensureIAMPolicyEngine() { + if iam.iamPolicyEngine == nil { + iam.iamPolicyEngine = policy_engine.NewPolicyEngine() + } +} + +// rebuildIAMPolicyEngineLocked rebuilds the entire IAM policy engine cache +// from the current policies map. Must be called with iam.m held. +func (iam *IdentityAccessManagement) rebuildIAMPolicyEngineLocked() { + if len(iam.policies) == 0 { + iam.iamPolicyEngine = nil + return + } + engine := policy_engine.NewPolicyEngine() + for name, p := range iam.policies { + if err := engine.SetBucketPolicy(name, p.Content); err != nil { + glog.Warningf("IAM policy cache rebuild: skipping invalid policy %q: %v", name, err) + } + } + iam.iamPolicyEngine = engine +} + // ListPolicies lists all policies func (iam *IdentityAccessManagement) ListPolicies() []*iam_pb.Policy { iam.m.RLock() diff --git a/weed/s3api/auth_credentials_test.go b/weed/s3api/auth_credentials_test.go index 1a4f5ac41..ad4f68f72 100644 --- a/weed/s3api/auth_credentials_test.go +++ b/weed/s3api/auth_credentials_test.go @@ -374,6 +374,28 @@ func TestVerifyActionPermissionPolicyFallback(t *testing.T) { assert.Equal(t, s3err.ErrNone, errCode) }) + t.Run("valid policy updated to invalid denies access", func(t *testing.T) { + iam := &IdentityAccessManagement{} + err := iam.PutPolicy("myPolicy", `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:GetObject","Resource":"arn:aws:s3:::test-bucket/*"}]}`) + assert.NoError(t, err) + + identity := &Identity{ + Name: "policy-user", + Account: &AccountAdmin, + PolicyNames: []string{"myPolicy"}, + } + + errCode := iam.VerifyActionPermission(buildRequest(t, http.MethodGet), identity, Action(ACTION_READ), "test-bucket", "test-object") + assert.Equal(t, s3err.ErrNone, errCode) + + // Update to invalid JSON — should revoke access. + err = iam.PutPolicy("myPolicy", "{broken") + assert.NoError(t, err) + + errCode = iam.VerifyActionPermission(buildRequest(t, http.MethodGet), identity, Action(ACTION_READ), "test-bucket", "test-object") + assert.Equal(t, s3err.ErrAccessDenied, errCode) + }) + t.Run("actions based path still works", func(t *testing.T) { iam := &IdentityAccessManagement{} identity := &Identity{