From 709d50a916bf3311eda2b3882ac5f2a0251f4223 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Tue, 15 Sep 2026 22:06:56 +0800 Subject: [PATCH 1/3] fix(iam): persist revocations across site replay and recovery Retain source-ordered tombstones and parent grant boundaries across both IAM backends, cache reloads, and deliberate identity recreation. Reconcile deletions through a versioned, bounded replication protocol with restart-aware acknowledgements. Cover inherited group grants, STS retention, same-key service recreation, absolute expiration, and failures after the durable commit. Document coordinated upgrades and the remaining consistency boundaries. Signed-off-by: Feng Ruohang --- .../rebrand-guard/compat-baseline.json | 2 + cmd/admin-router.go | 1 + cmd/iam-credential-retention_test.go | 111 +++ cmd/iam-etcd-store.go | 136 ++-- cmd/iam-group-revision.go | 164 ++++ cmd/iam-healing-performance_test.go | 60 ++ cmd/iam-object-store.go | 117 ++- cmd/iam-performance_test.go | 131 +++ cmd/iam-replay_test.go | 168 ++++ cmd/iam-replication-protocol.go | 321 ++++++++ cmd/iam-replication-protocol_test.go | 161 ++++ cmd/iam-revision-boundary_test.go | 393 +++++++++ cmd/iam-revision-index.go | 220 +++++ cmd/iam-revision-lock_test.go | 305 +++++++ cmd/iam-revision.go | 749 ++++++++++++++++++ cmd/iam-revision_test.go | 330 ++++++++ cmd/iam-revocation_test.go | 535 +++++++++++++ cmd/iam-service-recreation_test.go | 239 ++++++ cmd/iam-store.go | 558 +++++++------ cmd/iam.go | 77 +- cmd/metrics-v3-cluster-iam.go | 14 + cmd/metrics-v3.go | 4 + cmd/peer-rest-server.go | 8 +- cmd/site-replication-metadata-gate_test.go | 4 + cmd/site-replication.go | 98 ++- cmd/sts-handlers.go | 4 + docs/site-replication/iam-revocations.md | 218 +++++ 27 files changed, 4721 insertions(+), 407 deletions(-) create mode 100644 cmd/iam-credential-retention_test.go create mode 100644 cmd/iam-group-revision.go create mode 100644 cmd/iam-healing-performance_test.go create mode 100644 cmd/iam-performance_test.go create mode 100644 cmd/iam-replay_test.go create mode 100644 cmd/iam-replication-protocol.go create mode 100644 cmd/iam-replication-protocol_test.go create mode 100644 cmd/iam-revision-boundary_test.go create mode 100644 cmd/iam-revision-index.go create mode 100644 cmd/iam-revision-lock_test.go create mode 100644 cmd/iam-revision.go create mode 100644 cmd/iam-revision_test.go create mode 100644 cmd/iam-revocation_test.go create mode 100644 cmd/iam-service-recreation_test.go create mode 100644 docs/site-replication/iam-revocations.md diff --git a/buildscripts/rebrand-guard/compat-baseline.json b/buildscripts/rebrand-guard/compat-baseline.json index 0255143ef..33bd2b00c 100644 --- a/buildscripts/rebrand-guard/compat-baseline.json +++ b/buildscripts/rebrand-guard/compat-baseline.json @@ -829,6 +829,7 @@ "/site-replication/peer/bucket-ops", "/site-replication/peer/edit", "/site-replication/peer/iam-item", + "/site-replication/peer/iam-revisions", "/site-replication/peer/idp-settings", "/site-replication/peer/join", "/site-replication/peer/remove", @@ -877,6 +878,7 @@ "/v2/metrics/cluster", "/v2/metrics/node", "/v2/metrics/resource", + "/v3/site-replication/peer/iam-revisions", "/var/vcap/bosh", "/verifybinary", "/version", diff --git a/cmd/admin-router.go b/cmd/admin-router.go index d218e475e..ebda4d698 100644 --- a/cmd/admin-router.go +++ b/cmd/admin-router.go @@ -388,6 +388,7 @@ func registerAdminRouter(router *mux.Router, enableConfigOps bool) { adminRouter.Methods(http.MethodPut).Path(adminVersion + "/site-replication/peer/join").HandlerFunc(adminMiddleware(adminAPI.SRPeerJoin)) adminRouter.Methods(http.MethodPut).Path(adminVersion+"/site-replication/peer/bucket-ops").HandlerFunc(adminMiddleware(adminAPI.SRPeerBucketOps)).Queries("bucket", "{bucket:.*}").Queries("operation", "{operation:.*}") adminRouter.Methods(http.MethodPut).Path(adminVersion + "/site-replication/peer/iam-item").HandlerFunc(adminMiddleware(adminAPI.SRPeerReplicateIAMItem)) + adminRouter.Methods(http.MethodGet, http.MethodPut).Path(adminVersion + "/site-replication/peer/iam-revisions").HandlerFunc(adminMiddleware(adminAPI.SRPeerIAMRevisions)) adminRouter.Methods(http.MethodPut).Path(adminVersion + "/site-replication/peer/bucket-meta").HandlerFunc(adminMiddleware(adminAPI.SRPeerReplicateBucketItem)) adminRouter.Methods(http.MethodGet).Path(adminVersion + "/site-replication/peer/idp-settings").HandlerFunc(adminMiddleware(adminAPI.SRPeerGetIDPSettings)) adminRouter.Methods(http.MethodPut).Path(adminVersion + "/site-replication/edit").HandlerFunc(adminMiddleware(adminAPI.SiteReplicationEdit)) diff --git a/cmd/iam-credential-retention_test.go b/cmd/iam-credential-retention_test.go new file mode 100644 index 000000000..c4edcf169 --- /dev/null +++ b/cmd/iam-credential-retention_test.go @@ -0,0 +1,111 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-only + +package cmd + +import ( + "errors" + "testing" + "time" + + "github.com/minio/minio/internal/auth" +) + +func TestIAMCredentialRetention(t *testing.T) { + for _, backend := range []string{"object", "etcd"} { + t.Run(backend, func(t *testing.T) { + ctx, sys, _ := prepareIAMRevisionFixture(t, backend) + secret, err := getTokenSigningKey() + mustIAM(t, err) + parent := "external-idp-parent" + credential := func(exp time.Time) auth.Credentials { + cred, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": exp.Unix(), parentClaim: parent}, secret) + mustIAM(t, err) + cred.ParentUser = parent + return cred + } + // Disablement of an external identity must include cached STS, + // which are kept separately from regular and service accounts. + cred := credential(UTCNow().Add(time.Hour)) + _, err = sys.SetTempUser(ctx, cred.AccessKey, cred, "") + mustIAM(t, err) + mustIAM(t, sys.store.DeleteUsers(ctx, []string{parent})) + r, err := loadIAMRevision(ctx, sys.store, getUserIdentityPath(cred.AccessKey, stsUser)) + mustIAM(t, err) + if !r.Deleted || !r.ExpiresAt.Equal(cred.Expiration.Add(globalMaxSkewTime)) || r.Credentials.SessionToken != "" || r.Credentials.SecretKey != "" { + t.Fatal("early STS revocation lost its retention boundary or retained a secret") + } + if _, ok := sys.store.GetUser(cred.AccessKey); ok { + t.Fatal("external disablement left the STS cache live") + } + _, err = sys.SetTempUser(withIAMReplicationTime(ctx, UTCNow().Add(time.Minute)), cred.AccessKey, cred, "") + if !errors.Is(err, errIAMStaleUpdate) { + t.Fatalf("same revoked token was reissued by replay: %v", err) + } + var mp MappedPolicy + err = sys.store.loadIAMConfig(ctx, &mp, getMappedPolicyPath(cred.AccessKey, stsUser, false)) + if !errors.Is(err, errConfigNotFound) { + t.Fatalf("random STS key produced a permanent mapping: %v", err) + } + + // Seed genuinely expired immutable tokens, as an ordinary startup + // loader sees them. Natural expiry leaves no permanent tombstone. + expired := credential(UTCNow().Add(-time.Hour)) + path := getUserIdentityPath(expired.AccessKey, stsUser) + mustIAM(t, sys.store.saveIAMConfig(ctx, &UserIdentity{Version: 1, Credentials: expired, UpdatedAt: UTCNow().Add(-2 * time.Hour)}, path)) + _ = sys.store.loadUser(ctx, expired.AccessKey, stsUser, make(map[string]UserIdentity)) + var u UserIdentity + if err := sys.store.loadIAMConfig(ctx, &u, path); !errors.Is(err, errConfigNotFound) { + t.Fatalf("natural expiration retained a random key: %v", err) + } + // A retained early-revocation record is collectable only after the + // immutable token's expiration plus the skew allowance. + tomb := UserIdentity{Version: 1, Deleted: true, UpdatedAt: UTCNow().Add(-2 * time.Hour), ExpiresAt: expired.Expiration.Add(globalMaxSkewTime)} + mustIAM(t, sys.store.saveIAMConfig(ctx, &tomb, path)) + _ = sys.store.loadUser(ctx, expired.AccessKey, stsUser, make(map[string]UserIdentity)) + if err := sys.store.loadIAMConfig(ctx, &u, path); !errors.Is(err, errConfigNotFound) { + t.Fatalf("expired STS revocation not collected: %v", err) + } + if _, ok := sys.store.revisionIndex().snapshot()[path]; ok { + t.Fatal("expired STS retained an index entry") + } + }) + } +} + +func TestIAMPolicyDeletionRemainsExplicit(t *testing.T) { + for _, backend := range []string{"object", "etcd"} { + t.Run(backend, func(t *testing.T) { + ctx, sys, _ := prepareIAMRevisionFixture(t, backend) + mustIAM(t, sys.DeletePolicy(ctx, "misspelled-policy", true)) + r, err := loadIAMRevision(ctx, sys.store, getPolicyDocPath("misspelled-policy")) + mustIAM(t, err) + if r.Deleted { + t.Fatal("local nonexistent policy created a tombstone") + } + p, err := sys.store.GetPolicy("readwrite") + mustIAM(t, err) + if err := sys.DeletePolicy(ctx, "readwrite", true); err == nil { + t.Fatal("local pristine builtin policy became deletable") + } + _, err = sys.SetPolicy(ctx, "readwrite", p) + mustIAM(t, err) + mustIAM(t, sys.DeletePolicy(ctx, "readwrite", true)) + mustIAM(t, sys.store.LoadIAMCache(ctx, false)) + if _, err := sys.store.GetPolicy("readwrite"); !errors.Is(err, errNoSuchPolicy) { + t.Fatalf("reload restored an explicitly deleted override: %v", err) + } + _, err = sys.SetPolicy(ctx, "readwrite", p) + mustIAM(t, err) + if _, err := sys.store.GetPolicy("readwrite"); err != nil { + t.Fatal("explicit policy recreation failed", err) + } + mustIAM(t, globalSiteReplicationSys.PeerAddPolicyHandler(ctx, "remote-unknown-policy", nil, UTCNow())) + r, err = loadIAMRevision(ctx, sys.store, getPolicyDocPath("remote-unknown-policy")) + mustIAM(t, err) + if !r.Deleted { + t.Fatal("replicated unknown deletion lost its version") + } + }) + } +} diff --git a/cmd/iam-etcd-store.go b/cmd/iam-etcd-store.go index 850606fd5..0307fcac4 100644 --- a/cmd/iam-etcd-store.go +++ b/cmd/iam-etcd-store.go @@ -26,7 +26,6 @@ import ( "sync" "time" - jsoniter "github.com/json-iterator/go" "github.com/minio/minio-go/v7/pkg/set" "github.com/minio/minio/internal/config" "github.com/minio/minio/internal/kms" @@ -62,6 +61,7 @@ type IAMEtcdStore struct { sync.RWMutex *iamCache + index iamRevisionIndex usersSysType UsersSysType @@ -69,13 +69,17 @@ type IAMEtcdStore struct { } func newIAMEtcdStore(client *etcd.Client, usersSysType UsersSysType) *IAMEtcdStore { - return &IAMEtcdStore{ + store := &IAMEtcdStore{ iamCache: newIamCache(), client: client, usersSysType: usersSysType, } + store.revisions = &store.index + return store } +func (ies *IAMEtcdStore) revisionIndex() *iamRevisionIndex { return &ies.index } + func (ies *IAMEtcdStore) rlock() *iamCache { ies.RLock() return ies.iamCache @@ -103,6 +107,7 @@ func (ies *IAMEtcdStore) saveIAMConfig(ctx context.Context, item any, itemPath s if err != nil { return err } + plain := data if GlobalKMS != nil { data, err = config.EncryptBytes(GlobalKMS, data, kms.Context{ minioMetaBucket: path.Join(minioMetaBucket, itemPath), @@ -111,24 +116,28 @@ func (ies *IAMEtcdStore) saveIAMConfig(ctx context.Context, item any, itemPath s return err } } - return saveKeyEtcd(ctx, ies.client, itemPath, data, opts...) + if err := saveKeyEtcd(ctx, ies.client, itemPath, data, opts...); err != nil { + return err + } + ies.index.observe(itemPath, plain) + return nil } -func getIAMConfig(item any, data []byte, itemPath string) error { - data, err := decryptData(data, itemPath) +func (ies *IAMEtcdStore) decodeIAMConfig(item any, data []byte, path string) error { + data, err := decryptData(data, path) if err != nil { return err } - json := jsoniter.ConfigCompatibleWithStandardLibrary + ies.index.observe(path, data) return json.Unmarshal(data, item) } func (ies *IAMEtcdStore) loadIAMConfig(ctx context.Context, item any, path string) error { - data, err := readKeyEtcd(ctx, ies.client, path) + data, err := ies.loadIAMConfigBytes(ctx, path) if err != nil { return err } - return getIAMConfig(item, data, path) + return json.Unmarshal(data, item) } func (ies *IAMEtcdStore) loadIAMConfigBytes(ctx context.Context, path string) ([]byte, error) { @@ -136,11 +145,19 @@ func (ies *IAMEtcdStore) loadIAMConfigBytes(ctx context.Context, path string) ([ if err != nil { return nil, err } - return decryptData(data, path) + data, err = decryptData(data, path) + if err == nil { + ies.index.observe(path, data) + } + return data, err } func (ies *IAMEtcdStore) deleteIAMConfig(ctx context.Context, path string) error { - return deleteKeyEtcd(ctx, ies.client, path) + if err := deleteKeyEtcd(ctx, ies.client, path); err != nil { + return err + } + ies.index.forget(path) + return nil } func (ies *IAMEtcdStore) loadPolicyDocWithRetry(ctx context.Context, policy string, m map[string]PolicyDoc, _ int) error { @@ -162,6 +179,9 @@ func (ies *IAMEtcdStore) loadPolicyDoc(ctx context.Context, policy string, m map return err } + if p.Deleted { + return errNoSuchPolicy + } m[policy] = p return nil } @@ -181,7 +201,11 @@ func (ies *IAMEtcdStore) getPolicyDocKV(ctx context.Context, kvs *mvccpb.KeyValu return err } + ies.index.observe(string(kvs.Key), data) policy := extractPathPrefixAndSuffix(string(kvs.Key), iamConfigPoliciesPrefix, path.Base(string(kvs.Key))) + if p.Deleted { + return errNoSuchPolicy + } m[policy] = p return nil } @@ -207,7 +231,7 @@ func (ies *IAMEtcdStore) loadPolicyDocs(ctx context.Context, m map[string]Policy func (ies *IAMEtcdStore) getUserKV(ctx context.Context, userkv *mvccpb.KeyValue, userType IAMUserType, m map[string]UserIdentity, basePrefix string) error { var u UserIdentity - err := getIAMConfig(&u, userkv.Value, string(userkv.Key)) + err := ies.decodeIAMConfig(&u, userkv.Value, string(userkv.Key)) if err != nil { if err == errConfigNotFound { return errNoSuchUser @@ -219,10 +243,14 @@ func (ies *IAMEtcdStore) getUserKV(ctx context.Context, userkv *mvccpb.KeyValue, } func (ies *IAMEtcdStore) addUser(ctx context.Context, user string, userType IAMUserType, u UserIdentity, m map[string]UserIdentity) error { + if u.Deleted { + if userType == stsUser && !u.ExpiresAt.IsZero() && UTCNow().After(u.ExpiresAt) { + bestEffortIAMExpiration(ctx, ies, getUserIdentityPath(user, userType)) + } + return errNoSuchUser + } if u.Credentials.IsExpired() { - // Delete expired identity. - deleteKeyEtcd(ctx, ies.client, getUserIdentityPath(user, userType)) - deleteKeyEtcd(ctx, ies.client, getMappedPolicyPath(user, userType, false)) + bestEffortIAMExpiration(ctx, ies, getUserIdentityPath(user, userType)) return nil } if u.Credentials.AccessKey == "" { @@ -231,16 +259,17 @@ func (ies *IAMEtcdStore) addUser(ctx context.Context, user string, userType IAMU if u.Credentials.SessionToken != "" { jwtClaims, err := extractJWTClaims(u) if err != nil { - if u.Credentials.IsTemp() { - // We should delete such that the client can re-request - // for the expiring credentials. - deleteKeyEtcd(ctx, ies.client, getUserIdentityPath(user, userType)) - deleteKeyEtcd(ctx, ies.client, getMappedPolicyPath(user, userType, false)) - } + // A temporarily unavailable signing key is not proof of expiration. return nil } u.Credentials.Claims = jwtClaims.Map() } + if err := checkIAMParentRevision(ctx, ies, u.Credentials); err != nil { + if errors.Is(err, errIAMStaleUpdate) { + return errNoSuchUser + } + return err + } if u.Credentials.Description == "" { u.Credentials.Description = u.Credentials.Comment } @@ -258,6 +287,9 @@ func (ies *IAMEtcdStore) loadSecretKey(ctx context.Context, user string, userTyp } return "", err } + if u.Deleted { + return "", errNoSuchUser + } return u.Credentials.SecretKey, nil } @@ -274,6 +306,7 @@ func (ies *IAMEtcdStore) loadUser(ctx context.Context, user string, userType IAM } func (ies *IAMEtcdStore) loadUsers(ctx context.Context, userType IAMUserType, m map[string]UserIdentity) error { + ctx = withIAMExpirationCleanup(ctx) var basePrefix string switch userType { case svcUser: @@ -312,6 +345,9 @@ func (ies *IAMEtcdStore) loadGroup(ctx context.Context, group string, m map[stri } return err } + if gi.Deleted { + return errNoSuchGroup + } m[group] = gi return nil } @@ -349,13 +385,16 @@ func (ies *IAMEtcdStore) loadMappedPolicy(ctx context.Context, name string, user } return err } + if !ies.index.mappingAllowed(getMappedPolicyPath(name, userType, isGroup), p) { + return errNoSuchPolicy + } m.Store(name, p) return nil } -func getMappedPolicy(kv *mvccpb.KeyValue, m *xsync.MapOf[string, MappedPolicy], basePrefix string) error { +func (ies *IAMEtcdStore) getMappedPolicy(kv *mvccpb.KeyValue, m *xsync.MapOf[string, MappedPolicy], basePrefix string) error { var p MappedPolicy - err := getIAMConfig(&p, kv.Value, string(kv.Key)) + err := ies.decodeIAMConfig(&p, kv.Value, string(kv.Key)) if err != nil { if err == errConfigNotFound { return errNoSuchPolicy @@ -363,6 +402,9 @@ func getMappedPolicy(kv *mvccpb.KeyValue, m *xsync.MapOf[string, MappedPolicy], return err } name := extractPathPrefixAndSuffix(string(kv.Key), basePrefix, ".json") + if !ies.index.mappingAllowed(string(kv.Key), p) { + return errNoSuchPolicy + } m.Store(name, p) return nil } @@ -392,61 +434,13 @@ func (ies *IAMEtcdStore) loadMappedPolicies(ctx context.Context, userType IAMUse // Parse all policies mapping to create the proper data model for _, kv := range r.Kvs { - if err = getMappedPolicy(kv, m, basePrefix); err != nil && !errors.Is(err, errNoSuchPolicy) { + if err = ies.getMappedPolicy(kv, m, basePrefix); err != nil && !errors.Is(err, errNoSuchPolicy) { return err } } return nil } -func (ies *IAMEtcdStore) savePolicyDoc(ctx context.Context, policyName string, p PolicyDoc) error { - return ies.saveIAMConfig(ctx, &p, getPolicyDocPath(policyName)) -} - -func (ies *IAMEtcdStore) saveMappedPolicy(ctx context.Context, name string, userType IAMUserType, isGroup bool, mp MappedPolicy, opts ...options) error { - return ies.saveIAMConfig(ctx, mp, getMappedPolicyPath(name, userType, isGroup), opts...) -} - -func (ies *IAMEtcdStore) saveUserIdentity(ctx context.Context, name string, userType IAMUserType, u UserIdentity, opts ...options) error { - return ies.saveIAMConfig(ctx, u, getUserIdentityPath(name, userType), opts...) -} - -func (ies *IAMEtcdStore) saveGroupInfo(ctx context.Context, name string, gi GroupInfo) error { - return ies.saveIAMConfig(ctx, gi, getGroupInfoPath(name)) -} - -func (ies *IAMEtcdStore) deletePolicyDoc(ctx context.Context, name string) error { - err := ies.deleteIAMConfig(ctx, getPolicyDocPath(name)) - if err == errConfigNotFound { - err = errNoSuchPolicy - } - return err -} - -func (ies *IAMEtcdStore) deleteMappedPolicy(ctx context.Context, name string, userType IAMUserType, isGroup bool) error { - err := ies.deleteIAMConfig(ctx, getMappedPolicyPath(name, userType, isGroup)) - if err == errConfigNotFound { - err = errNoSuchPolicy - } - return err -} - -func (ies *IAMEtcdStore) deleteUserIdentity(ctx context.Context, name string, userType IAMUserType) error { - err := ies.deleteIAMConfig(ctx, getUserIdentityPath(name, userType)) - if err == errConfigNotFound { - err = errNoSuchUser - } - return err -} - -func (ies *IAMEtcdStore) deleteGroupInfo(ctx context.Context, name string) error { - err := ies.deleteIAMConfig(ctx, getGroupInfoPath(name)) - if err == errConfigNotFound { - err = errNoSuchGroup - } - return err -} - func (ies *IAMEtcdStore) watch(ctx context.Context, keyPath string) <-chan iamWatchEvent { ch := make(chan iamWatchEvent) diff --git a/cmd/iam-group-revision.go b/cmd/iam-group-revision.go new file mode 100644 index 000000000..db912f781 --- /dev/null +++ b/cmd/iam-group-revision.go @@ -0,0 +1,164 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-only + +package cmd + +import ( + "context" + "maps" + "slices" + "time" + + "github.com/minio/minio-go/v7/pkg/set" +) + +type ( + iamGroupGrantsKey struct{} + iamGroupMutationKey struct{} + iamGroupMutation struct { + Members []string + Remove bool + StatusOnly bool + } +) + +// Merge the intended mutation with the record read under the distributed +// revision lock, not the older cache used to prepare the request. +func mergeIAMGroupMutation(ctx context.Context, previous GroupInfo, next *GroupInfo) { + op, ok := ctx.Value(iamGroupMutationKey{}).(iamGroupMutation) + if !ok || previous.Deleted || previous.Version == 0 { + return + } + members := set.CreateStringSet(previous.Members...) + grants := maps.Clone(previous.MemberGrants) + if grants == nil { + grants = make(map[string]time.Time) + } + switch { + case op.StatusOnly: + // Only the status changes. + case op.Remove: + for _, member := range op.Members { + members.Remove(member) + delete(grants, member) + } + next.Status = previous.Status + default: + requested := set.CreateStringSet(next.Members...) + for _, member := range op.Members { + if !requested.Contains(member) { + continue + } + at := next.MemberGrants[member] + if at.Before(grants[member]) { + continue + } + members.Add(member) + grants[member] = at + } + next.Status = previous.Status + } + next.Members, next.MemberGrants = members.ToSlice(), grants + slices.Sort(next.Members) +} + +// A non-nil map is supplied by the versioned peer envelope, including for +// snapshots. Missing times are unknown, never the snapshot's newer timestamp. +func withIAMGroupGrants(ctx context.Context, grants map[string]time.Time) context.Context { + return context.WithValue(ctx, iamGroupGrantsKey{}, grants) +} + +func (c *iamCache) effectiveGroupMembers(gi GroupInfo) []string { + var members []string + for _, member := range gi.Members { + if c.groupMemberAllowed(member, gi.MemberGrants[member], gi.RevokedBefore) { + members = append(members, member) + } + } + return members +} + +func (c *iamCache) effectiveUserGroups(user string) []string { + var groups []string + for group := range c.iamUserGroupMemberships[user] { + gi, ok := c.iamGroupsMap[group] + r := c.revisions.get(getGroupInfoPath(group)) + if r.RevokedBefore.After(gi.RevokedBefore) { + gi.RevokedBefore = r.RevokedBefore + } + if ok && !r.Deleted && c.groupMemberAllowed(user, gi.MemberGrants[user], gi.RevokedBefore) { + groups = append(groups, group) + } + } + return groups +} + +func (c *iamCache) addGroupMembers(ctx context.Context, gi GroupInfo, members []string) (GroupInfo, error) { + grants, versioned := ctx.Value(iamGroupGrantsKey{}).(map[string]time.Time) + if boundary, ok := ctx.Value(iamRecordBoundaryKey{}).(time.Time); ok && boundary.After(gi.RevokedBefore) { + gi.RevokedBefore = boundary + } + origin, replicated := iamReplicationTime(ctx) + gi.Members = slices.Clone(gi.Members) + gi.MemberGrants = maps.Clone(gi.MemberGrants) + if gi.MemberGrants == nil { + gi.MemberGrants = make(map[string]time.Time) + } + current := set.CreateStringSet(gi.Members...) + gi.UpdatedAt = UTCNow() + if replicated { + gi.UpdatedAt = origin + } + for _, member := range members { + at := gi.UpdatedAt + r := c.userRevocation(member) + if replicated { + switch { + case versioned: + at = grants[member] + if at.After(origin) { + return gi, errInvalidArgument + } + case !r.RevokedBefore.IsZero() || r.Deleted || !gi.RevokedBefore.IsZero(): + // Legacy snapshots cannot prove a post-revocation grant. + continue + case current.Contains(member): + continue + } + if !c.groupMemberAllowed(member, at, gi.RevokedBefore) { + continue + } + } else { + if current.Contains(member) && c.groupMemberAllowed(member, gi.MemberGrants[member], gi.RevokedBefore) { + continue // Editing the group is not reissuing every grant. + } + if !at.After(gi.RevokedBefore) { + at = gi.RevokedBefore.Add(time.Nanosecond) + } + if !at.After(r.RevokedBefore) { + at = r.RevokedBefore.Add(time.Nanosecond) + } + if !at.After(gi.MemberGrants[member]) { + at = gi.MemberGrants[member].Add(time.Nanosecond) + } + if at.After(gi.UpdatedAt) { + gi.UpdatedAt = at + } + } + u, ok := c.iamUsersMap[member] + if !ok { + return gi, errNoSuchUser + } + if u.Credentials.IsTemp() || u.Credentials.IsServiceAccount() { + return gi, errIAMActionNotAllowed + } + if previous := gi.MemberGrants[member]; previous.After(at) { + continue + } + current.Add(member) + gi.MemberGrants[member] = at + } + gi.Members = current.ToSlice() + slices.Sort(gi.Members) + return gi, nil +} diff --git a/cmd/iam-healing-performance_test.go b/cmd/iam-healing-performance_test.go new file mode 100644 index 000000000..c4f463f58 --- /dev/null +++ b/cmd/iam-healing-performance_test.go @@ -0,0 +1,60 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-only + +package cmd + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/minio/madmin-go/v3" +) + +// Measures steady-state index traversal, sorting and the capability request. +// The network peer acknowledges real batches but performs no disk I/O; this +// benchmark deliberately does not claim durable catch-up throughput. +func BenchmarkIAMRevisionConvergedHealing(b *testing.B) { + for _, n := range []int{1000, 10000} { + b.Run(fmt.Sprint(n), func(b *testing.B) { + ctx, sys, _ := prepareIAMRevisionFixture(b) + _, err := sys.CreateUser(ctx, "benchmark-sync", madmin.AddOrUpdateUserReq{SecretKey: "valid-sync-password", Status: madmin.AccountEnabled}) + mustIAM(b, err) + for i := range n { + at := UTCNow().Add(time.Duration(i) * time.Nanosecond) + data, err := json.Marshal(iamRevision{Deleted: true, UpdatedAt: at, RevokedBefore: at}) + mustIAM(b, err) + sys.store.revisionIndex().observe(getUserIdentityPath(fmt.Sprintf("deleted-%06d", i), regUser), data) + } + var puts atomic.Int64 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/minio/health/live" { + w.WriteHeader(http.StatusOK) + return + } + if r.Method == http.MethodPut { + puts.Add(1) + } + _ = json.NewEncoder(w).Encode(iamRevisionResponse{iamRevisionStatus: iamRevisionStatus{Version: iamRevisionProtocol, Node: "node-1", Instance: "benchmark-peer", Digest: "constant"}}) + })) + defer server.Close() + c := &SiteReplicationSys{enabled: true, state: srState{ServiceAccountAccessKey: "benchmark-sync", Peers: map[string]madmin.PeerInfo{globalDeploymentID(): {DeploymentID: globalDeploymentID(), Name: "local"}, "remote": {DeploymentID: "remote", Name: "remote", Endpoint: server.URL}}}} + mustIAM(b, c.healIAMDeletions(ctx)) + before := puts.Load() + b.ReportAllocs() + b.ResetTimer() + for b.Loop() { + mustIAM(b, c.healIAMDeletions(ctx)) + } + b.StopTimer() + b.ReportMetric(float64(puts.Load()-before)/float64(b.N), "PUT/op") + if puts.Load() != before { + b.Fatal("steady-state healing replayed acknowledged records") + } + }) + } +} diff --git a/cmd/iam-object-store.go b/cmd/iam-object-store.go index b1865878f..c3ece5a2b 100644 --- a/cmd/iam-object-store.go +++ b/cmd/iam-object-store.go @@ -45,6 +45,7 @@ type IAMObjectStore struct { sync.RWMutex *iamCache + index iamRevisionIndex usersSysType UsersSysType @@ -52,13 +53,17 @@ type IAMObjectStore struct { } func newIAMObjectStore(objAPI ObjectLayer, usersSysType UsersSysType) *IAMObjectStore { - return &IAMObjectStore{ + store := &IAMObjectStore{ iamCache: newIamCache(), objAPI: objAPI, usersSysType: usersSysType, } + store.revisions = &store.index + return store } +func (iamOS *IAMObjectStore) revisionIndex() *iamRevisionIndex { return &iamOS.index } + func (iamOS *IAMObjectStore) rlock() *iamCache { iamOS.RLock() return iamOS.iamCache @@ -87,6 +92,7 @@ func (iamOS *IAMObjectStore) saveIAMConfig(ctx context.Context, item any, objPat if err != nil { return err } + plain := data if GlobalKMS != nil { data, err = config.EncryptBytes(GlobalKMS, data, kms.Context{ minioMetaBucket: path.Join(minioMetaBucket, objPath), @@ -95,7 +101,11 @@ func (iamOS *IAMObjectStore) saveIAMConfig(ctx context.Context, item any, objPat return err } } - return saveConfig(ctx, iamOS.objAPI, objPath, data) + if err := saveConfig(ctx, iamOS.objAPI, objPath, data); err != nil { + return err + } + iamOS.index.observe(objPath, plain) + return nil } func decryptData(data []byte, objPath string) ([]byte, error) { @@ -133,6 +143,7 @@ func (iamOS *IAMObjectStore) loadIAMConfigBytesWithMetadata(ctx context.Context, if err != nil { return nil, meta, err } + iamOS.index.observe(objPath, data) return data, meta, nil } @@ -146,7 +157,11 @@ func (iamOS *IAMObjectStore) loadIAMConfig(ctx context.Context, item any, objPat } func (iamOS *IAMObjectStore) deleteIAMConfig(ctx context.Context, path string) error { - return deleteConfig(ctx, iamOS.objAPI, path) + if err := deleteConfig(ctx, iamOS.objAPI, path); err != nil { + return err + } + iamOS.index.forget(path) + return nil } func (iamOS *IAMObjectStore) loadPolicyDocWithRetry(ctx context.Context, policy string, m map[string]PolicyDoc, retries int) error { @@ -171,6 +186,10 @@ func (iamOS *IAMObjectStore) loadPolicyDocWithRetry(ctx context.Context, policy return err } + if p.Deleted { + return errNoSuchPolicy + } + if p.Version == 0 { // This means that policy was in the old version (without any // timestamp info). We fetch the mod time of the file and save @@ -200,6 +219,10 @@ func (iamOS *IAMObjectStore) loadPolicy(ctx context.Context, policy string) (Pol return p, err } + if p.Deleted { + return PolicyDoc{}, errNoSuchPolicy + } + if p.Version == 0 { // This means that policy was in the old version (without any // timestamp info). We fetch the mod time of the file and save @@ -245,6 +268,9 @@ func (iamOS *IAMObjectStore) loadSecretKey(ctx context.Context, user string, use } return "", err } + if u.Deleted { + return "", errNoSuchUser + } return u.Credentials.SecretKey, nil } @@ -258,10 +284,15 @@ func (iamOS *IAMObjectStore) loadUserIdentity(ctx context.Context, user string, return u, err } + if u.Deleted { + if userType == stsUser && !u.ExpiresAt.IsZero() && UTCNow().After(u.ExpiresAt) { + bestEffortIAMExpiration(ctx, iamOS, getUserIdentityPath(user, userType)) + } + return UserIdentity{}, errNoSuchUser + } + if u.Credentials.IsExpired() { - // Delete expired identity - ignoring errors here. - iamOS.deleteIAMConfig(ctx, getUserIdentityPath(user, userType)) - iamOS.deleteIAMConfig(ctx, getMappedPolicyPath(user, userType, false)) + bestEffortIAMExpiration(ctx, iamOS, getUserIdentityPath(user, userType)) return u, errNoSuchUser } @@ -272,16 +303,18 @@ func (iamOS *IAMObjectStore) loadUserIdentity(ctx context.Context, user string, if u.Credentials.SessionToken != "" { jwtClaims, err := extractJWTClaims(u) if err != nil { - if u.Credentials.IsTemp() { - // We should delete such that the client can re-request - // for the expiring credentials. - iamOS.deleteIAMConfig(ctx, getUserIdentityPath(user, userType)) - iamOS.deleteIAMConfig(ctx, getMappedPolicyPath(user, userType, false)) - } - return u, errNoSuchUser + // During startup the site signing key may not be available yet. + // Reject this load without deleting a credential that has not expired. + return UserIdentity{}, errNoSuchUser } u.Credentials.Claims = jwtClaims.Map() } + if err := checkIAMParentRevision(ctx, iamOS, u.Credentials); err != nil { + if errors.Is(err, errIAMStaleUpdate) { + return UserIdentity{}, errNoSuchUser + } + return UserIdentity{}, err + } if u.Credentials.Description == "" { u.Credentials.Description = u.Credentials.Comment @@ -320,6 +353,7 @@ func (iamOS *IAMObjectStore) loadUser(ctx context.Context, user string, userType } func (iamOS *IAMObjectStore) loadUsers(ctx context.Context, userType IAMUserType, m map[string]UserIdentity) error { + ctx = withIAMExpirationCleanup(ctx) var basePrefix string switch userType { case svcUser: @@ -354,6 +388,9 @@ func (iamOS *IAMObjectStore) loadGroup(ctx context.Context, group string, m map[ } return err } + if g.Deleted { + return errNoSuchGroup + } m[group] = g return nil } @@ -391,6 +428,9 @@ func (iamOS *IAMObjectStore) loadMappedPolicyWithRetry(ctx context.Context, name goto retry } + if !iamOS.index.mappingAllowed(getMappedPolicyPath(name, userType, isGroup), p) { + return errNoSuchPolicy + } m.Store(name, p) return nil } @@ -405,6 +445,9 @@ func (iamOS *IAMObjectStore) loadMappedPolicyInternal(ctx context.Context, name } return p, err } + if !iamOS.index.mappingAllowed(getMappedPolicyPath(name, userType, isGroup), p) { + return MappedPolicy{}, errNoSuchPolicy + } return p, nil } @@ -824,54 +867,6 @@ func (iamOS *IAMObjectStore) loadAllFromObjStore(ctx context.Context, cache *iam return nil } -func (iamOS *IAMObjectStore) savePolicyDoc(ctx context.Context, policyName string, p PolicyDoc) error { - return iamOS.saveIAMConfig(ctx, &p, getPolicyDocPath(policyName)) -} - -func (iamOS *IAMObjectStore) saveMappedPolicy(ctx context.Context, name string, userType IAMUserType, isGroup bool, mp MappedPolicy, opts ...options) error { - return iamOS.saveIAMConfig(ctx, mp, getMappedPolicyPath(name, userType, isGroup), opts...) -} - -func (iamOS *IAMObjectStore) saveUserIdentity(ctx context.Context, name string, userType IAMUserType, u UserIdentity, opts ...options) error { - return iamOS.saveIAMConfig(ctx, u, getUserIdentityPath(name, userType), opts...) -} - -func (iamOS *IAMObjectStore) saveGroupInfo(ctx context.Context, name string, gi GroupInfo) error { - return iamOS.saveIAMConfig(ctx, gi, getGroupInfoPath(name)) -} - -func (iamOS *IAMObjectStore) deletePolicyDoc(ctx context.Context, name string) error { - err := iamOS.deleteIAMConfig(ctx, getPolicyDocPath(name)) - if err == errConfigNotFound { - err = errNoSuchPolicy - } - return err -} - -func (iamOS *IAMObjectStore) deleteMappedPolicy(ctx context.Context, name string, userType IAMUserType, isGroup bool) error { - err := iamOS.deleteIAMConfig(ctx, getMappedPolicyPath(name, userType, isGroup)) - if err == errConfigNotFound { - err = errNoSuchPolicy - } - return err -} - -func (iamOS *IAMObjectStore) deleteUserIdentity(ctx context.Context, name string, userType IAMUserType) error { - err := iamOS.deleteIAMConfig(ctx, getUserIdentityPath(name, userType)) - if err == errConfigNotFound { - err = errNoSuchUser - } - return err -} - -func (iamOS *IAMObjectStore) deleteGroupInfo(ctx context.Context, name string) error { - err := iamOS.deleteIAMConfig(ctx, getGroupInfoPath(name)) - if err == errConfigNotFound { - err = errNoSuchGroup - } - return err -} - // Lists objects in the minioMetaBucket at the given path prefix. All returned // items have the pathPrefix removed from their names. func listIAMConfigItems(ctx context.Context, objAPI ObjectLayer, pathPrefix string) <-chan itemOrErr[string] { diff --git a/cmd/iam-performance_test.go b/cmd/iam-performance_test.go new file mode 100644 index 000000000..71e4504c0 --- /dev/null +++ b/cmd/iam-performance_test.go @@ -0,0 +1,131 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-only + +package cmd + +import ( + "context" + "fmt" + "os" + "testing" + "time" + + "github.com/minio/madmin-go/v3" + "github.com/minio/minio/internal/auth" +) + +// Uses APIs shared with the pre-revision tree so the same benchmark can be +// overlaid on that tree for a comparable local baseline. +func prepareIAMPerformanceFixture(b *testing.B) (context.Context, *IAMSys) { + b.Helper() + resetTestGlobals() + ctx, cancel := context.WithCancel(context.Background()) + disks, err := getRandomDisks(1) + if err != nil { + b.Fatal(err) + } + obj, _, err := initObjectLayer(ctx, mustGetPoolEndpoints(0, disks...)) + if err != nil { + b.Fatal(err) + } + initAllSubsystems(ctx) + globalIAMSys.initStore(obj, nil) + if err := globalIAMSys.Load(ctx, true); err != nil { + b.Fatal(err) + } + b.Cleanup(func() { cancel(); obj.Shutdown(context.Background()); os.RemoveAll(disks[0]); resetTestGlobals() }) + return ctx, globalIAMSys +} + +func BenchmarkIAMCachedCredential(b *testing.B) { + for _, kind := range []string{"user", "service", "sts"} { + b.Run(kind, func(b *testing.B) { + ctx, sys := prepareIAMPerformanceFixture(b) + const parent = "benchmark-parent" + _, err := sys.CreateUser(ctx, parent, madmin.AddOrUpdateUserReq{SecretKey: "benchmark-user-password", Status: madmin.AccountEnabled}) + if err != nil { + b.Fatal(err) + } + key := parent + if kind == "service" { + c, _, err := sys.NewServiceAccount(ctx, parent, nil, newServiceAccountOpts{accessKey: "benchmark-service", secretKey: "benchmark-service-password"}) + if err != nil { + b.Fatal(err) + } + key = c.AccessKey + } + if kind == "sts" { + secret, err := getTokenSigningKey() + if err != nil { + b.Fatal(err) + } + c, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: parent}, secret) + if err != nil { + b.Fatal(err) + } + c.ParentUser = parent + if _, err := sys.SetTempUser(ctx, c.AccessKey, c, ""); err != nil { + b.Fatal(err) + } + key = c.AccessKey + } + b.ReportAllocs() + b.ResetTimer() + for b.Loop() { + if _, ok := sys.store.GetUser(key); !ok { + b.Fatal("credential missing") + } + } + }) + } +} + +func BenchmarkIAMSetTempUser(b *testing.B) { + ctx, sys := prepareIAMPerformanceFixture(b) + const parent = "benchmark-sts-parent" + _, err := sys.CreateUser(ctx, parent, madmin.AddOrUpdateUserReq{SecretKey: "benchmark-user-password", Status: madmin.AccountEnabled}) + if err != nil { + b.Fatal(err) + } + secret, err := getTokenSigningKey() + if err != nil { + b.Fatal(err) + } + cred, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: parent}, secret) + if err != nil { + b.Fatal(err) + } + cred.ParentUser = parent + b.ReportAllocs() + b.ResetTimer() + for b.Loop() { + if _, err := sys.SetTempUser(ctx, cred.AccessKey, cred, "readwrite"); err != nil { + b.Fatal(err) + } + } +} + +// Run with -benchtime=1x. Preparation is outside the timer; each measured load +// sees a fresh set of expired reusable service-account records. +func BenchmarkIAMColdLoadExpiredServices(b *testing.B) { + for _, count := range []int{100, 1000} { + b.Run(fmt.Sprint(count), func(b *testing.B) { + ctx, sys := prepareIAMPerformanceFixture(b) + b.ReportAllocs() + for i := 0; i < b.N; i++ { + b.StopTimer() + for j := 0; j < count; j++ { + key := fmt.Sprintf("expired-benchmark-%d-%d", i, j) + u := UserIdentity{Version: 1, UpdatedAt: UTCNow().Add(-2 * time.Hour), Credentials: auth.Credentials{AccessKey: key, SecretKey: "expired-benchmark-password", ParentUser: "absent-idp-parent", Expiration: UTCNow().Add(-time.Hour), Status: auth.AccountOn}} + if err := sys.store.saveIAMConfig(ctx, &u, getUserIdentityPath(key, svcUser)); err != nil { + b.Fatal(err) + } + } + b.StartTimer() + if err := sys.store.LoadIAMCache(ctx, true); err != nil { + b.Fatal(err) + } + } + }) + } +} diff --git a/cmd/iam-replay_test.go b/cmd/iam-replay_test.go new file mode 100644 index 000000000..6360cf516 --- /dev/null +++ b/cmd/iam-replay_test.go @@ -0,0 +1,168 @@ +package cmd + +import ( + "context" + "errors" + "os" + "testing" + "time" + + "github.com/minio/madmin-go/v3" + "github.com/pgsty/silo-pkg/v3/policy" +) + +func TestReviewIAMRevokedUserReplay(t *testing.T) { + resetTestGlobals() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + obj, disk, err := prepareFS(ctx) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(disk) + defer obj.Shutdown(ctx) + defer resetTestGlobals() + user := "review-revoked-user" + req := madmin.AddOrUpdateUserReq{SecretKey: "review-valid-password", Status: madmin.AccountEnabled} + created, err := globalIAMSys.CreateUser(ctx, user, req) + if err != nil { + t.Fatal(err) + } + policyAt, err := globalIAMSys.PolicyDBSet(ctx, user, "readwrite", regUser, false) + if err != nil { + t.Fatal(err) + } + args := policy.Args{AccountName: user, Action: policy.GetObjectAction, BucketName: "review-bucket", ObjectName: "review-object"} + if !globalIAMSys.IsAllowed(args) { + t.Fatal("seed must allow object read") + } + if err := globalIAMSys.DeleteUser(ctx, user, false); err != nil { + t.Fatal(err) + } + if err := globalIAMSys.store.LoadIAMCache(ctx, false); err != nil { + t.Fatal(err) + } + if globalIAMSys.IsAllowed(args) { + t.Fatal("deletion did not remove initial permission") + } + if err := globalSiteReplicationSys.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: user, UserReq: &req}, created); err != nil { + t.Fatal(err) + } + if _, err := globalIAMSys.GetUserInfo(ctx, user); !errors.Is(err, errNoSuchUser) { + t.Errorf("revoked user restored by an older replicated create, GetUserInfo error = %v", err) + } + if err := globalSiteReplicationSys.PeerPolicyMappingHandler(ctx, &madmin.SRPolicyMapping{UserOrGroup: user, UserType: int(regUser), Policy: "readwrite"}, policyAt); err != nil { + t.Fatal(err) + } + if globalIAMSys.IsAllowed(args) { + t.Error("older replicated identity and policy events restored revoked S3 read permission") + } +} + +func TestReviewIAMSourceTimestampOrder(t *testing.T) { + resetTestGlobals() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + obj, disk, err := prepareFS(ctx) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(disk) + defer obj.Shutdown(ctx) + defer resetTestGlobals() + user := "review-ordered-user" + req := madmin.AddOrUpdateUserReq{SecretKey: "review-valid-password", Status: madmin.AccountEnabled} + origin := UTCNow().Add(-time.Hour) + if err := globalSiteReplicationSys.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: user, UserReq: &req}, origin); err != nil { + t.Fatal(err) + } + if err := globalSiteReplicationSys.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: user, IsDeleteReq: true}, origin.Add(time.Minute)); err != nil { + t.Fatal(err) + } + if _, err := globalIAMSys.GetUserInfo(ctx, user); !errors.Is(err, errNoSuchUser) { + t.Fatalf("newer source deletion skipped after delayed creation, GetUserInfo error = %v", err) + } +} + +// A user's old group grant must not return after deletion and deliberate recreation. +func TestR3CandidateOldGroupReplayAfterRecreation(t *testing.T) { + resetTestGlobals() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + obj, disk, err := prepareFS(ctx) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(disk) + defer obj.Shutdown(ctx) + defer resetTestGlobals() + must := func(err error) { + t.Helper() + if err != nil { + t.Fatal(err) + } + } + user, group := "r3-group-member", "r3-granting-group" + origin := UTCNow().Add(-time.Hour) + req := madmin.AddOrUpdateUserReq{SecretKey: "valid-r3-user-password", Status: madmin.AccountEnabled} + peer := &globalSiteReplicationSys + must(peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: user, UserReq: &req}, origin)) + add := &madmin.SRGroupInfo{UpdateReq: madmin.GroupAddRemove{Group: group, Members: []string{user}}} + must(peer.PeerGroupInfoChangeHandler(ctx, add, origin.Add(time.Minute))) + must(peer.PeerPolicyMappingHandler(ctx, &madmin.SRPolicyMapping{UserOrGroup: group, IsGroup: true, UserType: int(regUser), Policy: "readwrite"}, origin.Add(time.Minute))) + args := policy.Args{AccountName: user, Action: policy.GetObjectAction, BucketName: "r3-bucket", ObjectName: "probe"} + if !globalIAMSys.IsAllowed(args) { + t.Fatal("fixture must grant through group") + } + must(peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: user, IsDeleteReq: true}, origin.Add(2*time.Minute))) + must(peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: user, UserReq: &req}, origin.Add(3*time.Minute))) + must(globalIAMSys.store.LoadIAMCache(ctx, false)) + if globalIAMSys.IsAllowed(args) { + t.Fatal("recreation must start without deleted group membership") + } + must(peer.PeerGroupInfoChangeHandler(ctx, add, origin.Add(time.Minute))) + must(globalIAMSys.store.LoadIAMCache(ctx, false)) + if globalIAMSys.IsAllowed(args) { + t.Fatal("old group event restored the deleted user's read grant after recreation and durable reload") + } +} + +// The user delete is also a revocation of its earlier group memberships. +func TestR3CandidateLateDeleteRetainsOldGroupGrant(t *testing.T) { + resetTestGlobals() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + obj, disk, err := prepareFS(ctx) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(disk) + defer obj.Shutdown(ctx) + defer resetTestGlobals() + must := func(err error) { + t.Helper() + if err != nil { + t.Fatal(err) + } + } + user, group := "r3-late-member", "r3-late-group" + origin := UTCNow().Add(-time.Hour) + req := madmin.AddOrUpdateUserReq{SecretKey: "valid-r3-user-password", Status: madmin.AccountEnabled} + peer := &globalSiteReplicationSys + must(peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: user, UserReq: &req}, origin)) + must(peer.PeerGroupInfoChangeHandler(ctx, &madmin.SRGroupInfo{UpdateReq: madmin.GroupAddRemove{Group: group, Members: []string{user}}}, origin.Add(time.Minute))) + must(peer.PeerPolicyMappingHandler(ctx, &madmin.SRPolicyMapping{UserOrGroup: group, IsGroup: true, UserType: int(regUser), Policy: "readwrite"}, origin.Add(time.Minute))) + args := policy.Args{AccountName: user, Action: policy.GetObjectAction, BucketName: "r3-bucket", ObjectName: "probe"} + if !globalIAMSys.IsAllowed(args) { + t.Fatal("fixture must grant through group") + } + must(peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: user, UserReq: &req}, origin.Add(3*time.Minute))) + must(peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: user, IsDeleteReq: true}, origin.Add(2*time.Minute))) + must(globalIAMSys.store.LoadIAMCache(ctx, false)) + if _, ok := globalIAMSys.GetUser(ctx, user); !ok { + t.Fatal("newer identity must survive") + } + if globalIAMSys.IsAllowed(args) { + t.Fatal("late user deletion retained the older group grant on the recreated identity") + } +} diff --git a/cmd/iam-replication-protocol.go b/cmd/iam-replication-protocol.go new file mode 100644 index 000000000..37d79e0a6 --- /dev/null +++ b/cmd/iam-replication-protocol.go @@ -0,0 +1,321 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-only + +package cmd + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "sort" + "strings" + "sync/atomic" + "time" + + "github.com/minio/madmin-go/v3" + xhttp "github.com/minio/minio/internal/http" + "github.com/pgsty/silo-pkg/v3/policy" +) + +const ( + iamRevisionProtocol = 1 + iamRevisionPeerPath = "/v3/site-replication/peer/iam-revisions" + iamUserBoundaryType = "silo-user-revocation" + iamGroupBoundaryType = "silo-group-revocation" + maxIAMRevisionBatch = 128 +) + +var iamRevisionInstance = mustGetUUID() + +type iamUserBoundary struct { + User string `json:"user"` + Before time.Time `json:"before"` +} + +type iamGroupBoundary struct { + Group string `json:"group"` + Before time.Time `json:"before"` +} + +// The server owns this additive protocol, without changing the client SDK or +// overloading a policy/document field. Old servers reject the dedicated route +// before applying any change that would lose revocation or member metadata. +type iamReplicationItem struct { + madmin.SRIAMItem + GroupGrants map[string]time.Time `json:"groupGrants,omitempty"` + GroupSnapshot bool `json:"groupSnapshot,omitempty"` + UserRevocation *iamUserBoundary `json:"userRevocation,omitempty"` + GroupRevocation *iamGroupBoundary `json:"groupRevocation,omitempty"` + RevokedBefore time.Time `json:"revokedBefore,omitempty"` +} + +type iamRevisionBatch struct { + Version int `json:"version"` + Items []iamReplicationItem `json:"items"` +} + +type iamRevisionStatus struct { + Version int `json:"version"` + Node string `json:"node"` + Instance string `json:"instance"` + Digest string `json:"digest"` +} + +type iamRevisionResponse struct { + iamRevisionStatus + Errors []string `json:"errors,omitempty"` +} + +type iamRevisionBatchError struct{ failures []string } + +func (e *iamRevisionBatchError) Error() string { + return "IAM revision batch: " + strings.Join(e.failures, "; ") +} + +type iamRevisionProgress struct { + Instances map[string]string + Acknowledged map[string]string +} + +type iamRevisionMetrics struct { + healFailures atomic.Uint64 + healLastSuccess atomic.Int64 + healDurationMillis atomic.Int64 +} + +func iamRevisionDigest(items map[string]iamRevision) string { + paths := make([]string, 0, len(items)) + for path := range items { + paths = append(paths, path) + } + sort.Strings(paths) + h := sha256.New() + for _, path := range paths { + r := items[path] + fmt.Fprintf(h, "%q %s %t %s\n", path, r.timestamp().UTC().Format(time.RFC3339Nano), r.Deleted, r.RevokedBefore.UTC().Format(time.RFC3339Nano)) + } + return hex.EncodeToString(h.Sum(nil)) +} + +func (store *IAMStoreSys) iamRevisionStatus() iamRevisionStatus { + node := globalLocalNodeName + if node == "" { + node = "local" + } + return iamRevisionStatus{Version: iamRevisionProtocol, Node: node, Instance: iamRevisionInstance, Digest: store.revisionIndex().digest()} +} + +func executeIAMRevisionRequest(ctx context.Context, client *madmin.AdminClient, method string, batch *iamRevisionBatch) (status iamRevisionStatus, err error) { + var content []byte + if batch != nil { + content, err = json.Marshal(batch) + if err != nil { + return status, err + } + } + resp, err := client.ExecuteMethod(ctx, method, madmin.RequestData{RelPath: iamRevisionPeerPath, QueryValues: url.Values{"api-version": {madmin.SiteReplAPIVersion}}, Content: content}) + if resp != nil { + defer xhttp.DrainBody(resp.Body) + } + if err != nil { + return status, err + } + if resp.StatusCode != http.StatusOK { + var remote madmin.ErrorResponse + if json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(&remote) == nil && remote.Code != "" { + return status, remote + } + return status, fmt.Errorf("IAM revision protocol requires upgraded peers: %s", resp.Status) + } + var response iamRevisionResponse + if err = json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(&response); err != nil { + return status, err + } + status = response.iamRevisionStatus + if status.Version != iamRevisionProtocol || status.Node == "" || status.Instance == "" || status.Digest == "" { + return status, errors.New("peer did not acknowledge the IAM revision protocol") + } + if len(response.Errors) != 0 { + return status, &iamRevisionBatchError{failures: response.Errors} + } + return status, nil +} + +type ( + iamRecordBoundaryKey struct{} + iamGroupSnapshotKey struct{} +) + +func (c *SiteReplicationSys) replicationItem(ctx context.Context, item madmin.SRIAMItem) (iamReplicationItem, error) { + out := iamReplicationItem{SRIAMItem: item} + if item.Type == madmin.SRIAMItemSvcAcc && item.SvcAccChange != nil { + var key string + if item.SvcAccChange.Create != nil { + key = item.SvcAccChange.Create.AccessKey + } else if item.SvcAccChange.Update != nil { + key = item.SvcAccChange.Update.AccessKey + } + if key != "" { + r, err := loadIAMRevision(ctx, globalIAMSys.store, getUserIdentityPath(key, svcUser)) + if err != nil { + return out, err + } + if r.Deleted || r.timestamp().After(item.UpdatedAt) { + return out, errIAMStaleUpdate + } + out.RevokedBefore = r.RevokedBefore + } + } + if item.Type == madmin.SRIAMItemGroupInfo && item.GroupInfo != nil && !item.GroupInfo.UpdateReq.IsRemove { + out.GroupSnapshot = true + var gi GroupInfo + if err := globalIAMSys.store.loadIAMConfig(ctx, &gi, getGroupInfoPath(item.GroupInfo.UpdateReq.Group)); err != nil { + return out, err + } + // The matching persisted snapshot carries member grant times. If a + // later write won before sending, propagate that whole newer state. + if gi.Deleted { + return out, errIAMStaleUpdate + } + out.UpdatedAt = gi.UpdatedAt + out.RevokedBefore = gi.RevokedBefore + out.GroupInfo = &madmin.SRGroupInfo{UpdateReq: madmin.GroupAddRemove{Group: item.GroupInfo.UpdateReq.Group, Status: madmin.GroupStatus(gi.Status)}} + cache := globalIAMSys.store.rlock() + out.GroupInfo.UpdateReq.Members = cache.effectiveGroupMembers(gi) + globalIAMSys.store.runlock() + out.GroupGrants = make(map[string]time.Time, len(out.GroupInfo.UpdateReq.Members)) + for _, member := range out.GroupInfo.UpdateReq.Members { + out.GroupGrants[member] = gi.MemberGrants[member] + } + } + if item.Type == madmin.SRIAMItemGroupInfo && item.GroupInfo != nil && item.GroupInfo.UpdateReq.IsRemove && len(item.GroupInfo.UpdateReq.Members) == 0 { + r, err := loadIAMRevision(ctx, globalIAMSys.store, getGroupInfoPath(item.GroupInfo.UpdateReq.Group)) + if err != nil { + return out, err + } + if !r.Deleted && !r.RevokedBefore.IsZero() { + out.Type, out.GroupInfo = iamGroupBoundaryType, nil + out.GroupRevocation = &iamGroupBoundary{Group: item.GroupInfo.UpdateReq.Group, Before: r.RevokedBefore} + out.UpdatedAt = r.RevokedBefore + } + } + if item.Type == madmin.SRIAMItemIAMUser && item.IAMUser != nil { + r, err := loadIAMRevision(ctx, globalIAMSys.store, getUserIdentityPath(item.IAMUser.AccessKey, regUser)) + if err != nil { + return out, err + } + if item.IAMUser.IsDeleteReq && !r.Deleted && !r.RevokedBefore.IsZero() { + out.Type = iamUserBoundaryType + out.IAMUser = nil + out.UserRevocation = &iamUserBoundary{User: item.IAMUser.AccessKey, Before: r.RevokedBefore} + out.UpdatedAt = r.RevokedBefore + } else if !item.IAMUser.IsDeleteReq { + if r.Deleted || r.timestamp().After(item.UpdatedAt) { + return out, errIAMStaleUpdate + } + out.RevokedBefore = r.RevokedBefore + } + } + return out, nil +} + +func applyIAMReplicationItem(ctx context.Context, item iamReplicationItem) error { + if item.GroupInfo != nil { + if item.GroupSnapshot { + ctx = context.WithValue(ctx, iamGroupSnapshotKey{}, true) + } + // A nil map also explicitly denotes unknown legacy grants. Do not + // turn an unrelated group edit into a new grant after a revocation. + ctx = withIAMGroupGrants(ctx, item.GroupGrants) + } + if !item.RevokedBefore.IsZero() { + if item.RevokedBefore.After(item.UpdatedAt) { + return errSRInvalidRequest(errInvalidArgument) + } + ctx = context.WithValue(ctx, iamRecordBoundaryKey{}, item.RevokedBefore) + } + switch item.Type { + case iamUserBoundaryType: + if item.UserRevocation == nil || item.UserRevocation.User == "" || item.UserRevocation.Before.IsZero() { + return errSRInvalidRequest(errInvalidArgument) + } + return iamReplicationError(globalIAMSys.DeleteUser(withIAMReplicationTime(ctx, item.UserRevocation.Before), item.UserRevocation.User, true)) + case iamGroupBoundaryType: + if item.GroupRevocation == nil || item.GroupRevocation.Group == "" || item.GroupRevocation.Before.IsZero() { + return errSRInvalidRequest(errInvalidArgument) + } + _, err := globalIAMSys.RemoveUsersFromGroup(withIAMReplicationTime(ctx, item.GroupRevocation.Before), item.GroupRevocation.Group, nil) + return iamReplicationError(err) + case madmin.SRIAMItemPolicy: + if len(item.Policy) == 0 { + return globalSiteReplicationSys.PeerAddPolicyHandler(ctx, item.Name, nil, item.UpdatedAt) + } + p, err := policy.ParseConfig(bytes.NewReader(item.Policy)) + if err != nil { + return err + } + if p.IsEmpty() { + p = nil + } + return globalSiteReplicationSys.PeerAddPolicyHandler(ctx, item.Name, p, item.UpdatedAt) + case madmin.SRIAMItemSvcAcc: + return globalSiteReplicationSys.PeerSvcAccChangeHandler(ctx, item.SvcAccChange, item.UpdatedAt) + case madmin.SRIAMItemPolicyMapping: + return globalSiteReplicationSys.PeerPolicyMappingHandler(ctx, item.PolicyMapping, item.UpdatedAt) + case madmin.SRIAMItemSTSAcc: + return globalSiteReplicationSys.PeerSTSAccHandler(ctx, item.STSCredential, item.UpdatedAt) + case madmin.SRIAMItemIAMUser: + return globalSiteReplicationSys.PeerIAMUserChangeHandler(ctx, item.IAMUser, item.UpdatedAt) + case madmin.SRIAMItemGroupInfo: + return globalSiteReplicationSys.PeerGroupInfoChangeHandler(ctx, item.GroupInfo, item.UpdatedAt) + default: + return errSRInvalidRequest(errInvalidArgument) + } +} + +func (a adminAPIHandlers) SRPeerIAMRevisions(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + if obj, _ := validateAdminReq(ctx, w, r, policy.SiteReplicationOperationAction); obj == nil { + return + } + var failures []string + if r.Method == http.MethodPut { + var batch iamRevisionBatch + if err := parseJSONBody(ctx, r.Body, &batch, ""); err != nil { + writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) + return + } + if batch.Version != iamRevisionProtocol || len(batch.Items) == 0 || len(batch.Items) > maxIAMRevisionBatch { + writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, errSRInvalidRequest(errInvalidArgument)), r.URL) + return + } + for i, item := range batch.Items { + if err := applyIAMReplicationItem(ctx, item); err != nil { + failures = append(failures, fmt.Sprintf("item %d (%s): %v", i, item.Type, err)) + } + } + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(iamRevisionResponse{iamRevisionStatus: globalIAMSys.store.iamRevisionStatus(), Errors: failures}) +} + +// A site endpoint can balance requests across nodes sharing durable IAM state. +// Switching between known node incarnations preserves ACKs; a new incarnation +// conservatively invalidates them so restoring an old backend cannot inherit +// acknowledgements from before the restore. +func (p *iamRevisionProgress) observePeer(status iamRevisionStatus) { + if p.Instances == nil { + p.Instances = make(map[string]string) + } + if p.Instances[status.Node] != status.Instance || p.Acknowledged == nil { + p.Instances[status.Node] = status.Instance + p.Acknowledged = make(map[string]string) + } +} diff --git a/cmd/iam-replication-protocol_test.go b/cmd/iam-replication-protocol_test.go new file mode 100644 index 000000000..5c62bec3a --- /dev/null +++ b/cmd/iam-replication-protocol_test.go @@ -0,0 +1,161 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-only + +package cmd + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/minio/madmin-go/v3" +) + +func TestIAMRevisionProtocolDoesNotFallBackToLegacy(t *testing.T) { + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + if r.URL.Path != "/minio/admin/v3/site-replication/peer/iam-revisions" { + t.Errorf("unsafe fallback path: %s", r.URL.Path) + } + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"Code":"NotImplemented","Message":"old server"}`)) + })) + defer server.Close() + client, err := madmin.New(strings.TrimPrefix(server.URL, "http://"), "test-access", "valid-test-secret", false) + mustIAM(t, err) + _, err = executeIAMRevisionRequest(context.Background(), client, http.MethodPut, &iamRevisionBatch{Version: iamRevisionProtocol, Items: []iamReplicationItem{{SRIAMItem: madmin.SRIAMItem{Type: iamUserBoundaryType}, UserRevocation: &iamUserBoundary{User: "recreated", Before: UTCNow()}}}}) + if err == nil || requests.Load() != 1 { + t.Fatalf("old peer must reject without fallback, err=%v requests=%d", err, requests.Load()) + } +} + +type iamNoHealingScanStore struct{ IAMStorageAPI } + +func (s *iamNoHealingScanStore) listIAMConfigPaths(context.Context) ([]string, error) { + panic("healing must use the loaded revision index") +} + +func TestIAMRevisionHealingAcknowledgements(t *testing.T) { + for _, balanced := range []bool{false, true} { + t.Run(fmt.Sprintf("load_balanced_%t", balanced), func(t *testing.T) { testIAMRevisionHealingAcknowledgements(t, balanced) }) + } +} + +func testIAMRevisionHealingAcknowledgements(t *testing.T, balanced bool) { + ctx, sys, _ := prepareIAMRevisionFixture(t) + _, err := sys.CreateUser(ctx, "ack-sync", madmin.AddOrUpdateUserReq{SecretKey: "valid-sync-password", Status: madmin.AccountEnabled}) + mustIAM(t, err) + for i := range maxIAMRevisionBatch*2 + 1 { + at := UTCNow().Add(time.Duration(i) * time.Nanosecond) + mustIAM(t, sys.store.saveIAMConfig(ctx, &UserIdentity{Version: 1, Deleted: true, UpdatedAt: at, RevokedBefore: at}, getUserIdentityPath(fmt.Sprintf("ack-%04d", i), regUser))) + } + sys.store.IAMStorageAPI = &iamNoHealingScanStore{IAMStorageAPI: sys.store.IAMStorageAPI} + var mu sync.Mutex + var puts, gets int + var applied int + instance := "boot-1" + failSecondBatch := true + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/minio/health/live" { + w.WriteHeader(http.StatusOK) + return + } + mu.Lock() + defer mu.Unlock() + if r.URL.Path != "/minio/admin/v3/site-replication/peer/iam-revisions" { + t.Errorf("unexpected request: %s", r.URL.Path) + w.WriteHeader(404) + return + } + var failures []string + if r.Method == http.MethodGet { + gets++ + } else { + puts++ + var batch iamRevisionBatch + if err := json.NewDecoder(r.Body).Decode(&batch); err != nil { + t.Error(err) + w.WriteHeader(400) + return + } + if len(batch.Items) > maxIAMRevisionBatch { + t.Error("batch exceeds limit") + } + if failSecondBatch && puts == 2 { + failures = []string{"injected item error"} + } else { + applied += len(batch.Items) + } + } + node := "node-1" + if balanced { + node = fmt.Sprintf("node-%d", (gets+puts)%2+1) + } + _ = json.NewEncoder(w).Encode(iamRevisionResponse{iamRevisionStatus: iamRevisionStatus{Version: iamRevisionProtocol, Node: node, Instance: node + instance, Digest: fmt.Sprintf("%d", applied)}, Errors: failures}) + })) + defer server.Close() + c := &SiteReplicationSys{enabled: true, state: srState{ServiceAccountAccessKey: "ack-sync", Peers: map[string]madmin.PeerInfo{globalDeploymentID(): {DeploymentID: globalDeploymentID(), Name: "local"}, "remote": {DeploymentID: "remote", Name: "remote", Endpoint: server.URL}}}} + if err := c.healIAMDeletions(ctx); err == nil { + t.Fatal("item failure was hidden") + } + mu.Lock() + if puts != 3 || applied != maxIAMRevisionBatch+1 { + t.Errorf("failed middle batch blocked later revocations: puts=%d applied=%d", puts, applied) + } + failSecondBatch = false + applied++ // Unrelated remote mutation changes its digest. + mu.Unlock() + at := UTCNow() + mustIAM(t, sys.store.saveIAMConfig(ctx, &UserIdentity{Version: 1, Deleted: true, UpdatedAt: at, RevokedBefore: at}, getUserIdentityPath("ack-new-local", regUser))) + mustIAM(t, c.healIAMDeletions(ctx)) + mu.Lock() + if puts != 5 { + t.Errorf("did not resume at unacknowledged batch: puts=%d", puts) + } + mu.Unlock() + mustIAM(t, c.healIAMDeletions(ctx)) + mu.Lock() + if puts != 5 || gets != 3 { + t.Errorf("converged records were replayed: puts=%d gets=%d", puts, gets) + } + instance = "boot-2" + mu.Unlock() + mustIAM(t, c.healIAMDeletions(ctx)) + mu.Lock() + defer mu.Unlock() + if puts != 8 { + t.Fatalf("peer restart reused an old acknowledgement: puts=%d", puts) + } +} + +func TestIAMRevisionIndexRebuildsFromStorage(t *testing.T) { + ctx, sys, obj := prepareIAMRevisionFixture(t) + const user = "index-parent" + req := madmin.AddOrUpdateUserReq{SecretKey: "valid-parent-password", Status: madmin.AccountEnabled} + _, err := sys.CreateUser(ctx, user, req) + mustIAM(t, err) + mustIAM(t, sys.DeleteUser(ctx, user, false)) + before := sys.store.revisionIndex().snapshot() + store := &IAMStoreSys{IAMStorageAPI: newIAMObjectStore(obj, MinIOUsersSysType)} + mustIAM(t, store.LoadIAMCache(ctx, true)) + if iamRevisionDigest(before) != iamRevisionDigest(store.revisionIndex().snapshot()) { + t.Fatal("ordinary IAM loading did not restore the deletion index") + } + _, err = store.AddUser(ctx, user, req) + mustIAM(t, err) + r := store.revisionIndex().get(getUserIdentityPath(user, regUser)) + if r.Deleted || r.RevokedBefore.IsZero() { + t.Fatal("recreation discarded the retained boundary") + } + if r.Credentials.SecretKey != "" || r.Credentials.SessionToken != "" { + t.Fatal("index retained credentials") + } +} diff --git a/cmd/iam-revision-boundary_test.go b/cmd/iam-revision-boundary_test.go new file mode 100644 index 000000000..98d978fc6 --- /dev/null +++ b/cmd/iam-revision-boundary_test.go @@ -0,0 +1,393 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-only + +package cmd + +import ( + "context" + "errors" + "fmt" + "os" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/minio/madmin-go/v3" + "github.com/minio/minio/internal/grid" + xnet "github.com/pgsty/silo-pkg/v3/net" + "github.com/pgsty/silo-pkg/v3/policy" + etcd "go.etcd.io/etcd/client/v3" + "go.etcd.io/etcd/client/v3/namespace" +) + +func prepareIAMRevisionFixture(t testing.TB, backend ...string) (context.Context, *IAMSys, ObjectLayer) { + t.Helper() + resetTestGlobals() + ctx, cancel := context.WithCancel(context.Background()) + disks, err := getRandomDisks(1) + mustIAM(t, err) + obj, _, err := initObjectLayer(ctx, mustGetPoolEndpoints(0, disks...)) + mustIAM(t, err) + initAllSubsystems(ctx) + // Deliberately omit the periodic refresh goroutine. Fault injection can + // replace this fixture's storage interface without racing initialization. + var client *etcd.Client + if len(backend) != 0 && backend[0] == "etcd" { + endpoint := os.Getenv("SILO_TEST_IAM_REVOCATION_ETCD") + if endpoint == "" { + cancel() + obj.Shutdown(context.Background()) + os.RemoveAll(disks[0]) + t.Skip("set SILO_TEST_IAM_REVOCATION_ETCD to a disposable etcd endpoint") + } + client, err = etcd.New(etcd.Config{Endpoints: strings.Split(endpoint, ","), DialTimeout: 5 * time.Second}) + mustIAM(t, err) + prefix := fmt.Sprintf("/silo-boundary-test/%d/", time.Now().UnixNano()) + client.KV = namespace.NewKV(client.KV, prefix) + client.Watcher = namespace.NewWatcher(client.Watcher, prefix) + t.Cleanup(func() { client.Delete(context.Background(), "", etcd.WithPrefix()); client.Close() }) + } + globalIAMSys.initStore(obj, client) + mustIAM(t, globalIAMSys.Load(ctx, true)) + t.Cleanup(func() { cancel(); obj.Shutdown(context.Background()); os.RemoveAll(disks[0]); resetTestGlobals() }) + return ctx, globalIAMSys, obj +} + +func mustIAM(t testing.TB, err error) { + t.Helper() + if err != nil { + t.Fatal(err) + } +} + +var errIAMInjectedWrite = errors.New("injected IAM persistence failure") + +type iamFailingCleanupStore struct { + IAMStorageAPI + parentPath string + beforeCommit bool +} + +func (s *iamFailingCleanupStore) saveIAMConfig(ctx context.Context, item any, path string, opts ...options) error { + if s.beforeCommit || path != s.parentPath { + return errIAMInjectedWrite + } + return s.IAMStorageAPI.saveIAMConfig(ctx, item, path, opts...) +} + +func TestIAMRevocationCommitBoundary(t *testing.T) { + for _, before := range []bool{true, false} { + name := "after_identity_commit" + if before { + name = "before_identity_commit" + } + t.Run(name, func(t *testing.T) { + ctx, sys, obj := prepareIAMRevisionFixture(t) + const user = "commit-boundary-user" + origin := UTCNow().Add(-time.Hour) + req := madmin.AddOrUpdateUserReq{SecretKey: "valid-test-password", Status: madmin.AccountEnabled} + _, err := sys.CreateUser(withIAMReplicationTime(ctx, origin), user, req) + mustIAM(t, err) + _, err = sys.PolicyDBSet(withIAMReplicationTime(ctx, origin.Add(time.Minute)), user, "readwrite", regUser, false) + mustIAM(t, err) + _, err = sys.AddUsersToGroup(withIAMReplicationTime(ctx, origin.Add(time.Minute)), "commit-group", []string{user}) + mustIAM(t, err) + _, err = sys.PolicyDBSet(ctx, "commit-group", "readwrite", regUser, true) + mustIAM(t, err) + child, _, err := sys.NewServiceAccount(withIAMReplicationTime(ctx, origin), user, nil, newServiceAccountOpts{accessKey: "commit-child", secretKey: "valid-child-password"}) + mustIAM(t, err) + args := policy.Args{AccountName: user, Action: policy.GetObjectAction, BucketName: "bucket", ObjectName: "object"} + if !sys.IsAllowed(args) { + t.Fatal("fixture has no grant") + } + siblingStore := &IAMStoreSys{IAMStorageAPI: newIAMObjectStore(obj, MinIOUsersSysType)} + mustIAM(t, siblingStore.LoadIAMCache(ctx, true)) + sibling := &IAMSys{store: siblingStore, usersSysType: MinIOUsersSysType} + tg, err := grid.SetupTestGrid(2) + mustIAM(t, err) + defer tg.Cleanup() + var notifications atomic.Int32 + mustIAM(t, deleteUserRPC.Register(tg.Managers[1], func(r *grid.MSS) (grid.NoPayload, *grid.RemoteErr) { + notifications.Add(1) + if err := sibling.LoadUserAfterDelete(ctx, r.Get(peerRESTUser)); err != nil { + return grid.NoPayload{}, grid.NewRemoteErr(err) + } + return grid.NoPayload{}, nil + })) + host, err := xnet.ParseHost(strings.TrimPrefix(tg.Hosts[1], "http://")) + mustIAM(t, err) + globalNotificationSys = &NotificationSys{peerClients: []*peerRESTClient{{host: host, gridConn: func() *grid.Connection { return tg.Managers[0].Connection(tg.Hosts[1]) }}}} + original := sys.store.IAMStorageAPI + sys.store.IAMStorageAPI = &iamFailingCleanupStore{IAMStorageAPI: original, parentPath: getUserIdentityPath(user, regUser), beforeCommit: before} + boundary := origin.Add(2 * time.Minute) + err = sys.DeleteUser(withIAMReplicationTime(ctx, boundary), user, true) + if !errors.Is(err, errIAMInjectedWrite) { + t.Fatalf("expected write failure, got %v", err) + } + sys.store.IAMStorageAPI = original + r, err := loadIAMRevision(ctx, original, getUserIdentityPath(user, regUser)) + mustIAM(t, err) + if before { + if r.Deleted || !sys.IsAllowed(args) || !sibling.IsAllowed(args) || notifications.Load() != 0 { + t.Fatal("failure before commit changed the identity or grant") + } + return + } + if !r.Deleted || !r.RevokedBefore.Equal(boundary) { + t.Fatal("cleanup failure lost durable revocation") + } + if sys.IsAllowed(args) || sibling.IsAllowed(args) || notifications.Load() != 1 { + t.Fatal("cleanup failure retained old permission") + } + // Subsequent fixture writes need no additional RPC handlers. + globalNotificationSys = &NotificationSys{} + // Recreate after the partial cleanup. The old mapping, group member + // and child still exist in storage; none may authorize this identity. + _, err = sys.CreateUser(withIAMReplicationTime(ctx, origin.Add(3*time.Minute)), user, req) + mustIAM(t, err) + reloaded := &IAMStoreSys{IAMStorageAPI: newIAMObjectStore(obj, MinIOUsersSysType)} + mustIAM(t, reloaded.LoadIAMCache(ctx, true)) + fresh := &IAMSys{store: reloaded, usersSysType: MinIOUsersSysType} + if fresh.IsAllowed(args) { + t.Fatal("cold reload restored partially cleaned-up grants") + } + if _, ok := reloaded.GetUser(child.AccessKey); ok { + t.Fatal("cold reload restored the old child") + } + gd, err := reloaded.GetGroupDescription("commit-group") + mustIAM(t, err) + if len(gd.Members) != 0 { + t.Fatalf("listing exposed a revoked group relation: %v", gd.Members) + } + _, err = sys.AddUsersToGroup(ctx, "commit-group", []string{user}) + mustIAM(t, err) + if !sys.IsAllowed(args) { + t.Fatal("explicit new group grant was not accepted") + } + }) + } +} + +func TestIAMGroupGrantVersionsSurviveSnapshotsAndRecreation(t *testing.T) { + ctx, sys, _ := prepareIAMRevisionFixture(t) + origin := UTCNow().Add(-time.Hour) + req := madmin.AddOrUpdateUserReq{SecretKey: "valid-test-password", Status: madmin.AccountEnabled} + for _, user := range []string{"grant-alice", "grant-bob"} { + _, err := sys.CreateUser(withIAMReplicationTime(ctx, origin), user, req) + mustIAM(t, err) + } + grant := origin.Add(time.Minute) + _, err := sys.AddUsersToGroup(withIAMReplicationTime(ctx, grant), "grant-group", []string{"grant-alice"}) + mustIAM(t, err) + _, err = sys.PolicyDBSet(ctx, "grant-group", "readwrite", regUser, true) + mustIAM(t, err) + boundary := origin.Add(2 * time.Minute) + mustIAM(t, sys.DeleteUser(withIAMReplicationTime(ctx, boundary), "grant-alice", false)) + _, err = sys.CreateUser(withIAMReplicationTime(ctx, origin.Add(3*time.Minute)), "grant-alice", req) + mustIAM(t, err) + _, err = sys.AddUsersToGroup(withIAMReplicationTime(ctx, origin.Add(4*time.Minute)), "grant-group", []string{"grant-bob"}) + mustIAM(t, err) + _, err = sys.SetGroupStatus(withIAMReplicationTime(ctx, origin.Add(5*time.Minute)), "grant-group", true) + mustIAM(t, err) + var gi GroupInfo + mustIAM(t, sys.store.loadIAMConfig(ctx, &gi, getGroupInfoPath("grant-group"))) + if !gi.MemberGrants["grant-alice"].Equal(grant) { + t.Fatal("unrelated group edits refreshed an old grant") + } + args := policy.Args{AccountName: "grant-alice", Action: policy.GetObjectAction, BucketName: "bucket", ObjectName: "object"} + for _, stale := range []time.Time{grant, boundary, {}} { + item := iamReplicationItem{SRIAMItem: madmin.SRIAMItem{Type: madmin.SRIAMItemGroupInfo, UpdatedAt: origin.Add(6 * time.Minute), GroupInfo: &madmin.SRGroupInfo{UpdateReq: madmin.GroupAddRemove{Group: "grant-group", Members: []string{"grant-alice", "grant-bob"}}}}, GroupSnapshot: true, GroupGrants: map[string]time.Time{"grant-alice": stale, "grant-bob": origin.Add(4 * time.Minute)}} + mustIAM(t, applyIAMReplicationItem(ctx, item)) + mustIAM(t, sys.store.LoadIAMCache(ctx, false)) + if sys.IsAllowed(args) { + t.Fatalf("snapshot restored revoked grant %s", stale) + } + gd, err := sys.GetGroupDescription("grant-group") + mustIAM(t, err) + if len(gd.Members) != 1 || gd.Members[0] != "grant-bob" { + t.Fatalf("inconsistent effective members: %v", gd.Members) + } + } + // Only an explicit post-revocation grant restores access. + freshAt, err := sys.AddUsersToGroup(ctx, "grant-group", []string{"grant-alice"}) + mustIAM(t, err) + if !sys.IsAllowed(args) { + t.Fatal("explicit regrant rejected") + } + mustIAM(t, sys.store.LoadIAMCache(ctx, false)) + mustIAM(t, sys.store.loadIAMConfig(ctx, &gi, getGroupInfoPath("grant-group"))) + if !gi.MemberGrants["grant-alice"].Equal(freshAt) { + t.Fatal("new grant version was not persisted") + } + if !gi.MemberGrants["grant-bob"].Equal(origin.Add(4 * time.Minute)) { + t.Fatal("regranting Alice changed Bob's grant") + } +} + +func TestIAMGroupRevocationCommitAndRecreation(t *testing.T) { + for _, backend := range []string{"object", "etcd"} { + t.Run(backend, func(t *testing.T) { testIAMGroupRevocationCommitAndRecreation(t, backend) }) + } +} + +func testIAMGroupRevocationCommitAndRecreation(t *testing.T, backend string) { + ctx, sys, obj := prepareIAMRevisionFixture(t, backend) + origin := UTCNow().Add(-time.Hour) + user, group := "group-boundary-user", "group-boundary" + _, err := sys.CreateUser(withIAMReplicationTime(ctx, origin), user, madmin.AddOrUpdateUserReq{SecretKey: "valid-user-password", Status: madmin.AccountEnabled}) + mustIAM(t, err) + grant, boundary := origin.Add(time.Minute), origin.Add(2*time.Minute) + _, err = sys.AddUsersToGroup(withIAMReplicationTime(ctx, grant), group, []string{user}) + mustIAM(t, err) + // A newer mapping must not veto the authoritative group deletion. + _, err = sys.PolicyDBSet(withIAMReplicationTime(ctx, origin.Add(3*time.Minute)), group, "readwrite", regUser, true) + mustIAM(t, err) + _, err = sys.RemoveUsersFromGroup(withIAMReplicationTime(ctx, boundary), group, nil) + mustIAM(t, err) + r, err := loadIAMRevision(ctx, sys.store, getGroupInfoPath(group)) + mustIAM(t, err) + if !r.Deleted || !r.RevokedBefore.Equal(boundary) { + t.Fatal("newer mapping swallowed group deletion") + } + _, err = sys.AddUsersToGroup(withIAMReplicationTime(ctx, origin.Add(4*time.Minute)), group, nil) + mustIAM(t, err) + for _, at := range []time.Time{grant, boundary, {}} { + item := iamReplicationItem{SRIAMItem: madmin.SRIAMItem{Type: madmin.SRIAMItemGroupInfo, UpdatedAt: origin.Add(5 * time.Minute), GroupInfo: &madmin.SRGroupInfo{UpdateReq: madmin.GroupAddRemove{Group: group, Members: []string{user}}}}, GroupSnapshot: true, GroupGrants: map[string]time.Time{user: at}} + mustIAM(t, applyIAMReplicationItem(ctx, item)) + gd, err := sys.GetGroupDescription(group) + mustIAM(t, err) + if len(gd.Members) != 0 { + t.Fatalf("group recreation restored grant %s", at) + } + } + _, err = sys.AddUsersToGroup(ctx, group, []string{user}) + mustIAM(t, err) + args := policy.Args{AccountName: user, Action: policy.GetObjectAction, BucketName: "bucket", ObjectName: "object"} + if !sys.IsAllowed(args) { + t.Fatal("explicit group regrant was rejected") + } + // The newer live snapshot may arrive before an older group deletion. + lateBoundary := origin.Add(6 * time.Minute) + _, err = sys.RemoveUsersFromGroup(withIAMReplicationTime(ctx, lateBoundary), group, nil) + mustIAM(t, err) + r, err = loadIAMRevision(ctx, sys.store, getGroupInfoPath(group)) + mustIAM(t, err) + if r.Deleted || !r.RevokedBefore.Equal(lateBoundary) { + t.Fatal("late deletion lost the live group's revocation boundary") + } + // The old mapping is now revoked; a new explicit mapping restores access. + if sys.IsAllowed(args) { + t.Fatal("late group boundary retained an old mapping") + } + _, err = sys.PolicyDBSet(ctx, group, "readwrite", regUser, true) + mustIAM(t, err) + store := &IAMStoreSys{IAMStorageAPI: newIAMObjectStore(obj, MinIOUsersSysType)} + if es, ok := sys.store.IAMStorageAPI.(*IAMEtcdStore); ok { + store.IAMStorageAPI = newIAMEtcdStore(es.client, MinIOUsersSysType) + } + mustIAM(t, store.LoadIAMCache(ctx, true)) + fresh := &IAMSys{store: store, usersSysType: MinIOUsersSysType} + if !fresh.IsAllowed(args) { + t.Fatal("reload lost explicit grants after a retained group boundary") + } + item, err := globalSiteReplicationSys.replicationItem(ctx, madmin.SRIAMItem{Type: madmin.SRIAMItemGroupInfo, GroupInfo: &madmin.SRGroupInfo{UpdateReq: madmin.GroupAddRemove{Group: group}}, UpdatedAt: r.timestamp()}) + mustIAM(t, err) + if !item.RevokedBefore.Equal(lateBoundary) || !item.GroupGrants[user].After(lateBoundary) { + t.Fatal("group snapshot lost revision metadata") + } +} + +// A committed revision is observable before all cached dependents have been +// cleaned up. Every authorization read must apply that boundary in this window. +func TestIAMCachedMappingHonorsCommittedRevision(t *testing.T) { + ctx, sys, _ := prepareIAMRevisionFixture(t) + origin := UTCNow().Add(-time.Hour) + parent := "cached-external-parent" + _, err := sys.PolicyDBSet(withIAMReplicationTime(ctx, origin), parent, "readwrite", stsUser, false) + mustIAM(t, err) + policies, err := sys.PolicyDBGet(parent) + mustIAM(t, err) + if len(policies) == 0 { + t.Fatal("fixture has no STS-parent mapping") + } + mustIAM(t, sys.store.saveIAMConfig(ctx, &MappedPolicy{Version: 1, Deleted: true, UpdatedAt: origin.Add(time.Minute)}, getMappedPolicyPath(parent, stsUser, false))) + policies, err = sys.PolicyDBGet(parent) + mustIAM(t, err) + if len(policies) != 0 { + t.Fatal("cached STS mapping ignored its own namespace tombstone") + } + + user, group := "cached-group-user", "cached-group" + _, err = sys.CreateUser(withIAMReplicationTime(ctx, origin), user, madmin.AddOrUpdateUserReq{SecretKey: "valid-user-password", Status: madmin.AccountEnabled}) + mustIAM(t, err) + grant := origin.Add(5 * time.Minute) + _, err = sys.AddUsersToGroup(withIAMReplicationTime(ctx, grant), group, []string{user}) + mustIAM(t, err) + _, err = sys.PolicyDBSet(withIAMReplicationTime(ctx, origin), group, "readwrite", regUser, true) + mustIAM(t, err) + args := policy.Args{AccountName: user, Action: policy.GetObjectAction, BucketName: "bucket", ObjectName: "object"} + if !sys.IsAllowed(args) { + t.Fatal("fixture has no group grant") + } + // A late deletion preserves the newer member grant but revokes the older + // policy mapping. Simulate the interval before mapping cleanup completes. + gi := GroupInfo{Version: 1, Status: statusEnabled, Members: []string{user}, MemberGrants: map[string]time.Time{user: grant}, UpdatedAt: grant, RevokedBefore: origin.Add(2 * time.Minute)} + mustIAM(t, sys.store.saveIAMConfig(ctx, &gi, getGroupInfoPath(group))) + if sys.IsAllowed(args) { + t.Fatal("cached group mapping ignored the committed group boundary") + } + gd, err := sys.GetGroupDescription(group) + mustIAM(t, err) + if gd.Policy != "" { + t.Fatal("group listing exposed a revoked mapping") + } +} + +type ( + iamExpiryLockFailure struct { + ObjectLayer + path string + } + iamFailedExpiryLock struct{ RWLocker } +) + +func (o *iamExpiryLockFailure) NewNSLock(bucket string, objects ...string) RWLocker { + lock := o.ObjectLayer.NewNSLock(bucket, objects...) + if bucket == minioMetaBucket && len(objects) == 1 && objects[0] == o.path+".revision-lock" { + return &iamFailedExpiryLock{RWLocker: lock} + } + return lock +} + +func (l *iamFailedExpiryLock) GetLock(context.Context, *dynamicTimeout) (LockContext, error) { + return LockContext{}, errIAMInjectedWrite +} + +func TestIAMExpiredCredentialCleanupDoesNotBlockLoading(t *testing.T) { + ctx, sys, obj := prepareIAMRevisionFixture(t) + _, err := sys.CreateUser(ctx, "healthy-user", madmin.AddOrUpdateUserReq{SecretKey: "healthy-user-password", Status: madmin.AccountEnabled}) + mustIAM(t, err) + _, err = sys.PolicyDBSet(ctx, "healthy-user", "readwrite", regUser, false) + mustIAM(t, err) + c, _, err := sys.NewServiceAccount(ctx, "healthy-user", nil, newServiceAccountOpts{accessKey: "expired-service", secretKey: "expired-service-password"}) + mustIAM(t, err) + c.Expiration = UTCNow().Add(-time.Hour) + path := getUserIdentityPath(c.AccessKey, svcUser) + mustIAM(t, sys.store.saveIAMConfig(ctx, &UserIdentity{Version: 1, Credentials: c, UpdatedAt: UTCNow()}, path)) + // A cold loader sees the existing version but cannot acquire the cleanup + // write lock. Healthy users must still load; the expired one stays denied. + fresh := &IAMStoreSys{IAMStorageAPI: newIAMObjectStore(&iamExpiryLockFailure{ObjectLayer: obj, path: path}, MinIOUsersSysType)} + mustIAM(t, fresh.LoadIAMCache(ctx, true)) + if _, ok := fresh.GetUser("healthy-user"); !ok { + t.Fatal("cleanup failure prevented healthy IAM state from loading") + } + if _, ok := fresh.GetUser(c.AccessKey); ok { + t.Fatal("cleanup failure admitted an expired service account") + } + r, err := loadIAMRevision(ctx, fresh, path) + mustIAM(t, err) + if r.Deleted || !r.Credentials.IsExpired() { + t.Fatal("failed cleanup lost the existing expired revision") + } +} diff --git a/cmd/iam-revision-index.go b/cmd/iam-revision-index.go new file mode 100644 index 000000000..4b4c60d76 --- /dev/null +++ b/cmd/iam-revision-index.go @@ -0,0 +1,220 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-only + +package cmd + +import ( + "encoding/json" + "fmt" + "maps" + "strings" + "sync" + "time" + + "github.com/minio/minio/internal/auth" +) + +// This index is rebuilt by the existing IAM loaders and updated by successful +// storage operations. It avoids a second full IAM walk during every heal pass. +// It is an optimization of the durable records, never a reason to delete them. +// The index contains no secrets or grants. +type iamParentRevision struct { + deleted bool + before time.Time +} + +type iamRevisionIndex struct { + mu sync.RWMutex + items map[string]iamRevision + parents map[string]iamParentRevision + floors map[string]time.Time + generation uint64 +} + +func (idx *iamRevisionIndex) observe(path string, data []byte) { + if !strings.HasPrefix(path, iamConfigPrefix+"/") { + return + } + var r iamRevision + if json.Unmarshal(data, &r) != nil { + return // The caller reports malformed data using its normal decoder. + } + r.Credentials = auth.Credentials{ParentUser: r.Credentials.ParentUser, Expiration: r.Credentials.Expiration} + idx.mu.Lock() + defer idx.mu.Unlock() + if strings.HasPrefix(path, iamConfigUsersPrefix) { + // Keep a compact name-keyed view for the authentication hot path; + // constructing a config path on every S3 request allocates needlessly. + defer func() { + name := strings.TrimSuffix(strings.TrimPrefix(path, iamConfigUsersPrefix), "/"+iamIdentityFile) + if current, ok := idx.items[path]; ok { + if idx.parents == nil { + idx.parents = make(map[string]iamParentRevision) + } + idx.parents[name] = iamParentRevision{deleted: current.Deleted, before: current.RevokedBefore} + } else { + delete(idx.parents, name) + } + }() + } + if floor, ok := idx.floors[path]; ok && r.timestamp().Before(floor) { + return + } + if previous, ok := idx.items[path]; ok { + // A concurrent read that began before a write must not roll it back. + if previous.timestamp().After(r.timestamp()) || (previous.Deleted && !r.Deleted && !r.timestamp().After(previous.timestamp())) { + return + } + if previous.RevokedBefore.After(r.RevokedBefore) { + r.RevokedBefore = previous.RevokedBefore + } + if previous.timestamp().Equal(r.timestamp()) && previous.Deleted == r.Deleted && previous.RevokedBefore.Equal(r.RevokedBefore) { + return + } + } + if r.Deleted && !r.ExpiresAt.IsZero() && UTCNow().After(r.ExpiresAt) { + if _, tracked := idx.items[path]; tracked { + delete(idx.items, path) + idx.generation++ + } + delete(idx.floors, path) + return + } + if !r.Deleted && r.RevokedBefore.IsZero() { + _, tracked := idx.items[path] + _, hasFloor := idx.floors[path] + if tracked || hasFloor { + if idx.floors == nil { + idx.floors = make(map[string]time.Time) + } + idx.floors[path] = r.timestamp() + } + if tracked { + delete(idx.items, path) + idx.generation++ + } + return + } + if idx.items == nil { + idx.items = make(map[string]iamRevision) + } + idx.items[path] = r + delete(idx.floors, path) + idx.generation++ +} + +func (idx *iamRevisionIndex) get(path string) iamRevision { + if idx == nil { + return iamRevision{} + } + idx.mu.RLock() + defer idx.mu.RUnlock() + return idx.items[path] +} + +func (idx *iamRevisionIndex) snapshot() map[string]iamRevision { + idx.mu.Lock() + defer idx.mu.Unlock() + for path, r := range idx.items { + if r.Deleted && !r.ExpiresAt.IsZero() && UTCNow().After(r.ExpiresAt) { + delete(idx.items, path) + delete(idx.floors, path) + idx.generation++ + } + } + return maps.Clone(idx.items) +} + +func (idx *iamRevisionIndex) count() int { + idx.mu.RLock() + defer idx.mu.RUnlock() + return len(idx.items) +} + +// A process-local generation plus the protocol's instance ID is sufficient +// for acknowledgements. Avoid hashing the entire index on every IAM write. +func (idx *iamRevisionIndex) digest() string { + idx.mu.RLock() + defer idx.mu.RUnlock() + return fmt.Sprintf("%x:%x", idx.generation, len(idx.items)) +} + +func (idx *iamRevisionIndex) forget(path string) { + idx.mu.Lock() + if _, ok := idx.items[path]; ok { + delete(idx.items, path) + idx.generation++ + } + delete(idx.floors, path) + if strings.HasPrefix(path, iamConfigUsersPrefix) { + delete(idx.parents, strings.TrimSuffix(strings.TrimPrefix(path, iamConfigUsersPrefix), "/"+iamIdentityFile)) + } + idx.mu.Unlock() +} + +func (c *iamCache) userRevocation(user string) iamRevision { + r := c.revisions.parentRevision(user) + if u, ok := c.iamUsersMap[user]; ok && u.RevokedBefore.After(r.RevokedBefore) { + r.RevokedBefore = u.RevokedBefore + } + return r +} + +func (c *iamCache) groupMemberAllowed(member string, grantedAt, groupBoundary time.Time) bool { + r := c.userRevocation(member) + return !r.Deleted && (r.RevokedBefore.IsZero() || grantedAt.After(r.RevokedBefore)) && (groupBoundary.IsZero() || grantedAt.After(groupBoundary)) +} + +func iamMappingParentPath(path string) string { + kind, name, ok := strings.Cut(strings.TrimPrefix(path, iamConfigPolicyDBPrefix), "/") + if !ok { + return "" + } + name = strings.TrimSuffix(name, ".json") + switch kind { + case "users", "sts-users": + return getUserIdentityPath(name, regUser) + case "service-accounts": + return getUserIdentityPath(name, svcUser) + case "groups": + return getGroupInfoPath(name) + } + return "" +} + +func (idx *iamRevisionIndex) mappingAllowed(path string, mp MappedPolicy) bool { + if mp.Deleted || idx.get(path).Deleted { + return false + } + r := idx.get(iamMappingParentPath(path)) + return !r.Deleted && (r.RevokedBefore.IsZero() || mp.UpdatedAt.After(r.RevokedBefore)) +} + +// Apply the persisted commit boundary even before dependent cache cleanup has +// completed. The map namespace is part of the authorization record's identity. +func (c *iamCache) cachedMappedPolicy(name string, userType IAMUserType, isGroup bool) (MappedPolicy, bool) { + var mp MappedPolicy + var ok bool + switch { + case isGroup: + mp, ok = c.iamGroupPolicyMap.Load(name) + case userType == stsUser: + mp, ok = c.iamSTSPolicyMap.Load(name) + default: + mp, ok = c.iamUserPolicyMap.Load(name) + } + if !ok || !c.revisions.mappingAllowed(getMappedPolicyPath(name, userType, isGroup), mp) { + return MappedPolicy{}, false + } + return mp, true +} + +func (idx *iamRevisionIndex) parentRevision(user string) iamRevision { + if idx == nil { + return iamRevision{} + } + idx.mu.RLock() + p := idx.parents[user] + idx.mu.RUnlock() + return iamRevision{Deleted: p.deleted, RevokedBefore: p.before} +} diff --git a/cmd/iam-revision-lock_test.go b/cmd/iam-revision-lock_test.go new file mode 100644 index 000000000..28c5e0cf5 --- /dev/null +++ b/cmd/iam-revision-lock_test.go @@ -0,0 +1,305 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-only + +package cmd + +import ( + "context" + "crypto/sha256" + "fmt" + "os" + "strings" + "sync" + "testing" + "time" + + "github.com/minio/madmin-go/v3" + etcd "go.etcd.io/etcd/client/v3" + "go.etcd.io/etcd/client/v3/concurrency" + "go.etcd.io/etcd/client/v3/namespace" +) + +type iamRevisionLockObserver struct { + ObjectLayer + path string + waiting chan struct{} + once sync.Once +} + +func (o *iamRevisionLockObserver) NewNSLock(bucket string, objects ...string) RWLocker { + lock := o.ObjectLayer.NewNSLock(bucket, objects...) + if bucket == minioMetaBucket && len(objects) == 1 && objects[0] == o.path { + return &iamRevisionObservedLock{RWLocker: lock, observe: func() { o.once.Do(func() { close(o.waiting) }) }} + } + return lock +} + +type iamRevisionObservedLock struct { + RWLocker + observe func() +} + +func (l *iamRevisionObservedLock) GetLock(ctx context.Context, timeout *dynamicTimeout) (LockContext, error) { + l.observe() + return l.RWLocker.GetLock(ctx, timeout) +} + +type iamRevisionWatchObserver struct { + etcd.Watcher + waiting chan struct{} + once sync.Once +} + +func (w *iamRevisionWatchObserver) Watch(ctx context.Context, key string, opts ...etcd.OpOption) etcd.WatchChan { + w.once.Do(func() { close(w.waiting) }) + return w.Watcher.Watch(ctx, key, opts...) +} + +// Simulate an unavailable cleanup RPC. Mutex.Lock calls Delete after its wait +// is canceled; that RPC must inherit a deadline too, not Client.Ctx() forever. +type iamRevisionCleanupBlocker struct { + etcd.KV + release chan struct{} +} + +func (b *iamRevisionCleanupBlocker) Delete(ctx context.Context, key string, opts ...etcd.OpOption) (*etcd.DeleteResponse, error) { + if strings.Contains(key, "/iam-revision-locks/") { + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-b.release: + } + } + return b.KV.Delete(ctx, key, opts...) +} + +type iamRevisionReadBlocker struct { + IAMStorageAPI + path string + after int + waiting chan struct{} +} + +func (b *iamRevisionReadBlocker) loadIAMConfig(ctx context.Context, item any, path string) error { + if path == b.path { + b.after-- + if b.after == 0 { + close(b.waiting) + <-ctx.Done() + return ctx.Err() + } + } + return b.IAMStorageAPI.loadIAMConfig(ctx, item, path) +} + +func TestIAMRevisionReadDoesNotBlockAuthentication(t *testing.T) { + for _, stage := range []struct { + name string + offset time.Duration + }{{"deletion", time.Minute}, {"retained_revocation", -time.Minute}} { + t.Run(stage.name, func(t *testing.T) { + resetTestGlobals() + t.Cleanup(resetTestGlobals) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + disks, err := getRandomDisks(1) + if err != nil { + t.Fatal(err) + } + obj, _, err := initObjectLayer(ctx, mustGetPoolEndpoints(0, disks...)) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + obj.Shutdown(context.Background()) + os.RemoveAll(disks[0]) + }) + store := &IAMStoreSys{IAMStorageAPI: newIAMObjectStore(obj, MinIOUsersSysType)} + const user = "read-blocked-parent" + created, err := store.AddUser(ctx, user, madmin.AddOrUpdateUserReq{SecretKey: "original-password", Status: madmin.AccountEnabled}) + if err != nil { + t.Fatal(err) + } + blocked := &iamRevisionReadBlocker{IAMStorageAPI: store.IAMStorageAPI, path: getUserIdentityPath(user, regUser), after: 1, waiting: make(chan struct{})} + store.IAMStorageAPI = blocked + done := make(chan error, 1) + go func() { + done <- store.DeleteUser(withIAMReplicationTime(ctx, created.Add(stage.offset)), user, regUser) + }() + defer func() { cancel(); <-done }() + select { + case <-blocked.waiting: + case <-time.After(5 * time.Second): + t.Fatal("revision read was not attempted") + } + read := make(chan bool, 1) + go func() { + u, ok := store.GetUser(user) + read <- ok && u.Credentials.SecretKey == "original-password" + }() + select { + case ok := <-read: + if !ok { + t.Fatal("pending revision read changed the cached identity") + } + case <-time.After(time.Second): + t.Fatal("revision read blocked cached authentication") + } + }) + } +} + +func TestIAMRevisionLockContention(t *testing.T) { + for _, backend := range []string{"object", "etcd"} { + t.Run(backend, func(t *testing.T) { + endpoint := os.Getenv("SILO_TEST_IAM_REVOCATION_ETCD") + if backend == "etcd" && endpoint == "" { + t.Skip("set SILO_TEST_IAM_REVOCATION_ETCD to a disposable etcd endpoint") + } + for _, outcome := range []string{"release", "cancel", "default_timeout"} { + t.Run(outcome, func(t *testing.T) { + resetTestGlobals() + t.Cleanup(resetTestGlobals) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + oldTimeout := defaultContextTimeout + defaultContextTimeout = 2 * time.Second + t.Cleanup(func() { defaultContextTimeout = oldTimeout }) + must := func(err error) { + t.Helper() + if err != nil { + t.Fatal(err) + } + } + const user = "contended-user" + path := getUserIdentityPath(user, regUser) + waiting := make(chan struct{}) + var store *IAMStoreSys + var hold func() func() + unblockCleanup := func() {} + if backend == "object" { + disks, err := getRandomDisks(1) + must(err) + obj, _, err := initObjectLayer(ctx, mustGetPoolEndpoints(0, disks...)) + must(err) + t.Cleanup(func() { + obj.Shutdown(context.Background()) + os.RemoveAll(disks[0]) + }) + observed := &iamRevisionLockObserver{ObjectLayer: obj, path: path + ".revision-lock", waiting: waiting} + store = &IAMStoreSys{IAMStorageAPI: newIAMObjectStore(obj, MinIOUsersSysType)} + hold = func() func() { + lock := obj.NewNSLock(minioMetaBucket, observed.path) + lc, err := lock.GetLock(ctx, newDynamicTimeout(time.Second, time.Second)) + must(err) + store.IAMStorageAPI.(*IAMObjectStore).objAPI = observed + return func() { lock.Unlock(lc) } + } + } else { + client, err := etcd.New(etcd.Config{Endpoints: strings.Split(endpoint, ","), DialTimeout: time.Second}) + must(err) + t.Cleanup(func() { client.Close() }) + prefix := fmt.Sprintf("/silo-lock-test/%d/", time.Now().UnixNano()) + client.KV = namespace.NewKV(client.KV, prefix) + client.Watcher = namespace.NewWatcher(client.Watcher, prefix) + store = &IAMStoreSys{IAMStorageAPI: newIAMEtcdStore(client, MinIOUsersSysType)} + hold = func() func() { + session, err := concurrency.NewSession(client, concurrency.WithContext(ctx)) + must(err) + lock := concurrency.NewMutex(session, fmt.Sprintf("%s/iam-revision-locks/%x", minioConfigPrefix, sha256.Sum256([]byte(path)))) + must(lock.Lock(ctx)) + client.Watcher = &iamRevisionWatchObserver{Watcher: client.Watcher, waiting: waiting} + blocker := &iamRevisionCleanupBlocker{KV: client.KV, release: make(chan struct{})} + client.KV = blocker + unblockCleanup = sync.OnceFunc(func() { close(blocker.release) }) + t.Cleanup(unblockCleanup) + return func() { session.Close() } + } + } + request := func(secret string) madmin.AddOrUpdateUserReq { + return madmin.AddOrUpdateUserReq{SecretKey: secret, Status: madmin.AccountEnabled} + } + _, err := store.AddUser(ctx, user, request("original-password")) + must(err) + release := sync.OnceFunc(hold()) + t.Cleanup(release) + writeCtx, cancelWrite := context.WithCancel(ctx) + defer cancelWrite() + first, second := make(chan error, 1), make(chan error, 1) + var writers sync.WaitGroup + t.Cleanup(func() { + cancelWrite() + unblockCleanup() + release() + writers.Wait() + }) + writers.Go(func() { + _, err := store.AddUser(writeCtx, user, request("first-password")) + first <- err + }) + select { + case <-waiting: + case <-time.After(5 * time.Second): + t.Fatal("writer did not attempt the held revision lock") + } + // A second writer must queue without taking the cache's RWMutex: + // Go's writer preference would otherwise block every new reader. + writers.Go(func() { + _, err := store.AddUser(ctx, user, request("second-password")) + second <- err + }) + select { + case err := <-second: + t.Fatalf("second writer bypassed the first: %v", err) + case <-time.After(50 * time.Millisecond): + } + read := make(chan UserIdentity, 1) + go func() { + u, _ := store.GetUser(user) + read <- u + }() + select { + case u := <-read: + if u.Credentials.SecretKey != "original-password" { + t.Fatal("pending write changed the cached credential") + } + case <-time.After(time.Second): + t.Fatal("distributed lock contention blocked cached authentication") + } + switch outcome { + case "release": + release() + case "cancel": + cancelWrite() + } + select { + case err := <-first: + if outcome == "release" { + must(err) + } else if err == nil { + t.Fatal("canceled or timed-out write succeeded") + } + case <-time.After(5 * time.Second): + t.Fatal("lock wait or cancellation cleanup exceeded its deadline") + } + release() + select { + case err := <-second: + must(err) + case <-time.After(5 * time.Second): + t.Fatal("queued writer did not recover after the first completed") + } + cached, ok := store.GetUser(user) + if !ok || cached.Credentials.SecretKey != "second-password" { + t.Fatal("cached write order was lost") + } + var persisted UserIdentity + must(store.loadIAMConfig(ctx, &persisted, path)) + if persisted.Credentials.SecretKey != cached.Credentials.SecretKey || !persisted.UpdatedAt.Equal(cached.UpdatedAt) { + t.Fatal("persistent and cached revisions differ") + } + }) + } + }) + } +} diff --git a/cmd/iam-revision.go b/cmd/iam-revision.go new file mode 100644 index 000000000..15e1e6898 --- /dev/null +++ b/cmd/iam-revision.go @@ -0,0 +1,749 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-only + +package cmd + +import ( + "context" + "crypto/sha256" + "errors" + "fmt" + "net/http" + "sort" + "strings" + "sync" + "sync/atomic" + "time" + + "github.com/minio/madmin-go/v3" + "github.com/minio/minio/internal/auth" + etcd "go.etcd.io/etcd/client/v3" + "go.etcd.io/etcd/client/v3/concurrency" +) + +var errIAMStaleUpdate = errors.New("IAM update predates a stored revision or revocation") + +// The parent is still live; callers must not broadcast a user deletion when +// only its revocation boundary was retained. +var errIAMRevocationRetained = errors.New("IAM revocation recorded without deleting the record") + +// A revocation advances the boundary even when a newer identity already +// exists. Keep this operation distinct from replacing/deleting that identity. +type iamUserRevocation struct { + UserIdentity + retained bool +} + +type iamGroupRevocation struct { + GroupInfo + retained bool + requireEmpty bool +} + +// Natural expiration is distinct from revoking a live credential. An expired +// immutable STS token can be removed; a reusable service-account key retains +// its revision so an older non-expiring credential cannot return. +type iamExpireIdentity struct{} + +// The authoritative revocation is durable even if dependent cleanup fails. +// Callers must publish it to sibling caches before returning the error. +type iamCommittedCleanupError struct { + err error + retained bool +} + +func (e *iamCommittedCleanupError) Error() string { + return "IAM revocation committed; cleanup failed: " + e.err.Error() +} +func (e *iamCommittedCleanupError) Unwrap() error { return e.err } + +type iamReplicationTimeKey struct{} + +func withIAMReplicationTime(ctx context.Context, at time.Time) context.Context { + return context.WithValue(ctx, iamReplicationTimeKey{}, at) +} + +func iamReplicationTime(ctx context.Context) (time.Time, bool) { + at, ok := ctx.Value(iamReplicationTimeKey{}).(time.Time) + return at, ok +} + +func iamReplicationError(err error) error { + if errors.Is(err, errIAMStaleUpdate) { + // Retrying an obsolete event cannot change the result. + return nil + } + return wrapSRErr(err) +} + +// Deletions occupy the original IAM config path. They contain no secret or +// grant and are hidden by the normal loaders, but remain available to heal +// and to timestamp comparisons after a restart. Do not age them out: a peer +// can be offline indefinitely. +type iamRevision struct { + UpdatedAt time.Time `json:"updatedAt"` + UpdateDate time.Time `json:"UpdateDate"` + Deleted bool `json:"deleted"` + RevokedBefore time.Time `json:"revokedBefore"` + ExpiresAt time.Time `json:"expiresAt,omitempty"` + Credentials auth.Credentials `json:"credentials"` +} + +func (r iamRevision) timestamp() time.Time { + if r.UpdateDate.After(r.UpdatedAt) { + return r.UpdateDate + } + return r.UpdatedAt +} + +func loadIAMRevision(ctx context.Context, store IAMStorageAPI, path string) (iamRevision, error) { + var r iamRevision + err := store.loadIAMConfig(ctx, &r, path) + if errors.Is(err, errConfigNotFound) { + err = nil + } + return r, err +} + +func (store *IAMStoreSys) checkIAMRevision(ctx context.Context, path string, deleting bool) error { + at, replicated := iamReplicationTime(ctx) + if !replicated { + return nil + } + return store.withIAMStorage(ctx, func(ctx context.Context) error { + r, err := loadIAMRevision(ctx, store.IAMStorageAPI, path) + if err != nil { + return err + } + if r.timestamp().After(at) || (r.Deleted && !deleting && !at.After(r.timestamp())) { + return errIAMStaleUpdate + } + return nil + }) +} + +// This signed claim records the parent's revocation boundary at issuance. +// Unlike UpdatedAt, it cannot advance when an offline site edits an old child. +// It travels in the existing service-account Claims and STS SessionToken fields. +const iamParentRevocationClaim = "siloParentRevocation" + +func setIAMParentRevocationClaim(ctx context.Context, store IAMStorageAPI, parent string, claims map[string]any) error { + delete(claims, iamParentRevocationClaim) + if parent == "" || parent == globalActiveCred.AccessKey { + return nil + } + r, err := loadIAMRevision(ctx, store, getUserIdentityPath(parent, regUser)) + if err != nil { + return err + } + if r.Deleted { + return errIAMStaleUpdate + } + if !r.RevokedBefore.IsZero() { + claims[iamParentRevocationClaim] = r.RevokedBefore.Format(time.RFC3339Nano) + } + return nil +} + +func iamCredentialSurvivesRevocation(cred auth.Credentials, at time.Time) bool { + if at.IsZero() { + return true + } + s, _ := cred.Claims[iamParentRevocationClaim].(string) + issuedAfter, err := time.Parse(time.RFC3339Nano, s) + return err == nil && !issuedAfter.Before(at) +} + +// Parent revocations delete old children even if an offline peer has edited +// them later. Preserve children that prove issuance after this revocation. +func iamChildDeletionContext(ctx context.Context, child UserIdentity) (context.Context, bool) { + if at, replicated := iamReplicationTime(ctx); replicated { + if !at.IsZero() && iamCredentialSurvivesRevocation(child.Credentials, at) { + return ctx, false + } + if child.UpdatedAt.After(at) { + ctx = withIAMReplicationTime(ctx, child.UpdatedAt) + } + } + return ctx, true +} + +// A delayed service account or STS event must not outlive deletion of its +// built-in parent. The caller must populate Claims from the verified token. +func checkIAMParentRevision(ctx context.Context, store IAMStorageAPI, cred auth.Credentials) error { + parent := cred.ParentUser + if parent == "" || parent == globalActiveCred.AccessKey { + return nil + } + r, err := loadIAMRevision(ctx, store, getUserIdentityPath(parent, regUser)) + if err != nil { + return err + } + if r.Deleted || !iamCredentialSurvivesRevocation(cred, r.RevokedBefore) { + return errIAMStaleUpdate + } + return nil +} + +// Called with the IAM writer mutex and cache lock held. Persistence only +// touches the caller's record, not the cache. Keep writers serialized while +// allowing cached authentication reads throughout storage and lock waits. +func (store *IAMStoreSys) withIAMStorage(ctx context.Context, fn func(context.Context) error) error { + store.IAMStorageAPI.unlock() + defer store.IAMStorageAPI.lock() + ctx, cancel := context.WithTimeout(ctx, defaultContextTimeout) + defer cancel() + return fn(ctx) +} + +func (store *IAMStoreSys) saveIAMRevision(ctx context.Context, path string, item any, opts ...options) error { + return store.withIAMStorage(ctx, func(ctx context.Context) error { + return saveIAMRevision(ctx, store.IAMStorageAPI, path, item, opts...) + }) +} + +func (store *IAMStoreSys) checkIAMParentRevision(ctx context.Context, cred auth.Credentials) error { + return store.withIAMStorage(ctx, func(ctx context.Context) error { + return checkIAMParentRevision(ctx, store.IAMStorageAPI, cred) + }) +} + +// Update the caller's record with the persisted revision before it is cached. +func saveIAMRevision(ctx context.Context, store IAMStorageAPI, path string, item any, opts ...options) error { + ctx, cancel := context.WithTimeout(ctx, defaultContextTimeout) + defer cancel() + + // Serialize compare-and-write across nodes, as well as goroutines. Use a + // separate lock name so saving the config does not reacquire this lock. + switch s := store.(type) { + case *IAMObjectStore: + lock := s.objAPI.NewNSLock(minioMetaBucket, path+".revision-lock") + lc, err := lock.GetLock(ctx, globalOperationTimeout) + if err != nil { + return err + } + defer lock.Unlock(lc) + ctx = lc.Context() + case *IAMEtcdStore: + // Mutex.Lock also uses Client.Ctx() for cleanup after cancellation. + // Borrow the existing services with the operation's bounded context; + // never close this facade, which does not own those services. + client := etcd.NewCtxClient(ctx, etcd.WithZapLogger(s.client.GetLogger())) + client.KV, client.Lease, client.Watcher = s.client.KV, s.client.Lease, s.client.Watcher + session, err := concurrency.NewSession(client, concurrency.WithContext(ctx)) + if err != nil { + return err + } + defer func() { + session.Orphan() + // A canceled operation must still release its lease when etcd is + // reachable. If it is unavailable, stop waiting and let it expire. + cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), defaultContextTimeout) + defer cancel() + _, _ = s.client.Revoke(cleanupCtx, session.Lease()) + }() + lock := concurrency.NewMutex(session, fmt.Sprintf("%s/iam-revision-locks/%x", minioConfigPrefix, sha256.Sum256([]byte(path)))) + if err = lock.Lock(ctx); err != nil { + return err + } + // Revoking the session lease releases the lock, including on cancellation. + } + previous, err := loadIAMRevision(ctx, store, path) + if err != nil { + return err + } + if _, expiring := item.(*iamExpireIdentity); expiring { + sts := strings.HasPrefix(path, iamConfigSTSPrefix) + if previous.Deleted { + if sts && !previous.ExpiresAt.IsZero() && UTCNow().After(previous.ExpiresAt) { + return expireIAMSTSConfig(ctx, store, path) + } + return nil + } + if previous.timestamp().IsZero() || !previous.Credentials.IsExpired() { + return nil + } + if sts { + return expireIAMSTSConfig(ctx, store, path) + } + item = &UserIdentity{Version: 1, Deleted: true} + ctx = withIAMReplicationTime(ctx, previous.timestamp()) + } + var revocation *iamUserRevocation + if op, ok := item.(*iamUserRevocation); ok { + revocation = op + op.UserIdentity = UserIdentity{Version: 1, Deleted: true} + if origin, replicated := iamReplicationTime(ctx); replicated && previous.timestamp().After(origin) { + if previous.Deleted || !origin.After(previous.RevokedBefore) { + return errIAMStaleUpdate + } + op.retained = true + op.UserIdentity = UserIdentity{Version: 1, Credentials: previous.Credentials, UpdatedAt: previous.timestamp(), RevokedBefore: origin} + ctx = withIAMReplicationTime(ctx, previous.timestamp()) + } + item = &op.UserIdentity + } + var groupRevocation *iamGroupRevocation + if op, ok := item.(*iamGroupRevocation); ok { + groupRevocation = op + var group GroupInfo + if err := store.loadIAMConfig(ctx, &group, path); err != nil && !errors.Is(err, errConfigNotFound) { + return err + } + if op.requireEmpty && !group.Deleted { + for _, member := range group.Members { + r := store.revisionIndex().get(getUserIdentityPath(member, regUser)) + at := group.MemberGrants[member] + if !r.Deleted && (r.RevokedBefore.IsZero() || at.After(r.RevokedBefore)) && (group.RevokedBefore.IsZero() || at.After(group.RevokedBefore)) { + return errGroupNotEmpty + } + } + } + op.GroupInfo = GroupInfo{Version: 1, Deleted: true} + if origin, replicated := iamReplicationTime(ctx); replicated && previous.timestamp().After(origin) { + if previous.Deleted || !origin.After(previous.RevokedBefore) { + return errIAMStaleUpdate + } + op.retained = true + op.GroupInfo = group + op.RevokedBefore = origin + ctx = withIAMReplicationTime(ctx, previous.timestamp()) + } + item = &op.GroupInfo + } + var at *time.Time + var deleted bool + switch v := item.(type) { + case *UserIdentity: + at, deleted = &v.UpdatedAt, v.Deleted + if boundary, ok := ctx.Value(iamRecordBoundaryKey{}).(time.Time); ok && boundary.After(v.RevokedBefore) { + v.RevokedBefore = boundary + } + if previous.RevokedBefore.After(v.RevokedBefore) { + v.RevokedBefore = previous.RevokedBefore + } + case *GroupInfo: + at, deleted = &v.UpdatedAt, v.Deleted + if boundary, ok := ctx.Value(iamRecordBoundaryKey{}).(time.Time); ok && boundary.After(v.RevokedBefore) { + v.RevokedBefore = boundary + } + if !deleted { + var group GroupInfo + if err := store.loadIAMConfig(ctx, &group, path); err != nil && !errors.Is(err, errConfigNotFound) { + return err + } + mergeIAMGroupMutation(ctx, group, v) + } + if previous.RevokedBefore.After(v.RevokedBefore) { + v.RevokedBefore = previous.RevokedBefore + } + case *MappedPolicy: + at, deleted = &v.UpdatedAt, v.Deleted + case *PolicyDoc: + at, deleted = &v.UpdateDate, v.Deleted + default: + return errInvalidArgument + } + if strings.HasPrefix(path, iamConfigSTSPrefix) && previous.Deleted && !deleted { + // STS access keys identify immutable tokens, not reusable user names. + return errIAMStaleUpdate + } + if origin, replicated := iamReplicationTime(ctx); replicated { + *at = origin + if previous.timestamp().After(origin) || (previous.Deleted && !deleted && !origin.After(previous.timestamp())) { + return errIAMStaleUpdate + } + if strings.HasPrefix(path, iamConfigServiceAccountsPrefix) && !deleted && previous.Credentials.AccessKey != "" && previous.timestamp().Equal(origin) { + // Duplicate service snapshots are acknowledgements, not new creates + // or edits. Reload the winner without writing, so even a stale + // sibling cache is refreshed by the retry before acknowledging it. + return store.loadIAMConfig(ctx, item, path) + } + if previous.Deleted && deleted && !origin.After(previous.timestamp()) { + // An already-applied tombstone needs no further persistent write. + if v, ok := item.(*UserIdentity); ok { + v.RevokedBefore = previous.RevokedBefore + } + if v, ok := item.(*GroupInfo); ok { + v.RevokedBefore = previous.RevokedBefore + } + return nil + } + } else { + if previous.Deleted && deleted { + // A peer notification without an originating revision must not + // advance a tombstone past a subsequent deliberate recreation. + *at = previous.timestamp() + if v, ok := item.(*UserIdentity); ok { + v.RevokedBefore = previous.RevokedBefore + } + if v, ok := item.(*GroupInfo); ok { + v.RevokedBefore = previous.RevokedBefore + } + return nil + } + if at.IsZero() { + *at = UTCNow() + } + if !at.After(previous.timestamp()) { + *at = previous.timestamp().Add(time.Nanosecond) + } + } + if v, ok := item.(*UserIdentity); ok { + if deleted { + // Retain only the parent name for root-account exclusion during heal. + v.Credentials = auth.Credentials{ParentUser: previous.Credentials.ParentUser} + v.RevokedBefore = *at + if strings.HasPrefix(path, iamConfigSTSPrefix) && !previous.Credentials.Expiration.IsZero() && !previous.Credentials.Expiration.Equal(timeSentinel) { + // The signed STS token cannot authorize beyond this time, even + // if an offline site replays it with a newer event timestamp. + v.ExpiresAt = previous.Credentials.Expiration.Add(globalMaxSkewTime) + opts = []options{{ttl: max(1, int64(time.Until(v.ExpiresAt).Seconds())+1)}} + } + } else { + if v.Credentials.SessionToken != "" && v.Credentials.Claims == nil { + claims, err := extractJWTClaims(*v) + if err != nil { + return err + } + v.Credentials.Claims = claims.Map() + } + if err = checkIAMParentRevision(ctx, store, v.Credentials); err != nil { + return err + } + } + } + if v, ok := item.(*GroupInfo); ok && deleted { + v.RevokedBefore = *at + v.Members, v.MemberGrants = nil, nil + } + if _, ok := item.(*MappedPolicy); ok && !deleted { + if parentPath := iamMappingParentPath(path); parentPath != "" { + parent, err := loadIAMRevision(ctx, store, parentPath) + if err != nil { + return err + } + if parent.Deleted { + return errIAMStaleUpdate + } + if !parent.RevokedBefore.IsZero() && !at.After(parent.RevokedBefore) { + if _, replicated := iamReplicationTime(ctx); replicated { + return errIAMStaleUpdate + } + *at = parent.RevokedBefore.Add(time.Nanosecond) + } + } + } + if err := store.saveIAMConfig(ctx, item, path, opts...); err != nil { + return err + } + if revocation != nil && revocation.retained { + return errIAMRevocationRetained + } + if groupRevocation != nil && groupRevocation.retained { + return errIAMRevocationRetained + } + return nil +} + +func (iamOS *IAMObjectStore) listIAMConfigPaths(ctx context.Context) ([]string, error) { + ctx, cancel := context.WithCancel(ctx) + defer cancel() + var paths []string + for item := range listIAMConfigItems(ctx, iamOS.objAPI, iamConfigPrefix+"/") { + if item.Err != nil { + return nil, item.Err + } + paths = append(paths, iamConfigPrefix+"/"+item.Item) + } + return paths, nil +} + +func (ies *IAMEtcdStore) listIAMConfigPaths(ctx context.Context) ([]string, error) { + ctx, cancel := context.WithTimeout(ctx, defaultContextTimeout) + defer cancel() + r, err := ies.client.Get(ctx, iamConfigPrefix+"/", etcd.WithPrefix(), etcd.WithKeysOnly()) + if err != nil { + return nil, err + } + paths := make([]string, 0, len(r.Kvs)) + for _, kv := range r.Kvs { + paths = append(paths, string(kv.Key)) + } + return paths, nil +} + +func iamDeletionItem(path string, r iamRevision) (item madmin.SRIAMItem, ok bool) { + if (strings.HasPrefix(path, iamConfigUsersPrefix) || strings.HasPrefix(path, iamConfigGroupsPrefix)) && !r.RevokedBefore.IsZero() { + // Recreating a parent does not cancel its older revocation of derived + // credentials. Replay this boundary even after the parent is live again. + r.Deleted = true + r.UpdatedAt, r.UpdateDate = r.RevokedBefore, time.Time{} + } + if !r.Deleted { + return item, false + } + item.UpdatedAt = r.timestamp() + switch { + case strings.HasPrefix(path, iamConfigUsersPrefix): + name := strings.TrimSuffix(strings.TrimPrefix(path, iamConfigUsersPrefix), "/"+iamIdentityFile) + item.Type = madmin.SRIAMItemIAMUser + item.IAMUser = &madmin.SRIAMUser{AccessKey: name, IsDeleteReq: true} + case strings.HasPrefix(path, iamConfigServiceAccountsPrefix): + name := strings.TrimSuffix(strings.TrimPrefix(path, iamConfigServiceAccountsPrefix), "/"+iamIdentityFile) + if name == siteReplicatorSvcAcc || r.Credentials.ParentUser == globalActiveCred.AccessKey { + return item, false + } + item.Type = madmin.SRIAMItemSvcAcc + item.SvcAccChange = &madmin.SRSvcAccChange{Delete: &madmin.SRSvcAccDelete{AccessKey: name}} + case strings.HasPrefix(path, iamConfigGroupsPrefix): + name := strings.TrimSuffix(strings.TrimPrefix(path, iamConfigGroupsPrefix), "/"+iamGroupMembersFile) + item.Type = madmin.SRIAMItemGroupInfo + item.GroupInfo = &madmin.SRGroupInfo{UpdateReq: madmin.GroupAddRemove{Group: name, IsRemove: true}} + case strings.HasPrefix(path, iamConfigPoliciesPrefix): + item.Type = madmin.SRIAMItemPolicy + item.Name = strings.TrimSuffix(strings.TrimPrefix(path, iamConfigPoliciesPrefix), "/"+iamPolicyFile) + case strings.HasPrefix(path, iamConfigPolicyDBPrefix): + prefix, name, found := strings.Cut(strings.TrimPrefix(path, iamConfigPolicyDBPrefix), "/") + if !found { + return item, false + } + typ := regUser + switch prefix { + case "sts-users": + typ = stsUser + case "service-accounts": + typ = svcUser + } + item.Type = madmin.SRIAMItemPolicyMapping + item.PolicyMapping = &madmin.SRPolicyMapping{UserOrGroup: strings.TrimSuffix(name, ".json"), UserType: int(typ), IsGroup: prefix == "groups"} + default: + // Expired STS credentials are not replayed. Parent revocations and + // their retained timestamp reject delayed copies of derived tokens. + return item, false + } + return item, true +} + +func iamDeletionPath(item madmin.SRIAMItem) string { + switch item.Type { + case madmin.SRIAMItemIAMUser: + if item.IAMUser != nil && item.IAMUser.IsDeleteReq { + return getUserIdentityPath(item.IAMUser.AccessKey, regUser) + } + case madmin.SRIAMItemSvcAcc: + if item.SvcAccChange != nil && item.SvcAccChange.Delete != nil { + return getUserIdentityPath(item.SvcAccChange.Delete.AccessKey, svcUser) + } + case madmin.SRIAMItemGroupInfo: + if item.GroupInfo != nil && item.GroupInfo.UpdateReq.IsRemove && len(item.GroupInfo.UpdateReq.Members) == 0 { + return getGroupInfoPath(item.GroupInfo.UpdateReq.Group) + } + case madmin.SRIAMItemPolicy: + if len(item.Policy) == 0 { + return getPolicyDocPath(item.Name) + } + case madmin.SRIAMItemPolicyMapping: + if p := item.PolicyMapping; p != nil && p.Policy == "" { + return getMappedPolicyPath(p.UserOrGroup, IAMUserType(p.UserType), p.IsGroup) + } + } + return "" +} + +func (c *SiteReplicationSys) healIAMDeletions(ctx context.Context) (err error) { + started := time.Now() + defer func() { + c.iamRevisionMetrics.healDurationMillis.Store(time.Since(started).Milliseconds()) + if err != nil { + c.iamRevisionMetrics.healFailures.Add(1) + } else { + c.iamRevisionMetrics.healLastSuccess.Store(time.Now().Unix()) + } + }() + c.iamHealMu.Lock() + defer c.iamHealMu.Unlock() + c.RLock() + defer c.RUnlock() + if !c.enabled { + return nil + } + snapshot := globalIAMSys.store.revisionIndex().snapshot() + paths := make([]string, 0, len(snapshot)) + for path := range snapshot { + paths = append(paths, path) + } + sort.Strings(paths) + byType := make(map[string][]iamReplicationItem) + for _, path := range paths { + r := snapshot[path] + item, ok := iamDeletionItem(path, r) + if !ok { + continue + } + out := iamReplicationItem{SRIAMItem: item} + if item.Type == madmin.SRIAMItemIAMUser && !r.Deleted { + out.Type, out.IAMUser = iamUserBoundaryType, nil + out.UserRevocation = &iamUserBoundary{User: item.IAMUser.AccessKey, Before: r.RevokedBefore} + } + if item.Type == madmin.SRIAMItemGroupInfo && !r.Deleted { + out.Type, out.GroupInfo = iamGroupBoundaryType, nil + out.GroupRevocation = &iamGroupBoundary{Group: item.GroupInfo.UpdateReq.Group, Before: r.RevokedBefore} + } + byType[item.Type] = append(byType[item.Type], out) + } + var items []iamReplicationItem + for _, typ := range []string{madmin.SRIAMItemPolicyMapping, madmin.SRIAMItemIAMUser, madmin.SRIAMItemSvcAcc, madmin.SRIAMItemGroupInfo, madmin.SRIAMItemPolicy} { + items = append(items, byType[typ]...) + } + if len(items) == 0 { + return nil + } + if c.iamRevisionProgress == nil { + c.iamRevisionProgress = make(map[string]iamRevisionProgress) + } + for id := range c.iamRevisionProgress { + if _, present := c.state.Peers[id]; !present { + delete(c.iamRevisionProgress, id) + } + } + var progressMu sync.Mutex + cerr := c.concDo(nil, func(id string, p madmin.PeerInfo) error { + // Bound each pass, but retain acknowledgements independently of the + // pass deadline or unrelated changes at either site. + peerCtx, cancel := context.WithTimeout(ctx, defaultContextTimeout) + defer cancel() + client, err := c.getAdminClient(peerCtx, id) + if err != nil { + return err + } + remote, err := executeIAMRevisionRequest(peerCtx, client, http.MethodGet, nil) + if err != nil { + return err + } + progressMu.Lock() + progress := c.iamRevisionProgress[id] + progressMu.Unlock() + progress.observePeer(remote) + defer func() { + progressMu.Lock() + c.iamRevisionProgress[id] = progress + progressMu.Unlock() + }() + var pending []iamReplicationItem + for _, item := range items { + path, version := iamReplicationMarker(item) + if progress.Acknowledged[path] != version { + pending = append(pending, item) + } + } + // Acknowledgements are only a replay optimization, never GC proof. + for path := range progress.Acknowledged { + if _, retained := snapshot[path]; !retained { + delete(progress.Acknowledged, path) + } + } + var failures []error + for next := 0; next < len(pending); { + end := min(next+maxIAMRevisionBatch, len(pending)) + batch := pending[next:end] + remote, err = executeIAMRevisionRequest(peerCtx, client, http.MethodPut, &iamRevisionBatch{Version: iamRevisionProtocol, Items: batch}) + if err != nil { + var batchErr *iamRevisionBatchError + if !errors.As(err, &batchErr) { + return errors.Join(append(failures, err)...) + } + failures = append(failures, err) + } else { + progress.observePeer(remote) + for _, item := range batch { + path, version := iamReplicationMarker(item) + progress.Acknowledged[path] = version + } + } + next = end + } + return errors.Join(failures...) + }, "IAM revision convergence") + return errors.Unwrap(cerr) +} + +func iamReplicationMarker(item iamReplicationItem) (path, version string) { + path = iamDeletionPath(item.SRIAMItem) + if item.UserRevocation != nil { + path = getUserIdentityPath(item.UserRevocation.User, regUser) + } + if item.GroupRevocation != nil { + path = getGroupInfoPath(item.GroupRevocation.Group) + } + return path, item.Type + ":" + item.UpdatedAt.UTC().Format(time.RFC3339Nano) +} + +func (store *IAMStoreSys) savePolicyDoc(ctx context.Context, policyName string, p *PolicyDoc) error { + return store.saveIAMRevision(ctx, getPolicyDocPath(policyName), p) +} + +func (store *IAMStoreSys) saveMappedPolicy(ctx context.Context, name string, userType IAMUserType, isGroup bool, mp *MappedPolicy, opts ...options) error { + return store.saveIAMRevision(ctx, getMappedPolicyPath(name, userType, isGroup), mp, opts...) +} + +func (store *IAMStoreSys) saveUserIdentity(ctx context.Context, name string, userType IAMUserType, u *UserIdentity, opts ...options) error { + return store.saveIAMRevision(ctx, getUserIdentityPath(name, userType), u, opts...) +} + +func (store *IAMStoreSys) saveGroupInfo(ctx context.Context, name string, gi *GroupInfo) error { + return store.saveIAMRevision(ctx, getGroupInfoPath(name), gi) +} + +func (store *IAMStoreSys) deletePolicyDoc(ctx context.Context, name string) error { + return store.saveIAMRevision(ctx, getPolicyDocPath(name), &PolicyDoc{Version: 1, Deleted: true}) +} + +func (store *IAMStoreSys) deleteMappedPolicy(ctx context.Context, name string, userType IAMUserType, isGroup bool) error { + return store.saveIAMRevision(ctx, getMappedPolicyPath(name, userType, isGroup), &MappedPolicy{Version: 1, Deleted: true}) +} + +func (store *IAMStoreSys) deleteUserIdentity(ctx context.Context, name string, userType IAMUserType) error { + return store.saveIAMRevision(ctx, getUserIdentityPath(name, userType), &UserIdentity{Version: 1, Deleted: true}) +} + +// Called under the identity's distributed revision lock, after verifying that +// its immutable STS token (or early-revocation retention) has expired. Only the +// old token-key mapping is removed; the reusable parent mapping is unaffected. +func expireIAMSTSConfig(ctx context.Context, store IAMStorageAPI, path string) error { + key := strings.TrimSuffix(strings.TrimPrefix(path, iamConfigSTSPrefix), "/"+iamIdentityFile) + if err := store.deleteIAMConfig(ctx, getMappedPolicyPath(key, stsUser, false)); err != nil && !errors.Is(err, errConfigNotFound) { + return err + } + return store.deleteIAMConfig(ctx, path) +} + +type ( + iamExpirationCleanupKey struct{} + iamExpirationCleanupState struct{ failed atomic.Bool } +) + +func withIAMExpirationCleanup(ctx context.Context) context.Context { + if _, ok := ctx.Value(iamExpirationCleanupKey{}).(*iamExpirationCleanupState); ok { + return ctx + } + return context.WithValue(ctx, iamExpirationCleanupKey{}, &iamExpirationCleanupState{}) +} + +func bestEffortIAMExpiration(ctx context.Context, store IAMStorageAPI, path string) { + state, _ := ctx.Value(iamExpirationCleanupKey{}).(*iamExpirationCleanupState) + if ctx.Err() != nil || (state != nil && state.failed.Load()) { + return + } + ctx, cancel := context.WithTimeout(ctx, time.Second) + defer cancel() + // Failure leaves the expired record and its existing version intact. + // Stop optional reclamation for this load, while still loading healthy + // users. Healthy cleanup has no per-scan quota that could build a backlog. + if err := saveIAMRevision(ctx, store, path, &iamExpireIdentity{}); err != nil { + if state != nil { + state.failed.Store(true) + } + iamLogIf(ctx, err) + } +} diff --git a/cmd/iam-revision_test.go b/cmd/iam-revision_test.go new file mode 100644 index 000000000..279e7cfd3 --- /dev/null +++ b/cmd/iam-revision_test.go @@ -0,0 +1,330 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-only + +package cmd + +import ( + "context" + "encoding/json" + "errors" + "os" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/minio/madmin-go/v3" + "github.com/minio/minio/internal/grid" + xnet "github.com/pgsty/silo-pkg/v3/net" +) + +// Count physical saves: comparing timestamps alone would miss identical +// tombstones being rewritten on every heal pass. +type iamRevisionWriteCounter struct { + IAMStorageAPI + data []byte + writes int +} + +func (s *iamRevisionWriteCounter) loadIAMConfig(_ context.Context, item any, _ string) error { + return json.Unmarshal(s.data, item) +} + +func (s *iamRevisionWriteCounter) saveIAMConfig(_ context.Context, item any, _ string, _ ...options) error { + data, err := json.Marshal(item) + if err == nil { + s.data = data + s.writes++ + } + return err +} + +func TestIAMRevocationTombstoneReplayIsIdempotent(t *testing.T) { + at := time.Date(2026, 9, 14, 12, 0, 0, 0, time.UTC) + for _, record := range []struct { + name string + new func(bool) any + }{ + {"user", func(deleted bool) any { return &UserIdentity{Version: 1, Deleted: deleted} }}, + {"group", func(deleted bool) any { return &GroupInfo{Version: 1, Deleted: deleted} }}, + {"policy", func(deleted bool) any { return &PolicyDoc{Version: 1, Deleted: deleted} }}, + {"mapping", func(deleted bool) any { return &MappedPolicy{Version: 1, Deleted: deleted} }}, + } { + t.Run(record.name, func(t *testing.T) { + data, err := json.Marshal(iamRevision{Deleted: true, UpdatedAt: at, RevokedBefore: at}) + if err != nil { + t.Fatal(err) + } + store := &iamRevisionWriteCounter{data: data} + ctx := context.Background() + for range 3 { + // Site heal carries the original timestamp. Sibling notifications + // have no timestamp; both must leave an applied deletion untouched. + for _, replay := range []context.Context{withIAMReplicationTime(ctx, at), ctx} { + if err := saveIAMRevision(replay, store, record.name, record.new(true)); err != nil { + t.Fatal(err) + } + } + } + if store.writes != 0 || string(store.data) != string(data) { + t.Fatalf("replayed tombstone changed storage: writes=%d, record=%s", store.writes, store.data) + } + for _, deleted := range []bool{false, true} { + err := saveIAMRevision(withIAMReplicationTime(ctx, at.Add(-time.Second)), store, record.name, record.new(deleted)) + if !errors.Is(err, errIAMStaleUpdate) { + t.Fatalf("older event accepted, deleted=%t: %v", deleted, err) + } + } + if err := saveIAMRevision(withIAMReplicationTime(ctx, at), store, record.name, record.new(false)); !errors.Is(err, errIAMStaleUpdate) { + t.Fatalf("equal-time recreation accepted: %v", err) + } + newer := at.Add(time.Minute) + if err := saveIAMRevision(withIAMReplicationTime(ctx, newer), store, record.name, record.new(true)); err != nil { + t.Fatal(err) + } + r, err := loadIAMRevision(ctx, store, record.name) + if err != nil || store.writes != 1 || !r.timestamp().Equal(newer) || !r.Deleted { + t.Fatalf("newer deletion did not advance storage: writes=%d, revision=%+v, error=%v", store.writes, r, err) + } + if err := saveIAMRevision(withIAMReplicationTime(ctx, newer.Add(time.Minute)), store, record.name, record.new(false)); err != nil { + t.Fatalf("newer recreation rejected: %v", err) + } + }) + } +} + +// Run with both object storage and etcd through TestIAMRevocation*Lifecycle. +// The object-store case uses the real peer RPC and deletion handler, so a +// spurious notification actually destroys the parent instead of only counting it. +func testIAMRevocationReplayAfterRecreation(ctx context.Context, t *testing.T, sys *IAMSys) { + t.Helper() + peer := &globalSiteReplicationSys + must := func(err error) { + t.Helper() + if err != nil { + t.Fatal(err) + } + } + user := "heal-recreated-parent" + req := madmin.AddOrUpdateUserReq{SecretKey: "valid-test-password", Status: madmin.AccountEnabled} + origin := UTCNow().Add(-time.Hour).Truncate(time.Millisecond) + deleted, recreated := origin.Add(time.Minute), origin.Add(3*time.Minute) + create := func(at time.Time) { + must(peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: user, UserReq: &req}, at)) + } + revoke := func(at time.Time) { + must(peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: user, IsDeleteReq: true}, at)) + } + create(origin) + revoke(deleted) + create(recreated) + child, _, err := sys.NewServiceAccount(ctx, user, nil, newServiceAccountOpts{ + accessKey: "heal-recreated-child", secretKey: "valid-service-password", + }) + must(err) + + tg, err := grid.SetupTestGrid(2) + must(err) + t.Cleanup(tg.Cleanup) + var deletes atomic.Int32 + server := &peerRESTServer{} + must(deleteUserRPC.Register(tg.Managers[1], func(req *grid.MSS) (grid.NoPayload, *grid.RemoteErr) { + deletes.Add(1) + return server.DeleteUserHandler(req) + })) + // Future user updates still use the normal peer reload notification. + must(loadUserRPC.Register(tg.Managers[1], server.LoadUserHandler)) + host, err := xnet.ParseHost(strings.TrimPrefix(tg.Hosts[1], "http://")) + must(err) + previousNotifications := globalNotificationSys + globalNotificationSys = &NotificationSys{peerClients: []*peerRESTClient{{ + host: host, + gridConn: func() *grid.Connection { + return tg.Managers[0].Connection(tg.Hosts[1]) + }, + }}} + t.Cleanup(func() { globalNotificationSys = previousNotifications }) + assertLive := func(key string) { + t.Helper() + if _, ok := sys.GetUser(ctx, key); !ok { + t.Fatalf("live credential %s lost during deletion replay", key) + } + } + assertNoDelete := func() { + t.Helper() + if n := deletes.Load(); n != 0 { + t.Fatalf("retained revocation sent %d destructive sibling notifications", n) + } + } + for range 3 { + r, err := loadIAMRevision(ctx, sys.store, getUserIdentityPath(user, regUser)) + must(err) + item, ok := iamDeletionItem(getUserIdentityPath(user, regUser), r) + if !ok || item.IAMUser == nil || !item.UpdatedAt.Equal(deleted) { + t.Fatal("recreated user lost its durable revocation replay") + } + must(peer.PeerIAMUserChangeHandler(ctx, item.IAMUser, item.UpdatedAt)) + must(sys.store.LoadIAMCache(ctx, false)) + assertLive(user) + assertLive(child.AccessKey) + assertNoDelete() + } + + // A divergent site sends a previously unseen revocation between our old + // boundary and recreation. Retain it and revoke old children, but never + // turn it into an unversioned delete of the recreated parent. + delayed := deleted.Add(time.Minute) + revoke(delayed) + assertNoDelete() + must(sys.store.LoadIAMCache(ctx, false)) + assertLive(user) + if _, ok := sys.GetUser(ctx, child.AccessKey); ok { + t.Fatal("child from before the delayed revocation remains usable") + } + r, err := loadIAMRevision(ctx, sys.store, getUserIdentityPath(user, regUser)) + must(err) + if r.Deleted || !r.RevokedBefore.Equal(delayed) || !r.timestamp().Equal(recreated) { + t.Fatalf("retained revocation damaged the recreated identity: %+v", r) + } + + // A genuinely newer deletion must still reach siblings and remove the + // parent plus credentials issued under its latest revocation boundary. + fresh, _, err := sys.NewServiceAccount(ctx, user, nil, newServiceAccountOpts{ + accessKey: "heal-fresh-child", secretKey: "valid-service-password", + }) + must(err) + latest := recreated.Add(time.Minute) + revoke(latest) + wantDeletes := int32(1) + if sys.HasWatcher() { + wantDeletes = 0 + } + if n := deletes.Load(); n != wantDeletes { + t.Fatalf("new deletion notifications=%d, want %d", n, wantDeletes) + } + for _, key := range []string{user, fresh.AccessKey} { + if _, ok := sys.GetUser(ctx, key); ok { + t.Fatalf("newer deletion left credential %s usable", key) + } + } + // Exercise the actual sibling handler again against the already persisted + // tombstone. Its context has no revision; it must not re-stamp the record. + _, remoteErr := server.DeleteUserHandler(grid.NewMSSWith(map[string]string{peerRESTUser: user})) + if remoteErr != nil { + t.Fatal(remoteErr) + } + r, err = loadIAMRevision(ctx, sys.store, getUserIdentityPath(user, regUser)) + must(err) + if !r.Deleted || !r.timestamp().Equal(latest) { + t.Fatalf("sibling re-stamped the tombstone: got %s, want %s", r.timestamp(), latest) + } + create(latest.Add(time.Minute)) + _, remoteErr = server.DeleteUserHandler(grid.NewMSSWith(map[string]string{peerRESTUser: user})) + if remoteErr != nil { + t.Fatal(remoteErr) + } + assertLive(user) + revoke(latest) + must(sys.store.LoadIAMCache(ctx, false)) + assertLive(user) +} + +// Counts what a retained revocation actually sends to sibling nodes. +func TestIAMRevocationRetainedReloadsSibling(t *testing.T) { + resetTestGlobals() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + disks, err := getRandomDisks(1) + if err != nil { + t.Fatal(err) + } + obj, _, err := initObjectLayer(ctx, mustGetPoolEndpoints(0, disks...)) + if err != nil { + t.Fatal(err) + } + initAllSubsystems(ctx) + globalIAMSys.Init(ctx, obj, nil, 2*time.Second) + defer os.RemoveAll(disks[0]) + defer obj.Shutdown(ctx) + defer resetTestGlobals() + + sys, peer := globalIAMSys, &globalSiteReplicationSys + must := func(err error) { + t.Helper() + if err != nil { + t.Fatal(err) + } + } + user := "retained-parent" + req := madmin.AddOrUpdateUserReq{SecretKey: "valid-test-password", Status: madmin.AccountEnabled} + origin := UTCNow().Add(-time.Hour).Truncate(time.Millisecond) + deleted, recreated := origin.Add(time.Minute), origin.Add(3*time.Minute) + create := func(at time.Time) { + must(peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: user, UserReq: &req}, at)) + } + revoke := func(at time.Time) { + must(peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: user, IsDeleteReq: true}, at)) + } + create(origin) + revoke(deleted) + create(recreated) + child, _, err := sys.NewServiceAccount(ctx, user, nil, newServiceAccountOpts{ + accessKey: "retained-child", secretKey: "valid-service-password", + }) + must(err) + + // A sibling shares persistent state but has an independent IAM cache. + sibling := &IAMStoreSys{IAMStorageAPI: newIAMObjectStore(obj, sys.usersSysType)} + must(sibling.LoadIAMCache(ctx, false)) + if _, ok := sibling.GetUser(child.AccessKey); !ok { + t.Fatal("sibling fixture did not load child") + } + tg, err := grid.SetupTestGrid(2) + must(err) + t.Cleanup(tg.Cleanup) + var deletes, loads atomic.Int32 + server := &peerRESTServer{} + must(deleteUserRPC.Register(tg.Managers[1], func(r *grid.MSS) (grid.NoPayload, *grid.RemoteErr) { + deletes.Add(1) + return server.DeleteUserHandler(r) + })) + must(loadUserRPC.Register(tg.Managers[1], func(r *grid.MSS) (grid.NoPayload, *grid.RemoteErr) { + loads.Add(1) + // LoadUserHandler delegates to this same cache reload method. + if err := sibling.UserNotificationHandler(ctx, r.Get(peerRESTUser), regUser); err != nil { + return grid.NoPayload{}, grid.NewRemoteErr(err) + } + return grid.NoPayload{}, nil + })) + host, err := xnet.ParseHost(strings.TrimPrefix(tg.Hosts[1], "http://")) + must(err) + prev := globalNotificationSys + globalNotificationSys = &NotificationSys{peerClients: []*peerRESTClient{{ + host: host, + gridConn: func() *grid.Connection { return tg.Managers[0].Connection(tg.Hosts[1]) }, + }}} + t.Cleanup(func() { globalNotificationSys = prev }) + + delayed := deleted.Add(time.Minute) + revoke(delayed) + + t.Logf("sibling notifications after a retained revocation: destructive=%d reload=%d", deletes.Load(), loads.Load()) + if deletes.Load() != 0 { + t.Errorf("destructive sibling delete sent: %d", deletes.Load()) + } + if loads.Load() == 0 { + t.Errorf("retained revocation did not notify the sibling") + } + if _, ok := sibling.GetUser(child.AccessKey); ok { + t.Error("sibling still resolves revoked child") + } + if _, ok := sibling.GetUser(user); !ok { + t.Error("sibling lost live parent") + } + if _, ok := sys.store.GetUser(user); !ok { + t.Error("live parent lost") + } + if _, ok := sys.store.GetUser(child.AccessKey); ok { + t.Error("revoked child still resolves on the receiving node") + } +} diff --git a/cmd/iam-revocation_test.go b/cmd/iam-revocation_test.go new file mode 100644 index 000000000..9ac756b6f --- /dev/null +++ b/cmd/iam-revocation_test.go @@ -0,0 +1,535 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-only + +package cmd + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "os" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/minio/madmin-go/v3" + "github.com/minio/minio/internal/auth" + etcd "go.etcd.io/etcd/client/v3" + "go.etcd.io/etcd/client/v3/namespace" +) + +// Exercise the persisted IAM store and the same peer handler used by site heal. +// A delete must survive a cache reload and an older create arriving afterwards. +func TestIAMRevocationRejectsOfflineUser(t *testing.T) { + resetTestGlobals() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + obj, disk, err := prepareFS(ctx) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(disk) + defer obj.Shutdown(ctx) + defer resetTestGlobals() + user := "offline-revoked-user" + req := madmin.AddOrUpdateUserReq{SecretKey: "test-password-valid", Status: madmin.AccountEnabled} + created, err := globalIAMSys.CreateUser(ctx, user, req) + if err != nil { + t.Fatal(err) + } + if err = globalIAMSys.DeleteUser(ctx, user, false); err != nil { + t.Fatal(err) + } + if err = globalIAMSys.store.LoadIAMCache(ctx, false); err != nil { + t.Fatal(err) + } + if err = globalSiteReplicationSys.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: user, UserReq: &req}, created); err != nil { + t.Fatal(err) + } + if _, err = globalIAMSys.GetUserInfo(ctx, user); !errors.Is(err, errNoSuchUser) { + t.Fatalf("revoked user restored by old peer event: %v", err) + } +} + +func TestIAMRevocationHealingContinuesAfterPeerRejectsDelete(t *testing.T) { + resetTestGlobals() + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + obj, disk, err := prepareFS(ctx) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(disk) + defer obj.Shutdown(ctx) + defer resetTestGlobals() + req := madmin.AddOrUpdateUserReq{SecretKey: "valid-test-password", Status: madmin.AccountEnabled} + if _, err := globalIAMSys.CreateUser(ctx, "heal-sync", req); err != nil { + t.Fatal(err) + } + if _, err := globalIAMSys.CreateUser(ctx, "heal-deleted", req); err != nil { + t.Fatal(err) + } + if err := globalIAMSys.DeleteUser(ctx, "heal-deleted", false); err != nil { + t.Fatal(err) + } + p, err := globalIAMSys.store.GetPolicy("readwrite") + if err != nil { + t.Fatal(err) + } + if _, err := globalIAMSys.SetPolicy(ctx, "heal-new-policy", p); err != nil { + t.Fatal(err) + } + var liveUpdates atomic.Int32 + peer := func(id string, rejectDelete bool) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case strings.HasSuffix(r.URL.Path, "/metainfo"): + _ = json.NewEncoder(w).Encode(madmin.SRInfo{DeploymentID: id}) + case r.URL.Path == "/minio/admin/v3/site-replication/peer/iam-revisions": + if r.Method == http.MethodGet { + _ = json.NewEncoder(w).Encode(iamRevisionStatus{Version: iamRevisionProtocol, Node: "node-1", Instance: id, Digest: "fixture"}) + return + } + var batch iamRevisionBatch + if err := json.NewDecoder(r.Body).Decode(&batch); err != nil { + t.Error(err) + w.WriteHeader(http.StatusBadRequest) + return + } + for _, item := range batch.Items { + if rejectDelete && iamDeletionPath(item.SRIAMItem) != "" { + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"Code":"AccessDenied","Message":"delete rejected"}`)) + return + } + if id == "healthy" && item.Type == madmin.SRIAMItemPolicy && item.Name == "heal-new-policy" && len(item.Policy) > 0 { + liveUpdates.Add(1) + } + } + _ = json.NewEncoder(w).Encode(iamRevisionStatus{Version: iamRevisionProtocol, Node: "node-1", Instance: id, Digest: "fixture"}) + default: + t.Errorf("unexpected peer request %s", r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + })) + } + healthy, rejected := peer("healthy", false), peer("rejected", true) + defer healthy.Close() + defer rejected.Close() + c := &SiteReplicationSys{enabled: true, state: srState{ + ServiceAccountAccessKey: "heal-sync", + Peers: map[string]madmin.PeerInfo{ + globalDeploymentID(): {Name: "local", DeploymentID: globalDeploymentID()}, + "healthy": {Name: "healthy", DeploymentID: "healthy", Endpoint: healthy.URL}, + "rejected": {Name: "rejected", DeploymentID: "rejected", Endpoint: rejected.URL}, + }, + }} + if err := c.healIAMSystem(ctx, obj); err == nil { + t.Fatal("deletion failure was not reported") + } + if liveUpdates.Load() == 0 { + t.Fatal("one peer rejecting a deletion blocked unrelated live IAM healing to a healthy peer") + } +} + +func TestIAMRevocationLifecycle(t *testing.T) { + testIAMRevocationLifecycle(t, nil) +} + +func TestIAMRevocationEtcdLifecycle(t *testing.T) { + endpoint := os.Getenv("SILO_TEST_IAM_REVOCATION_ETCD") + if endpoint == "" { + t.Skip("set SILO_TEST_IAM_REVOCATION_ETCD to a disposable etcd endpoint") + } + connection, err := etcd.New(etcd.Config{Endpoints: strings.Split(endpoint, ","), DialTimeout: 5 * time.Second}) + if err != nil { + t.Fatal(err) + } + defer connection.Close() + // The facade borrows the connection's services. Close the owning client, + // not namespace.Watcher while IAM's canceled watch loop is winding down. + ctx, cancel := context.WithCancel(connection.Ctx()) + defer cancel() + client := etcd.NewCtxClient(ctx, etcd.WithZapLogger(connection.GetLogger())) + prefix := fmt.Sprintf("/silo-revocation-test/%d/", time.Now().UnixNano()) + client.KV = namespace.NewKV(connection.KV, prefix) + client.Watcher = namespace.NewWatcher(connection.Watcher, prefix) + client.Lease = connection.Lease + testIAMRevocationLifecycle(t, client) +} + +func testIAMRevocationLifecycle(t *testing.T, client *etcd.Client) { + resetTestGlobals() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + disks, err := getRandomDisks(1) + if err != nil { + t.Fatal(err) + } + disk := disks[0] + obj, _, err := initObjectLayer(ctx, mustGetPoolEndpoints(0, disks...)) + if err == nil { + initAllSubsystems(ctx) + globalIAMSys.Init(ctx, obj, client, 2*time.Second) + } + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(disk) + defer obj.Shutdown(ctx) + defer resetTestGlobals() + sys, peer := globalIAMSys, &globalSiteReplicationSys + must := func(t *testing.T, err error) { + t.Helper() + if err != nil { + t.Fatal(err) + } + } + reload := func(t *testing.T) { t.Helper(); must(t, sys.store.LoadIAMCache(ctx, false)) } + req := madmin.AddOrUpdateUserReq{SecretKey: "valid-test-password", Status: madmin.AccountEnabled} + origin := UTCNow().Add(-time.Hour).Truncate(time.Millisecond) + createUser := func(t *testing.T, name string) { + t.Helper() + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: name, UserReq: &req}, origin)) + } + assertAbsent := func(t *testing.T, name string) { + t.Helper() + if _, ok := sys.GetUser(ctx, name); ok { + t.Fatalf("revoked credential %s is usable", name) + } + } + + t.Run("replay after recreation", func(t *testing.T) { + testIAMRevocationReplayAfterRecreation(ctx, t, sys) + }) + + t.Run("origin timestamp and recreation", func(t *testing.T) { + name := "revocation-recreate" + createUser(t, name) + ui, ok := sys.store.GetUser(name) + if !ok || !ui.UpdatedAt.Equal(origin) { + t.Fatalf("origin time changed: %v", ui.UpdatedAt) + } + must(t, sys.DeleteUser(ctx, name, false)) + reload(t) + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: name, UserReq: &req}, time.Time{})) + assertAbsent(t, name) + newTime := UTCNow().Add(time.Minute) + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: name, UserReq: &req}, newTime)) + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: name, IsDeleteReq: true}, origin.Add(time.Second))) + reload(t) + ui, ok = sys.store.GetUser(name) + if !ok || !ui.UpdatedAt.Equal(newTime) || ui.RevokedBefore.IsZero() { + t.Fatalf("newer recreation lost, or deletion boundary missing: present=%v", ok) + } + }) + + t.Run("groups policies and mappings", func(t *testing.T) { + user, group, name := "revocation-member", "revocation-group", "revocation-policy" + createUser(t, user) + p, err := sys.store.GetPolicy("readwrite") + must(t, err) + must(t, peer.PeerAddPolicyHandler(ctx, name, &p, origin)) + add := &madmin.SRGroupInfo{UpdateReq: madmin.GroupAddRemove{Group: group, Members: []string{user}}} + must(t, peer.PeerGroupInfoChangeHandler(ctx, add, origin)) + for _, isGroup := range []bool{false, true} { + entity := user + if isGroup { + entity = group + } + mp := &madmin.SRPolicyMapping{UserOrGroup: entity, Policy: name, UserType: int(regUser), IsGroup: isGroup} + must(t, peer.PeerPolicyMappingHandler(ctx, mp, origin)) + _, err = sys.PolicyDBSet(ctx, entity, "", regUser, isGroup) + must(t, err) + must(t, peer.PeerPolicyMappingHandler(ctx, mp, origin)) + if _, ok := sys.store.GetMappedPolicy(entity, isGroup); ok { + t.Fatal("old grant restored") + } + } + // This receiver never saw the member-removal event preceding deletion. + must(t, peer.PeerGroupInfoChangeHandler(ctx, &madmin.SRGroupInfo{UpdateReq: madmin.GroupAddRemove{Group: group, IsRemove: true}}, UTCNow())) + must(t, sys.DeletePolicy(ctx, name, true)) + reload(t) + must(t, peer.PeerGroupInfoChangeHandler(ctx, add, origin)) + must(t, peer.PeerAddPolicyHandler(ctx, name, &p, origin)) + if _, err = sys.GetGroupDescription(group); !errors.Is(err, errNoSuchGroup) { + t.Fatalf("group restored: %v", err) + } + if _, err = sys.store.GetPolicyDoc(name); !errors.Is(err, errNoSuchPolicy) { + t.Fatalf("policy restored: %v", err) + } + paths, err := sys.store.listIAMConfigPaths(ctx) + must(t, err) + found := make(map[string]bool) + for _, path := range paths { + r, err := loadIAMRevision(ctx, sys.store, path) + must(t, err) + if item, ok := iamDeletionItem(path, r); ok { + found[iamDeletionPath(item)] = true + if item.UpdatedAt.IsZero() { + t.Fatal("undated delete replay") + } + } + } + for _, path := range []string{getGroupInfoPath(group), getPolicyDocPath(name), getMappedPolicyPath(user, regUser, false), getMappedPolicyPath(group, regUser, true)} { + if !found[path] { + t.Errorf("deletion missing from heal: %s", path) + } + } + }) + + t.Run("parent revokes service accounts and STS", func(t *testing.T) { + parent := "revocation-parent" + createUser(t, parent) + svc, svcAt, err := sys.NewServiceAccount(withIAMReplicationTime(ctx, origin), parent, nil, newServiceAccountOpts{accessKey: "revocation-service", secretKey: "valid-service-password"}) + must(t, err) + secret, err := getTokenSigningKey() + must(t, err) + sts, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: parent}, secret) + must(t, err) + sts.ParentUser = parent + _, err = sys.SetTempUser(withIAMReplicationTime(ctx, origin), sts.AccessKey, sts, "readwrite") + must(t, err) + must(t, sys.DeleteUser(ctx, parent, false)) + reload(t) + assertAbsent(t, parent) + assertAbsent(t, svc.AccessKey) + assertAbsent(t, sts.AccessKey) + // Recreate the parent, then deliver old child events from the offline site. + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: parent, UserReq: &req}, UTCNow())) + must(t, peer.PeerSvcAccChangeHandler(ctx, &madmin.SRSvcAccChange{Create: &madmin.SRSvcAccCreate{Parent: parent, AccessKey: svc.AccessKey, SecretKey: svc.SecretKey}}, svcAt)) + must(t, peer.PeerSTSAccHandler(ctx, &madmin.SRSTSCredential{AccessKey: sts.AccessKey, SecretKey: sts.SecretKey, ParentUser: parent, SessionToken: sts.SessionToken, ParentPolicyMapping: "readwrite"}, origin)) + reload(t) + assertAbsent(t, svc.AccessKey) + assertAbsent(t, sts.AccessKey) + // A freshly issued credential is still supported after deliberate recreation. + _, _, err = sys.NewServiceAccount(ctx, parent, nil, newServiceAccountOpts{accessKey: "new-service", secretKey: "valid-service-password"}) + must(t, err) + if _, ok := sys.GetUser(ctx, "new-service"); !ok { + t.Fatal("fresh service account rejected") + } + }) + + t.Run("delete before first create", func(t *testing.T) { + name := "revocation-unseen" + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: name, IsDeleteReq: true}, UTCNow())) + createUser(t, name) + assertAbsent(t, name) + svc := "unseen-service" + must(t, peer.PeerSvcAccChangeHandler(ctx, &madmin.SRSvcAccChange{Delete: &madmin.SRSvcAccDelete{AccessKey: svc}}, UTCNow())) + must(t, peer.PeerSvcAccChangeHandler(ctx, &madmin.SRSvcAccChange{Create: &madmin.SRSvcAccCreate{Parent: "revocation-recreate", AccessKey: svc, SecretKey: "valid-service-password"}}, origin)) + assertAbsent(t, svc) + }) + + t.Run("recreation arrives before revocation", func(t *testing.T) { + parent := "reordered-parent" + createUser(t, parent) + child, _, err := sys.NewServiceAccount(withIAMReplicationTime(ctx, origin), parent, nil, newServiceAccountOpts{accessKey: "reordered-child", secretKey: "valid-service-password"}) + must(t, err) + newTime, deleteTime := UTCNow(), origin.Add(time.Minute) + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: parent, UserReq: &req}, newTime)) + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: parent, IsDeleteReq: true}, deleteTime)) + assertAbsent(t, child.AccessKey) + reload(t) + assertAbsent(t, child.AccessKey) + u, ok := sys.GetUser(ctx, parent) + if !ok || !u.UpdatedAt.Equal(newTime) || !u.RevokedBefore.Equal(deleteTime) { + t.Fatal("reordered revocation damaged the new parent or lost its boundary") + } + r, err := loadIAMRevision(ctx, sys.store, getUserIdentityPath(parent, regUser)) + must(t, err) + item, ok := iamDeletionItem(getUserIdentityPath(parent, regUser), r) + if !ok || !item.UpdatedAt.Equal(deleteTime) { + t.Fatal("recreation erased deletion replay") + } + }) + + t.Run("user cleanup does not supersede group deletion", func(t *testing.T) { + user, group := "cascade-user", "cascade-group" + createUser(t, user) + must(t, peer.PeerGroupInfoChangeHandler(ctx, &madmin.SRGroupInfo{UpdateReq: madmin.GroupAddRemove{Group: group, Members: []string{user}}}, origin)) + // On the origin site the group was removed before the user, but the + // recovering receiver processes those independent events in reverse. + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: user, IsDeleteReq: true}, origin.Add(2*time.Minute))) + must(t, peer.PeerGroupInfoChangeHandler(ctx, &madmin.SRGroupInfo{UpdateReq: madmin.GroupAddRemove{Group: group, IsRemove: true}}, origin.Add(time.Minute))) + reload(t) + if _, err := sys.GetGroupDescription(group); !errors.Is(err, errNoSuchGroup) { + t.Fatalf("deleted group survived reordered cleanup: %v", err) + } + groups, err := sys.ListGroups(ctx) + must(t, err) + for _, name := range groups { + if name == group { + t.Fatal("deleted group listed") + } + } + }) + + t.Run("parent revocation covers later updates to existing children", func(t *testing.T) { + parent, key := "late-update-parent", "late-update-child" + createUser(t, parent) + _, _, err := sys.NewServiceAccount(withIAMReplicationTime(ctx, origin), parent, nil, newServiceAccountOpts{accessKey: key, secretKey: "valid-service-password"}) + must(t, err) + // This site missed the deletion and subsequently edited an old child. + _, err = sys.UpdateServiceAccount(withIAMReplicationTime(ctx, origin.Add(2*time.Minute)), key, updateServiceAccountOpts{description: "edited while the peer was offline"}) + must(t, err) + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: parent, IsDeleteReq: true}, origin.Add(time.Minute))) + assertAbsent(t, key) + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: parent, UserReq: &req}, origin.Add(3*time.Minute))) + reload(t) + assertAbsent(t, key) + }) + + t.Run("old generation cannot return with a newer event timestamp", func(t *testing.T) { + parent := "generation-parent" + createUser(t, parent) + deleteTime := origin.Add(time.Minute) + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: parent, IsDeleteReq: true}, deleteTime)) + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: parent, UserReq: &req}, origin.Add(2*time.Minute))) + // Another offline site issued this child under the original parent, + // after this site's delete/recreate. Wall-clock ordering cannot identify it. + late := origin.Add(3 * time.Minute) + must(t, peer.PeerSvcAccChangeHandler(ctx, &madmin.SRSvcAccChange{Create: &madmin.SRSvcAccCreate{Parent: parent, AccessKey: "old-gen-service", SecretKey: "valid-service-password"}}, late)) + secret, err := getTokenSigningKey() + must(t, err) + sts, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: parent}, secret) + must(t, err) + must(t, peer.PeerSTSAccHandler(ctx, &madmin.SRSTSCredential{AccessKey: sts.AccessKey, SecretKey: sts.SecretKey, ParentUser: parent, SessionToken: sts.SessionToken}, late)) + reload(t) + assertAbsent(t, "old-gen-service") + assertAbsent(t, sts.AccessKey) + // A local issuer knows the new boundary and signs it into both kinds + // of child. Untrusted inherited claims cannot select that boundary. + child, _, err := sys.NewServiceAccount(ctx, parent, nil, newServiceAccountOpts{accessKey: "new-gen-service", secretKey: "valid-service-password", claims: map[string]any{iamParentRevocationClaim: "forged"}}) + must(t, err) + newClaims := map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: parent} + must(t, setIAMParentRevocationClaim(ctx, sys.store, parent, newClaims)) + fresh, err := auth.GetNewCredentialsWithMetadata(newClaims, secret) + must(t, err) + fresh.ParentUser = parent + _, err = sys.SetTempUser(ctx, fresh.AccessKey, fresh, "") + must(t, err) + reload(t) + // A periodic reload retains the STS cache. Explicitly clear it to + // exercise the cold credential load performed after process restart. + cache := sys.store.lock() + cache.iamSTSAccountsMap = make(map[string]UserIdentity) + sys.store.unlock() + for _, key := range []string{child.AccessKey, fresh.AccessKey} { + u, ok := sys.GetUser(ctx, key) + if !ok || !iamCredentialSurvivesRevocation(u.Credentials, deleteTime) { + t.Fatalf("new-generation credential %s rejected", key) + } + } + }) + + t.Run("late revocation preserves proven new-generation children", func(t *testing.T) { + for _, recreateFirst := range []bool{false, true} { + parent := fmt.Sprintf("gen-parent-%t", recreateFirst) + createUser(t, parent) + deleteTime, createTime := origin.Add(time.Minute), origin.Add(2*time.Minute) + if recreateFirst { + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: parent, UserReq: &req}, createTime)) + } + key := fmt.Sprintf("gen-child-%t", recreateFirst) + must(t, peer.PeerSvcAccChangeHandler(ctx, &madmin.SRSvcAccChange{Create: &madmin.SRSvcAccCreate{Parent: parent, AccessKey: key, SecretKey: "valid-service-password", Claims: map[string]any{iamParentRevocationClaim: deleteTime.Format(time.RFC3339Nano)}}}, createTime.Add(time.Second))) + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: parent, IsDeleteReq: true}, deleteTime)) + if !recreateFirst { + assertAbsent(t, key) + must(t, peer.PeerIAMUserChangeHandler(ctx, &madmin.SRIAMUser{AccessKey: parent, UserReq: &req}, createTime)) + } + reload(t) + if _, ok := sys.GetUser(ctx, key); !ok { + t.Fatal("late revocation deleted a child issued by the recreated parent") + } + } + }) + + t.Run("cold loading preserves site-signed STS", func(t *testing.T) { + parent := "cold-sts-parent" + createUser(t, parent) + secret := "site-signing-key-valid" + _, _, err := sys.NewServiceAccount(ctx, globalActiveCred.AccessKey, nil, newServiceAccountOpts{ + accessKey: siteReplicatorSvcAcc, secretKey: secret, allowSiteReplicatorAccount: true, + }) + must(t, err) + setReplication := func(enabled bool) { + globalSiteReplicationSys.Lock() + globalSiteReplicationSys.enabled = enabled + globalSiteReplicationSys.Unlock() + globalSiteReplicatorCred.Set("") + } + setReplication(true) + defer setReplication(false) + cred, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: parent}, secret) + must(t, err) + cred.ParentUser = parent + _, err = sys.SetTempUser(ctx, cred.AccessKey, cred, "") + must(t, err) + // IAM can load before the site replication manager during startup. + // A signing key that is not available yet must not delete live tokens. + setReplication(false) + for range 3 { + unverified := make(map[string]UserIdentity) + _ = sys.store.loadUser(ctx, cred.AccessKey, stsUser, unverified) + if _, ok := unverified[cred.AccessKey]; ok { + t.Fatal("accepted STS before the signing key became available") + } + } + r, err := loadIAMRevision(ctx, sys.store, getUserIdentityPath(cred.AccessKey, stsUser)) + must(t, err) + if r.Credentials.SessionToken == "" { + t.Fatal("cold IAM load physically deleted a non-expired site-signed STS credential") + } + setReplication(true) + loaded := make(map[string]UserIdentity) + must(t, sys.store.loadUser(ctx, cred.AccessKey, stsUser, loaded)) + if _, ok := loaded[cred.AccessKey]; !ok { + t.Fatal("STS credential did not recover when the signing key became available") + } + }) + + t.Run("unverifiable STS stay denied and expired STS are removed", func(t *testing.T) { + parent := "invalid-sts-parent" + createUser(t, parent) + // Keep the etcd watcher from cleaning half of the fixture before the + // second record is seeded; this subtest exercises the loader directly. + sys.store.lock() + defer sys.store.unlock() + for _, expired := range []bool{false, true} { + cred, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: parent}, "unavailable-test-signing-key") + must(t, err) + cred.ParentUser = parent + if expired { + cred.Expiration = UTCNow().Add(-time.Minute) + } + identityPath := getUserIdentityPath(cred.AccessKey, stsUser) + mappingPath := getMappedPolicyPath(cred.AccessKey, stsUser, false) + // Seed disk directly to exercise loading, including existing records + // whose key is unknown. The write API should not accept such tokens. + must(t, sys.store.saveIAMConfig(ctx, &UserIdentity{Version: 1, Credentials: cred, UpdatedAt: UTCNow()}, identityPath)) + must(t, sys.store.saveIAMConfig(ctx, &MappedPolicy{Version: 1, Policies: "readwrite"}, mappingPath)) + loaded := make(map[string]UserIdentity) + _ = sys.store.loadUser(ctx, cred.AccessKey, stsUser, loaded) + if _, ok := loaded[cred.AccessKey]; ok { + t.Fatalf("invalid STS accepted, expired=%t", expired) + } + for _, path := range []string{identityPath, mappingPath} { + var record map[string]any + err := sys.store.loadIAMConfig(ctx, &record, path) + if expired { + if !errors.Is(err, errConfigNotFound) { + t.Fatalf("expired STS data not cleaned up at %s: %v", path, err) + } + } else { + must(t, err) + } + } + } + }) +} diff --git a/cmd/iam-service-recreation_test.go b/cmd/iam-service-recreation_test.go new file mode 100644 index 000000000..02c977858 --- /dev/null +++ b/cmd/iam-service-recreation_test.go @@ -0,0 +1,239 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-only + +package cmd + +import ( + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/minio/madmin-go/v3" + "github.com/minio/minio/internal/auth" +) + +// The receiver missed a deletion and still has the previous service key. +// A later full snapshot must replace it, including when the owner changed. +func TestIAMServiceAccountRecreation(t *testing.T) { + for _, backend := range []string{"object", "etcd"} { + t.Run(backend, func(t *testing.T) { + ctx, sys, _ := prepareIAMRevisionFixture(t, backend) + origin := UTCNow().Add(-time.Hour) + for _, parent := range []string{"old-owner", "new-owner"} { + _, err := sys.CreateUser(withIAMReplicationTime(ctx, origin), parent, madmin.AddOrUpdateUserReq{SecretKey: "valid-owner-password", Status: madmin.AccountEnabled}) + mustIAM(t, err) + } + const key = "reusable-service" + old := &madmin.SRSvcAccChange{Create: &madmin.SRSvcAccCreate{Parent: "old-owner", AccessKey: key, SecretKey: "old-service-password"}} + mustIAM(t, globalSiteReplicationSys.PeerSvcAccChangeHandler(ctx, old, origin)) + oldIdentity, _ := sys.store.GetUser(key) + _, err := sys.PolicyDBSet(withIAMReplicationTime(ctx, origin), key, "readwrite", svcUser, false) + mustIAM(t, err) + boundary, newer := origin.Add(time.Minute), origin.Add(2*time.Minute) + fresh := iamReplicationItem{SRIAMItem: madmin.SRIAMItem{Type: madmin.SRIAMItemSvcAcc, UpdatedAt: newer, SvcAccChange: &madmin.SRSvcAccChange{Create: &madmin.SRSvcAccCreate{Parent: "new-owner", AccessKey: key, SecretKey: "new-service-password", Status: auth.AccountOff}}}, RevokedBefore: boundary} + mustIAM(t, applyIAMReplicationItem(ctx, fresh)) + // A sibling may have missed the notification of the committed + // replacement. An equal-version retry must refresh that cache too. + staleCache := sys.store.lock() + staleCache.iamUsersMap[key] = oldIdentity + sys.store.unlock() + mustIAM(t, applyIAMReplicationItem(ctx, fresh)) // duplicate delivery is acknowledged + if current, _ := sys.store.GetUser(key); current.Credentials.SecretKey != fresh.SvcAccChange.Create.SecretKey { + t.Fatal("duplicate snapshot acknowledged without refreshing the stale cache") + } + mustIAM(t, globalSiteReplicationSys.PeerSvcAccChangeHandler(ctx, old, origin)) + mustIAM(t, globalSiteReplicationSys.PeerSvcAccChangeHandler(ctx, &madmin.SRSvcAccChange{Delete: &madmin.SRSvcAccDelete{AccessKey: key}}, boundary)) + mustIAM(t, sys.store.LoadIAMCache(ctx, false)) + u, ok := sys.store.GetUser(key) + if !ok || u.Credentials.SecretKey != fresh.SvcAccChange.Create.SecretKey || u.Credentials.ParentUser != "new-owner" || u.Credentials.Status != auth.AccountOff || !u.UpdatedAt.Equal(newer) || !u.RevokedBefore.Equal(boundary) { + t.Fatal("recreation did not retain the new identity, disabled status, source version and revocation") + } + if _, ok := sys.GetUser(ctx, key); ok { + t.Fatal("replicated disabled service can authenticate") + } + cache := sys.store.rlock() + _, mapped := cache.cachedMappedPolicy(key, svcUser, false) + sys.store.runlock() + if mapped { + t.Fatal("recreated service inherited an older mapping") + } + _, err = sys.PolicyDBSet(withIAMReplicationTime(ctx, origin), key, "readwrite", svcUser, false) + if !errors.Is(err, errIAMStaleUpdate) { + t.Fatalf("old service mapping replay was accepted: %v", err) + } + _, _, err = sys.NewServiceAccount(ctx, "new-owner", nil, newServiceAccountOpts{accessKey: key, secretKey: "local-service-password"}) + if !errors.Is(err, errIAMServiceAccountNotAllowed) { + t.Fatalf("local duplicate creation must remain rejected: %v", err) + } + // Outbound snapshots must carry the retained service boundary too. + out, err := globalSiteReplicationSys.replicationItem(ctx, fresh.SRIAMItem) + mustIAM(t, err) + if !out.RevokedBefore.Equal(boundary) { + t.Fatal("outbound service snapshot lost its revocation") + } + }) + } +} + +func TestIAMServiceAccountReplicationRejectsOtherCredentialKinds(t *testing.T) { + ctx, sys, _ := prepareIAMRevisionFixture(t) + _, err := sys.CreateUser(ctx, "builtin-collision", madmin.AddOrUpdateUserReq{SecretKey: "valid-user-password", Status: madmin.AccountEnabled}) + mustIAM(t, err) + secret, err := getTokenSigningKey() + mustIAM(t, err) + token, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: "builtin-collision"}, secret) + mustIAM(t, err) + token.ParentUser = "builtin-collision" + _, err = sys.SetTempUser(ctx, token.AccessKey, token, "") + mustIAM(t, err) + for _, key := range []string{"builtin-collision", token.AccessKey} { + _, _, err := sys.NewServiceAccount(withIAMReplicationTime(ctx, UTCNow().Add(time.Minute)), "another-owner", nil, newServiceAccountOpts{accessKey: key, secretKey: "valid-service-password"}) + if !errors.Is(err, errIAMServiceAccountNotAllowed) { + t.Fatalf("service replication replaced another credential kind: %v", err) + } + } +} + +// SR configuration can be temporarily unreadable even though a service token +// is signed with its own valid secret. Do not acknowledge a failed cache load. +func TestIAMServiceAccountRetryReportsClaimLoadFailure(t *testing.T) { + ctx, sys, _ := prepareIAMRevisionFixture(t) + globalSiteReplicatorCred.RLock() + previousSigningKey := globalSiteReplicatorCred.secretKey + globalSiteReplicatorCred.RUnlock() + globalSiteReplicatorCred.Set("") + t.Cleanup(func() { globalSiteReplicatorCred.Set(previousSigningKey) }) + _, err := sys.CreateUser(ctx, "retry-owner", madmin.AddOrUpdateUserReq{SecretKey: "valid-owner-password", Status: madmin.AccountEnabled}) + mustIAM(t, err) + opts := newServiceAccountOpts{accessKey: "retry-service", secretKey: "valid-service-password"} + _, _, err = sys.NewServiceAccount(ctx, "retry-owner", nil, opts) + mustIAM(t, err) + old, _ := sys.store.GetUser(opts.accessKey) + opts.secretKey = "replacement-service-password" + at, err := sys.UpdateServiceAccount(ctx, opts.accessKey, updateServiceAccountOpts{secretKey: opts.secretKey}) + mustIAM(t, err) + cache := sys.store.lock() + cache.iamUsersMap[opts.accessKey] = old // Missed sibling notification. + sys.store.unlock() + globalSiteReplicationSys.Lock() + globalSiteReplicationSys.enabled = true // No site-replicator credential is installed. + globalSiteReplicationSys.Unlock() + _, _, err = sys.NewServiceAccount(withIAMReplicationTime(ctx, at), "retry-owner", nil, opts) + if err == nil { + t.Fatal("acknowledged service retry despite failed claims loading") + } + if _, ok := sys.store.GetUser(opts.accessKey); ok { + t.Fatal("failed cache refresh retained the superseded service secret") + } + globalSiteReplicationSys.Lock() + globalSiteReplicationSys.enabled = false + globalSiteReplicationSys.Unlock() + _, _, err = sys.NewServiceAccount(withIAMReplicationTime(ctx, at), "retry-owner", nil, opts) + mustIAM(t, err) +} + +// A delayed snapshot still has its original absolute expiration. Reapplying +// the local minimum issuance lifetime would leave the old unexpired key alive. +func TestIAMServiceAccountReplicationPreservesExpiration(t *testing.T) { + for _, backend := range []string{"object", "etcd"} { + for _, action := range []string{"create", "update"} { + for _, expired := range []bool{false, true} { + name := backend + "/" + action + "/near_expiry" + if expired { + name = backend + "/" + action + "/expired" + } + t.Run(name, func(t *testing.T) { + ctx, sys, _ := prepareIAMRevisionFixture(t, backend) + origin := UTCNow().Add(-time.Hour) + _, err := sys.CreateUser(ctx, "expiry-owner", madmin.AddOrUpdateUserReq{SecretKey: "valid-owner-password", Status: madmin.AccountEnabled}) + mustIAM(t, err) + old := &madmin.SRSvcAccChange{Create: &madmin.SRSvcAccCreate{Parent: "expiry-owner", AccessKey: "expiry-service", SecretKey: "old-service-password"}} + mustIAM(t, globalSiteReplicationSys.PeerSvcAccChangeHandler(ctx, old, origin)) + expires := UTCNow().Add(time.Minute) + if expired { + expires = UTCNow().Add(-time.Minute) + } + change := &madmin.SRSvcAccChange{Create: &madmin.SRSvcAccCreate{Parent: "expiry-owner", AccessKey: "expiry-service", SecretKey: "new-service-password", Expiration: &expires}} + if action == "update" { + change = &madmin.SRSvcAccChange{Update: &madmin.SRSvcAccUpdate{AccessKey: "expiry-service", SecretKey: "new-service-password", Expiration: &expires}} + } + mustIAM(t, globalSiteReplicationSys.PeerSvcAccChangeHandler(ctx, change, origin.Add(2*time.Minute))) + u, ok := sys.store.GetUser("expiry-service") + if !ok || u.Credentials.SecretKey != "new-service-password" || !u.Credentials.Expiration.Equal(expires) { + t.Fatal("delayed snapshot lost its new secret or absolute expiration") + } + _, allowed := sys.GetUser(ctx, "expiry-service") + if allowed == expired { + t.Fatal("credential validity disagrees with its absolute expiration") + } + mustIAM(t, sys.store.LoadIAMCache(ctx, false)) + mustIAM(t, globalSiteReplicationSys.PeerSvcAccChangeHandler(ctx, old, origin)) + r, err := loadIAMRevision(ctx, sys.store, getUserIdentityPath("expiry-service", svcUser)) + mustIAM(t, err) + if r.Credentials.SecretKey == old.Create.SecretKey || (expired && !r.Deleted) { + t.Fatal("old non-expiring credential returned after reload") + } + _, _, err = sys.NewServiceAccount(ctx, "expiry-owner", nil, newServiceAccountOpts{accessKey: "local-expiry", secretKey: "valid-service-password", expiration: &expires}) + if !errors.Is(err, errInvalidSvcAcctExpiration) { + t.Fatalf("local issuance lifetime check changed: %v", err) + } + }) + } + } + } +} + +// Status-only summaries intentionally omit secrets. Different revisions must +// still trigger live healing, and disabled identities must be eligible sources. +func TestIAMServiceAccountHealingNewerSnapshot(t *testing.T) { + ctx, sys, obj := prepareIAMRevisionFixture(t) + origin := UTCNow().Add(-time.Hour) + _, err := sys.CreateUser(ctx, "heal-svc-owner", madmin.AddOrUpdateUserReq{SecretKey: "valid-owner-password", Status: madmin.AccountEnabled}) + mustIAM(t, err) + _, _, err = sys.NewServiceAccount(withIAMReplicationTime(ctx, origin), "heal-svc-owner", nil, newServiceAccountOpts{accessKey: "heal-service", secretKey: "valid-service-password"}) + mustIAM(t, err) + at, err := sys.UpdateServiceAccount(ctx, "heal-service", updateServiceAccountOpts{status: auth.AccountOff}) + mustIAM(t, err) + var sent atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/minio/health/live" { + return + } + if r.URL.Path != "/minio/admin/v3/site-replication/peer/iam-revisions" || r.Method != http.MethodPut { + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + w.WriteHeader(http.StatusNotFound) + return + } + var batch iamRevisionBatch + if err := json.NewDecoder(r.Body).Decode(&batch); err != nil { + t.Error(err) + w.WriteHeader(http.StatusBadRequest) + return + } + for _, item := range batch.Items { + if item.SvcAccChange != nil && item.SvcAccChange.Create != nil && item.SvcAccChange.Create.AccessKey == "heal-service" && item.SvcAccChange.Create.Status == auth.AccountOff && item.UpdatedAt.Equal(at) { + sent.Add(1) + } + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(iamRevisionResponse{iamRevisionStatus: iamRevisionStatus{Version: iamRevisionProtocol, Node: "node", Instance: "boot", Digest: "fixture"}}) + })) + defer server.Close() + peers := map[string]madmin.PeerInfo{globalDeploymentID(): {Name: "local", DeploymentID: globalDeploymentID()}, "remote": {Name: "remote", DeploymentID: "remote", Endpoint: server.URL}} + c := &SiteReplicationSys{enabled: true, state: srState{ServiceAccountAccessKey: "heal-svc-owner", Peers: peers}} + local := madmin.UserInfo{Status: madmin.AccountStatus(auth.AccountOff), UpdatedAt: at} + remote := local + remote.UpdatedAt = origin + if isUserInfoReplicated(2, 2, []madmin.UserInfo{local, remote}) { + t.Fatal("status-only summaries concealed different service revisions") + } + info := srStatusInfo{Sites: peers, UserStats: map[string]map[string]srUserStatsSummary{"heal-service": {globalDeploymentID(): {userInfo: srUserInfo{UserInfo: local}}, "remote": {SRUserStatsSummary: madmin.SRUserStatsSummary{UserInfoMismatch: true}, userInfo: srUserInfo{UserInfo: remote}}}}} + mustIAM(t, c.healUsers(ctx, obj, "heal-service", info)) + if sent.Load() == 0 { + t.Fatal("disabled newer service snapshot was not healed") + } +} diff --git a/cmd/iam-store.go b/cmd/iam-store.go index 2d017a808..fca9cbb1e 100644 --- a/cmd/iam-store.go +++ b/cmd/iam-store.go @@ -158,9 +158,12 @@ func getMappedPolicyPath(name string, userType IAMUserType, isGroup bool) string // UserIdentity represents a user's secret key and their status type UserIdentity struct { - Version int `json:"version"` - Credentials auth.Credentials `json:"credentials"` - UpdatedAt time.Time `json:"updatedAt"` + Deleted bool `json:"deleted,omitempty"` + RevokedBefore time.Time `json:"revokedBefore,omitempty"` + ExpiresAt time.Time `json:"expiresAt,omitempty"` + Version int `json:"version"` + Credentials auth.Credentials `json:"credentials"` + UpdatedAt time.Time `json:"updatedAt"` } func newUserIdentity(cred auth.Credentials) UserIdentity { @@ -169,10 +172,13 @@ func newUserIdentity(cred auth.Credentials) UserIdentity { // GroupInfo contains info about a group type GroupInfo struct { - Version int `json:"version"` - Status string `json:"status"` - Members []string `json:"members"` - UpdatedAt time.Time `json:"updatedAt"` + RevokedBefore time.Time `json:"revokedBefore,omitempty"` + MemberGrants map[string]time.Time `json:"memberGrants,omitempty"` + Deleted bool `json:"deleted,omitempty"` + Version int `json:"version"` + Status string `json:"status"` + Members []string `json:"members"` + UpdatedAt time.Time `json:"updatedAt"` } func newGroupInfo(members []string) GroupInfo { @@ -181,6 +187,7 @@ func newGroupInfo(members []string) GroupInfo { // MappedPolicy represents a policy name mapped to a user or group type MappedPolicy struct { + Deleted bool `json:"deleted,omitempty"` Version int `json:"version"` Policies string `json:"policy"` UpdatedAt time.Time `json:"updatedAt"` @@ -218,7 +225,8 @@ func newMappedPolicy(policy string) MappedPolicy { // PolicyDoc represents an IAM policy with some metadata. type PolicyDoc struct { - Version int `json:",omitempty"` + Deleted bool `json:"deleted,omitempty"` + Version int `json:",omitempty"` Policy policy.Policy CreateDate time.Time UpdateDate time.Time @@ -287,6 +295,7 @@ type iamWatchEvent struct { // iamCache contains in-memory cache of IAM data. type iamCache struct { updatedAt time.Time + revisions *iamRevisionIndex // map of policy names to policy definitions iamPolicyDocsMap map[string]PolicyDoc @@ -338,6 +347,9 @@ func (c *iamCache) updateGroupMembershipsMap(group string, gi *GroupInfo) { return } for _, member := range gi.Members { + if !c.groupMemberAllowed(member, gi.MemberGrants[member], gi.RevokedBefore) { + continue + } v := c.iamUserGroupMemberships[member] if v == nil { v = set.CreateStringSet(group) @@ -376,7 +388,7 @@ func (c *iamCache) policyDBGetGroups(store *IAMStoreSys, userPolicyPresent bool, } } - policy, ok := c.iamGroupPolicyMap.Load(group) + policy, ok := c.cachedMappedPolicy(group, regUser, true) if !ok { continue } @@ -409,7 +421,7 @@ func (c *iamCache) policyDBGetGroups(store *IAMStoreSys, userPolicyPresent bool, if errors.Is(err, errNoSuchPolicy) { return nil } - policy, _ := c.iamGroupPolicyMap.Load(groups[index]) + policy, _ := c.cachedMappedPolicy(groups[index], regUser, true) mu.Lock() policies = append(policies, policy.toSlice()...) mu.Unlock() @@ -453,7 +465,7 @@ func (c *iamCache) policyDBGet(store *IAMStoreSys, name string, isGroup bool, po } } - policy, ok := c.iamGroupPolicyMap.Load(name) + policy, ok := c.cachedMappedPolicy(name, regUser, true) if ok { return policy.toSlice(), policy.UpdatedAt, nil } @@ -461,7 +473,7 @@ func (c *iamCache) policyDBGet(store *IAMStoreSys, name string, isGroup bool, po if err := store.loadMappedPolicy(context.TODO(), name, regUser, true, c.iamGroupPolicyMap); err != nil && !errors.Is(err, errNoSuchPolicy) { return nil, time.Time{}, err } - policy, _ = c.iamGroupPolicyMap.Load(name) + policy, _ = c.cachedMappedPolicy(name, regUser, true) return policy.toSlice(), policy.UpdatedAt, nil } return nil, time.Time{}, nil @@ -474,13 +486,16 @@ func (c *iamCache) policyDBGet(store *IAMStoreSys, name string, isGroup bool, po if store.getUsersSysType() == LDAPUsersSysType { // For LDAP policy mapping is part of STS users, we only need to lookup // those mappings. - mp, ok := c.iamSTSPolicyMap.Load(name) + mp, ok := c.cachedMappedPolicy(name, stsUser, false) if !ok { // Attempt to load parent user mapping for STS accounts if err := store.loadMappedPolicy(context.TODO(), name, stsUser, false, c.iamSTSPolicyMap); err != nil && !errors.Is(err, errNoSuchPolicy) { return nil, time.Time{}, err } - mp, _ = c.iamSTSPolicyMap.Load(name) + mp, _ = c.cachedMappedPolicy(name, stsUser, false) + } + if !c.revisions.mappingAllowed(getMappedPolicyPath(name, stsUser, false), mp) { + mp = MappedPolicy{} } policies = set.CreateStringSet(mp.toSlice()...) updatedAt = mp.UpdatedAt @@ -497,22 +512,22 @@ func (c *iamCache) policyDBGet(store *IAMStoreSys, name string, isGroup bool, po // For internal IDP regular/service account user accounts, the policy // mapping is iamUserPolicyMap. For STS accounts, the parent user would be // passed here and we lookup the mapping in iamSTSPolicyMap. - mp, ok := c.iamUserPolicyMap.Load(name) + mp, ok := c.cachedMappedPolicy(name, regUser, false) if !ok { if err := store.loadMappedPolicy(context.TODO(), name, regUser, false, c.iamUserPolicyMap); err != nil && !errors.Is(err, errNoSuchPolicy) { return nil, time.Time{}, err } - mp, ok = c.iamUserPolicyMap.Load(name) + mp, ok = c.cachedMappedPolicy(name, regUser, false) if !ok { // Since user "name" could be a parent user of an STS account, we look up // mappings for those too. - mp, ok = c.iamSTSPolicyMap.Load(name) + mp, ok = c.cachedMappedPolicy(name, stsUser, false) if !ok { // Attempt to load parent user mapping for STS accounts if err := store.loadMappedPolicy(context.TODO(), name, stsUser, false, c.iamSTSPolicyMap); err != nil && !errors.Is(err, errNoSuchPolicy) { return nil, time.Time{}, err } - mp, _ = c.iamSTSPolicyMap.Load(name) + mp, _ = c.cachedMappedPolicy(name, stsUser, false) } } } @@ -528,12 +543,12 @@ func (c *iamCache) policyDBGet(store *IAMStoreSys, name string, isGroup bool, po } } - policy, ok := c.iamGroupPolicyMap.Load(group) + policy, ok := c.cachedMappedPolicy(group, regUser, true) if !ok { if err := store.loadMappedPolicy(context.TODO(), group, regUser, true, c.iamGroupPolicyMap); err != nil && !errors.Is(err, errNoSuchPolicy) { return nil, time.Time{}, err } - policy, _ = c.iamGroupPolicyMap.Load(group) + policy, _ = c.cachedMappedPolicy(group, regUser, true) } for _, p := range policy.toSlice() { @@ -543,7 +558,7 @@ func (c *iamCache) policyDBGet(store *IAMStoreSys, name string, isGroup bool, po updatedAt = mp.UpdatedAt } - for _, group := range c.iamUserGroupMemberships[name].ToSlice() { + for _, group := range c.effectiveUserGroups(name) { if store.getUsersSysType() == MinIOUsersSysType { g, ok := c.iamGroupsMap[group] if ok { @@ -555,12 +570,12 @@ func (c *iamCache) policyDBGet(store *IAMStoreSys, name string, isGroup bool, po } } - policy, ok := c.iamGroupPolicyMap.Load(group) + policy, ok := c.cachedMappedPolicy(group, regUser, true) if !ok { if err := store.loadMappedPolicy(context.TODO(), group, regUser, true, c.iamGroupPolicyMap); err != nil && !errors.Is(err, errNoSuchPolicy) { return nil, time.Time{}, err } - policy, _ = c.iamGroupPolicyMap.Load(group) + policy, _ = c.cachedMappedPolicy(group, regUser, true) } for _, p := range policy.toSlice() { @@ -613,14 +628,8 @@ type IAMStorageAPI interface { saveIAMConfig(ctx context.Context, item any, path string, opts ...options) error loadIAMConfig(ctx context.Context, item any, path string) error deleteIAMConfig(ctx context.Context, path string) error - savePolicyDoc(ctx context.Context, policyName string, p PolicyDoc) error - saveMappedPolicy(ctx context.Context, name string, userType IAMUserType, isGroup bool, mp MappedPolicy, opts ...options) error - saveUserIdentity(ctx context.Context, name string, userType IAMUserType, u UserIdentity, opts ...options) error - saveGroupInfo(ctx context.Context, group string, gi GroupInfo) error - deletePolicyDoc(ctx context.Context, policyName string) error - deleteMappedPolicy(ctx context.Context, name string, userType IAMUserType, isGroup bool) error - deleteUserIdentity(ctx context.Context, name string, userType IAMUserType) error - deleteGroupInfo(ctx context.Context, name string) error + listIAMConfigPaths(ctx context.Context) ([]string, error) + revisionIndex() *iamRevisionIndex } // iamStorageWatcher is implemented by `IAMStorageAPI` implementers that @@ -641,6 +650,7 @@ func setDefaultCannedPolicies(policies map[string]PolicyDoc) { // LoadIAMCache reads all IAM items and populates a new iamCache object and // replaces the in-memory cache object. func (store *IAMStoreSys) LoadIAMCache(ctx context.Context, firstTime bool) error { + ctx = withIAMExpirationCleanup(ctx) bootstrapTraceMsgFirstTime := func(s string) { if firstTime { bootstrapTraceMsg(s) @@ -649,6 +659,7 @@ func (store *IAMStoreSys) LoadIAMCache(ctx context.Context, firstTime bool) erro bootstrapTraceMsgFirstTime("loading IAM data") newCache := newIamCache() + newCache.revisions = store.revisionIndex() loadedAt := time.Now() @@ -683,6 +694,11 @@ func (store *IAMStoreSys) LoadIAMCache(ctx context.Context, firstTime bool) erro if err := store.loadMappedPolicies(ctx, regUser, false, newCache.iamUserPolicyMap); err != nil { return err } + // External-IDP mappings are durable authorization records even when + // no local service account currently refers to their parent. + if err := store.loadMappedPolicies(ctx, stsUser, false, newCache.iamSTSPolicyMap); err != nil { + return err + } bootstrapTraceMsgFirstTime("loading group policy mapping") // load policies mapped to groups @@ -699,6 +715,12 @@ func (store *IAMStoreSys) LoadIAMCache(ctx context.Context, firstTime bool) erro newCache.buildUserGroupMemberships() } + for name := range newCache.iamPolicyDocsMap { + if store.revisionIndex().get(getPolicyDocPath(name)).Deleted { + delete(newCache.iamPolicyDocsMap, name) + } + } + cache := store.lock() defer store.unlock() @@ -740,10 +762,24 @@ func (store *IAMStoreSys) LoadIAMCache(ctx context.Context, firstTime bool) erro type IAMStoreSys struct { IAMStorageAPI + // Serialize cache writers independently of readers, so persistence can + // release the cache lock without another writer invalidating its state. + writeMu sync.Mutex + group *singleflight.Group policy *singleflight.Group } +func (store *IAMStoreSys) lock() *iamCache { + store.writeMu.Lock() + return store.IAMStorageAPI.lock() +} + +func (store *IAMStoreSys) unlock() { + store.IAMStorageAPI.unlock() + store.writeMu.Unlock() +} + // HasWatcher - returns if the storage system has a watcher. func (store *IAMStoreSys) HasWatcher() bool { _, ok := store.IAMStorageAPI.(iamStorageWatcher) @@ -760,6 +796,12 @@ func (store *IAMStoreSys) GetUser(user string) (UserIdentity, bool) { // Check the sts map u, ok = cache.iamSTSAccountsMap[user] } + if ok && u.Credentials.ParentUser != "" { + r := cache.userRevocation(u.Credentials.ParentUser) + if r.Deleted || !iamCredentialSurvivesRevocation(u.Credentials, r.RevokedBefore) { + return UserIdentity{}, false + } + } return u, ok } @@ -769,10 +811,10 @@ func (store *IAMStoreSys) GetMappedPolicy(name string, isGroup bool) (MappedPoli defer store.runlock() if isGroup { - v, ok := cache.iamGroupPolicyMap.Load(name) + v, ok := cache.cachedMappedPolicy(name, regUser, true) return v, ok } - return cache.iamUserPolicyMap.Load(name) + return cache.cachedMappedPolicy(name, regUser, false) } // GroupNotificationHandler - updates in-memory cache on notification of @@ -863,44 +905,32 @@ func (store *IAMStoreSys) AddUsersToGroup(ctx context.Context, group string, mem cache := store.lock() defer store.unlock() - // Validate that all members exist. - for _, member := range members { - u, ok := cache.iamUsersMap[member] - if !ok { - return updatedAt, errNoSuchUser - } - cr := u.Credentials - if cr.IsTemp() || cr.IsServiceAccount() { - return updatedAt, errIAMActionNotAllowed - } + if err := store.checkIAMRevision(ctx, getGroupInfoPath(group), false); err != nil { + return updatedAt, err } gi, ok := cache.iamGroupsMap[group] if !ok { // Set group as enabled by default when it doesn't // exist. - gi = newGroupInfo(members) - } else { - gi.Members = set.CreateStringSet(append(gi.Members, members...)...).ToSlice() - gi.UpdatedAt = UTCNow() + gi = newGroupInfo(nil) + } + if r := store.revisionIndex().get(getGroupInfoPath(group)); r.RevokedBefore.After(gi.RevokedBefore) { + gi.RevokedBefore = r.RevokedBefore + } + gi, err = cache.addGroupMembers(ctx, gi, members) + if err != nil { + return updatedAt, err } - if err := store.saveGroupInfo(ctx, group, gi); err != nil { + if err := store.saveGroupInfo(context.WithValue(ctx, iamGroupMutationKey{}, iamGroupMutation{Members: members}), group, &gi); err != nil { return updatedAt, err } cache.iamGroupsMap[group] = gi - // update user-group membership map - for _, member := range members { - gset := cache.iamUserGroupMemberships[member] - if gset == nil { - gset = set.CreateStringSet(group) - } else { - gset.Add(group) - } - cache.iamUserGroupMemberships[member] = gset - } + cache.removeGroupFromMembershipsMap(group) + cache.updateGroupMembershipsMap(group, &gi) cache.updatedAt = time.Now() return gi.UpdatedAt, nil @@ -918,13 +948,13 @@ func removeMembersFromGroup(ctx context.Context, store *IAMStoreSys, cache *iamC d := set.CreateStringSet(members...) gi.Members = s.Difference(d).ToSlice() + gi.UpdatedAt = UTCNow() if !updateCacheOnly { - err := store.saveGroupInfo(ctx, group, gi) + err := store.saveGroupInfo(context.WithValue(ctx, iamGroupMutationKey{}, iamGroupMutation{Members: members, Remove: true}), group, &gi) if err != nil { return updatedAt, err } } - gi.UpdatedAt = UTCNow() cache.iamGroupsMap[group] = gi // update user-group membership map @@ -950,6 +980,13 @@ func (store *IAMStoreSys) RemoveUsersFromGroup(ctx context.Context, group string cache := store.lock() defer store.unlock() + _, replicated := iamReplicationTime(ctx) + if len(members) != 0 { + if err := store.checkIAMRevision(ctx, getGroupInfoPath(group), false); err != nil { + return updatedAt, err + } + } + // Validate that all members exist. for _, member := range members { u, ok := cache.iamUsersMap[member] @@ -963,32 +1000,41 @@ func (store *IAMStoreSys) RemoveUsersFromGroup(ctx context.Context, group string } gi, ok := cache.iamGroupsMap[group] - if !ok { + if !ok && (!replicated || len(members) != 0) { return updatedAt, errNoSuchGroup } // Check if attempting to delete a non-empty group. - if len(members) == 0 && len(gi.Members) != 0 { + if !replicated && len(members) == 0 && len(cache.effectiveGroupMembers(gi)) != 0 { return updatedAt, errGroupNotEmpty } if len(members) == 0 { - // len(gi.Members) == 0 here. - - // Remove the group from storage. First delete the - // mapped policy. No-mapped-policy case is ignored. - if err := store.deleteMappedPolicy(ctx, group, regUser, true); err != nil && !errors.Is(err, errNoSuchPolicy) { + // Commit the group revision before attempting dependent cleanup. A + // newer mapping cannot veto a deletion or conceal an older boundary. + op := &iamGroupRevocation{requireEmpty: !replicated} + err := store.saveIAMRevision(ctx, getGroupInfoPath(group), op) + retained := errors.Is(err, errIAMRevocationRetained) + if err != nil && !retained { return updatedAt, err } - if err := store.deleteGroupInfo(ctx, group); err != nil && err != errNoSuchGroup { - return updatedAt, err + updatedAt = op.RevokedBefore + cache.removeGroupFromMembershipsMap(group) + if retained { + cache.iamGroupsMap[group] = op.GroupInfo + cache.updateGroupMembershipsMap(group, &op.GroupInfo) + } else { + delete(cache.iamGroupsMap, group) } - - // Delete from server memory - delete(cache.iamGroupsMap, group) - cache.iamGroupPolicyMap.Delete(group) cache.updatedAt = time.Now() - return cache.updatedAt, nil + cleanupErr := store.deleteMappedPolicy(withIAMReplicationTime(ctx, updatedAt), group, regUser, true) + if mp, ok := cache.iamGroupPolicyMap.Load(group); ok && !mp.UpdatedAt.After(updatedAt) { + cache.iamGroupPolicyMap.Delete(group) + } + if cleanupErr != nil && !errors.Is(cleanupErr, errIAMStaleUpdate) { + return updatedAt, &iamCommittedCleanupError{err: cleanupErr, retained: retained} + } + return updatedAt, nil } return removeMembersFromGroup(ctx, store, cache, group, members, false) @@ -1014,7 +1060,7 @@ func (store *IAMStoreSys) SetGroupStatus(ctx context.Context, group string, enab gi.Status = statusDisabled } gi.UpdatedAt = UTCNow() - if err := store.saveGroupInfo(ctx, group, gi); err != nil { + if err := store.saveGroupInfo(context.WithValue(ctx, iamGroupMutationKey{}, iamGroupMutation{StatusOnly: true}), group, &gi); err != nil { return gi.UpdatedAt, err } @@ -1052,7 +1098,7 @@ func (store *IAMStoreSys) GetGroupDescription(group string) (gd madmin.GroupDesc return madmin.GroupDesc{ Name: group, Status: gi.Status, - Members: gi.Members, + Members: cache.effectiveGroupMembers(gi), Policy: policy, UpdatedAt: gi.UpdatedAt, }, nil @@ -1073,6 +1119,10 @@ func (store *IAMStoreSys) updateGroups(ctx context.Context, cache *iamCache) (re if err = iamOS.loadGroup(ctx, group, cache.iamGroupsMap); err != nil && !errors.Is(err, errNoSuchGroup) { return nil, fmt.Errorf("unable to load the group: %w", err) } + if errors.Is(err, errNoSuchGroup) { + delete(cache.iamGroupsMap, group) + continue + } groupSet.Add(group) } } @@ -1083,6 +1133,10 @@ func (store *IAMStoreSys) updateGroups(ctx context.Context, cache *iamCache) (re if err = iamOS.loadMappedPolicy(ctx, group, regUser, true, cache.iamGroupPolicyMap); err != nil && !errors.Is(err, errNoSuchPolicy) { return nil, fmt.Errorf("unable to load the policy mapping for the group: %w", err) } + if errors.Is(err, errNoSuchPolicy) { + cache.iamGroupPolicyMap.Delete(group) + continue + } groupSet.Add(group) } @@ -1209,7 +1263,8 @@ func (store *IAMStoreSys) PolicyDBUpdate(ctx context.Context, name string, isGro // In case of detach operation, it is possible that no policies are mapped - // in this case, we delete the mapping from the store. if len(newPolicies) == 0 { - if err = store.deleteMappedPolicy(ctx, name, userType, isGroup); err != nil && !errors.Is(err, errNoSuchPolicy) { + newPolicyMapping.Deleted = true + if err = store.saveMappedPolicy(ctx, name, userType, isGroup, &newPolicyMapping); err != nil && !errors.Is(err, errNoSuchPolicy) { return updatedAt, addedOrRemoved, effectivePolicies, err } if !isGroup { @@ -1222,7 +1277,7 @@ func (store *IAMStoreSys) PolicyDBUpdate(ctx context.Context, name string, isGro cache.iamGroupPolicyMap.Delete(name) } } else { - if err = store.saveMappedPolicy(ctx, name, userType, isGroup, newPolicyMapping); err != nil { + if err = store.saveMappedPolicy(ctx, name, userType, isGroup, &newPolicyMapping); err != nil { return updatedAt, addedOrRemoved, effectivePolicies, err } if !isGroup { @@ -1237,7 +1292,7 @@ func (store *IAMStoreSys) PolicyDBUpdate(ctx context.Context, name string, isGro } cache.updatedAt = UTCNow() - return cache.updatedAt, addedOrRemoved, newPolicies, nil + return newPolicyMapping.UpdatedAt, addedOrRemoved, newPolicies, nil } // PolicyDBSet - update the policy mapping for the given user or group in @@ -1253,6 +1308,10 @@ func (store *IAMStoreSys) PolicyDBSet(ctx context.Context, name, policy string, cache := store.lock() defer store.unlock() + if err := store.checkIAMRevision(ctx, getMappedPolicyPath(name, userType, isGroup), policy == ""); err != nil { + return updatedAt, err + } + // Handle policy mapping removal. if policy == "" { if store.getUsersSysType() == LDAPUsersSysType { @@ -1261,7 +1320,8 @@ func (store *IAMStoreSys) PolicyDBSet(ctx context.Context, name, policy string, // introduced in PR #11840 store.deleteMappedPolicy(ctx, name, regUser, false) } - err := store.deleteMappedPolicy(ctx, name, userType, isGroup) + mp := MappedPolicy{Version: 1, Deleted: true} + err := store.saveMappedPolicy(ctx, name, userType, isGroup, &mp) if err != nil && !errors.Is(err, errNoSuchPolicy) { return updatedAt, err } @@ -1275,7 +1335,7 @@ func (store *IAMStoreSys) PolicyDBSet(ctx context.Context, name, policy string, cache.iamGroupPolicyMap.Delete(name) } cache.updatedAt = time.Now() - return cache.updatedAt, nil + return mp.UpdatedAt, nil } // Handle policy mapping set/update @@ -1286,7 +1346,7 @@ func (store *IAMStoreSys) PolicyDBSet(ctx context.Context, name, policy string, } } - if err := store.saveMappedPolicy(ctx, name, userType, isGroup, mp); err != nil { + if err := store.saveMappedPolicy(ctx, name, userType, isGroup, &mp); err != nil { return updatedAt, err } if !isGroup { @@ -1369,35 +1429,45 @@ func (store *IAMStoreSys) DeletePolicy(ctx context.Context, policy string, isFro defer store.unlock() if !isFromNotification { - // Check if policy is mapped to any existing user or group. If so, we do not - // allow deletion of the policy. If the policy is mapped to an STS account, - // we do allow deletion. - users := []string{} - groups := []string{} - cache.iamUserPolicyMap.Range(func(u string, mp MappedPolicy) bool { - pset := mp.policySet() - if store.getUsersSysType() == MinIOUsersSysType { - if _, ok := cache.iamUsersMap[u]; !ok { - // This case can happen when a temporary account is - // deleted or expired - remove it from userPolicyMap. - cache.iamUserPolicyMap.Delete(u) - return true + if err := store.checkIAMRevision(ctx, getPolicyDocPath(policy), true); err != nil { + return err + } + if _, replicated := iamReplicationTime(ctx); !replicated { + if _, exists := cache.iamPolicyDocsMap[policy]; !exists { + // Preserve idempotent local deletion without creating a durable + // revocation for a misspelled/nonexistent policy name. + return nil + } + // Check if policy is mapped to any existing user or group. If so, we do not + // allow deletion of the policy. If the policy is mapped to an STS account, + // we do allow deletion. + users := []string{} + groups := []string{} + cache.iamUserPolicyMap.Range(func(u string, mp MappedPolicy) bool { + pset := mp.policySet() + if store.getUsersSysType() == MinIOUsersSysType { + if _, ok := cache.iamUsersMap[u]; !ok { + // This case can happen when a temporary account is + // deleted or expired - remove it from userPolicyMap. + cache.iamUserPolicyMap.Delete(u) + return true + } } + if pset.Contains(policy) { + users = append(users, u) + } + return true + }) + cache.iamGroupPolicyMap.Range(func(g string, mp MappedPolicy) bool { + pset := mp.policySet() + if pset.Contains(policy) { + groups = append(groups, g) + } + return true + }) + if len(users) != 0 || len(groups) != 0 { + return errPolicyInUse } - if pset.Contains(policy) { - users = append(users, u) - } - return true - }) - cache.iamGroupPolicyMap.Range(func(g string, mp MappedPolicy) bool { - pset := mp.policySet() - if pset.Contains(policy) { - groups = append(groups, g) - } - return true - }) - if len(users) != 0 || len(groups) != 0 { - return errPolicyInUse } err := store.deletePolicyDoc(ctx, policy) @@ -1481,7 +1551,7 @@ func (store *IAMStoreSys) SetPolicy(ctx context.Context, name string, policy pol d = newPolicyDoc(policy) } - if err := store.savePolicyDoc(ctx, name, d); err != nil { + if err := store.savePolicyDoc(ctx, name, &d); err != nil { return d.UpdateDate, err } @@ -1644,11 +1714,11 @@ func (store *IAMStoreSys) GetBucketUsers(bucket string) (map[string]madmin.UserI continue } var policies []string - mp, ok := cache.iamUserPolicyMap.Load(k) + mp, ok := cache.cachedMappedPolicy(k, regUser, false) if ok { policies = append(policies, mp.Policies) - for _, group := range cache.iamUserGroupMemberships[k].ToSlice() { - if nmp, ok := cache.iamGroupPolicyMap.Load(group); ok { + for _, group := range cache.effectiveUserGroups(k) { + if nmp, ok := cache.cachedMappedPolicy(group, regUser, true); ok { policies = append(policies, nmp.Policies) } } @@ -1663,7 +1733,7 @@ func (store *IAMStoreSys) GetBucketUsers(bucket string) (map[string]madmin.UserI } return madmin.AccountDisabled }(), - MemberOf: cache.iamUserGroupMemberships[k].ToSlice(), + MemberOf: cache.effectiveUserGroups(k), } } } @@ -1683,7 +1753,7 @@ func (store *IAMStoreSys) GetUsers() map[string]madmin.UserInfo { if v.IsTemp() || v.IsServiceAccount() { continue } - pl, _ := cache.iamUserPolicyMap.Load(k) + pl, _ := cache.cachedMappedPolicy(k, regUser, false) result[k] = madmin.UserInfo{ PolicyName: pl.Policies, Status: func() madmin.AccountStatus { @@ -1692,7 +1762,7 @@ func (store *IAMStoreSys) GetUsers() map[string]madmin.UserInfo { } return madmin.AccountDisabled }(), - MemberOf: cache.iamUserGroupMemberships[k].ToSlice(), + MemberOf: cache.effectiveUserGroups(k), UpdatedAt: pl.UpdatedAt, } } @@ -1742,14 +1812,14 @@ func (store *IAMStoreSys) GetUserInfo(name string) (u madmin.UserInfo, err error break } } - mappedPolicy, ok := cache.iamUserPolicyMap.Load(name) + mappedPolicy, ok := cache.cachedMappedPolicy(name, regUser, false) if !ok { - mappedPolicy, ok = cache.iamSTSPolicyMap.Load(name) + mappedPolicy, ok = cache.cachedMappedPolicy(name, stsUser, false) } if !ok { // Attempt to load parent user mapping for STS accounts store.loadMappedPolicy(context.TODO(), name, stsUser, false, cache.iamSTSPolicyMap) - mappedPolicy, ok = cache.iamSTSPolicyMap.Load(name) + mappedPolicy, ok = cache.cachedMappedPolicy(name, stsUser, false) if !ok { return u, errNoSuchUser } @@ -1770,7 +1840,7 @@ func (store *IAMStoreSys) GetUserInfo(name string) (u madmin.UserInfo, err error if cred.IsTemp() || cred.IsServiceAccount() { return u, errIAMActionNotAllowed } - pl, _ := cache.iamUserPolicyMap.Load(name) + pl, _ := cache.cachedMappedPolicy(name, regUser, false) return madmin.UserInfo{ PolicyName: pl.Policies, Status: func() madmin.AccountStatus { @@ -1779,8 +1849,8 @@ func (store *IAMStoreSys) GetUserInfo(name string) (u madmin.UserInfo, err error } return madmin.AccountDisabled }(), - MemberOf: cache.iamUserGroupMemberships[name].ToSlice(), - UpdatedAt: pl.UpdatedAt, + MemberOf: cache.effectiveUserGroups(name), + UpdatedAt: ui.UpdatedAt, }, nil } @@ -1974,65 +2044,89 @@ func (store *IAMStoreSys) DeleteUser(ctx context.Context, accessKey string, user if accessKey == "" { return errInvalidArgument } - cache := store.lock() defer store.unlock() - // first we remove the user from their groups. - if store.getUsersSysType() == MinIOUsersSysType && userType == regUser { - memberOf := cache.iamUserGroupMemberships[accessKey].ToSlice() - for _, group := range memberOf { - _, removeErr := removeMembersFromGroup(ctx, store, cache, group, []string{accessKey}, false) - if removeErr != nil { - return removeErr - } - } - } - - // Now we can remove the user from memory and IAM store - - // Delete any STS and service account derived from this credential - // first. + // The identity record is the commit point. A failure in later cleanup + // cannot undo its durable revocation or resurrect old dependent grants. + var identity UserIdentity + var err error + retained := false if userType == regUser { - for _, ui := range cache.iamUsersMap { - u := ui.Credentials - if u.ParentUser == accessKey { - switch { - case u.IsServiceAccount(): - _ = store.deleteUserIdentity(ctx, u.AccessKey, svcUser) - delete(cache.iamUsersMap, u.AccessKey) - case u.IsTemp(): - _ = store.deleteUserIdentity(ctx, u.AccessKey, stsUser) - delete(cache.iamSTSAccountsMap, u.AccessKey) - delete(cache.iamUsersMap, u.AccessKey) - } - if store.group != nil { - store.group.Forget(u.AccessKey) - } - } - } + op := &iamUserRevocation{} + err = store.saveIAMRevision(ctx, getUserIdentityPath(accessKey, userType), op) + identity, retained = op.UserIdentity, errors.Is(err, errIAMRevocationRetained) + } else { + identity = UserIdentity{Version: 1, Deleted: true} + err = store.saveUserIdentity(ctx, accessKey, userType, &identity) } - - // It is ok to ignore deletion error on the mapped policy - store.deleteMappedPolicy(ctx, accessKey, userType, false) - cache.iamUserPolicyMap.Delete(accessKey) - - err := store.deleteUserIdentity(ctx, accessKey, userType) - if err == errNoSuchUser { - // ignore if user is already deleted. - err = nil + if err != nil && !retained { + return err } - if userType == stsUser { + if retained { + cache.iamUsersMap[accessKey] = identity + } else { + delete(cache.iamUsersMap, accessKey) delete(cache.iamSTSAccountsMap, accessKey) } - delete(cache.iamUsersMap, accessKey) + cache.updatedAt = time.Now() if store.group != nil { store.group.Forget(accessKey) } + boundary := identity.RevokedBefore + if boundary.IsZero() { + boundary = identity.UpdatedAt + } + cleanupCtx := withIAMReplicationTime(ctx, boundary) - cache.updatedAt = time.Now() - - return err + // Membership is derived from the grant and parent revocation versions. + // Do not rewrite a whole group as a side effect of deleting one member. + for group, gi := range cache.iamGroupsMap { + cache.removeGroupFromMembershipsMap(group) + cache.updateGroupMembershipsMap(group, &gi) + } + var cleanupErrors []error + if userType == regUser { + for _, users := range []map[string]UserIdentity{cache.iamUsersMap, cache.iamSTSAccountsMap} { + for key, child := range users { + if child.Credentials.ParentUser != accessKey || iamCredentialSurvivesRevocation(child.Credentials, boundary) { + continue + } + delete(users, key) + if store.group != nil { + store.group.Forget(key) + } + // The durable parent boundary suffices for STS. Avoid one + // permanent tombstone per short-lived random access key. + if child.Credentials.IsTemp() { + continue + } + if child.Credentials.IsServiceAccount() { + childCtx, _ := iamChildDeletionContext(cleanupCtx, child) + if err := store.deleteUserIdentity(childCtx, key, svcUser); err != nil && !errors.Is(err, errIAMStaleUpdate) { + cleanupErrors = append(cleanupErrors, err) + } + } + } + } + } + // A mapping newer than the revocation is an explicit new grant. Keep it. + if userType != stsUser { + // STS policies belong to the parent, not each random token key. + if err := store.deleteMappedPolicy(cleanupCtx, accessKey, userType, false); err != nil && !errors.Is(err, errIAMStaleUpdate) { + cleanupErrors = append(cleanupErrors, err) + } + } + if mp, ok := cache.iamUserPolicyMap.Load(accessKey); ok && !mp.UpdatedAt.After(boundary) { + cache.iamUserPolicyMap.Delete(accessKey) + } + if err := errors.Join(cleanupErrors...); err != nil { + return &iamCommittedCleanupError{err: err, retained: retained} + } + if retained { + return errIAMRevocationRetained + } + return nil } // SetTempUser - saves temporary (STS) credential to storage and cache. If a @@ -2048,6 +2142,19 @@ func (store *IAMStoreSys) SetTempUser(ctx context.Context, accessKey string, cre cache := store.lock() defer store.unlock() + if err := store.checkIAMRevision(ctx, getUserIdentityPath(accessKey, stsUser), false); err != nil { + return time.Time{}, err + } + u := newUserIdentity(cred) + claims, err := extractJWTClaims(u) + if err != nil { + return time.Time{}, err + } + u.Credentials.Claims = claims.Map() + if err := store.checkIAMParentRevision(ctx, u.Credentials); err != nil { + return time.Time{}, err + } + if policyName != "" { mp := newMappedPolicy(policyName) _, combinedPolicyStmt := filterPolicies(cache, mp.Policies, "") @@ -2056,7 +2163,7 @@ func (store *IAMStoreSys) SetTempUser(ctx context.Context, accessKey string, cre return time.Time{}, fmt.Errorf("specified policy %s, not found %w", policyName, errNoSuchPolicy) } - err := store.saveMappedPolicy(ctx, cred.ParentUser, stsUser, false, mp, options{ttl: ttl}) + err := store.saveMappedPolicy(ctx, cred.ParentUser, stsUser, false, &mp, options{ttl: ttl}) if err != nil { return time.Time{}, err } @@ -2064,8 +2171,7 @@ func (store *IAMStoreSys) SetTempUser(ctx context.Context, accessKey string, cre cache.iamSTSPolicyMap.Store(cred.ParentUser, mp) } - u := newUserIdentity(cred) - err := store.saveUserIdentity(ctx, accessKey, stsUser, u, options{ttl: ttl}) + err = store.saveUserIdentity(ctx, accessKey, stsUser, &u, options{ttl: ttl}) if err != nil { return time.Time{}, err } @@ -2124,46 +2230,34 @@ func (store *IAMStoreSys) RevokeTokens(ctx context.Context, parentUser string, t // any derived credentials (STS or service accounts) and any associated policy // mappings. func (store *IAMStoreSys) DeleteUsers(ctx context.Context, users []string) error { - cache := store.lock() - defer store.unlock() - - var deleted bool + cache := store.rlock() usersToDelete := set.CreateStringSet(users...) - for user, ui := range cache.iamUsersMap { - userType := regUser - cred := ui.Credentials - - if cred.IsServiceAccount() { - userType = svcUser - } else if cred.IsTemp() { - userType = stsUser - } - - if usersToDelete.Contains(user) || usersToDelete.Contains(cred.ParentUser) { - // Delete this user account and its policy mapping - store.deleteMappedPolicy(ctx, user, userType, false) - cache.iamUserPolicyMap.Delete(user) - - // we are only logging errors, not handling them. - err := store.deleteUserIdentity(ctx, user, userType) - iamLogIf(GlobalContext, err) - if userType == stsUser { - delete(cache.iamSTSAccountsMap, user) + targets := make(map[string]IAMUserType) + for _, identities := range []map[string]UserIdentity{cache.iamUsersMap, cache.iamSTSAccountsMap} { + for key, ui := range identities { + cred := ui.Credentials + if !usersToDelete.Contains(key) && !usersToDelete.Contains(cred.ParentUser) { + continue } - delete(cache.iamUsersMap, user) - if store.group != nil { - store.group.Forget(user) + typ := regUser + if cred.IsServiceAccount() { + typ = svcUser + } else if cred.IsTemp() { + typ = stsUser } - - deleted = true + targets[key] = typ } } - - if deleted { - cache.updatedAt = time.Now() + store.runlock() + var failures []error + for key, typ := range targets { + // External-IDP disablement is an early revocation, not natural token + // expiration. Preserve the credential's durable ordering boundary. + if err := store.DeleteUser(ctx, key, typ); err != nil && !errors.Is(err, errIAMRevocationRetained) { + failures = append(failures, err) + } } - - return nil + return errors.Join(failures...) } // ParentUserInfo contains extra info about a the parent user. @@ -2596,7 +2690,7 @@ func (store *IAMStoreSys) SetUserStatus(ctx context.Context, accessKey string, s }(), }) - if err := store.saveUserIdentity(ctx, accessKey, regUser, uinfo); err != nil { + if err := store.saveUserIdentity(ctx, accessKey, regUser, &uinfo); err != nil { return updatedAt, err } @@ -2615,15 +2709,21 @@ func (store *IAMStoreSys) AddServiceAccount(ctx context.Context, cred auth.Crede accessKey := cred.AccessKey parentUser := cred.ParentUser - // Found newly requested service account, to be an existing account - - // reject such operation (updates to the service account are handled in - // a different API). + // Local creates cannot replace an existing key. Replication also sends + // full snapshots after deletion/recreation, possibly with a new owner; + // their source revision is compared atomically by saveUserIdentity. + _, replicated := iamReplicationTime(ctx) if su, found := cache.iamUsersMap[accessKey]; found { scred := su.Credentials - if scred.ParentUser != parentUser { - return updatedAt, fmt.Errorf("%w: the service account access key is taken by another user", errIAMServiceAccountNotAllowed) + if !replicated || !scred.IsServiceAccount() { + if scred.ParentUser != parentUser { + return updatedAt, fmt.Errorf("%w: the service account access key is taken by another user", errIAMServiceAccountNotAllowed) + } + return updatedAt, fmt.Errorf("%w: the service account access key already taken", errIAMServiceAccountNotAllowed) } - return updatedAt, fmt.Errorf("%w: the service account access key already taken", errIAMServiceAccountNotAllowed) + } + if _, found := cache.iamSTSAccountsMap[accessKey]; found { + return updatedAt, fmt.Errorf("%w: the service account access key is taken by a temporary credential", errIAMServiceAccountNotAllowed) } // Parent user must not be a service account. @@ -2632,12 +2732,18 @@ func (store *IAMStoreSys) AddServiceAccount(ctx context.Context, cred auth.Crede } u := newUserIdentity(cred) - err = store.saveUserIdentity(ctx, u.Credentials.AccessKey, svcUser, u) + err = store.saveUserIdentity(ctx, u.Credentials.AccessKey, svcUser, &u) if err != nil { return updatedAt, err } - cache.updateUserWithClaims(u.Credentials.AccessKey, u) + if err := cache.updateUserWithClaims(u.Credentials.AccessKey, u); err != nil { + // Persistence already replaced the credential. A failed refresh must + // not leave its superseded secret usable through this node's cache. + delete(cache.iamUsersMap, accessKey) + cache.updatedAt = UTCNow() + return updatedAt, err + } return u.UpdatedAt, nil } @@ -2670,7 +2776,7 @@ func (store *IAMStoreSys) UpdateServiceAccount(ctx context.Context, accessKey st if opts.expiration != nil { expirationInUTC := opts.expiration.UTC() - if err := validateSvcExpirationInUTC(expirationInUTC); err != nil { + if err := validateSvcExpirationInUTC(ctx, expirationInUTC); err != nil { return updatedAt, err } cr.Expiration = expirationInUTC @@ -2735,11 +2841,13 @@ func (store *IAMStoreSys) UpdateServiceAccount(ctx context.Context, accessKey st } u := newUserIdentity(cr) - if err := store.saveUserIdentity(ctx, u.Credentials.AccessKey, svcUser, u); err != nil { + if err := store.saveUserIdentity(ctx, u.Credentials.AccessKey, svcUser, &u); err != nil { return updatedAt, err } if err := cache.updateUserWithClaims(u.Credentials.AccessKey, u); err != nil { + delete(cache.iamUsersMap, accessKey) + cache.updatedAt = UTCNow() return updatedAt, err } @@ -2872,7 +2980,7 @@ func (store *IAMStoreSys) AddUser(ctx context.Context, accessKey string, ureq ma }(), }) - if err := store.saveUserIdentity(ctx, accessKey, regUser, u); err != nil { + if err := store.saveUserIdentity(ctx, accessKey, regUser, &u); err != nil { return updatedAt, err } if err := cache.updateUserWithClaims(accessKey, u); err != nil { @@ -2896,7 +3004,7 @@ func (store *IAMStoreSys) UpdateUserSecretKey(ctx context.Context, accessKey, se cred := ui.Credentials cred.SecretKey = secretKey u := newUserIdentity(cred) - if err := store.saveUserIdentity(ctx, accessKey, regUser, u); err != nil { + if err := store.saveUserIdentity(ctx, accessKey, regUser, &u); err != nil { return err } @@ -2942,7 +3050,7 @@ func (store *IAMStoreSys) UpdateUserIdentity(ctx context.Context, cred auth.Cred ui := newUserIdentity(cred) // Overwrite the user identity here. As store should be // atomic, it shouldn't cause any corruption. - if err := store.saveUserIdentity(ctx, cred.AccessKey, userType, ui); err != nil { + if err := store.saveUserIdentity(ctx, cred.AccessKey, userType, &ui); err != nil { return err } @@ -2955,6 +3063,7 @@ func (store *IAMStoreSys) LoadUser(ctx context.Context, accessKey string) error newCachePopulate := func() (val any, err error) { newCache := newIamCache() + newCache.revisions = store.revisionIndex() // Check for service account first store.loadUser(ctx, accessKey, svcUser, newCache.iamUsersMap) @@ -3101,7 +3210,14 @@ func extractJWTClaims(u UserIdentity) (jwtClaims *jwt.MapClaims, err error) { return jwtClaims, err } -func validateSvcExpirationInUTC(expirationInUTC time.Time) error { +func validateSvcExpirationInUTC(ctx context.Context, expirationInUTC time.Time) error { + if _, replicated := iamReplicationTime(ctx); replicated { + // Issuance limits were checked at the source. Keep its absolute + // expiration, even if it has passed: persisting the newer expired + // identity denies the older credential and lets ordinary cleanup + // retain a tombstone. Rejecting it could leave an old key alive. + return nil + } if expirationInUTC.IsZero() || expirationInUTC.Equal(timeSentinel) { // Service accounts might not have expiration in older releases. return nil diff --git a/cmd/iam.go b/cmd/iam.go index e912de903..4e52b72ba 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -605,10 +605,22 @@ func (sys *IAMSys) DeletePolicy(ctx context.Context, policyName string, notifyPe return errServerNotInitialized } - for _, v := range policy.DefaultPolicies { - if v.Name == policyName { - if err := checkConfig(ctx, globalObjectAPI, getPolicyDocPath(policyName)); err != nil && err == errConfigNotFound { - return fmt.Errorf("inbuilt policy `%s` not allowed to be deleted", policyName) + if _, replicated := iamReplicationTime(ctx); !replicated && notifyPeers { + for _, v := range policy.DefaultPolicies { + if v.Name == policyName { + var err error + if objectStore, ok := sys.store.IAMStorageAPI.(*IAMObjectStore); ok { + err = checkConfig(ctx, objectStore.objAPI, getPolicyDocPath(policyName)) + } else { + var r iamRevision + err = sys.store.loadIAMConfig(ctx, &r, getPolicyDocPath(policyName)) + } + if errors.Is(err, errConfigNotFound) { + return fmt.Errorf("inbuilt policy `%s` not allowed to be deleted", policyName) + } + if err != nil { + return err + } } } } @@ -714,20 +726,30 @@ func (sys *IAMSys) DeleteUser(ctx context.Context, accessKey string, notifyPeers return errServerNotInitialized } - if err := sys.store.DeleteUser(ctx, accessKey, regUser); err != nil { + err := sys.store.DeleteUser(ctx, accessKey, regUser) + var cleanupErr *iamCommittedCleanupError + retained := errors.Is(err, errIAMRevocationRetained) + if errors.As(err, &cleanupErr) { + retained = cleanupErr.retained + } else if err != nil && !retained { return err } - - // Notify all other MinIO peers to delete user. + // Publish the committed state even when dependent cleanup must be retried. if notifyPeers && !sys.HasWatcher() { - for _, nerr := range globalNotificationSys.DeleteUser(ctx, accessKey) { - if nerr.Err != nil { - logger.GetReqInfo(ctx).SetTags("peerAddress", nerr.Host.String()) - iamLogIf(ctx, nerr.Err) + if retained { + sys.notifyForUser(ctx, accessKey, false) + } else { + for _, nerr := range globalNotificationSys.DeleteUser(ctx, accessKey) { + if nerr.Err != nil { + logger.GetReqInfo(ctx).SetTags("peerAddress", nerr.Host.String()) + iamLogIf(ctx, nerr.Err) + } } } } - + if cleanupErr != nil { + return cleanupErr + } return nil } @@ -1061,6 +1083,7 @@ type newServiceAccountOpts struct { sessionPolicy *policy.Policy accessKey string secretKey string + status string // Used by replication snapshots; local creates default to enabled. name, description string expiration *time.Time allowSiteReplicatorAccount bool // allow creating internal service account for site-replication. @@ -1125,6 +1148,11 @@ func (sys *IAMSys) NewServiceAccount(ctx context.Context, parentUser string, gro m[k] = v } } + if _, replicated := iamReplicationTime(ctx); !replicated { + if err := setIAMParentRevocationClaim(ctx, sys.store, parentUser, m); err != nil { + return auth.Credentials{}, time.Time{}, err + } + } var accessKey, secretKey string var err error @@ -1143,12 +1171,19 @@ func (sys *IAMSys) NewServiceAccount(ctx context.Context, parentUser string, gro cred.ParentUser = parentUser cred.Groups = groups cred.Status = string(auth.AccountOn) + switch opts.status { + case "", auth.AccountOn, string(madmin.AccountEnabled): + case auth.AccountOff, string(madmin.AccountDisabled): + cred.Status = auth.AccountOff + default: + return auth.Credentials{}, time.Time{}, errInvalidArgument + } cred.Name = opts.name cred.Description = opts.description if opts.expiration != nil { expirationInUTC := opts.expiration.UTC() - if err := validateSvcExpirationInUTC(expirationInUTC); err != nil { + if err := validateSvcExpirationInUTC(ctx, expirationInUTC); err != nil { return auth.Credentials{}, time.Time{}, err } cred.Expiration = expirationInUTC @@ -1379,7 +1414,7 @@ func (sys *IAMSys) DeleteServiceAccount(ctx context.Context, accessKey string, n } sa, ok := sys.store.GetUser(accessKey) - if !ok || !sa.Credentials.IsServiceAccount() { + if _, replicated := iamReplicationTime(ctx); (!ok || !sa.Credentials.IsServiceAccount()) && !replicated { return nil } @@ -1483,8 +1518,8 @@ func (sys *IAMSys) purgeExpiredCredentialsForExternalSSO(ctx context.Context) { } } - // We ignore any errors - _ = sys.store.DeleteUsers(ctx, expiredUsers) + // Keep failed revocations visible so the next purge can retry. + iamLogIf(ctx, sys.store.DeleteUsers(ctx, expiredUsers)) } // purgeExpiredCredentialsForLDAP - validates if local credentials are still @@ -1512,8 +1547,8 @@ func (sys *IAMSys) purgeExpiredCredentialsForLDAP(ctx context.Context) { return } - // We ignore any errors - _ = sys.store.DeleteUsers(ctx, expiredUsers) + // Keep failed revocations visible so the next purge can retry. + iamLogIf(ctx, sys.store.DeleteUsers(ctx, expiredUsers)) } // updateGroupMembershipsForLDAP - updates the list of groups associated with the credential. @@ -1934,12 +1969,12 @@ func (sys *IAMSys) RemoveUsersFromGroup(ctx context.Context, group string, membe } updatedAt, err = sys.store.RemoveUsersFromGroup(ctx, group, members) - if err != nil { + var cleanupErr *iamCommittedCleanupError + if err != nil && !errors.As(err, &cleanupErr) { return updatedAt, err } - sys.notifyForGroup(ctx, group) - return updatedAt, nil + return updatedAt, err } // SetGroupStatus - enable/disabled a group diff --git a/cmd/metrics-v3-cluster-iam.go b/cmd/metrics-v3-cluster-iam.go index 6689fe558..82b29c13c 100644 --- a/cmd/metrics-v3-cluster-iam.go +++ b/cmd/metrics-v3-cluster-iam.go @@ -34,6 +34,10 @@ const ( sinceLastSyncMillis = "since_last_sync_millis" syncFailures = "sync_failures" syncSuccesses = "sync_successes" + revocationRecords = "revocation_records" + revocationHealFailures = "revocation_heal_failures" + revocationHealDurationMillis = "revocation_heal_duration_millis" + revocationHealLastSuccess = "revocation_heal_last_success_timestamp_seconds" ) var ( @@ -47,10 +51,20 @@ var ( sinceLastSyncMillisMD = NewCounterMD(sinceLastSyncMillis, "Time (in milliseconds) since last successful IAM data sync.") syncFailuresMD = NewCounterMD(syncFailures, "Number of failed IAM data syncs since server start.") syncSuccessesMD = NewCounterMD(syncSuccesses, "Number of successful IAM data syncs since server start.") + revocationRecordsMD = NewGaugeMD(revocationRecords, "Retained IAM deletion records and revocation boundaries in this node's index.") + revocationHealFailuresMD = NewCounterMD(revocationHealFailures, "Failed IAM revocation convergence passes since server start.") + revocationHealDurationMillisMD = NewGaugeMD(revocationHealDurationMillis, "Duration of the last IAM revocation convergence pass in milliseconds.") + revocationHealLastSuccessMD = NewGaugeMD(revocationHealLastSuccess, "Unix timestamp of the last successful IAM revocation convergence pass.") ) // loadClusterIAMMetrics - `MetricsLoaderFn` for cluster IAM metrics. func loadClusterIAMMetrics(_ context.Context, m MetricValues, _ *metricsCache) error { + if globalIAMSys.Initialized() { + m.Set(revocationRecords, float64(globalIAMSys.store.revisionIndex().count())) + } + m.Set(revocationHealFailures, float64(globalSiteReplicationSys.iamRevisionMetrics.healFailures.Load())) + m.Set(revocationHealDurationMillis, float64(globalSiteReplicationSys.iamRevisionMetrics.healDurationMillis.Load())) + m.Set(revocationHealLastSuccess, float64(globalSiteReplicationSys.iamRevisionMetrics.healLastSuccess.Load())) m.Set(lastSyncDurationMillis, float64(atomic.LoadUint64(&globalIAMSys.LastRefreshDurationMilliseconds))) pluginAuthNMetrics := globalAuthNPlugin.Metrics() m.Set(pluginAuthnServiceFailedRequestsMinute, float64(pluginAuthNMetrics.FailedRequests)) diff --git a/cmd/metrics-v3.go b/cmd/metrics-v3.go index 49daeac25..62e4d89fd 100644 --- a/cmd/metrics-v3.go +++ b/cmd/metrics-v3.go @@ -323,6 +323,10 @@ func newMetricGroups(r *prometheus.Registry) *metricsV3Collection { sinceLastSyncMillisMD, syncFailuresMD, syncSuccessesMD, + revocationRecordsMD, + revocationHealFailuresMD, + revocationHealDurationMillisMD, + revocationHealLastSuccessMD, }, loadClusterIAMMetrics, ) diff --git a/cmd/peer-rest-server.go b/cmd/peer-rest-server.go index 0ef0eb0c0..b28c8db44 100644 --- a/cmd/peer-rest-server.go +++ b/cmd/peer-rest-server.go @@ -204,7 +204,9 @@ func (s *peerRESTServer) DeleteServiceAccountHandler(mss *grid.MSS) (np grid.NoP return np, grid.NewRemoteErr(errors.New("service account name is missing")) } - if err := globalIAMSys.DeleteServiceAccount(context.Background(), accessKey, false); err != nil { + ctx, cancel := context.WithTimeout(GlobalContext, defaultContextTimeout) + defer cancel() + if err := globalIAMSys.LoadServiceAccount(ctx, accessKey); err != nil { return np, grid.NewRemoteErr(err) } @@ -274,7 +276,9 @@ func (s *peerRESTServer) LoadUserHandler(mss *grid.MSS) (np grid.NoPayload, nerr userType = stsUser } - if err = globalIAMSys.LoadUser(context.Background(), objAPI, accessKey, userType); err != nil { + ctx, cancel := context.WithTimeout(GlobalContext, defaultContextTimeout) + defer cancel() + if err = globalIAMSys.LoadUser(ctx, objAPI, accessKey, userType); err != nil { return np, grid.NewRemoteErr(err) } diff --git a/cmd/site-replication-metadata-gate_test.go b/cmd/site-replication-metadata-gate_test.go index de9a37bb6..ea5d8dfbe 100644 --- a/cmd/site-replication-metadata-gate_test.go +++ b/cmd/site-replication-metadata-gate_test.go @@ -274,6 +274,10 @@ func TestBucketMetadataInitialSyncPhysicalCreated(t *testing.T) { } events = append(events, event) } + if r.URL.Path == "/minio/admin/v3/site-replication/peer/iam-revisions" { + _ = json.NewEncoder(w).Encode(iamRevisionResponse{iamRevisionStatus: iamRevisionStatus{Version: iamRevisionProtocol, Node: "initial-peer", Instance: "initial-boot", Digest: "ack"}}) + return + } w.WriteHeader(http.StatusOK) })) defer peer.Close() diff --git a/cmd/site-replication.go b/cmd/site-replication.go index 41dce704e..6592dd14e 100644 --- a/cmd/site-replication.go +++ b/cmd/site-replication.go @@ -28,6 +28,7 @@ import ( "fmt" "maps" "math/rand" + "net/http" "net/url" "reflect" "runtime" @@ -209,7 +210,10 @@ type SiteReplicationSys struct { // In-memory and persisted multi-site replication state. state srState - iamMetaCache srIAMCache + iamMetaCache srIAMCache + iamHealMu sync.Mutex + iamRevisionProgress map[string]iamRevisionProgress + iamRevisionMetrics iamRevisionMetrics } type srState srStateV1 @@ -1257,13 +1261,36 @@ func (c *SiteReplicationSys) IAMChangeHook(ctx context.Context, item madmin.SRIA return nil } + if path := iamDeletionPath(item); path != "" { + r, err := loadIAMRevision(ctx, globalIAMSys.store, path) + if err != nil { + return err + } + if !r.Deleted && ((item.Type != madmin.SRIAMItemIAMUser && item.Type != madmin.SRIAMItemGroupInfo) || r.RevokedBefore.IsZero()) { + // A concurrent recreation has already superseded this delete. + return nil + } + item.UpdatedAt = r.timestamp() + if !r.Deleted { + item.UpdatedAt = r.RevokedBefore + } + } + + versioned, err := c.replicationItem(ctx, item) + if errors.Is(err, errIAMStaleUpdate) { + return nil + } + if err != nil { + return err + } cerr := c.concDo(nil, func(d string, p madmin.PeerInfo) error { admClient, err := c.getAdminClient(ctx, d) if err != nil { return wrapSRErr(err) } - return c.annotatePeerErr(p.Name, replicateIAMItem, admClient.SRPeerReplicateIAMItem(ctx, item)) + _, err = executeIAMRevisionRequest(ctx, admClient, http.MethodPut, &iamRevisionBatch{Version: iamRevisionProtocol, Items: []iamReplicationItem{versioned}}) + return c.annotatePeerErr(p.Name, replicateIAMItem, err) }, replicateIAMItem, ) @@ -1273,6 +1300,7 @@ func (c *SiteReplicationSys) IAMChangeHook(ctx context.Context, item madmin.SRIA // PeerAddPolicyHandler - copies IAM policy to local. A nil policy argument, // causes the named policy to be deleted. func (c *SiteReplicationSys) PeerAddPolicyHandler(ctx context.Context, policyName string, p *policy.Policy, updatedAt time.Time) error { + ctx = withIAMReplicationTime(ctx, updatedAt) var err error // skip overwrite of local update if peer sent stale info if !updatedAt.IsZero() { @@ -1286,18 +1314,19 @@ func (c *SiteReplicationSys) PeerAddPolicyHandler(ctx context.Context, policyNam _, err = globalIAMSys.SetPolicy(ctx, policyName, *p) } if err != nil { - return wrapSRErr(err) + return iamReplicationError(err) } return nil } // PeerIAMUserChangeHandler - copies IAM user to local. func (c *SiteReplicationSys) PeerIAMUserChangeHandler(ctx context.Context, change *madmin.SRIAMUser, updatedAt time.Time) error { + ctx = withIAMReplicationTime(ctx, updatedAt) if change == nil { return errSRInvalidRequest(errInvalidArgument) } // skip overwrite of local update if peer sent stale info - if !updatedAt.IsZero() { + if !change.IsDeleteReq && !updatedAt.IsZero() { if ui, err := globalIAMSys.GetUserInfo(ctx, change.AccessKey); err == nil && ui.UpdatedAt.After(updatedAt) { return nil } @@ -1326,13 +1355,14 @@ func (c *SiteReplicationSys) PeerIAMUserChangeHandler(ctx context.Context, chang } } if err != nil { - return wrapSRErr(err) + return iamReplicationError(err) } return nil } // PeerGroupInfoChangeHandler - copies group changes to local. func (c *SiteReplicationSys) PeerGroupInfoChangeHandler(ctx context.Context, change *madmin.SRGroupInfo, updatedAt time.Time) error { + ctx = withIAMReplicationTime(ctx, updatedAt) if change == nil { return errSRInvalidRequest(errInvalidArgument) } @@ -1340,7 +1370,7 @@ func (c *SiteReplicationSys) PeerGroupInfoChangeHandler(ctx context.Context, cha var err error // skip overwrite of local update if peer sent stale info - if !updatedAt.IsZero() { + if !updatedAt.IsZero() && (!updReq.IsRemove || len(updReq.Members) != 0) { if gd, err := globalIAMSys.GetGroupDescription(updReq.Group); err == nil && gd.UpdatedAt.After(updatedAt) { return nil } @@ -1349,7 +1379,8 @@ func (c *SiteReplicationSys) PeerGroupInfoChangeHandler(ctx context.Context, cha if updReq.IsRemove { _, err = globalIAMSys.RemoveUsersFromGroup(ctx, updReq.Group, updReq.Members) } else { - if updReq.Status != "" && len(updReq.Members) == 0 { + snapshot, _ := ctx.Value(iamGroupSnapshotKey{}).(bool) + if !snapshot && updReq.Status != "" && len(updReq.Members) == 0 { _, err = globalIAMSys.SetGroupStatus(ctx, updReq.Group, updReq.Status == madmin.GroupEnabled) } else { if globalIAMSys.LDAPConfig.Enabled() { @@ -1365,13 +1396,14 @@ func (c *SiteReplicationSys) PeerGroupInfoChangeHandler(ctx context.Context, cha } } if err != nil && !errors.Is(err, errNoSuchGroup) { - return wrapSRErr(err) + return iamReplicationError(err) } return nil } // PeerSvcAccChangeHandler - copies service-account change to local. func (c *SiteReplicationSys) PeerSvcAccChangeHandler(ctx context.Context, change *madmin.SRSvcAccChange, updatedAt time.Time) error { + ctx = withIAMReplicationTime(ctx, updatedAt) if change == nil { return errSRInvalidRequest(errInvalidArgument) } @@ -1382,7 +1414,7 @@ func (c *SiteReplicationSys) PeerSvcAccChangeHandler(ctx context.Context, change if len(change.Create.SessionPolicy) > 0 { sp, err = policy.ParseConfig(bytes.NewReader(change.Create.SessionPolicy)) if err != nil { - return wrapSRErr(err) + return iamReplicationError(err) } } // skip overwrite of local update if peer sent stale info @@ -1394,6 +1426,7 @@ func (c *SiteReplicationSys) PeerSvcAccChangeHandler(ctx context.Context, change opts := newServiceAccountOpts{ accessKey: change.Create.AccessKey, secretKey: change.Create.SecretKey, + status: change.Create.Status, sessionPolicy: sp, claims: change.Create.Claims, name: change.Create.Name, @@ -1402,7 +1435,7 @@ func (c *SiteReplicationSys) PeerSvcAccChangeHandler(ctx context.Context, change } _, _, err = globalIAMSys.NewServiceAccount(ctx, change.Create.Parent, change.Create.Groups, opts) if err != nil { - return wrapSRErr(err) + return iamReplicationError(err) } case change.Update != nil: @@ -1411,7 +1444,7 @@ func (c *SiteReplicationSys) PeerSvcAccChangeHandler(ctx context.Context, change if len(change.Update.SessionPolicy) > 0 { sp, err = policy.ParseConfig(bytes.NewReader(change.Update.SessionPolicy)) if err != nil { - return wrapSRErr(err) + return iamReplicationError(err) } } // skip overwrite of local update if peer sent stale info @@ -1431,7 +1464,7 @@ func (c *SiteReplicationSys) PeerSvcAccChangeHandler(ctx context.Context, change _, err = globalIAMSys.UpdateServiceAccount(ctx, change.Update.AccessKey, opts) if err != nil { - return wrapSRErr(err) + return iamReplicationError(err) } case change.Delete != nil: @@ -1442,7 +1475,7 @@ func (c *SiteReplicationSys) PeerSvcAccChangeHandler(ctx context.Context, change } } if err := globalIAMSys.DeleteServiceAccount(ctx, change.Delete.AccessKey, true); err != nil { - return wrapSRErr(err) + return iamReplicationError(err) } } @@ -1451,24 +1484,17 @@ func (c *SiteReplicationSys) PeerSvcAccChangeHandler(ctx context.Context, change // PeerPolicyMappingHandler - copies policy mapping to local. func (c *SiteReplicationSys) PeerPolicyMappingHandler(ctx context.Context, mapping *madmin.SRPolicyMapping, updatedAt time.Time) error { + ctx = withIAMReplicationTime(ctx, updatedAt) if mapping == nil { return errSRInvalidRequest(errInvalidArgument) } - // skip overwrite of local update if peer sent stale info - if !updatedAt.IsZero() { - mp, ok := globalIAMSys.store.GetMappedPolicy(mapping.Policy, mapping.IsGroup) - if ok && mp.UpdatedAt.After(updatedAt) { - return nil - } - } - // When LDAP is enabled, we verify that the user or group exists in LDAP and // use the normalized form of the entityName (which will be an LDAP DN). userType := IAMUserType(mapping.UserType) isGroup := mapping.IsGroup entityName := mapping.UserOrGroup - if globalIAMSys.GetUsersSysType() == LDAPUsersSysType && userType == stsUser { + if mapping.Policy != "" && globalIAMSys.GetUsersSysType() == LDAPUsersSysType && userType == stsUser { // Validate that the user or group exists in LDAP and use the normalized // form of the entityName (which will be an LDAP DN). var err error @@ -1491,19 +1517,20 @@ func (c *SiteReplicationSys) PeerPolicyMappingHandler(ctx context.Context, mappi entityName = foundUserDN.NormDN } if err != nil { - return wrapSRErr(err) + return iamReplicationError(err) } } _, err := globalIAMSys.PolicyDBSet(ctx, entityName, mapping.Policy, userType, isGroup) if err != nil { - return wrapSRErr(err) + return iamReplicationError(err) } return nil } // PeerSTSAccHandler - replicates STS credential locally. func (c *SiteReplicationSys) PeerSTSAccHandler(ctx context.Context, stsCred *madmin.SRSTSCredential, updatedAt time.Time) error { + ctx = withIAMReplicationTime(ctx, updatedAt) if stsCred == nil { return errSRInvalidRequest(errInvalidArgument) } @@ -1555,7 +1582,7 @@ func (c *SiteReplicationSys) PeerSTSAccHandler(ctx context.Context, stsCred *mad // Set these credentials to IAM. if _, err := globalIAMSys.SetTempUser(ctx, cred.AccessKey, cred, stsCred.ParentPolicyMapping); err != nil { - return fmt.Errorf("unable to save STS credential and/or parent policy mapping: %w", err) + return iamReplicationError(fmt.Errorf("unable to save STS credential and/or parent policy mapping: %w", err)) } return nil @@ -4193,7 +4220,8 @@ func (c *SiteReplicationSys) SiteReplicationMetaInfo(ctx context.Context, objAPI } info.UserInfoMap[k] = madmin.UserInfo{ - Status: madmin.AccountStatus(v.Credentials.Status), + Status: madmin.AccountStatus(v.Credentials.Status), + UpdatedAt: v.UpdatedAt, } } } @@ -5170,13 +5198,16 @@ func (c *SiteReplicationSys) healBucketReplicationConfig(ctx context.Context, ob } func (c *SiteReplicationSys) healIAMSystem(ctx context.Context, objAPI ObjectLayer) error { + // A peer rejecting a deletion must not stop unrelated live updates from + // reaching healthy peers. Retain and report the error for the next retry. + deletionErr := c.healIAMDeletions(ctx) info, err := c.siteReplicationStatus(ctx, objAPI, madmin.SRStatusOptions{ Users: true, Policies: true, Groups: true, }) if err != nil { - return err + return errors.Join(deletionErr, err) } for policy := range info.PolicyStats { c.healPolicies(ctx, objAPI, policy, info) @@ -5194,7 +5225,7 @@ func (c *SiteReplicationSys) healIAMSystem(ctx context.Context, objAPI ObjectLay c.healGroupPolicies(ctx, objAPI, group, info) } - return nil + return deletionErr } // heal iam policies present on this site to peers, provided current cluster has the most recent update. @@ -5429,8 +5460,10 @@ func (c *SiteReplicationSys) healUsers(ctx context.Context, objAPI ObjectLayer, peerName := info.Sites[dID].Name - u, ok := globalIAMSys.GetUser(ctx, user) - if !ok { + // Disabled identities are valid replication sources. CheckKey returns + // their stored record even though authentication is denied. + u, _, err := globalIAMSys.CheckKey(ctx, user) + if err != nil || u.Credentials.AccessKey == "" || u.Credentials.IsExpired() { continue } creds := u.Credentials @@ -5629,7 +5662,10 @@ func isGroupDescEqual(g1, g2 madmin.GroupDesc) bool { } func isUserInfoEqual(u1, u2 madmin.UserInfo) bool { - if u1.PolicyName != u2.PolicyName || + // Full-site summaries omit secrets and claims. Equal status alone cannot + // distinguish a recreated identity or an edited service-account policy. + if !u1.UpdatedAt.Equal(u2.UpdatedAt) || + u1.PolicyName != u2.PolicyName || u1.Status != u2.Status || u1.SecretKey != u2.SecretKey { return false diff --git a/cmd/sts-handlers.go b/cmd/sts-handlers.go index 421d4d71b..f52a18c86 100644 --- a/cmd/sts-handlers.go +++ b/cmd/sts-handlers.go @@ -624,6 +624,10 @@ func (sts *stsAPIHandlers) AssumeRole(w http.ResponseWriter, r *http.Request) { claims[expClaim] = UTCNow().Add(duration).Unix() claims[parentClaim] = user.AccessKey + if err := setIAMParentRevocationClaim(ctx, globalIAMSys.store, user.AccessKey, claims); err != nil { + writeSTSErrorResponse(ctx, w, ErrSTSInternalError, err) + return + } tokenRevokeType := r.Form.Get(stsRevokeTokenType) if tokenRevokeType != "" { diff --git a/docs/site-replication/iam-revocations.md b/docs/site-replication/iam-revocations.md new file mode 100644 index 000000000..6908d88b8 --- /dev/null +++ b/docs/site-replication/iam-revocations.md @@ -0,0 +1,218 @@ +# Durable IAM revocations in site replication + +SILO retains the version of a deleted IAM record so an offline site cannot +restore an older identity or grant when it reconnects. This covers built-in +users, service accounts, groups, policy documents, and policy mappings in their +actual user/STS-parent/group namespaces. It also revokes the deleted built-in +user's older service accounts, STS credentials and group grants across deliberate +same-name recreation. + +## Ordering and persistence + +Deletion records occupy the original IAM configuration paths. They contain the +originating timestamp, `Deleted`, and, where required, `RevokedBefore`; identity +deletion records contain no secret key or session token. Normal IAM listings and +authorization hide deleted records. Object storage and etcd both serialize each +path's version comparison and write with a distributed lock. The source timestamp +is persisted without replacing it with the receiving node's clock. + +Older events cannot overwrite a newer revision. At an identical timestamp, a +deletion wins over a live record. A local deliberate recreation receives a +version newer than the stored deletion. An older user/group deletion arriving +after recreation retains its revocation boundary while preserving the newer +live record. Receiving an already-applied tombstone does not rewrite or advance +it. These rules also apply after cold loading persistent IAM state. + +The user or group revision is the commit point of deletion. Cleanup of mappings +and children follows that commit; cleanup failure cannot undo it. The API returns +the cleanup error and still notifies sibling nodes to reload the committed +state. Such an error does **not** mean the identity is still active. Retrying the +operation is safe. When the API returns a committed-cleanup error, the admin +handler does not send its immediate cross-site hook; cross-site propagation +relies on the normal deletion-healing retry. Sibling deletion notifications reload current shared state, +so a delayed notification cannot delete a subsequently recreated identity. +Etcd siblings also receive persistent changes through watches. Failed +notifications/watches remain eventual propagation, not a distributed +instantaneous revocation transaction. + +Keep node clocks synchronized and monitor offsets. Ordering uses source wall +clock timestamps, with monotonic advancement for local writes to the same path. +It does not establish causal order between concurrent writers at different sites +or resolve conflicting live updates with identical timestamps deterministically. + +## Users, child credentials and groups + +A recreated user retains `RevokedBefore`. New service accounts and built-in STS +issuances carry a signed `siloParentRevocation` claim identifying the parent +boundary known at issuance. Editing or replaying an old child does not update +this claim. Old children remain invalid even when their own update timestamp is +newer than the parent deletion; children issued for the recreated parent remain +valid. Claims are read from verified tokens on credential load/write. + +A newer replicated service-account snapshot can replace an older service with +the same access key, including a deliberate change of owner or secret. Local +duplicate creates remain rejected. Snapshots preserve disabled status and the +service's own revocation boundary, so earlier mappings cannot attach to the new +service. Equal-version retries reload the committed identity without rewriting +it. Periodic live healing compares source versions even when the public status +summary is unchanged, and includes disabled identities as healing sources. +Service snapshots also preserve their absolute expiration. The receiving site +does not reapply the local minimum lifetime for a newly issued credential. A +newer already-expired snapshot still supersedes the old key, is denied by +authentication, and is collected into a durable service tombstone by normal +loading. Cache/claims loading failures are returned for retry, not acknowledged; +after a committed replacement the stale cached secret is evicted immediately. +Collisions with an existing built-in or cached STS identity report an error and +require an explicit administrative resolution; replication cannot change its +credential kind. Concurrent conflicting service updates with exactly the same +timestamp can retain different winners at different sites; the status summary +does not resolve that case. + +Each group member has its own `MemberGrants` timestamp. Changing another member +or the group's enabled status does not reissue everyone else's grants. Effective +membership requires the grant to be newer than both the user's and the group's +retained boundaries. Listings and policy evaluation use the same effective +membership. Peer snapshots preserve grant times, including unknown legacy grant +times; they cannot treat a recent snapshot time as a fresh grant to a revoked +identity. A new explicit administrative group grant can restore access. + +This does not implement a general conflict-resolution protocol for all group +membership edits. In particular, the inherited live-group snapshot merge adds +members and does not reconcile a missed ordinary member removal. Removing a +member from a live group during a site outage is a separate known limitation; +do not infer that this change resolves it. User/group deletion boundaries and +same-name recreation are covered here. + +## Retention and expiration + +Permanent identities, groups, policy documents and mappings have no automatic +tombstone TTL. A disconnected peer or an old backup may return arbitrarily late. +Successful replay acknowledgements are an optimization, **not** permission to +garbage-collect this history. + +Natural expiration of an immutable STS token physically removes its token-key +record and any legacy token-key mapping, without generating a permanent +tombstone. An early STS revocation is retained until that token's expiration plus +the existing clock-skew allowance. Replaying the same revoked token with a later +event timestamp cannot recreate it. Etcd uses an expiration lease; object storage +collects expired STS tombstones during its existing credential loading/purge. +A record with unknown expiration is retained conservatively. Cleanup writes are +best effort and use a short lock budget. If one fails, that load stops optional +reclamation, reports the error and still loads healthy users; expired credentials +stay denied and retain their existing durable version. The next load retries. +Healthy cleanup has no fixed record quota. The reusable STS +parent policy mapping is not assigned the token's TTL by deletion cleanup. +External-IDP disablement is an early revocation, not natural token expiration, +and its cached STS and service accounts are included in cleanup. Expiring service +accounts retain a durable revision because their access keys are reusable and +an older version might have no expiration. + +Direct per-token `RevokeTokens` delivery between sites is not a new guarantee of +this change. The guarantee for built-in parent deletion follows from the durable +parent boundary, including children not currently present in the deleting node's +cache. + +## Healing, failures and operational cost + +The normal IAM loaders maintain an in-memory index of deletion records and +retained boundaries, without secrets. Healing uses this index; it does not add a +second full walk of `config/iam/` every cycle. Existing full IAM loading still +scans persistent records, including tombstones, at startup and on refresh. + +Each peer receives batches of at most 128 records. The sender remembers which +path/version each peer acknowledged. Unrelated new changes at either site do not +reset that progress. A failed batch remains pending while later independent +batches can progress; a lost response may cause safe idempotent replay. A pass +has a bounded duration, and its successful acknowledgements survive that timeout. +The protocol reports each node name and process instance. Switching between +known node instances behind a load balancer preserves acknowledgements. A new +node instance conservatively invalidates prior acknowledgements once; repeated +switches among those known instances do not reset progress. Restore persistent state only with the affected processes +stopped, so a restore cannot reuse an old process acknowledgement. + +Steady-state healing still traverses/sorts the retained in-memory set and checks +the peer's protocol status. It suppresses repeated deletion PUTs once acknowledged. +Memory use scales with retained paths and peers; startup storage reads scale with +history. This release does not provide general history compaction. +After upgrading, older live records whose receiving sites originally assigned +different timestamps can require an initial reconciliation wave. Allow for its +storage writes and sibling notifications when planning the maintenance window. + +The cluster IAM metrics include `revocation_records`, +`revocation_heal_failures`, `revocation_heal_duration_millis`, and +`revocation_heal_last_success_timestamp_seconds`. Errors are also logged. A +nominal 30-second scheduler interval is not a convergence deadline: outages, +large backlogs, lock contention and failed requests can require more passes. + +Cached credential lookup checks the in-memory parent revision index and performs +no additional storage read. STS issuance and cold credential loading still consult +the persistent parent revision. Revision I/O and distributed lock waits release +the IAM cache lock while a separate local writer mutex preserves write order. +These operations have bounded contexts, including etcd lock and lease cleanup. + +## Protocol and supported upgrade + +The server-owned versioned route is +`/minio/admin/v3/site-replication/peer/iam-revisions`. It carries source versions, +member grant times and distinct user/group revocation items without changing the +admin client SDK or S3 API. Older servers reject this route. The sender reports +the failure and does not fall back to a route that would discard the metadata. +The existing legacy IAM route remains readable for best-effort compatibility; +this does not confer the new guarantees on an older peer. + +All participating servers must be upgraded for the guarantee in this document. +Mixed old/new nodes sharing an IAM backend and rolling downgrade are unsupported: +older binaries do not interpret tombstones or signed parent boundaries correctly. +Use a maintenance window for coordinated upgrade: + +1. Pause IAM changes and isolate any offline site or backup whose state is unknown. +2. Back up each site's complete IAM storage and required encryption material. A + live IAM admin export omits deletion history and is not an adequate backup. +3. Stop all nodes sharing each site's IAM backend, replace their binaries, and + restart them on the upgraded version. Complete this for every participating + site before relying on the new revocation semantics. +4. Check IAM loading, site-replication errors and revocation convergence. Verify + representative old credentials are denied and deliberately reissued ones work. +5. Resolve pre-upgrade revocations explicitly. Absence cannot reconstruct an + already-lost deletion version: remove surviving old records on the sites that + still have them, and rebuild stale offline peers from approved state before + admitting them. Do not reconnect an unknown old snapshot just to discover its + deleted credentials. + +Credentials issued by an older server for a recreated parent lack the required +signed boundary and must be reissued by an upgraded server. Parents without any +retained revocation history preserve existing credential behavior. + +A pristine built-in policy remains protected from local deletion. If an +administrator explicitly overrides that policy and later deletes the override, +the durable deletion now suppresses automatic recreation of the built-in policy +on reload. This prevents reload from undoing the deletion. Restore the policy by +an explicit policy-create operation if desired. Local deletion of a nonexistent +policy remains idempotent and does not create a new tombstone; replicated +unknown deletions retain their version. + +For rollback, stop and isolate the affected sites and assess changes since the +backup before restoring compatible state. Restoring an older backup can itself +lose later revocations and requires reconciliation/rekeying before access is +reopened. Do not delete tombstones online or convert only live IAM records to +make an older binary start. Server, client, Console, package and deployment +acceptance remain separate delivery gates of the maintained PGSTY stack. + +## Regression and performance checks + +Focused coverage is in `iam-revocation_test.go`, `iam-revision_test.go`, +`iam-revision-lock_test.go`, `iam-revision-boundary_test.go`, +`iam-replication-protocol_test.go`, `iam-credential-retention_test.go`, +`iam-peer-reload_test.go`, and `iam-replay_test.go`. Set +`SILO_TEST_IAM_REVOCATION_ETCD` to a disposable etcd endpoint to include backend +lifecycle/locking/boundary tests; they use isolated key namespaces. + +`BenchmarkIAMCachedCredential` and `BenchmarkIAMSetTempUser` can be run against +the pre-change source for a comparable local baseline. +`BenchmarkIAMRevisionConvergedHealing` covers 1,000 and 10,000 retained records; +it measures steady-state index/network work and asserts zero repeated PUTs. Its +fake peer does not measure durable catch-up throughput. +`BenchmarkIAMColdLoadExpiredServices` measures loading and cleanup with 100 or +1,000 expired reusable credentials; run it with `-benchtime=1x`. Use actual multi-site +signed S3/STS tests and deployment-specific latency/scale measurements in +addition to these component tests. From a0dd7dae9b7340b8dfc10f0234596ae6b2d2b3ff Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Tue, 15 Sep 2026 22:16:12 +0800 Subject: [PATCH 2/3] fix(iam): resume site healing after leadership changes Keep one healing loop per process across replication configuration reloads. Reacquire leadership after a lease is canceled and allow shutdown while waiting, so temporary quorum loss cannot permanently stop revocation propagation. Signed-off-by: Feng Ruohang --- cmd/iam-healing-leadership_test.go | 75 ++++++++++++++++++++++++ cmd/site-replication.go | 23 +++++++- docs/site-replication/iam-revocations.md | 6 ++ 3 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 cmd/iam-healing-leadership_test.go diff --git a/cmd/iam-healing-leadership_test.go b/cmd/iam-healing-leadership_test.go new file mode 100644 index 000000000..5c30010f2 --- /dev/null +++ b/cmd/iam-healing-leadership_test.go @@ -0,0 +1,75 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-only + +package cmd + +import ( + "context" + "testing" + "time" +) + +func TestIAMHealingResumesAfterLeadershipLoss(t *testing.T) { + previous := globalLeaderLock + locks := make(chan LockContext) + globalLeaderLock = &sharedLock{lockContext: locks} + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + c := &SiteReplicationSys{} + go func() { c.startHealRoutine(ctx, nil); close(done) }() + t.Cleanup(func() { + cancel() + select { + case <-done: + case locks <- LockContext{ctx: ctx}: + } + <-done + globalLeaderLock = previous + }) + first, loseFirst := context.WithCancel(ctx) + defer loseFirst() + select { + case locks <- LockContext{ctx: first}: + case <-time.After(time.Second): + t.Fatal("healer did not acquire its first leader context") + } + loseFirst() // A transient quorum loss cancels the distributed lease. + select { + case <-done: + t.Fatal("healer permanently exited after temporary leadership loss") + case locks <- LockContext{ctx: ctx}: + case <-time.After(time.Second): + t.Fatal("healer did not wait for reacquired leadership") + } + // Shutdown must also interrupt the wait for leadership after lease loss. + cancel() + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("healer did not stop with its owning context") + } +} + +func TestIAMHealingLeadershipWaitCancels(t *testing.T) { + previous := globalLeaderLock + locks := make(chan LockContext) + globalLeaderLock = &sharedLock{lockContext: locks} + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + c := &SiteReplicationSys{} + go func() { c.startHealRoutine(ctx, nil); close(done) }() + cancel() + t.Cleanup(func() { + select { + case <-done: + case locks <- LockContext{ctx: ctx}: + } + <-done + globalLeaderLock = previous + }) + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("healer ignored shutdown while waiting for leadership") + } +} diff --git a/cmd/site-replication.go b/cmd/site-replication.go index 6592dd14e..6c82fa52b 100644 --- a/cmd/site-replication.go +++ b/cmd/site-replication.go @@ -211,6 +211,7 @@ type SiteReplicationSys struct { state srState iamMetaCache srIAMCache + healOnce sync.Once // Configuration reloads must not spawn more healing loops. iamHealMu sync.Mutex iamRevisionProgress map[string]iamRevisionProgress iamRevisionMetrics iamRevisionMetrics @@ -238,7 +239,7 @@ type srStateData struct { // Init - initialize the site replication manager. func (c *SiteReplicationSys) Init(ctx context.Context, objAPI ObjectLayer) error { - go c.startHealRoutine(ctx, objAPI) + c.healOnce.Do(func() { go c.startHealRoutine(ctx, objAPI) }) r := rand.New(rand.NewSource(time.Now().UnixNano())) for { err := c.loadFromDisk(ctx, objAPI) @@ -4577,9 +4578,25 @@ func (c *SiteReplicationSys) PeerStateEditReq(ctx context.Context, arg madmin.SR const siteHealTimeInterval = 30 * time.Second func (c *SiteReplicationSys) startHealRoutine(ctx context.Context, objAPI ObjectLayer) { - ctx, cancel := globalLeaderLock.GetLock(ctx) - defer cancel() + for ctx.Err() == nil { + var leadership LockContext + select { + case <-ctx.Done(): + return + case leadership = <-globalLeaderLock.lockContext: + } + if leadership.Context().Err() != nil { + continue + } + leaderCtx, cancel := mergeContext(leadership.Context(), ctx) + c.healWithLeadership(leaderCtx, objAPI) + cancel() + // Quorum loss cancels a leadership lease, not the subsystem. Wait + // for a new lease so revocations can still reach offline sites. + } +} +func (c *SiteReplicationSys) healWithLeadership(ctx context.Context, objAPI ObjectLayer) { healTimer := time.NewTimer(siteHealTimeInterval) defer healTimer.Stop() diff --git a/docs/site-replication/iam-revocations.md b/docs/site-replication/iam-revocations.md index 6908d88b8..d2ee4303c 100644 --- a/docs/site-replication/iam-revocations.md +++ b/docs/site-replication/iam-revocations.md @@ -114,6 +114,12 @@ cache. ## Healing, failures and operational cost +Each process starts one healing loop. Losing its distributed leadership lease +pauses work until leadership is reacquired; it does not permanently terminate +healing. Configuration reloads do not create extra loops. The 30-second interval +starts after leadership is acquired and after each completed pass. Initial lock +retries and endpoint recovery can add further delay; it is not a convergence SLA. + The normal IAM loaders maintain an in-memory index of deletion records and retained boundaries, without secrets. Healing uses this index; it does not add a second full walk of `config/iam/` every cycle. Existing full IAM loading still From 7b4cacc392994f72817d59eff2b6bbd919996741 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Tue, 15 Sep 2026 22:19:18 +0800 Subject: [PATCH 3/3] chore: align IAM file notices with contribution policy Signed-off-by: Feng Ruohang --- cmd/iam-credential-retention_test.go | 2 +- cmd/iam-group-revision.go | 2 +- cmd/iam-healing-leadership_test.go | 2 +- cmd/iam-healing-performance_test.go | 2 +- cmd/iam-performance_test.go | 2 +- cmd/iam-replication-protocol.go | 2 +- cmd/iam-replication-protocol_test.go | 2 +- cmd/iam-revision-boundary_test.go | 2 +- cmd/iam-revision-index.go | 2 +- cmd/iam-revision-lock_test.go | 2 +- cmd/iam-revision.go | 2 +- cmd/iam-revision_test.go | 2 +- cmd/iam-revocation_test.go | 2 +- cmd/iam-service-recreation_test.go | 2 +- 14 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cmd/iam-credential-retention_test.go b/cmd/iam-credential-retention_test.go index c4edcf169..09392e86e 100644 --- a/cmd/iam-credential-retention_test.go +++ b/cmd/iam-credential-retention_test.go @@ -1,5 +1,5 @@ // Copyright (c) 2026 PGSTY -// SPDX-License-Identifier: AGPL-3.0-only +// SPDX-License-Identifier: AGPL-3.0-or-later package cmd diff --git a/cmd/iam-group-revision.go b/cmd/iam-group-revision.go index db912f781..257e87811 100644 --- a/cmd/iam-group-revision.go +++ b/cmd/iam-group-revision.go @@ -1,5 +1,5 @@ // Copyright (c) 2026 PGSTY -// SPDX-License-Identifier: AGPL-3.0-only +// SPDX-License-Identifier: AGPL-3.0-or-later package cmd diff --git a/cmd/iam-healing-leadership_test.go b/cmd/iam-healing-leadership_test.go index 5c30010f2..f10611dc6 100644 --- a/cmd/iam-healing-leadership_test.go +++ b/cmd/iam-healing-leadership_test.go @@ -1,5 +1,5 @@ // Copyright (c) 2026 PGSTY -// SPDX-License-Identifier: AGPL-3.0-only +// SPDX-License-Identifier: AGPL-3.0-or-later package cmd diff --git a/cmd/iam-healing-performance_test.go b/cmd/iam-healing-performance_test.go index c4f463f58..a4bda282d 100644 --- a/cmd/iam-healing-performance_test.go +++ b/cmd/iam-healing-performance_test.go @@ -1,5 +1,5 @@ // Copyright (c) 2026 PGSTY -// SPDX-License-Identifier: AGPL-3.0-only +// SPDX-License-Identifier: AGPL-3.0-or-later package cmd diff --git a/cmd/iam-performance_test.go b/cmd/iam-performance_test.go index 71e4504c0..f24baca0d 100644 --- a/cmd/iam-performance_test.go +++ b/cmd/iam-performance_test.go @@ -1,5 +1,5 @@ // Copyright (c) 2026 PGSTY -// SPDX-License-Identifier: AGPL-3.0-only +// SPDX-License-Identifier: AGPL-3.0-or-later package cmd diff --git a/cmd/iam-replication-protocol.go b/cmd/iam-replication-protocol.go index 37d79e0a6..c26d3586b 100644 --- a/cmd/iam-replication-protocol.go +++ b/cmd/iam-replication-protocol.go @@ -1,5 +1,5 @@ // Copyright (c) 2026 PGSTY -// SPDX-License-Identifier: AGPL-3.0-only +// SPDX-License-Identifier: AGPL-3.0-or-later package cmd diff --git a/cmd/iam-replication-protocol_test.go b/cmd/iam-replication-protocol_test.go index 5c62bec3a..a7da45f9b 100644 --- a/cmd/iam-replication-protocol_test.go +++ b/cmd/iam-replication-protocol_test.go @@ -1,5 +1,5 @@ // Copyright (c) 2026 PGSTY -// SPDX-License-Identifier: AGPL-3.0-only +// SPDX-License-Identifier: AGPL-3.0-or-later package cmd diff --git a/cmd/iam-revision-boundary_test.go b/cmd/iam-revision-boundary_test.go index 98d978fc6..9627e9ed5 100644 --- a/cmd/iam-revision-boundary_test.go +++ b/cmd/iam-revision-boundary_test.go @@ -1,5 +1,5 @@ // Copyright (c) 2026 PGSTY -// SPDX-License-Identifier: AGPL-3.0-only +// SPDX-License-Identifier: AGPL-3.0-or-later package cmd diff --git a/cmd/iam-revision-index.go b/cmd/iam-revision-index.go index 4b4c60d76..120b46697 100644 --- a/cmd/iam-revision-index.go +++ b/cmd/iam-revision-index.go @@ -1,5 +1,5 @@ // Copyright (c) 2026 PGSTY -// SPDX-License-Identifier: AGPL-3.0-only +// SPDX-License-Identifier: AGPL-3.0-or-later package cmd diff --git a/cmd/iam-revision-lock_test.go b/cmd/iam-revision-lock_test.go index 28c5e0cf5..3c745eb73 100644 --- a/cmd/iam-revision-lock_test.go +++ b/cmd/iam-revision-lock_test.go @@ -1,5 +1,5 @@ // Copyright (c) 2026 PGSTY -// SPDX-License-Identifier: AGPL-3.0-only +// SPDX-License-Identifier: AGPL-3.0-or-later package cmd diff --git a/cmd/iam-revision.go b/cmd/iam-revision.go index 15e1e6898..8e4a106a2 100644 --- a/cmd/iam-revision.go +++ b/cmd/iam-revision.go @@ -1,5 +1,5 @@ // Copyright (c) 2026 PGSTY -// SPDX-License-Identifier: AGPL-3.0-only +// SPDX-License-Identifier: AGPL-3.0-or-later package cmd diff --git a/cmd/iam-revision_test.go b/cmd/iam-revision_test.go index 279e7cfd3..70f5d71a5 100644 --- a/cmd/iam-revision_test.go +++ b/cmd/iam-revision_test.go @@ -1,5 +1,5 @@ // Copyright (c) 2026 PGSTY -// SPDX-License-Identifier: AGPL-3.0-only +// SPDX-License-Identifier: AGPL-3.0-or-later package cmd diff --git a/cmd/iam-revocation_test.go b/cmd/iam-revocation_test.go index 9ac756b6f..cc3c3a83f 100644 --- a/cmd/iam-revocation_test.go +++ b/cmd/iam-revocation_test.go @@ -1,5 +1,5 @@ // Copyright (c) 2026 PGSTY -// SPDX-License-Identifier: AGPL-3.0-only +// SPDX-License-Identifier: AGPL-3.0-or-later package cmd diff --git a/cmd/iam-service-recreation_test.go b/cmd/iam-service-recreation_test.go index 02c977858..adbc13f81 100644 --- a/cmd/iam-service-recreation_test.go +++ b/cmd/iam-service-recreation_test.go @@ -1,5 +1,5 @@ // Copyright (c) 2026 PGSTY -// SPDX-License-Identifier: AGPL-3.0-only +// SPDX-License-Identifier: AGPL-3.0-or-later package cmd