OpenBao AWS Plugin Vulnerable to Cross-Account IAM Role Impersonation in AWS Auth Method

@@ -73,17 +73,17 @@ type backend struct {
 	// of tidyCooldownPeriod.
 	nextTidyTime time.Time
 
-	// Map to hold the EC2 client objects indexed by region and STS role.
+	// Map to hold the EC2 client objects indexed by region, account ID, and STS role.
 	// This avoids the overhead of creating a client object for every login request.
 	// When the credentials are modified or deleted, all the cached client objects
 	// will be flushed. The empty STS role signifies the master account
-	EC2ClientsMap map[string]map[string]*ec2.EC2
+	EC2ClientsMap map[string]map[string]map[string]*ec2.EC2
 
-	// Map to hold the IAM client objects indexed by region and STS role.
+	// Map to hold the IAM client objects indexed by region, account ID, and STS role.
 	// This avoids the overhead of creating a client object for every login request.
 	// When the credentials are modified or deleted, all the cached client objects
 	// will be flushed. The empty STS role signifies the master account
-	IAMClientsMap map[string]map[string]*iam.IAM
+	IAMClientsMap map[string]map[string]map[string]*iam.IAM
 
 	// Map to associate a partition to a random region in that partition. Users of
 	// this don't care what region in the partition they use, but there is some client
@@ -122,8 +122,8 @@ func Backend(_ *logical.BackendConfig) (*backend, error) {
 		// Setting the periodic func to be run once in an hour.
 		// If there is a real need, this can be made configurable.
 		tidyCooldownPeriod:     time.Hour,
-		EC2ClientsMap:          make(map[string]map[string]*ec2.EC2),
-		IAMClientsMap:          make(map[string]map[string]*iam.IAM),
+		EC2ClientsMap:          make(map[string]map[string]map[string]*ec2.EC2),
+		IAMClientsMap:          make(map[string]map[string]map[string]*iam.IAM),
 		iamUserIdToArnCache:    cache.New(7*24*time.Hour, 24*time.Hour),
 		tidyDenyListCASGuard:   new(uint32),
 		tidyAccessListCASGuard: new(uint32),

@@ -190,6 +190,12 @@ func (b *backend) stsRoleForAccount(ctx context.Context, s logical.Storage, acco
 	if sts != nil {
 		return sts.StsRole, nil
 	}
+
+	// Return an error if there's no STS config for an account which is not the default one
+	if b.defaultAWSAccountID != "" && b.defaultAWSAccountID != accountID {
+		return "", fmt.Errorf("no STS configuration found for account ID %q", accountID)
+	}
+
 	return "", nil
 }
 
@@ -200,20 +206,26 @@ func (b *backend) clientEC2(ctx context.Context, s logical.Storage, region, acco
 		return nil, err
 	}
 	b.configMutex.RLock()
-	if b.EC2ClientsMap[region] != nil && b.EC2ClientsMap[region][stsRole] != nil {
+	if b.EC2ClientsMap[region] != nil &&
+		b.EC2ClientsMap[region][accountID] != nil &&
+		b.EC2ClientsMap[region][accountID][stsRole] != nil {
 		defer b.configMutex.RUnlock()
 		// If the client object was already created, return it
-		return b.EC2ClientsMap[region][stsRole], nil
+		b.Logger().Debug(fmt.Sprintf("returning cached client for region %s, account %s and stsRole %s", region, accountID, stsRole))
+		return b.EC2ClientsMap[region][accountID][stsRole], nil
 	}
+	b.Logger().Debug(fmt.Sprintf("no cached client for region %s, account %s and stsRole %s", region, accountID, stsRole))
 
 	// Release the read lock and acquire the write lock
 	b.configMutex.RUnlock()
 	b.configMutex.Lock()
 	defer b.configMutex.Unlock()
 
 	// If the client gets created while switching the locks, return it
-	if b.EC2ClientsMap[region] != nil && b.EC2ClientsMap[region][stsRole] != nil {
-		return b.EC2ClientsMap[region][stsRole], nil
+	if b.EC2ClientsMap[region] != nil &&
+		b.EC2ClientsMap[region][accountID] != nil &&
+		b.EC2ClientsMap[region][accountID][stsRole] != nil {
+		return b.EC2ClientsMap[region][accountID][stsRole], nil
 	}
 
 	// Create an AWS config object using a chain of providers
@@ -237,13 +249,16 @@ func (b *backend) clientEC2(ctx context.Context, s logical.Storage, region, acco
 	if client == nil {
 		return nil, fmt.Errorf("could not obtain ec2 client")
 	}
+
 	if _, ok := b.EC2ClientsMap[region]; !ok {
-		b.EC2ClientsMap[region] = map[string]*ec2.EC2{stsRole: client}
-	} else {
-		b.EC2ClientsMap[region][stsRole] = client
+		b.EC2ClientsMap[region] = make(map[string]map[string]*ec2.EC2)
 	}
+	if _, ok := b.EC2ClientsMap[region][accountID]; !ok {
+		b.EC2ClientsMap[region][accountID] = make(map[string]*ec2.EC2)
+	}
+	b.EC2ClientsMap[region][accountID][stsRole] = client
 
-	return b.EC2ClientsMap[region][stsRole], nil
+	return b.EC2ClientsMap[region][accountID][stsRole], nil
 }
 
 // clientIAM creates a client to interact with AWS IAM API
@@ -258,22 +273,26 @@ func (b *backend) clientIAM(ctx context.Context, s logical.Storage, region, acco
 		b.Logger().Debug(fmt.Sprintf("found stsRole %s for account %s", stsRole, accountID))
 	}
 	b.configMutex.RLock()
-	if b.IAMClientsMap[region] != nil && b.IAMClientsMap[region][stsRole] != nil {
+	if b.IAMClientsMap[region] != nil &&
+		b.IAMClientsMap[region][accountID] != nil &&
+		b.IAMClientsMap[region][accountID][stsRole] != nil {
 		defer b.configMutex.RUnlock()
 		// If the client object was already created, return it
-		b.Logger().Debug(fmt.Sprintf("returning cached client for region %s and stsRole %s", region, stsRole))
-		return b.IAMClientsMap[region][stsRole], nil
+		b.Logger().Debug(fmt.Sprintf("returning cached client for region %s, account %s and stsRole %s", region, accountID, stsRole))
+		return b.IAMClientsMap[region][accountID][stsRole], nil
 	}
-	b.Logger().Debug(fmt.Sprintf("no cached client for region %s and stsRole %s", region, stsRole))
+	b.Logger().Debug(fmt.Sprintf("no cached client for region %s, account %s and stsRole %s", region, accountID, stsRole))
 
 	// Release the read lock and acquire the write lock
 	b.configMutex.RUnlock()
 	b.configMutex.Lock()
 	defer b.configMutex.Unlock()
 
 	// If the client gets created while switching the locks, return it
-	if b.IAMClientsMap[region] != nil && b.IAMClientsMap[region][stsRole] != nil {
-		return b.IAMClientsMap[region][stsRole], nil
+	if b.IAMClientsMap[region] != nil &&
+		b.IAMClientsMap[region][accountID] != nil &&
+		b.IAMClientsMap[region][accountID][stsRole] != nil {
+		return b.IAMClientsMap[region][accountID][stsRole], nil
 	}
 
 	// Create an AWS config object using a chain of providers
@@ -297,10 +316,14 @@ func (b *backend) clientIAM(ctx context.Context, s logical.Storage, region, acco
 	if client == nil {
 		return nil, fmt.Errorf("could not obtain iam client")
 	}
+
 	if _, ok := b.IAMClientsMap[region]; !ok {
-		b.IAMClientsMap[region] = map[string]*iam.IAM{stsRole: client}
-	} else {
-		b.IAMClientsMap[region][stsRole] = client
+		b.IAMClientsMap[region] = make(map[string]map[string]*iam.IAM)
 	}
-	return b.IAMClientsMap[region][stsRole], nil
+	if _, ok := b.IAMClientsMap[region][accountID]; !ok {
+		b.IAMClientsMap[region][accountID] = make(map[string]*iam.IAM)
+	}
+	b.IAMClientsMap[region][accountID][stsRole] = client
+
+	return b.IAMClientsMap[region][accountID][stsRole], nil
 }

@@ -0,0 +1,72 @@
+// Copyright (c) 2025 OpenBao a Series of LF Projects, LLC
+// SPDX-License-Identifier: MPL-2.0
+
+package aws
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/openbao/openbao/sdk/v2/logical"
+)
+
+// TestClientCache verifies that IAM clients for different
+// AWS accounts are properly isolated in the cache
+func TestClientCache(t *testing.T) {
+	config := logical.TestBackendConfig()
+	storage := &logical.InmemStorage{}
+	config.StorageView = storage
+
+	b, err := Backend(config)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ctx := context.Background()
+	if err := b.Setup(ctx, config); err != nil {
+		t.Fatal(err)
+	}
+
+	account1 := "111111111111"
+	account2 := "222222222222"
+
+	b.defaultAWSAccountID = account1
+
+	// This should work - same account as default
+	stsRole, err := b.stsRoleForAccount(ctx, storage, account1)
+	if err != nil {
+		t.Fatalf("Expected success for default account, got error: %v", err)
+	}
+	if stsRole != "" {
+		t.Fatalf("Expected empty STS role for default account, got: %v", stsRole)
+	}
+
+	// This should fail - different account without STS config
+	_, err = b.stsRoleForAccount(ctx, storage, account2)
+	if err == nil {
+		t.Fatal("Expected error for cross-account access without STS config")
+	}
+
+	// Verify the error message contains the expected error
+	expectedError := fmt.Sprintf("no STS configuration found for account ID %q", account2)
+	if err.Error() != expectedError {
+		t.Fatalf("Expected specific error message, got: %v", err)
+	}
+
+	stsEntry := &awsStsEntry{
+		StsRole: "arn:aws:iam::222222222222:role/cross-account-role",
+	}
+	err = b.lockedSetAwsStsEntry(ctx, storage, account2, stsEntry)
+	if err != nil {
+		t.Fatalf("Failed to set STS entry: %v", err)
+	}
+
+	stsRole, err = b.stsRoleForAccount(ctx, storage, account2)
+	if err != nil {
+		t.Fatalf("Expected success for account with STS config, got error: %v", err)
+	}
+	if stsRole != stsEntry.StsRole {
+		t.Fatalf("Expected STS role %v, got: %v", stsEntry.StsRole, stsRole)
+	}
+}

@@ -177,8 +177,8 @@ func (b *backend) pathConfigRotateRootUpdate(ctx context.Context, req *logical.R
 
 	// Previous cached clients need to be cleared because they may have been made using
 	// the soon-to-be-obsolete credentials.
-	b.IAMClientsMap = make(map[string]map[string]*iam.IAM)
-	b.EC2ClientsMap = make(map[string]map[string]*ec2.EC2)
+	b.IAMClientsMap = make(map[string]map[string]map[string]*iam.IAM)
+	b.EC2ClientsMap = make(map[string]map[string]map[string]*ec2.EC2)
 
 	// Now to clean up the old key.
 	deleteAccessKeyInput := iam.DeleteAccessKeyInput{