From b508264ac44c652f861143ab1ddc86b0fa7dfb86 Mon Sep 17 00:00:00 2001 From: Anis Eleuch Date: Thu, 15 Aug 2024 02:29:20 +0100 Subject: [PATCH] sr: Avoid recursion when loading site replicator credentials (#20262) If the site replication is enabled and the code tries to extract jwt claims while the site replication service account credentials are still not loaded yet, the code will enter an infinite loop, causing in a high CPU usage. Another possibility of the infinite loop is having some service accounts created by an old deployment version where the service account JWT was signed by the root credentials, but not anymore. This commit will remove the possibility of the infinite loop in the code and add root credential fallback to extract claims from old service accounts. --- cmd/iam-etcd-store.go | 12 ++++++++++++ cmd/iam-object-store.go | 12 ++++++++++++ cmd/iam-store.go | 32 +++++++++++++++++++++----------- cmd/iam.go | 2 +- cmd/site-replication.go | 34 ++++++++++++++++++---------------- cmd/sts-handlers.go | 4 ++-- 6 files changed, 66 insertions(+), 30 deletions(-) diff --git a/cmd/iam-etcd-store.go b/cmd/iam-etcd-store.go index ea80e9064..925399ea2 100644 --- a/cmd/iam-etcd-store.go +++ b/cmd/iam-etcd-store.go @@ -249,6 +249,18 @@ func (ies *IAMEtcdStore) addUser(ctx context.Context, user string, userType IAMU return nil } +func (ies *IAMEtcdStore) loadSecretKey(ctx context.Context, user string, userType IAMUserType) (string, error) { + var u UserIdentity + err := ies.loadIAMConfig(ctx, &u, getUserIdentityPath(user, userType)) + if err != nil { + if errors.Is(err, errConfigNotFound) { + return "", errNoSuchUser + } + return "", err + } + return u.Credentials.SecretKey, nil +} + func (ies *IAMEtcdStore) loadUser(ctx context.Context, user string, userType IAMUserType, m map[string]UserIdentity) error { var u UserIdentity err := ies.loadIAMConfig(ctx, &u, getUserIdentityPath(user, userType)) diff --git a/cmd/iam-object-store.go b/cmd/iam-object-store.go index c874b728a..93a488ab2 100644 --- a/cmd/iam-object-store.go +++ b/cmd/iam-object-store.go @@ -225,6 +225,18 @@ func (iamOS *IAMObjectStore) loadPolicyDocs(ctx context.Context, m map[string]Po return nil } +func (iamOS *IAMObjectStore) loadSecretKey(ctx context.Context, user string, userType IAMUserType) (string, error) { + var u UserIdentity + err := iamOS.loadIAMConfig(ctx, &u, getUserIdentityPath(user, userType)) + if err != nil { + if errors.Is(err, errConfigNotFound) { + return "", errNoSuchUser + } + return "", err + } + return u.Credentials.SecretKey, nil +} + func (iamOS *IAMObjectStore) loadUser(ctx context.Context, user string, userType IAMUserType, m map[string]UserIdentity) error { var u UserIdentity err := iamOS.loadIAMConfig(ctx, &u, getUserIdentityPath(user, userType)) diff --git a/cmd/iam-store.go b/cmd/iam-store.go index b47d3509e..30add72ab 100644 --- a/cmd/iam-store.go +++ b/cmd/iam-store.go @@ -535,6 +535,7 @@ type IAMStorageAPI interface { loadPolicyDocWithRetry(ctx context.Context, policy string, m map[string]PolicyDoc, retries int) error loadPolicyDocs(ctx context.Context, m map[string]PolicyDoc) error loadUser(ctx context.Context, user string, userType IAMUserType, m map[string]UserIdentity) error + loadSecretKey(ctx context.Context, user string, userType IAMUserType) (string, error) loadUsers(ctx context.Context, userType IAMUserType, m map[string]UserIdentity) error loadGroup(ctx context.Context, group string, m map[string]GroupInfo) error loadGroups(ctx context.Context, m map[string]GroupInfo) error @@ -2820,20 +2821,29 @@ func (store *IAMStoreSys) LoadUser(ctx context.Context, accessKey string) error return err } -func extractJWTClaims(u UserIdentity) (*jwt.MapClaims, error) { - jwtClaims, err := auth.ExtractClaims(u.Credentials.SessionToken, u.Credentials.SecretKey) - if err != nil { - secretKey, err := getTokenSigningKey() - if err != nil { - return nil, err +func extractJWTClaims(u UserIdentity) (jwtClaims *jwt.MapClaims, err error) { + keys := make([]string, 0, 3) + // Append credentials secret key itself + keys = append(keys, u.Credentials.SecretKey) + // Use site-replication credentials if found + if globalSiteReplicationSys.isEnabled() { + siteReplSecretKey, e := globalSiteReplicatorCred.Get(GlobalContext) + if e != nil { + return nil, e } - // Session tokens for STS creds will be generated with root secret or site-replicator-0 secret - jwtClaims, err = auth.ExtractClaims(u.Credentials.SessionToken, secretKey) - if err != nil { - return nil, err + keys = append(keys, siteReplSecretKey) + } + // Use root credentials for credentials created with older deployments + keys = append(keys, globalActiveCred.SecretKey) + + // Iterate over all keys and return with the first successful claim extraction + for _, key := range keys { + jwtClaims, err = auth.ExtractClaims(u.Credentials.SessionToken, key) + if err == nil { + break } } - return jwtClaims, nil + return } func validateSvcExpirationInUTC(expirationInUTC time.Time) error { diff --git a/cmd/iam.go b/cmd/iam.go index 51120e543..8de0904d9 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -211,7 +211,7 @@ func (sys *IAMSys) Load(ctx context.Context, firstTime bool) error { if !globalSiteReplicatorCred.IsValid() { sa, _, err := sys.getServiceAccount(ctx, siteReplicatorSvcAcc) if err == nil { - globalSiteReplicatorCred.Set(sa.Credentials) + globalSiteReplicatorCred.Set(sa.Credentials.SecretKey) } } diff --git a/cmd/site-replication.go b/cmd/site-replication.go index 48fa78a3d..265c99821 100644 --- a/cmd/site-replication.go +++ b/cmd/site-replication.go @@ -597,7 +597,7 @@ func (c *SiteReplicationSys) AddPeerClusters(ctx context.Context, psites []madmi } if !globalSiteReplicatorCred.IsValid() { - globalSiteReplicatorCred.Set(svcCred) + globalSiteReplicatorCred.Set(svcCred.SecretKey) } result := madmin.ReplicateAddStatus{ Success: true, @@ -659,7 +659,7 @@ func (c *SiteReplicationSys) PeerJoinReq(ctx context.Context, arg madmin.SRPeerJ return errSRBackendIssue(fmt.Errorf("unable to save cluster-replication state to drive on %s: %v", ourName, err)) } if !globalSiteReplicatorCred.IsValid() { - globalSiteReplicatorCred.Set(sa) + globalSiteReplicatorCred.Set(sa.SecretKey) } return nil @@ -6269,34 +6269,36 @@ func ilmExpiryReplicationEnabled(sites map[string]madmin.PeerInfo) bool { } type siteReplicatorCred struct { - Creds auth.Credentials + secretKey string sync.RWMutex } // Get or attempt to load site replicator credentials from disk. -func (s *siteReplicatorCred) Get(ctx context.Context) (auth.Credentials, error) { +func (s *siteReplicatorCred) Get(ctx context.Context) (string, error) { s.RLock() - if s.Creds.IsValid() { - s.RUnlock() - return s.Creds, nil - } + secretKey := s.secretKey s.RUnlock() - m := make(map[string]UserIdentity) - if err := globalIAMSys.store.loadUser(ctx, siteReplicatorSvcAcc, svcUser, m); err != nil { - return auth.Credentials{}, err + + if secretKey != "" { + return secretKey, nil } - s.Set(m[siteReplicatorSvcAcc].Credentials) - return m[siteReplicatorSvcAcc].Credentials, nil + + secretKey, err := globalIAMSys.store.loadSecretKey(ctx, siteReplicatorSvcAcc, svcUser) + if err != nil { + return "", err + } + s.Set(secretKey) + return secretKey, nil } -func (s *siteReplicatorCred) Set(c auth.Credentials) { +func (s *siteReplicatorCred) Set(secretKey string) { s.Lock() defer s.Unlock() - s.Creds = c + s.secretKey = secretKey } func (s *siteReplicatorCred) IsValid() bool { s.RLock() defer s.RUnlock() - return s.Creds.IsValid() + return s.secretKey != "" } diff --git a/cmd/sts-handlers.go b/cmd/sts-handlers.go index 3fee6bfbe..10621d064 100644 --- a/cmd/sts-handlers.go +++ b/cmd/sts-handlers.go @@ -241,11 +241,11 @@ func parseForm(r *http.Request) error { func getTokenSigningKey() (string, error) { secret := globalActiveCred.SecretKey if globalSiteReplicationSys.isEnabled() { - c, err := globalSiteReplicatorCred.Get(GlobalContext) + secretKey, err := globalSiteReplicatorCred.Get(GlobalContext) if err != nil { return "", err } - return c.SecretKey, nil + return secretKey, nil } return secret, nil }