diff --git a/agent/imdscreds/refresher.go b/agent/imdscreds/refresher.go index 94ba9aa0a27..f38647bb4b9 100644 --- a/agent/imdscreds/refresher.go +++ b/agent/imdscreds/refresher.go @@ -96,7 +96,7 @@ func (r *IMDSCredentialsRefresher) refresh() { return } - creds, err := r.scanner.Scan(r.ctx) + result, err := r.scanner.Scan(r.ctx) if err != nil { logger.Error("IMDS credentials refresh: scan failed", logger.Fields{ field.Error: err, @@ -106,7 +106,7 @@ func (r *IMDSCredentialsRefresher) refresh() { // upsertedCredCount tallies credentials written to the credentials manager. upsertedCredCount := 0 - for _, cred := range creds { + for _, cred := range result.Credentials { task, ok := nonTerminalTasks[cred.TaskID] if !ok { // Credential for a task that's either terminal or unknown @@ -128,10 +128,11 @@ func (r *IMDSCredentialsRefresher) refresh() { upsertedCredCount++ } - if len(creds) > 0 { + if len(result.Credentials) > 0 || len(result.AssumeRoleUnauthorizedAccessRoles) > 0 { logger.Info("IMDS credentials refresh: scan complete", logger.Fields{ - "retrievedCredentialCount": len(creds), - "upsertedCredentialCount": upsertedCredCount, + "retrievedCredentialCount": len(result.Credentials), + "upsertedCredentialCount": upsertedCredCount, + "assumeRoleUnauthorizedAccessRoleCount": len(result.AssumeRoleUnauthorizedAccessRoles), }) } } diff --git a/agent/imdscreds/refresher_integ_test.go b/agent/imdscreds/refresher_integ_test.go index 7b8a6fd59b4..9a703d00d06 100644 --- a/agent/imdscreds/refresher_integ_test.go +++ b/agent/imdscreds/refresher_integ_test.go @@ -120,15 +120,18 @@ func TestIMDSCredentialsRefresh(t *testing.T) { mockIMDS.AddCredential( "iam-ecs-1", taskID1, credentials.ApplicationRoleType, roleARN1, "AKID_IMDS_A_APP", + ecsagentimds.CredentialCodeSuccess, ) // Namespace 2: taskB with application + execution roles. mockIMDS.AddCredential( "iam-ecs-2", taskID2, credentials.ApplicationRoleType, roleARN2, "AKID_IMDS_B_APP", + ecsagentimds.CredentialCodeSuccess, ) mockIMDS.AddCredential( "iam-ecs-2", taskID2, credentials.ExecutionRoleType, roleARN2, "AKID_IMDS_B_EXEC", + ecsagentimds.CredentialCodeSuccess, ) // Wait for the refresher to pick up the new credentials from IMDS diff --git a/agent/imdscreds/refresher_test.go b/agent/imdscreds/refresher_test.go index fce854621ad..c327a3574ff 100644 --- a/agent/imdscreds/refresher_test.go +++ b/agent/imdscreds/refresher_test.go @@ -79,12 +79,13 @@ func newTestTask( func TestRefresh(t *testing.T) { tests := []struct { - name string - listTasksErr error - tasks []*apitask.Task - scanResult []imds.TaskCredential - scanErr error - expectedUpserts []*credentials.TaskIAMRoleCredentials + name string + listTasksErr error + tasks []*apitask.Task + scanCreds []imds.TaskCredential + scanAssumeRoleUnauthorizedAccess []imds.AssumeRoleUnauthorizedAccessIAMRole + scanErr error + expectedUpserts []*credentials.TaskIAMRoleCredentials }{ { name: "no tasks skips scan", @@ -117,7 +118,7 @@ func TestRefresh(t *testing.T) { execCredID: testCredID2, execRoleArn: testRoleARN2, }), }, - scanResult: []imds.TaskCredential{ + scanCreds: []imds.TaskCredential{ { TaskID: testTaskID1, RoleType: credentials.ApplicationRoleType, @@ -151,7 +152,7 @@ func TestRefresh(t *testing.T) { execCredID: testCredID2, execRoleArn: testRoleARN2, }), }, - scanResult: []imds.TaskCredential{ + scanCreds: []imds.TaskCredential{ { TaskID: testTaskID1, RoleType: credentials.ExecutionRoleType, @@ -183,7 +184,7 @@ func TestRefresh(t *testing.T) { newTestTask(testTaskARN1, status.TaskRunning, testTaskOpts{credID: testCredID1, roleArn: testRoleARN1}), }, - scanResult: []imds.TaskCredential{ + scanCreds: []imds.TaskCredential{ { TaskID: "unknown00000000000000000000000000", RoleType: credentials.ApplicationRoleType, @@ -196,7 +197,7 @@ func TestRefresh(t *testing.T) { tasks: []*apitask.Task{ newTestTask(testTaskARN1, status.TaskRunning, testTaskOpts{}), }, - scanResult: []imds.TaskCredential{ + scanCreds: []imds.TaskCredential{ { TaskID: testTaskID1, RoleType: credentials.ApplicationRoleType, @@ -210,7 +211,7 @@ func TestRefresh(t *testing.T) { newTestTask(testTaskARN1, status.TaskRunning, testTaskOpts{credID: testCredID1, roleArn: testRoleARN1}), }, - scanResult: []imds.TaskCredential{ + scanCreds: []imds.TaskCredential{ { TaskID: testTaskID1, RoleType: credentials.ApplicationRoleType, @@ -226,7 +227,7 @@ func TestRefresh(t *testing.T) { newTestTask(testTaskARN2, status.TaskRunning, testTaskOpts{credID: testCredID3, roleArn: testRoleARN3}), }, - scanResult: []imds.TaskCredential{ + scanCreds: []imds.TaskCredential{ { TaskID: testTaskID1, RoleType: credentials.ApplicationRoleType, @@ -281,7 +282,7 @@ func TestRefresh(t *testing.T) { execCredID: testCredID2, execRoleArn: testRoleARN1, }), }, - scanResult: []imds.TaskCredential{ + scanCreds: []imds.TaskCredential{ { TaskID: testTaskID1, RoleType: credentials.ApplicationRoleType, @@ -328,6 +329,47 @@ func TestRefresh(t *testing.T) { }, }, }, + { + name: "unauthorized roles are not upserted", + tasks: []*apitask.Task{ + newTestTask(testTaskARN1, status.TaskRunning, testTaskOpts{ + credID: testCredID1, roleArn: testRoleARN1, + execCredID: testCredID2, execRoleArn: testRoleARN2, + }), + }, + scanCreds: []imds.TaskCredential{ + { + TaskID: testTaskID1, + RoleType: credentials.ApplicationRoleType, + RoleArn: testRoleARN1, + AccessKeyID: "AKID_TASK", + SecretAccessKey: "secret_task", + SessionToken: "token_task", + Expiration: "2026-05-05T12:00:00Z", + }, + }, + scanAssumeRoleUnauthorizedAccess: []imds.AssumeRoleUnauthorizedAccessIAMRole{ + { + TaskID: testTaskID1, + RoleType: credentials.ExecutionRoleType, + RoleArn: testRoleARN2, + }, + }, + expectedUpserts: []*credentials.TaskIAMRoleCredentials{ + { + ARN: testTaskARN1, + IAMRoleCredentials: credentials.IAMRoleCredentials{ + CredentialsID: testCredID1, + RoleArn: testRoleARN1, + AccessKeyID: "AKID_TASK", + SecretAccessKey: "secret_task", + SessionToken: "token_task", + Expiration: "2026-05-05T12:00:00Z", + RoleType: credentials.ApplicationRoleType, + }, + }, + }, + }, } for _, tc := range tests { @@ -344,7 +386,10 @@ func TestRefresh(t *testing.T) { if tc.listTasksErr == nil && len(nonTerminalTasksByID(tc.tasks)) > 0 { mockScanner.EXPECT(). Scan(gomock.Any()). - Return(tc.scanResult, tc.scanErr) + Return(imds.ScanResult{ + Credentials: tc.scanCreds, + AssumeRoleUnauthorizedAccessRoles: tc.scanAssumeRoleUnauthorizedAccess, + }, tc.scanErr) } if len(tc.expectedUpserts) > 0 { diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/mocks/imds_mocks.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/mocks/imds_mocks.go index e79bc637d61..848a38c484b 100644 --- a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/mocks/imds_mocks.go +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/mocks/imds_mocks.go @@ -50,10 +50,10 @@ func (m *MockScanner) EXPECT() *MockScannerMockRecorder { } // Scan mocks base method. -func (m *MockScanner) Scan(arg0 context.Context) ([]imds.TaskCredential, error) { +func (m *MockScanner) Scan(arg0 context.Context) (imds.ScanResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Scan", arg0) - ret0, _ := ret[0].([]imds.TaskCredential) + ret0, _ := ret[0].(imds.ScanResult) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/scanner.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/scanner.go index cb65dabb965..54f86d09e02 100644 --- a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/scanner.go +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/scanner.go @@ -74,8 +74,9 @@ const ( // Scanner fetches task credentials from IMDS iam-ecs-* namespaces. type Scanner interface { // Scan discovers all ECS IAM namespaces, reads their info files, and - // fetches credentials from namespaces that have changed since the last scan. - Scan(ctx context.Context) ([]TaskCredential, error) + // fetches credentials from namespaces that have changed since the last + // scan. + Scan(ctx context.Context) (ScanResult, error) } // scanner implements the Scanner interface. @@ -102,22 +103,22 @@ func NewScanner(ec2MetadataClient ec2.EC2MetadataClient, // Scan discovers all ECS IAM namespaces, reads their info files, and // fetches credentials from namespaces that have changed since the last scan. -func (s *scanner) Scan(ctx context.Context) ([]TaskCredential, error) { +func (s *scanner) Scan(ctx context.Context) (ScanResult, error) { namespaces, err := s.discoverNamespaces(ctx) if err != nil { - return nil, fmt.Errorf("imds scan: discover namespaces: %w", err) + return ScanResult{}, fmt.Errorf("imds scan: discover namespaces: %w", err) } // No namespaces is expected when IMDS does not have ECS task credentials yet. if len(namespaces) == 0 { logger.Debug("IMDS credentials scan: no iam-ecs namespace found") - return nil, nil + return ScanResult{}, nil } - var creds []TaskCredential + var result ScanResult var scanErrors []error for _, ns := range namespaces { - nsCreds, err := s.scanNamespace(ctx, ns) + nsResult, err := s.scanNamespace(ctx, ns) if err != nil { logger.Error("IMDS credentials scan: failed to scan namespace", logger.Fields{ "namespace": ns, @@ -128,17 +129,18 @@ func (s *scanner) Scan(ctx context.Context) ([]TaskCredential, error) { // namespaces before returning. continue } - creds = append(creds, nsCreds...) + result.Credentials = append(result.Credentials, nsResult.Credentials...) + result.AssumeRoleUnauthorizedAccessRoles = append(result.AssumeRoleUnauthorizedAccessRoles, nsResult.AssumeRoleUnauthorizedAccessRoles...) } - // Surface an error when scanning all namespaces failed so - // the caller doesn't mistake it for "no credentials yet". - if len(creds) == 0 && len(scanErrors) > 0 { - return nil, fmt.Errorf("imds scan: all %d namespace(s) failed: %w", + // Surface an error only when neither credentials nor unauthorized roles were + // read, so the caller doesn't mistake a total failure for "no credentials yet". + if len(result.Credentials) == 0 && len(result.AssumeRoleUnauthorizedAccessRoles) == 0 && len(scanErrors) > 0 { + return ScanResult{}, fmt.Errorf("imds scan: all %d namespace(s) failed: %w", len(scanErrors), errors.Join(scanErrors...)) } - return creds, nil + return result, nil } // discoverNamespaces lists the IMDS metadata root and returns all @@ -164,10 +166,10 @@ func (s *scanner) discoverNamespaces(ctx context.Context) ([]string, error) { return namespaces, nil } -// scanNamespace reads the info file for a namespace and fetches -// credentials for each entry. Only successfully fetched credentials -// are returned. -func (s *scanner) scanNamespace(ctx context.Context, namespace string) ([]TaskCredential, error) { +// scanNamespace reads the info file for a namespace and fetches credentials for each entry. +// It returns a list of successfully fetched credentials and a list of IAM roles for which +// the "Code" in the info file was AssumeRoleUnauthorizedAccess. +func (s *scanner) scanNamespace(ctx context.Context, namespace string) (ScanResult, error) { infoPath := fmt.Sprintf(infoPathFormat, namespace) infoResp, err := s.getMetadata(ctx, infoPath) if err != nil { @@ -175,7 +177,7 @@ func (s *scanner) scanNamespace(ctx context.Context, namespace string) ([]TaskCr WithFields(map[string]any{ metricFieldNamespace: namespace, }).Done(err) - return nil, fmt.Errorf("fetch info for %s: %w", namespace, err) + return ScanResult{}, fmt.Errorf("fetch info for %s: %w", namespace, err) } var info NamespaceInfo @@ -184,7 +186,7 @@ func (s *scanner) scanNamespace(ctx context.Context, namespace string) ([]TaskCr WithFields(map[string]any{ metricFieldNamespace: namespace, }).Done(err) - return nil, fmt.Errorf("parse info for %s: %w", namespace, err) + return ScanResult{}, fmt.Errorf("parse info for %s: %w", namespace, err) } // Skip credential fetches if the namespace hasn't been updated since the last scan. @@ -194,16 +196,16 @@ func (s *scanner) scanNamespace(ctx context.Context, namespace string) ([]TaskCr WithFields(map[string]any{ metricFieldNamespace: namespace, }).Done(err) - return nil, fmt.Errorf("parse LastUpdated for %s: %w", namespace, err) + return ScanResult{}, fmt.Errorf("parse LastUpdated for %s: %w", namespace, err) } if cached, ok := s.lastUpdated[namespace]; ok && lastUpdated.Equal(cached) { logger.Debug("IMDS credentials scan: skipping namespace with unchanged LastUpdated", logger.Fields{ "namespace": namespace, }) - return nil, nil + return ScanResult{}, nil } - var creds []TaskCredential + var result ScanResult var hasErrors bool for key, entry := range info.TaskCredentials { taskID, roleType, err := parseCredentialKey(key) @@ -221,6 +223,37 @@ func (s *scanner) scanNamespace(ctx context.Context, namespace string) ([]TaskCr continue } + // A Code of AssumeRoleUnauthorizedAccess means the provider was not + // authorized to assume the role, so no credential file was written for + // this entry. + if isCredentialAssumeRoleUnauthorizedAccess(entry.Code) { + logger.Debug("IMDS credentials scan: provider not authorized to assume the IAM role", + logger.Fields{ + field.TaskID: taskID, + "roleType": roleType, + "namespace": namespace, + }) + result.AssumeRoleUnauthorizedAccessRoles = append(result.AssumeRoleUnauthorizedAccessRoles, AssumeRoleUnauthorizedAccessIAMRole{ + TaskID: taskID, + RoleType: roleType, + RoleArn: entry.RoleArn, + }) + continue + } + + // Success and AssumeRoleUnauthorizedAccess are the only codes the agent + // interprets. An unrecognized code is logged and then fetched anyway: a + // credential file may still exist for it. + if !isCredentialDelivered(entry.Code) { + logger.Warn("IMDS credentials scan: unrecognized credential code", + logger.Fields{ + field.TaskID: taskID, + "roleType": roleType, + "namespace": namespace, + "code": entry.Code, + }) + } + credPath := fmt.Sprintf(credentialPathFormat, namespace, key) credResp, err := s.getMetadata(ctx, credPath) if err != nil { @@ -284,7 +317,7 @@ func (s *scanner) scanNamespace(ctx context.Context, namespace string) ([]TaskCr "namespace": namespace, "expiration": imdsCred.Expiration, }) - creds = append(creds, TaskCredential{ + result.Credentials = append(result.Credentials, TaskCredential{ TaskID: taskID, RoleType: roleType, RoleArn: entry.RoleArn, @@ -301,19 +334,32 @@ func (s *scanner) scanNamespace(ctx context.Context, namespace string) ([]TaskCr s.lastUpdated[namespace] = lastUpdated } - // Surface an error when the namespace yielded no credentials and also had + // Surface an error when the namespace yielded nothing and also had // failures, so callers don't mistake it for "no credentials yet". - if len(creds) == 0 && hasErrors { - return nil, fmt.Errorf("all credential processing failed for %s", namespace) + if len(result.Credentials) == 0 && len(result.AssumeRoleUnauthorizedAccessRoles) == 0 && hasErrors { + return ScanResult{}, fmt.Errorf("all credential processing failed for %s", namespace) } logger.Info("IMDS credentials scan: namespace scan complete", logger.Fields{ - "namespace": namespace, - "retrievedCredentialCount": len(creds), - "lastUpdated": info.LastUpdated, + "namespace": namespace, + "retrievedCredentialCount": len(result.Credentials), + "assumeRoleUnauthorizedAccessRoleCount": len(result.AssumeRoleUnauthorizedAccessRoles), + "lastUpdated": info.LastUpdated, }) - return creds, nil + return result, nil +} + +// isCredentialDelivered reports whether an info file entry's Code says a +// credential file was successfully written for it. +func isCredentialDelivered(code string) bool { + return strings.EqualFold(code, CredentialCodeSuccess) +} + +// isCredentialAssumeRoleUnauthorizedAccess reports whether an info file entry's +// Code says the provider was not authorized to assume the IAM role. +func isCredentialAssumeRoleUnauthorizedAccess(code string) bool { + return strings.EqualFold(code, CredentialCodeAssumeRoleUnauthorizedAccess) } // parseCredentialKey extracts the task ID and role type from an IMDS key. diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/testutil/mock_imds_server.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/testutil/mock_imds_server.go index 577933d735c..73ef258e683 100644 --- a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/testutil/mock_imds_server.go +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/testutil/mock_imds_server.go @@ -27,6 +27,8 @@ import ( "sync" "testing" "time" + + "github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds" ) // MockIMDSServer is an HTTP server that mimics the EC2 IMDS @@ -43,6 +45,7 @@ type mockNamespace struct { } type mockCredential struct { + Code string RoleArn string AccessKeyID string SecretAccessKey string @@ -69,23 +72,33 @@ func (s *MockIMDSServer) Close() { s.server.Close() } -// AddCredential registers a credential in the mock IMDS server. -// The namespace is auto-created if it doesn't exist. +// AddCredential registers an entry with the given info file Code in the mock +// IMDS server. The namespace is auto-created if it doesn't exist. accessKeyID is +// ignored unless the Code is Success, since no credential file is served for any +// other Code. func (s *MockIMDSServer) AddCredential( - namespace, taskID, roleType, roleArn, accessKeyID string, + namespace, taskID, roleType, roleArn, accessKeyID, code string, ) { s.mu.Lock() defer s.mu.Unlock() ns := s.getOrCreateNamespace(namespace) key := taskID + "-" + roleType - ns.credentials[key] = &mockCredential{ - RoleArn: roleArn, - AccessKeyID: accessKeyID, - SecretAccessKey: "secret-" + accessKeyID, - SessionToken: "token-" + accessKeyID, - Expiration: time.Now().Add(1 * time.Hour).UTC().Format(time.RFC3339), + cred := &mockCredential{ + Code: code, + RoleArn: roleArn, + } + + // A non-Success entry is advertised in the info file with no credential file + // behind it, so it carries no credential material. + if code == imds.CredentialCodeSuccess { + cred.AccessKeyID = accessKeyID + cred.SecretAccessKey = "secret-" + accessKeyID + cred.SessionToken = "token-" + accessKeyID + cred.Expiration = time.Now().Add(1 * time.Hour).UTC().Format(time.RFC3339) } + + ns.credentials[key] = cred ns.lastUpdated = time.Now().UTC() } @@ -110,6 +123,7 @@ func (s *MockIMDSServer) RotateCredential( t.Fatalf("mock IMDS: credential %q not found in namespace %q", key, namespace) } + cred.Code = imds.CredentialCodeSuccess cred.AccessKeyID = newAccessKeyID cred.SecretAccessKey = "secret-" + newAccessKeyID cred.SessionToken = "token-" + newAccessKeyID @@ -193,6 +207,7 @@ func (s *MockIMDSServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { // LastUpdated and the TaskCredentials map. func (s *MockIMDSServer) serveInfo(w http.ResponseWriter, ns *mockNamespace) { type taskCredInfo struct { + Code string `json:"Code"` RoleARN string `json:"RoleARN"` } info := struct { @@ -203,7 +218,10 @@ func (s *MockIMDSServer) serveInfo(w http.ResponseWriter, ns *mockNamespace) { TaskCredentials: make(map[string]taskCredInfo), } for key, cred := range ns.credentials { - info.TaskCredentials[key] = taskCredInfo{RoleARN: cred.RoleArn} + info.TaskCredentials[key] = taskCredInfo{ + Code: cred.Code, + RoleARN: cred.RoleArn, + } } w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(info) @@ -220,6 +238,13 @@ func (s *MockIMDSServer) serveCredential( http.NotFound(w, r) return } + // An undelivered entry has no credential file, matching a provider that + // could not assume the role. + if cred.Code != imds.CredentialCodeSuccess { + http.NotFound(w, r) + return + } + resp := struct { AccessKeyId string `json:"AccessKeyId"` SecretAccessKey string `json:"SecretAccessKey"` diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/types.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/types.go index 2014f069ab8..2467909bad5 100644 --- a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/types.go +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds/types.go @@ -13,6 +13,17 @@ package imds +const ( + // CredentialCodeSuccess is the info file Code for a credential the provider + // successfully assumed the role for and wrote to IMDS. + CredentialCodeSuccess = "Success" + + // CredentialCodeAssumeRoleUnauthorizedAccess is the info file Code for a credential the + // provider was not authorized to assume the role for, for example because + // the role's trust policy rejects it. + CredentialCodeAssumeRoleUnauthorizedAccess = "AssumeRoleUnauthorizedAccess" +) + // NamespaceInfo represents the parsed info file from an iam-ecs-* namespace. // JSON tags match the IMDS response format. type NamespaceInfo struct { @@ -22,6 +33,7 @@ type NamespaceInfo struct { // TaskCredentialInfo represents a single entry in the namespace info file. type TaskCredentialInfo struct { + Code string `json:"Code"` RoleArn string `json:"RoleARN"` } @@ -36,6 +48,26 @@ type TaskCredential struct { Expiration string } +// AssumeRoleUnauthorizedAccessIAMRole identifies a task's IAM role that the +// provider was not authorized to assume, as reported by a namespace info file +// entry with Code AssumeRoleUnauthorizedAccess. It carries no credential +// material because the provider wrote none. +type AssumeRoleUnauthorizedAccessIAMRole struct { + TaskID string + RoleType string + RoleArn string +} + +// ScanResult holds the outcome of a single IMDS credentials scan. +type ScanResult struct { + // Credentials are the task credentials retrieved from IMDS. + Credentials []TaskCredential + // AssumeRoleUnauthorizedAccessRoles are the roles the provider was not + // authorized to assume. A consumer uses them to attribute a stale credential + // to that, as opposed to a broken delivery path. + AssumeRoleUnauthorizedAccessRoles []AssumeRoleUnauthorizedAccessIAMRole +} + // imdsCredential is used internally by the scanner to deserialize IMDS // credential files, which use different field names than TaskCredential // (e.g. "Token" vs SessionToken). JSON tags match the IMDS response format. diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/interface.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/interface.go index 4d1c626208d..91236515666 100644 --- a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/interface.go +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/interface.go @@ -15,11 +15,13 @@ package credentials // Manager is responsible for saving and retrieving credentials. A single // instance of the credentials manager is created in the agent, and shared -// between the task engine, acs and credentials handlers +// between the task engine, acs and credentials handlers. type Manager interface { SetTaskCredentials(*TaskIAMRoleCredentials) error GetTaskCredentials(string) (TaskIAMRoleCredentials, bool) RemoveCredentials(string) IsCredentialsPending(string) bool AddKnownCredentialsID(string) + SetAssumeRoleUnauthorizedAccessCredentials(string) + IsCredentialsAssumeRoleUnauthorizedAccess(string) bool } diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/manager.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/manager.go index 3829f0cbad9..9181edcf04f 100644 --- a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/manager.go +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/manager.go @@ -90,7 +90,10 @@ type credentialsManager struct { idToTaskCredentials map[string]TaskIAMRoleCredentials // knownCredentialsIDs tracks all credentials IDs we know about knownCredentialsIDs map[string]bool - taskCredentialsLock sync.RWMutex + // assumeRoleUnauthorizedAccessCredentialsIDs holds the credentials ids the provider reported + // being denied to assume the role for. + assumeRoleUnauthorizedAccessCredentialsIDs map[string]bool + taskCredentialsLock sync.RWMutex } // IAMRoleCredentialsFromACS translates ecsacs.IAMRoleCredentials object to @@ -110,8 +113,9 @@ func IAMRoleCredentialsFromACS(roleCredentials *ecsacs.IAMRoleCredentials, roleT // NewManager creates a new credentials manager object func NewManager() Manager { return &credentialsManager{ - idToTaskCredentials: make(map[string]TaskIAMRoleCredentials), - knownCredentialsIDs: make(map[string]bool), + idToTaskCredentials: make(map[string]TaskIAMRoleCredentials), + knownCredentialsIDs: make(map[string]bool), + assumeRoleUnauthorizedAccessCredentialsIDs: make(map[string]bool), } } @@ -138,6 +142,10 @@ func (manager *credentialsManager) SetTaskCredentials(taskCredentials *TaskIAMRo manager.knownCredentialsIDs[credentials.CredentialsID] = true + // Credentials arriving for this id mean the provider was able to assume the role. + // This could happen, for instance, when a previously misconfigured IAM role is now rectified. + delete(manager.assumeRoleUnauthorizedAccessCredentialsIDs, credentials.CredentialsID) + return nil } @@ -164,6 +172,28 @@ func (manager *credentialsManager) RemoveCredentials(id string) { delete(manager.idToTaskCredentials, id) delete(manager.knownCredentialsIDs, id) + delete(manager.assumeRoleUnauthorizedAccessCredentialsIDs, id) +} + +// SetAssumeRoleUnauthorizedAccessCredentials records that the credentials +// provider was not authorized to assume the role for the given credentials id. +// Any credentials already held for the id are left in place; they stay +// serviceable until they expire. +func (manager *credentialsManager) SetAssumeRoleUnauthorizedAccessCredentials(id string) { + manager.taskCredentialsLock.Lock() + defer manager.taskCredentialsLock.Unlock() + + manager.assumeRoleUnauthorizedAccessCredentialsIDs[id] = true +} + +// IsCredentialsAssumeRoleUnauthorizedAccess returns true if the provider +// reported that it was not authorized to assume the role for the given +// credentials id, and has not since delivered credentials for it. +func (manager *credentialsManager) IsCredentialsAssumeRoleUnauthorizedAccess(id string) bool { + manager.taskCredentialsLock.RLock() + defer manager.taskCredentialsLock.RUnlock() + + return manager.assumeRoleUnauthorizedAccessCredentialsIDs[id] } // IsCredentialsPending returns true if credentials ID is known but has not yet arrived from ACS. diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/mocks/credentials_mocks.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/mocks/credentials_mocks.go index 12d63378b9a..ea19134043d 100644 --- a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/mocks/credentials_mocks.go +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/credentials/mocks/credentials_mocks.go @@ -75,6 +75,20 @@ func (mr *MockManagerMockRecorder) GetTaskCredentials(arg0 interface{}) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTaskCredentials", reflect.TypeOf((*MockManager)(nil).GetTaskCredentials), arg0) } +// IsCredentialsAssumeRoleUnauthorizedAccess mocks base method. +func (m *MockManager) IsCredentialsAssumeRoleUnauthorizedAccess(arg0 string) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsCredentialsAssumeRoleUnauthorizedAccess", arg0) + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsCredentialsAssumeRoleUnauthorizedAccess indicates an expected call of IsCredentialsAssumeRoleUnauthorizedAccess. +func (mr *MockManagerMockRecorder) IsCredentialsAssumeRoleUnauthorizedAccess(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsCredentialsAssumeRoleUnauthorizedAccess", reflect.TypeOf((*MockManager)(nil).IsCredentialsAssumeRoleUnauthorizedAccess), arg0) +} + // IsCredentialsPending mocks base method. func (m *MockManager) IsCredentialsPending(arg0 string) bool { m.ctrl.T.Helper() @@ -101,6 +115,18 @@ func (mr *MockManagerMockRecorder) RemoveCredentials(arg0 interface{}) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveCredentials", reflect.TypeOf((*MockManager)(nil).RemoveCredentials), arg0) } +// SetAssumeRoleUnauthorizedAccessCredentials mocks base method. +func (m *MockManager) SetAssumeRoleUnauthorizedAccessCredentials(arg0 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetAssumeRoleUnauthorizedAccessCredentials", arg0) +} + +// SetAssumeRoleUnauthorizedAccessCredentials indicates an expected call of SetAssumeRoleUnauthorizedAccessCredentials. +func (mr *MockManagerMockRecorder) SetAssumeRoleUnauthorizedAccessCredentials(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetAssumeRoleUnauthorizedAccessCredentials", reflect.TypeOf((*MockManager)(nil).SetAssumeRoleUnauthorizedAccessCredentials), arg0) +} + // SetTaskCredentials mocks base method. func (m *MockManager) SetTaskCredentials(arg0 *credentials.TaskIAMRoleCredentials) error { m.ctrl.T.Helper() diff --git a/ecs-agent/credentials/imds/mocks/imds_mocks.go b/ecs-agent/credentials/imds/mocks/imds_mocks.go index e79bc637d61..848a38c484b 100644 --- a/ecs-agent/credentials/imds/mocks/imds_mocks.go +++ b/ecs-agent/credentials/imds/mocks/imds_mocks.go @@ -50,10 +50,10 @@ func (m *MockScanner) EXPECT() *MockScannerMockRecorder { } // Scan mocks base method. -func (m *MockScanner) Scan(arg0 context.Context) ([]imds.TaskCredential, error) { +func (m *MockScanner) Scan(arg0 context.Context) (imds.ScanResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Scan", arg0) - ret0, _ := ret[0].([]imds.TaskCredential) + ret0, _ := ret[0].(imds.ScanResult) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/ecs-agent/credentials/imds/scanner.go b/ecs-agent/credentials/imds/scanner.go index cb65dabb965..54f86d09e02 100644 --- a/ecs-agent/credentials/imds/scanner.go +++ b/ecs-agent/credentials/imds/scanner.go @@ -74,8 +74,9 @@ const ( // Scanner fetches task credentials from IMDS iam-ecs-* namespaces. type Scanner interface { // Scan discovers all ECS IAM namespaces, reads their info files, and - // fetches credentials from namespaces that have changed since the last scan. - Scan(ctx context.Context) ([]TaskCredential, error) + // fetches credentials from namespaces that have changed since the last + // scan. + Scan(ctx context.Context) (ScanResult, error) } // scanner implements the Scanner interface. @@ -102,22 +103,22 @@ func NewScanner(ec2MetadataClient ec2.EC2MetadataClient, // Scan discovers all ECS IAM namespaces, reads their info files, and // fetches credentials from namespaces that have changed since the last scan. -func (s *scanner) Scan(ctx context.Context) ([]TaskCredential, error) { +func (s *scanner) Scan(ctx context.Context) (ScanResult, error) { namespaces, err := s.discoverNamespaces(ctx) if err != nil { - return nil, fmt.Errorf("imds scan: discover namespaces: %w", err) + return ScanResult{}, fmt.Errorf("imds scan: discover namespaces: %w", err) } // No namespaces is expected when IMDS does not have ECS task credentials yet. if len(namespaces) == 0 { logger.Debug("IMDS credentials scan: no iam-ecs namespace found") - return nil, nil + return ScanResult{}, nil } - var creds []TaskCredential + var result ScanResult var scanErrors []error for _, ns := range namespaces { - nsCreds, err := s.scanNamespace(ctx, ns) + nsResult, err := s.scanNamespace(ctx, ns) if err != nil { logger.Error("IMDS credentials scan: failed to scan namespace", logger.Fields{ "namespace": ns, @@ -128,17 +129,18 @@ func (s *scanner) Scan(ctx context.Context) ([]TaskCredential, error) { // namespaces before returning. continue } - creds = append(creds, nsCreds...) + result.Credentials = append(result.Credentials, nsResult.Credentials...) + result.AssumeRoleUnauthorizedAccessRoles = append(result.AssumeRoleUnauthorizedAccessRoles, nsResult.AssumeRoleUnauthorizedAccessRoles...) } - // Surface an error when scanning all namespaces failed so - // the caller doesn't mistake it for "no credentials yet". - if len(creds) == 0 && len(scanErrors) > 0 { - return nil, fmt.Errorf("imds scan: all %d namespace(s) failed: %w", + // Surface an error only when neither credentials nor unauthorized roles were + // read, so the caller doesn't mistake a total failure for "no credentials yet". + if len(result.Credentials) == 0 && len(result.AssumeRoleUnauthorizedAccessRoles) == 0 && len(scanErrors) > 0 { + return ScanResult{}, fmt.Errorf("imds scan: all %d namespace(s) failed: %w", len(scanErrors), errors.Join(scanErrors...)) } - return creds, nil + return result, nil } // discoverNamespaces lists the IMDS metadata root and returns all @@ -164,10 +166,10 @@ func (s *scanner) discoverNamespaces(ctx context.Context) ([]string, error) { return namespaces, nil } -// scanNamespace reads the info file for a namespace and fetches -// credentials for each entry. Only successfully fetched credentials -// are returned. -func (s *scanner) scanNamespace(ctx context.Context, namespace string) ([]TaskCredential, error) { +// scanNamespace reads the info file for a namespace and fetches credentials for each entry. +// It returns a list of successfully fetched credentials and a list of IAM roles for which +// the "Code" in the info file was AssumeRoleUnauthorizedAccess. +func (s *scanner) scanNamespace(ctx context.Context, namespace string) (ScanResult, error) { infoPath := fmt.Sprintf(infoPathFormat, namespace) infoResp, err := s.getMetadata(ctx, infoPath) if err != nil { @@ -175,7 +177,7 @@ func (s *scanner) scanNamespace(ctx context.Context, namespace string) ([]TaskCr WithFields(map[string]any{ metricFieldNamespace: namespace, }).Done(err) - return nil, fmt.Errorf("fetch info for %s: %w", namespace, err) + return ScanResult{}, fmt.Errorf("fetch info for %s: %w", namespace, err) } var info NamespaceInfo @@ -184,7 +186,7 @@ func (s *scanner) scanNamespace(ctx context.Context, namespace string) ([]TaskCr WithFields(map[string]any{ metricFieldNamespace: namespace, }).Done(err) - return nil, fmt.Errorf("parse info for %s: %w", namespace, err) + return ScanResult{}, fmt.Errorf("parse info for %s: %w", namespace, err) } // Skip credential fetches if the namespace hasn't been updated since the last scan. @@ -194,16 +196,16 @@ func (s *scanner) scanNamespace(ctx context.Context, namespace string) ([]TaskCr WithFields(map[string]any{ metricFieldNamespace: namespace, }).Done(err) - return nil, fmt.Errorf("parse LastUpdated for %s: %w", namespace, err) + return ScanResult{}, fmt.Errorf("parse LastUpdated for %s: %w", namespace, err) } if cached, ok := s.lastUpdated[namespace]; ok && lastUpdated.Equal(cached) { logger.Debug("IMDS credentials scan: skipping namespace with unchanged LastUpdated", logger.Fields{ "namespace": namespace, }) - return nil, nil + return ScanResult{}, nil } - var creds []TaskCredential + var result ScanResult var hasErrors bool for key, entry := range info.TaskCredentials { taskID, roleType, err := parseCredentialKey(key) @@ -221,6 +223,37 @@ func (s *scanner) scanNamespace(ctx context.Context, namespace string) ([]TaskCr continue } + // A Code of AssumeRoleUnauthorizedAccess means the provider was not + // authorized to assume the role, so no credential file was written for + // this entry. + if isCredentialAssumeRoleUnauthorizedAccess(entry.Code) { + logger.Debug("IMDS credentials scan: provider not authorized to assume the IAM role", + logger.Fields{ + field.TaskID: taskID, + "roleType": roleType, + "namespace": namespace, + }) + result.AssumeRoleUnauthorizedAccessRoles = append(result.AssumeRoleUnauthorizedAccessRoles, AssumeRoleUnauthorizedAccessIAMRole{ + TaskID: taskID, + RoleType: roleType, + RoleArn: entry.RoleArn, + }) + continue + } + + // Success and AssumeRoleUnauthorizedAccess are the only codes the agent + // interprets. An unrecognized code is logged and then fetched anyway: a + // credential file may still exist for it. + if !isCredentialDelivered(entry.Code) { + logger.Warn("IMDS credentials scan: unrecognized credential code", + logger.Fields{ + field.TaskID: taskID, + "roleType": roleType, + "namespace": namespace, + "code": entry.Code, + }) + } + credPath := fmt.Sprintf(credentialPathFormat, namespace, key) credResp, err := s.getMetadata(ctx, credPath) if err != nil { @@ -284,7 +317,7 @@ func (s *scanner) scanNamespace(ctx context.Context, namespace string) ([]TaskCr "namespace": namespace, "expiration": imdsCred.Expiration, }) - creds = append(creds, TaskCredential{ + result.Credentials = append(result.Credentials, TaskCredential{ TaskID: taskID, RoleType: roleType, RoleArn: entry.RoleArn, @@ -301,19 +334,32 @@ func (s *scanner) scanNamespace(ctx context.Context, namespace string) ([]TaskCr s.lastUpdated[namespace] = lastUpdated } - // Surface an error when the namespace yielded no credentials and also had + // Surface an error when the namespace yielded nothing and also had // failures, so callers don't mistake it for "no credentials yet". - if len(creds) == 0 && hasErrors { - return nil, fmt.Errorf("all credential processing failed for %s", namespace) + if len(result.Credentials) == 0 && len(result.AssumeRoleUnauthorizedAccessRoles) == 0 && hasErrors { + return ScanResult{}, fmt.Errorf("all credential processing failed for %s", namespace) } logger.Info("IMDS credentials scan: namespace scan complete", logger.Fields{ - "namespace": namespace, - "retrievedCredentialCount": len(creds), - "lastUpdated": info.LastUpdated, + "namespace": namespace, + "retrievedCredentialCount": len(result.Credentials), + "assumeRoleUnauthorizedAccessRoleCount": len(result.AssumeRoleUnauthorizedAccessRoles), + "lastUpdated": info.LastUpdated, }) - return creds, nil + return result, nil +} + +// isCredentialDelivered reports whether an info file entry's Code says a +// credential file was successfully written for it. +func isCredentialDelivered(code string) bool { + return strings.EqualFold(code, CredentialCodeSuccess) +} + +// isCredentialAssumeRoleUnauthorizedAccess reports whether an info file entry's +// Code says the provider was not authorized to assume the IAM role. +func isCredentialAssumeRoleUnauthorizedAccess(code string) bool { + return strings.EqualFold(code, CredentialCodeAssumeRoleUnauthorizedAccess) } // parseCredentialKey extracts the task ID and role type from an IMDS key. diff --git a/ecs-agent/credentials/imds/scanner_test.go b/ecs-agent/credentials/imds/scanner_test.go index 41c33f19af4..17f15ba06d4 100644 --- a/ecs-agent/credentials/imds/scanner_test.go +++ b/ecs-agent/credentials/imds/scanner_test.go @@ -53,28 +53,39 @@ func testCredentialJSON(accessKeyID string) string { // testInfoJSONWithTimestamp returns a mock IMDS info file JSON // with the given LastUpdated value. func testInfoJSONWithTimestamp( - lastUpdated string, entries map[string]string, + lastUpdated string, entries map[string]TaskCredentialInfo, ) string { - entriesJSON := "" - for key, roleARN := range entries { - if entriesJSON != "" { - entriesJSON += "," - } - entriesJSON += fmt.Sprintf( - `"%s": {"RoleARN": "%s"}`, key, roleARN, - ) + encoded := make([]string, 0, len(entries)) + for key, entry := range entries { + encoded = append(encoded, fmt.Sprintf( + `"%s": {"Code": "%s", "RoleARN": "%s"}`, key, entry.Code, entry.RoleArn)) } return fmt.Sprintf( `{"LastUpdated": "%s", "TaskCredentials": {%s}}`, - lastUpdated, entriesJSON, + lastUpdated, strings.Join(encoded, ","), ) } // testInfoJSON returns a mock IMDS info file JSON with a default timestamp. -func testInfoJSON(entries map[string]string) string { +func testInfoJSON(entries map[string]TaskCredentialInfo) string { return testInfoJSONWithTimestamp("2026-04-28T00:00:00Z", entries) } +// testEntryWithCode returns an info file entry with the given role ARN and Code. +func testEntryWithCode(roleARN, code string) TaskCredentialInfo { + return TaskCredentialInfo{Code: code, RoleArn: roleARN} +} + +// testAssumeRoleUnauthorizedAccess is a helper func that returns an +// AssumeRoleUnauthorizedAccessIAMRole with the given fields. +func testAssumeRoleUnauthorizedAccess(taskID, roleType, roleArn string) AssumeRoleUnauthorizedAccessIAMRole { + return AssumeRoleUnauthorizedAccessIAMRole{ + TaskID: taskID, + RoleType: roleType, + RoleArn: roleArn, + } +} + // testCred is a helper func that returns a TaskCredential with the given fields. func testCred(taskID, roleType, roleArn, accessKeyID string) TaskCredential { return TaskCredential{ @@ -201,12 +212,13 @@ func TestScanNamespace(t *testing.T) { key2 := testTaskID2 + "-" + credentials.ExecutionRoleType tests := []struct { - name string - setupMock func(*mockec2.MockEC2MetadataClient) - lastUpdated map[string]time.Time - expectedCreds []TaskCredential - expectedErrSubstring string - expectedMetrics []metricExpectation + name string + setupMock func(*mockec2.MockEC2MetadataClient) + lastUpdated map[string]time.Time + expectedCreds []TaskCredential + expectedAssumeRoleUnauthorizedAccessRoles []AssumeRoleUnauthorizedAccessIAMRole + expectedErrSubstring string + expectedMetrics []metricExpectation // expectLastUpdatedCached is a pointer to distinguish "don't check" (nil) // from "assert not cached" (false). expectLastUpdatedCached *bool @@ -215,7 +227,9 @@ func TestScanNamespace(t *testing.T) { name: "single credential", setupMock: func(m *mockec2.MockEC2MetadataClient) { m.EXPECT().GetMetadata("iam-ecs-1/info").Return( - testInfoJSON(map[string]string{key1: testRoleARN}), nil) + testInfoJSON(map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, CredentialCodeSuccess), + }), nil) m.EXPECT().GetMetadata("iam-ecs-1/security-credentials/"+key1).Return( testCredentialJSON("AKID1"), nil) }, @@ -228,9 +242,9 @@ func TestScanNamespace(t *testing.T) { name: "multiple credentials", setupMock: func(m *mockec2.MockEC2MetadataClient) { m.EXPECT().GetMetadata("iam-ecs-1/info").Return( - testInfoJSON(map[string]string{ - key1: testRoleARN, - key2: testRoleARN, + testInfoJSON(map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, CredentialCodeSuccess), + key2: testEntryWithCode(testRoleARN, CredentialCodeSuccess), }), nil) m.EXPECT().GetMetadata("iam-ecs-1/security-credentials/"+key1).Return( testCredentialJSON("AKID1"), nil) @@ -242,6 +256,112 @@ func TestScanNamespace(t *testing.T) { testCred(testTaskID2, credentials.ExecutionRoleType, testRoleARN, "AKID2"), }, }, + { + name: "absent Code is treated as delivered", + setupMock: func(m *mockec2.MockEC2MetadataClient) { + m.EXPECT().GetMetadata("iam-ecs-1/info").Return( + testInfoJSON(map[string]TaskCredentialInfo{ + key1: {RoleArn: testRoleARN}, + }), nil) + m.EXPECT().GetMetadata("iam-ecs-1/security-credentials/"+key1).Return( + testCredentialJSON("AKID1"), nil) + }, + expectedCreds: []TaskCredential{ + testCred(testTaskID1, credentials.ApplicationRoleType, testRoleARN, "AKID1"), + }, + }, + { + name: "lowercase Code is treated as delivered", + setupMock: func(m *mockec2.MockEC2MetadataClient) { + m.EXPECT().GetMetadata("iam-ecs-1/info").Return( + testInfoJSON(map[string]TaskCredentialInfo{ + key1: {Code: "success", RoleArn: testRoleARN}, + }), nil) + m.EXPECT().GetMetadata("iam-ecs-1/security-credentials/"+key1).Return( + testCredentialJSON("AKID1"), nil) + }, + expectedCreds: []TaskCredential{ + testCred(testTaskID1, credentials.ApplicationRoleType, testRoleARN, "AKID1"), + }, + }, + { + name: "unauthorized credential is reported without a fetch", + setupMock: func(m *mockec2.MockEC2MetadataClient) { + m.EXPECT().GetMetadata("iam-ecs-1/info").Return( + testInfoJSON(map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, CredentialCodeAssumeRoleUnauthorizedAccess), + }), nil) + }, + expectedAssumeRoleUnauthorizedAccessRoles: []AssumeRoleUnauthorizedAccessIAMRole{ + testAssumeRoleUnauthorizedAccess(testTaskID1, credentials.ApplicationRoleType, + testRoleARN), + }, + // An unauthorized entry is a delivery outcome, not a scan failure, + // so the namespace stays eligible for LastUpdated caching. + expectLastUpdatedCached: aws.Bool(true), + }, + { + name: "delivered and unauthorized credentials in one namespace", + setupMock: func(m *mockec2.MockEC2MetadataClient) { + m.EXPECT().GetMetadata("iam-ecs-1/info").Return( + testInfoJSON(map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, CredentialCodeSuccess), + key2: testEntryWithCode(testRoleARN, CredentialCodeAssumeRoleUnauthorizedAccess), + }), nil) + m.EXPECT().GetMetadata("iam-ecs-1/security-credentials/"+key1).Return( + testCredentialJSON("AKID1"), nil) + }, + expectedCreds: []TaskCredential{ + testCred(testTaskID1, credentials.ApplicationRoleType, testRoleARN, "AKID1"), + }, + expectedAssumeRoleUnauthorizedAccessRoles: []AssumeRoleUnauthorizedAccessIAMRole{ + testAssumeRoleUnauthorizedAccess(testTaskID2, credentials.ExecutionRoleType, + testRoleARN), + }, + expectLastUpdatedCached: aws.Bool(true), + }, + { + name: "unrecognized code is fetched anyway", + setupMock: func(m *mockec2.MockEC2MetadataClient) { + m.EXPECT().GetMetadata("iam-ecs-1/info").Return( + testInfoJSON(map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, "SomethingElse"), + }), nil) + m.EXPECT().GetMetadata("iam-ecs-1/security-credentials/"+key1).Return( + testCredentialJSON("AKID1"), nil) + }, + // A credential file may exist for a code the agent does not + // interpret, and discarding a usable credential is worse than a + // wasted fetch. + expectedCreds: []TaskCredential{ + testCred(testTaskID1, credentials.ApplicationRoleType, testRoleARN, "AKID1"), + }, + expectLastUpdatedCached: aws.Bool(true), + }, + { + name: "unrecognized code with no credential file fails the fetch", + setupMock: func(m *mockec2.MockEC2MetadataClient) { + m.EXPECT().GetMetadata("iam-ecs-1/info").Return( + testInfoJSON(map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, "SomethingElse"), + }), nil) + m.EXPECT().GetMetadata("iam-ecs-1/security-credentials/"+key1).Return( + "", errors.New("not found")) + }, + expectedErrSubstring: "all credential processing failed", + expectedMetrics: []metricExpectation{ + { + name: metrics.IMDSCredentialsScannerCredentialFailureMetricName, + fields: map[string]any{ + metricFieldNamespace: "iam-ecs-1", + metricFieldTaskID: testTaskID1, + metricFieldRoleType: credentials.ApplicationRoleType, + }, + doneErr: errMessageContains("not found"), + }, + }, + expectLastUpdatedCached: aws.Bool(false), + }, { name: "info file fetch fails", setupMock: func(m *mockec2.MockEC2MetadataClient) { @@ -277,7 +397,9 @@ func TestScanNamespace(t *testing.T) { setupMock: func(m *mockec2.MockEC2MetadataClient) { m.EXPECT().GetMetadata("iam-ecs-1/info").Return( testInfoJSONWithTimestamp("not-a-timestamp", - map[string]string{key1: testRoleARN}), nil) + map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, CredentialCodeSuccess), + }), nil) }, expectedErrSubstring: "parse LastUpdated for", expectedMetrics: []metricExpectation{ @@ -292,9 +414,9 @@ func TestScanNamespace(t *testing.T) { name: "fetch for one credential fails, other succeeds", setupMock: func(m *mockec2.MockEC2MetadataClient) { m.EXPECT().GetMetadata("iam-ecs-1/info").Return( - testInfoJSON(map[string]string{ - key1: testRoleARN, - key2: testRoleARN, + testInfoJSON(map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, CredentialCodeSuccess), + key2: testEntryWithCode(testRoleARN, CredentialCodeSuccess), }), nil) m.EXPECT().GetMetadata("iam-ecs-1/security-credentials/"+key1).Return( "", errors.New("timeout")) @@ -321,7 +443,9 @@ func TestScanNamespace(t *testing.T) { name: "credential response invalid JSON", setupMock: func(m *mockec2.MockEC2MetadataClient) { m.EXPECT().GetMetadata("iam-ecs-1/info").Return( - testInfoJSON(map[string]string{key1: testRoleARN}), nil) + testInfoJSON(map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, CredentialCodeSuccess), + }), nil) m.EXPECT().GetMetadata("iam-ecs-1/security-credentials/"+key1).Return( "not json", nil) }, @@ -343,7 +467,9 @@ func TestScanNamespace(t *testing.T) { name: "credential missing required fields", setupMock: func(m *mockec2.MockEC2MetadataClient) { m.EXPECT().GetMetadata("iam-ecs-1/info").Return( - testInfoJSON(map[string]string{key1: testRoleARN}), nil) + testInfoJSON(map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, CredentialCodeSuccess), + }), nil) m.EXPECT().GetMetadata("iam-ecs-1/security-credentials/"+key1).Return( `{"AccessKeyId": "AKID1"}`, nil) }, @@ -366,8 +492,8 @@ func TestScanNamespace(t *testing.T) { name: "invalid credential key format", setupMock: func(m *mockec2.MockEC2MetadataClient) { m.EXPECT().GetMetadata("iam-ecs-1/info").Return( - testInfoJSON(map[string]string{ - "nodelimiterkey": testRoleARN, + testInfoJSON(map[string]TaskCredentialInfo{ + "nodelimiterkey": testEntryWithCode(testRoleARN, CredentialCodeSuccess), }), nil) }, expectedErrSubstring: "all credential processing failed", @@ -380,22 +506,63 @@ func TestScanNamespace(t *testing.T) { }, expectLastUpdatedCached: aws.Bool(false), }, + { + name: "malformed key alongside an unauthorized entry still reports the entry", + setupMock: func(m *mockec2.MockEC2MetadataClient) { + m.EXPECT().GetMetadata("iam-ecs-1/info").Return( + testInfoJSON(map[string]TaskCredentialInfo{ + "nodelimiterkey": testEntryWithCode(testRoleARN, CredentialCodeSuccess), + key1: testEntryWithCode(testRoleARN, CredentialCodeAssumeRoleUnauthorizedAccess), + }), nil) + }, + expectedAssumeRoleUnauthorizedAccessRoles: []AssumeRoleUnauthorizedAccessIAMRole{ + testAssumeRoleUnauthorizedAccess(testTaskID1, credentials.ApplicationRoleType, + testRoleARN), + }, + expectedMetrics: []metricExpectation{ + { + name: metrics.IMDSCredentialsScannerCredentialFailureMetricName, + fields: map[string]any{metricFieldNamespace: "iam-ecs-1"}, + doneErr: errMessageContains("unexpected credential key format"), + }, + }, + expectLastUpdatedCached: aws.Bool(false), + }, { name: "unchanged LastUpdated skips credential fetches", setupMock: func(m *mockec2.MockEC2MetadataClient) { m.EXPECT().GetMetadata("iam-ecs-1/info").Return( - testInfoJSON(map[string]string{key1: testRoleARN}), nil) + testInfoJSON(map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, CredentialCodeSuccess), + }), nil) + }, + lastUpdated: map[string]time.Time{ + "iam-ecs-1": time.Date(2026, 4, 28, 0, 0, 0, 0, time.UTC), + }, + }, + { + name: "unchanged LastUpdated skips an unauthorized entry too", + setupMock: func(m *mockec2.MockEC2MetadataClient) { + m.EXPECT().GetMetadata("iam-ecs-1/info").Return( + testInfoJSON(map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, CredentialCodeAssumeRoleUnauthorizedAccess), + }), nil) }, lastUpdated: map[string]time.Time{ "iam-ecs-1": time.Date(2026, 4, 28, 0, 0, 0, 0, time.UTC), }, + // A Code change rewrites the info file and moves LastUpdated, so an + // unchanged LastUpdated means this denial was already reported on an + // earlier scan and the caller still holds it. }, { name: "changed LastUpdated re-fetches credentials", setupMock: func(m *mockec2.MockEC2MetadataClient) { m.EXPECT().GetMetadata("iam-ecs-1/info").Return( testInfoJSONWithTimestamp("2026-04-28T01:00:00Z", - map[string]string{key1: testRoleARN}), nil) + map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, CredentialCodeSuccess), + }), nil) m.EXPECT().GetMetadata("iam-ecs-1/security-credentials/"+key1).Return( testCredentialJSON("AKID_NEW"), nil) }, @@ -428,14 +595,16 @@ func TestScanNamespace(t *testing.T) { s.lastUpdated = tc.lastUpdated } - creds, err := s.scanNamespace(context.Background(), "iam-ecs-1") + result, err := s.scanNamespace(context.Background(), "iam-ecs-1") if tc.expectedErrSubstring != "" { assert.ErrorContains(t, err, tc.expectedErrSubstring) - assert.Nil(t, creds) + assert.Empty(t, result.Credentials) + assert.Empty(t, result.AssumeRoleUnauthorizedAccessRoles) } else { assert.NoError(t, err) - assert.ElementsMatch(t, tc.expectedCreds, creds) + assert.ElementsMatch(t, tc.expectedCreds, result.Credentials) + assert.ElementsMatch(t, tc.expectedAssumeRoleUnauthorizedAccessRoles, result.AssumeRoleUnauthorizedAccessRoles) } if tc.expectLastUpdatedCached != nil { if *tc.expectLastUpdatedCached { @@ -447,6 +616,86 @@ func TestScanNamespace(t *testing.T) { }) } } + +func TestIsCredentialDelivered(t *testing.T) { + tests := []struct { + name string + code string + expected bool + }{ + { + name: "success", + code: CredentialCodeSuccess, + expected: true, + }, + { + name: "lowercase success", + code: "success", + expected: true, + }, + { + name: "assume role unauthorized access", + code: CredentialCodeAssumeRoleUnauthorizedAccess, + expected: false, + }, + { + name: "unrecognized code", + code: "SomethingElse", + expected: false, + }, + { + name: "absent code", + code: "", + expected: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, isCredentialDelivered(tc.code)) + }) + } +} + +func TestIsCredentialAssumeRoleUnauthorizedAccess(t *testing.T) { + tests := []struct { + name string + code string + expected bool + }{ + { + name: "assume role unauthorized access", + code: CredentialCodeAssumeRoleUnauthorizedAccess, + expected: true, + }, + { + name: "lowercase assume role unauthorized access", + code: "assumeroleunauthorizedaccess", + expected: true, + }, + { + name: "success", + code: CredentialCodeSuccess, + expected: false, + }, + { + name: "absent code", + code: "", + expected: false, + }, + { + name: "unrecognized code", + code: "SomethingElse", + expected: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, isCredentialAssumeRoleUnauthorizedAccess(tc.code)) + }) + } +} func TestParseCredentialKey(t *testing.T) { tests := []struct { name string @@ -571,12 +820,13 @@ func TestValidateCredential(t *testing.T) { func TestScan(t *testing.T) { tests := []struct { - name string - setupMock func(*mockec2.MockEC2MetadataClient) - ctx context.Context - expectedCreds []TaskCredential - expectedErrSubstring string - expectedMetrics []metricExpectation + name string + setupMock func(*mockec2.MockEC2MetadataClient) + ctx context.Context + expectedCreds []TaskCredential + expectedAssumeRoleUnauthorizedAccessRoles []AssumeRoleUnauthorizedAccessIAMRole + expectedErrSubstring string + expectedMetrics []metricExpectation }{ { name: "no namespaces", @@ -591,11 +841,15 @@ func TestScan(t *testing.T) { key2 := testTaskID2 + "-" + credentials.ExecutionRoleType m.EXPECT().GetMetadata("").Return("iam-ecs-1\niam-ecs-2", nil) m.EXPECT().GetMetadata("iam-ecs-1/info").Return( - testInfoJSON(map[string]string{key1: testRoleARN}), nil) + testInfoJSON(map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, CredentialCodeSuccess), + }), nil) m.EXPECT().GetMetadata("iam-ecs-1/security-credentials/"+key1).Return( testCredentialJSON("AKID1"), nil) m.EXPECT().GetMetadata("iam-ecs-2/info").Return( - testInfoJSON(map[string]string{key2: testRoleARN}), nil) + testInfoJSON(map[string]TaskCredentialInfo{ + key2: testEntryWithCode(testRoleARN, CredentialCodeSuccess), + }), nil) m.EXPECT().GetMetadata("iam-ecs-2/security-credentials/"+key2).Return( testCredentialJSON("AKID2"), nil) }, @@ -604,6 +858,55 @@ func TestScan(t *testing.T) { testCred(testTaskID2, credentials.ExecutionRoleType, testRoleARN, "AKID2"), }, }, + { + name: "unauthorized entries are aggregated across namespaces", + setupMock: func(m *mockec2.MockEC2MetadataClient) { + key1 := testTaskID1 + "-" + credentials.ApplicationRoleType + key2 := testTaskID2 + "-" + credentials.ExecutionRoleType + m.EXPECT().GetMetadata("").Return("iam-ecs-1\niam-ecs-2", nil) + m.EXPECT().GetMetadata("iam-ecs-1/info").Return( + testInfoJSON(map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, CredentialCodeAssumeRoleUnauthorizedAccess), + }), nil) + m.EXPECT().GetMetadata("iam-ecs-2/info").Return( + testInfoJSON(map[string]TaskCredentialInfo{ + key2: testEntryWithCode(testRoleARN, CredentialCodeSuccess), + }), nil) + m.EXPECT().GetMetadata("iam-ecs-2/security-credentials/"+key2).Return( + testCredentialJSON("AKID2"), nil) + }, + expectedCreds: []TaskCredential{ + testCred(testTaskID2, credentials.ExecutionRoleType, testRoleARN, "AKID2"), + }, + expectedAssumeRoleUnauthorizedAccessRoles: []AssumeRoleUnauthorizedAccessIAMRole{ + testAssumeRoleUnauthorizedAccess(testTaskID1, credentials.ApplicationRoleType, + testRoleARN), + }, + }, + { + name: "unauthorized entries survive a failing namespace", + setupMock: func(m *mockec2.MockEC2MetadataClient) { + key1 := testTaskID1 + "-" + credentials.ApplicationRoleType + m.EXPECT().GetMetadata("").Return("iam-ecs-1\niam-ecs-2", nil) + m.EXPECT().GetMetadata("iam-ecs-1/info").Return( + testInfoJSON(map[string]TaskCredentialInfo{ + key1: testEntryWithCode(testRoleARN, CredentialCodeAssumeRoleUnauthorizedAccess), + }), nil) + m.EXPECT().GetMetadata("iam-ecs-2/info").Return( + "", errors.New("timeout")) + }, + expectedAssumeRoleUnauthorizedAccessRoles: []AssumeRoleUnauthorizedAccessIAMRole{ + testAssumeRoleUnauthorizedAccess(testTaskID1, credentials.ApplicationRoleType, + testRoleARN), + }, + expectedMetrics: []metricExpectation{ + { + name: metrics.IMDSCredentialsScannerNamespaceInfoFailureMetricName, + fields: map[string]any{metricFieldNamespace: "iam-ecs-2"}, + doneErr: errMessageContains("timeout"), + }, + }, + }, { name: "namespace discovery fails", setupMock: func(m *mockec2.MockEC2MetadataClient) { @@ -667,14 +970,16 @@ func TestScan(t *testing.T) { } s := NewScanner(mock, mockMetricsFactory) - creds, err := s.Scan(ctx) + result, err := s.Scan(ctx) if tc.expectedErrSubstring != "" { assert.ErrorContains(t, err, tc.expectedErrSubstring) - assert.Nil(t, creds) + assert.Empty(t, result.Credentials) + assert.Empty(t, result.AssumeRoleUnauthorizedAccessRoles) } else { assert.NoError(t, err) - assert.ElementsMatch(t, tc.expectedCreds, creds) + assert.ElementsMatch(t, tc.expectedCreds, result.Credentials) + assert.ElementsMatch(t, tc.expectedAssumeRoleUnauthorizedAccessRoles, result.AssumeRoleUnauthorizedAccessRoles) } }) } diff --git a/ecs-agent/credentials/imds/testutil/mock_imds_server.go b/ecs-agent/credentials/imds/testutil/mock_imds_server.go index 577933d735c..73ef258e683 100644 --- a/ecs-agent/credentials/imds/testutil/mock_imds_server.go +++ b/ecs-agent/credentials/imds/testutil/mock_imds_server.go @@ -27,6 +27,8 @@ import ( "sync" "testing" "time" + + "github.com/aws/amazon-ecs-agent/ecs-agent/credentials/imds" ) // MockIMDSServer is an HTTP server that mimics the EC2 IMDS @@ -43,6 +45,7 @@ type mockNamespace struct { } type mockCredential struct { + Code string RoleArn string AccessKeyID string SecretAccessKey string @@ -69,23 +72,33 @@ func (s *MockIMDSServer) Close() { s.server.Close() } -// AddCredential registers a credential in the mock IMDS server. -// The namespace is auto-created if it doesn't exist. +// AddCredential registers an entry with the given info file Code in the mock +// IMDS server. The namespace is auto-created if it doesn't exist. accessKeyID is +// ignored unless the Code is Success, since no credential file is served for any +// other Code. func (s *MockIMDSServer) AddCredential( - namespace, taskID, roleType, roleArn, accessKeyID string, + namespace, taskID, roleType, roleArn, accessKeyID, code string, ) { s.mu.Lock() defer s.mu.Unlock() ns := s.getOrCreateNamespace(namespace) key := taskID + "-" + roleType - ns.credentials[key] = &mockCredential{ - RoleArn: roleArn, - AccessKeyID: accessKeyID, - SecretAccessKey: "secret-" + accessKeyID, - SessionToken: "token-" + accessKeyID, - Expiration: time.Now().Add(1 * time.Hour).UTC().Format(time.RFC3339), + cred := &mockCredential{ + Code: code, + RoleArn: roleArn, + } + + // A non-Success entry is advertised in the info file with no credential file + // behind it, so it carries no credential material. + if code == imds.CredentialCodeSuccess { + cred.AccessKeyID = accessKeyID + cred.SecretAccessKey = "secret-" + accessKeyID + cred.SessionToken = "token-" + accessKeyID + cred.Expiration = time.Now().Add(1 * time.Hour).UTC().Format(time.RFC3339) } + + ns.credentials[key] = cred ns.lastUpdated = time.Now().UTC() } @@ -110,6 +123,7 @@ func (s *MockIMDSServer) RotateCredential( t.Fatalf("mock IMDS: credential %q not found in namespace %q", key, namespace) } + cred.Code = imds.CredentialCodeSuccess cred.AccessKeyID = newAccessKeyID cred.SecretAccessKey = "secret-" + newAccessKeyID cred.SessionToken = "token-" + newAccessKeyID @@ -193,6 +207,7 @@ func (s *MockIMDSServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { // LastUpdated and the TaskCredentials map. func (s *MockIMDSServer) serveInfo(w http.ResponseWriter, ns *mockNamespace) { type taskCredInfo struct { + Code string `json:"Code"` RoleARN string `json:"RoleARN"` } info := struct { @@ -203,7 +218,10 @@ func (s *MockIMDSServer) serveInfo(w http.ResponseWriter, ns *mockNamespace) { TaskCredentials: make(map[string]taskCredInfo), } for key, cred := range ns.credentials { - info.TaskCredentials[key] = taskCredInfo{RoleARN: cred.RoleArn} + info.TaskCredentials[key] = taskCredInfo{ + Code: cred.Code, + RoleARN: cred.RoleArn, + } } w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(info) @@ -220,6 +238,13 @@ func (s *MockIMDSServer) serveCredential( http.NotFound(w, r) return } + // An undelivered entry has no credential file, matching a provider that + // could not assume the role. + if cred.Code != imds.CredentialCodeSuccess { + http.NotFound(w, r) + return + } + resp := struct { AccessKeyId string `json:"AccessKeyId"` SecretAccessKey string `json:"SecretAccessKey"` diff --git a/ecs-agent/credentials/imds/types.go b/ecs-agent/credentials/imds/types.go index 2014f069ab8..2467909bad5 100644 --- a/ecs-agent/credentials/imds/types.go +++ b/ecs-agent/credentials/imds/types.go @@ -13,6 +13,17 @@ package imds +const ( + // CredentialCodeSuccess is the info file Code for a credential the provider + // successfully assumed the role for and wrote to IMDS. + CredentialCodeSuccess = "Success" + + // CredentialCodeAssumeRoleUnauthorizedAccess is the info file Code for a credential the + // provider was not authorized to assume the role for, for example because + // the role's trust policy rejects it. + CredentialCodeAssumeRoleUnauthorizedAccess = "AssumeRoleUnauthorizedAccess" +) + // NamespaceInfo represents the parsed info file from an iam-ecs-* namespace. // JSON tags match the IMDS response format. type NamespaceInfo struct { @@ -22,6 +33,7 @@ type NamespaceInfo struct { // TaskCredentialInfo represents a single entry in the namespace info file. type TaskCredentialInfo struct { + Code string `json:"Code"` RoleArn string `json:"RoleARN"` } @@ -36,6 +48,26 @@ type TaskCredential struct { Expiration string } +// AssumeRoleUnauthorizedAccessIAMRole identifies a task's IAM role that the +// provider was not authorized to assume, as reported by a namespace info file +// entry with Code AssumeRoleUnauthorizedAccess. It carries no credential +// material because the provider wrote none. +type AssumeRoleUnauthorizedAccessIAMRole struct { + TaskID string + RoleType string + RoleArn string +} + +// ScanResult holds the outcome of a single IMDS credentials scan. +type ScanResult struct { + // Credentials are the task credentials retrieved from IMDS. + Credentials []TaskCredential + // AssumeRoleUnauthorizedAccessRoles are the roles the provider was not + // authorized to assume. A consumer uses them to attribute a stale credential + // to that, as opposed to a broken delivery path. + AssumeRoleUnauthorizedAccessRoles []AssumeRoleUnauthorizedAccessIAMRole +} + // imdsCredential is used internally by the scanner to deserialize IMDS // credential files, which use different field names than TaskCredential // (e.g. "Token" vs SessionToken). JSON tags match the IMDS response format. diff --git a/ecs-agent/credentials/interface.go b/ecs-agent/credentials/interface.go index 4d1c626208d..91236515666 100644 --- a/ecs-agent/credentials/interface.go +++ b/ecs-agent/credentials/interface.go @@ -15,11 +15,13 @@ package credentials // Manager is responsible for saving and retrieving credentials. A single // instance of the credentials manager is created in the agent, and shared -// between the task engine, acs and credentials handlers +// between the task engine, acs and credentials handlers. type Manager interface { SetTaskCredentials(*TaskIAMRoleCredentials) error GetTaskCredentials(string) (TaskIAMRoleCredentials, bool) RemoveCredentials(string) IsCredentialsPending(string) bool AddKnownCredentialsID(string) + SetAssumeRoleUnauthorizedAccessCredentials(string) + IsCredentialsAssumeRoleUnauthorizedAccess(string) bool } diff --git a/ecs-agent/credentials/manager.go b/ecs-agent/credentials/manager.go index 3829f0cbad9..9181edcf04f 100644 --- a/ecs-agent/credentials/manager.go +++ b/ecs-agent/credentials/manager.go @@ -90,7 +90,10 @@ type credentialsManager struct { idToTaskCredentials map[string]TaskIAMRoleCredentials // knownCredentialsIDs tracks all credentials IDs we know about knownCredentialsIDs map[string]bool - taskCredentialsLock sync.RWMutex + // assumeRoleUnauthorizedAccessCredentialsIDs holds the credentials ids the provider reported + // being denied to assume the role for. + assumeRoleUnauthorizedAccessCredentialsIDs map[string]bool + taskCredentialsLock sync.RWMutex } // IAMRoleCredentialsFromACS translates ecsacs.IAMRoleCredentials object to @@ -110,8 +113,9 @@ func IAMRoleCredentialsFromACS(roleCredentials *ecsacs.IAMRoleCredentials, roleT // NewManager creates a new credentials manager object func NewManager() Manager { return &credentialsManager{ - idToTaskCredentials: make(map[string]TaskIAMRoleCredentials), - knownCredentialsIDs: make(map[string]bool), + idToTaskCredentials: make(map[string]TaskIAMRoleCredentials), + knownCredentialsIDs: make(map[string]bool), + assumeRoleUnauthorizedAccessCredentialsIDs: make(map[string]bool), } } @@ -138,6 +142,10 @@ func (manager *credentialsManager) SetTaskCredentials(taskCredentials *TaskIAMRo manager.knownCredentialsIDs[credentials.CredentialsID] = true + // Credentials arriving for this id mean the provider was able to assume the role. + // This could happen, for instance, when a previously misconfigured IAM role is now rectified. + delete(manager.assumeRoleUnauthorizedAccessCredentialsIDs, credentials.CredentialsID) + return nil } @@ -164,6 +172,28 @@ func (manager *credentialsManager) RemoveCredentials(id string) { delete(manager.idToTaskCredentials, id) delete(manager.knownCredentialsIDs, id) + delete(manager.assumeRoleUnauthorizedAccessCredentialsIDs, id) +} + +// SetAssumeRoleUnauthorizedAccessCredentials records that the credentials +// provider was not authorized to assume the role for the given credentials id. +// Any credentials already held for the id are left in place; they stay +// serviceable until they expire. +func (manager *credentialsManager) SetAssumeRoleUnauthorizedAccessCredentials(id string) { + manager.taskCredentialsLock.Lock() + defer manager.taskCredentialsLock.Unlock() + + manager.assumeRoleUnauthorizedAccessCredentialsIDs[id] = true +} + +// IsCredentialsAssumeRoleUnauthorizedAccess returns true if the provider +// reported that it was not authorized to assume the role for the given +// credentials id, and has not since delivered credentials for it. +func (manager *credentialsManager) IsCredentialsAssumeRoleUnauthorizedAccess(id string) bool { + manager.taskCredentialsLock.RLock() + defer manager.taskCredentialsLock.RUnlock() + + return manager.assumeRoleUnauthorizedAccessCredentialsIDs[id] } // IsCredentialsPending returns true if credentials ID is known but has not yet arrived from ACS. diff --git a/ecs-agent/credentials/manager_test.go b/ecs-agent/credentials/manager_test.go index f38a9ec14b1..4899614d511 100644 --- a/ecs-agent/credentials/manager_test.go +++ b/ecs-agent/credentials/manager_test.go @@ -23,6 +23,7 @@ import ( "github.com/aws/amazon-ecs-agent/ecs-agent/acs/model/ecsacs" "github.com/aws/aws-sdk-go-v2/aws" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // TestIAMRoleCredentialsFromACS tests if credentials sent from ACS can be @@ -102,8 +103,12 @@ func TestSetAndGetTaskCredentialsHappyPath(t *testing.T) { }, } + // A denial recorded before the credentials arrive is retired by their arrival. + manager.SetAssumeRoleUnauthorizedAccessCredentials("cid1") + err := manager.SetTaskCredentials(&credentials) assert.NoError(t, err, "Error adding credentials") + assert.False(t, manager.IsCredentialsAssumeRoleUnauthorizedAccess("cid1")) credentialsFromManager, ok := manager.GetTaskCredentials("cid1") assert.True(t, ok, "GetTaskCredentials returned false for existing credentials") @@ -166,11 +171,14 @@ func TestRemoveExistingCredentials(t *testing.T) { assert.True(t, ok, "GetTaskCredentials returned false for existing credentials") assert.Equal(t, credentials, credentialsFromManager, "Mismatch between added and retrieved credentials") + manager.SetAssumeRoleUnauthorizedAccessCredentials("cid1") + manager.RemoveCredentials("cid1") _, ok = manager.GetTaskCredentials("cid1") if ok { t.Error("Expected GetTaskCredentials to return false for removed credentials") } + assert.False(t, manager.IsCredentialsAssumeRoleUnauthorizedAccess("cid1")) } // TestAddKnownCredentialsID tests that AddKnownCredentialsID properly tracks credentials IDs @@ -223,3 +231,108 @@ func TestIsCredentialsPending(t *testing.T) { manager.RemoveCredentials(credentialsID) assert.False(t, manager.IsCredentialsPending(credentialsID)) } + +// testTaskCredentials returns a valid TaskIAMRoleCredentials for the given +// credentials id. +func testTaskCredentials(credentialsID string) *TaskIAMRoleCredentials { + return &TaskIAMRoleCredentials{ + ARN: "t1", + IAMRoleCredentials: IAMRoleCredentials{ + CredentialsID: credentialsID, + AccessKeyID: "akid1", + }, + } +} + +func TestSetAssumeRoleUnauthorizedAccessCredentials(t *testing.T) { + const credentialsID = "cid1" + + tests := []struct { + name string + setup func(*testing.T, Manager) + expectCredentialsHeld bool + }{ + { + name: "records a denial for an id with no credentials", + setup: func(*testing.T, Manager) {}, + }, + { + name: "recording the same denial twice is idempotent", + setup: func(t *testing.T, manager Manager) { + manager.SetAssumeRoleUnauthorizedAccessCredentials(credentialsID) + }, + }, + { + name: "leaves credentials already held for the id in place", + setup: func(t *testing.T, manager Manager) { + require.NoError(t, manager.SetTaskCredentials(testTaskCredentials(credentialsID))) + }, + expectCredentialsHeld: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + manager := NewManager() + tc.setup(t, manager) + + manager.SetAssumeRoleUnauthorizedAccessCredentials(credentialsID) + + assert.True(t, manager.IsCredentialsAssumeRoleUnauthorizedAccess(credentialsID)) + creds, ok := manager.GetTaskCredentials(credentialsID) + assert.Equal(t, tc.expectCredentialsHeld, ok) + if tc.expectCredentialsHeld { + assert.Equal(t, "akid1", creds.IAMRoleCredentials.AccessKeyID) + } + }) + } +} + +// TestIsCredentialsAssumeRoleUnauthorizedAccess tests that +// IsCredentialsAssumeRoleUnauthorizedAccess reports an unauthorized role only +// for the id it was recorded against. +func TestIsCredentialsAssumeRoleUnauthorizedAccess(t *testing.T) { + const credentialsID = "cid1" + + tests := []struct { + name string + setup func(*testing.T, Manager) + expected bool + }{ + { + name: "no denial recorded", + setup: func(*testing.T, Manager) {}, + expected: false, + }, + { + name: "denial recorded for the id", + setup: func(t *testing.T, manager Manager) { + manager.SetAssumeRoleUnauthorizedAccessCredentials(credentialsID) + }, + expected: true, + }, + { + name: "denial recorded for a different id", + setup: func(t *testing.T, manager Manager) { + manager.SetAssumeRoleUnauthorizedAccessCredentials("cid2") + }, + expected: false, + }, + { + name: "credentials held for the id but no denial recorded", + setup: func(t *testing.T, manager Manager) { + require.NoError(t, manager.SetTaskCredentials(testTaskCredentials(credentialsID))) + }, + expected: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + manager := NewManager() + tc.setup(t, manager) + + assert.Equal(t, tc.expected, manager.IsCredentialsAssumeRoleUnauthorizedAccess(credentialsID)) + }) + } +} diff --git a/ecs-agent/credentials/mocks/credentials_mocks.go b/ecs-agent/credentials/mocks/credentials_mocks.go index 12d63378b9a..ea19134043d 100644 --- a/ecs-agent/credentials/mocks/credentials_mocks.go +++ b/ecs-agent/credentials/mocks/credentials_mocks.go @@ -75,6 +75,20 @@ func (mr *MockManagerMockRecorder) GetTaskCredentials(arg0 interface{}) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTaskCredentials", reflect.TypeOf((*MockManager)(nil).GetTaskCredentials), arg0) } +// IsCredentialsAssumeRoleUnauthorizedAccess mocks base method. +func (m *MockManager) IsCredentialsAssumeRoleUnauthorizedAccess(arg0 string) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsCredentialsAssumeRoleUnauthorizedAccess", arg0) + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsCredentialsAssumeRoleUnauthorizedAccess indicates an expected call of IsCredentialsAssumeRoleUnauthorizedAccess. +func (mr *MockManagerMockRecorder) IsCredentialsAssumeRoleUnauthorizedAccess(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsCredentialsAssumeRoleUnauthorizedAccess", reflect.TypeOf((*MockManager)(nil).IsCredentialsAssumeRoleUnauthorizedAccess), arg0) +} + // IsCredentialsPending mocks base method. func (m *MockManager) IsCredentialsPending(arg0 string) bool { m.ctrl.T.Helper() @@ -101,6 +115,18 @@ func (mr *MockManagerMockRecorder) RemoveCredentials(arg0 interface{}) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveCredentials", reflect.TypeOf((*MockManager)(nil).RemoveCredentials), arg0) } +// SetAssumeRoleUnauthorizedAccessCredentials mocks base method. +func (m *MockManager) SetAssumeRoleUnauthorizedAccessCredentials(arg0 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetAssumeRoleUnauthorizedAccessCredentials", arg0) +} + +// SetAssumeRoleUnauthorizedAccessCredentials indicates an expected call of SetAssumeRoleUnauthorizedAccessCredentials. +func (mr *MockManagerMockRecorder) SetAssumeRoleUnauthorizedAccessCredentials(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetAssumeRoleUnauthorizedAccessCredentials", reflect.TypeOf((*MockManager)(nil).SetAssumeRoleUnauthorizedAccessCredentials), arg0) +} + // SetTaskCredentials mocks base method. func (m *MockManager) SetTaskCredentials(arg0 *credentials.TaskIAMRoleCredentials) error { m.ctrl.T.Helper()