From c698cbdc936c401f3c21705f15cf624f22221497 Mon Sep 17 00:00:00 2001 From: Anuj Singh Date: Fri, 4 Sep 2026 09:29:59 -0700 Subject: [PATCH] Track unauthorized roles from IMDS scans The IMDS info file reports a Code per credential. Success means the provider assumed the role and wrote a credential file; AssumeRoleUnauthorizedAccess means it was not authorized to assume it. Parse the Code in the shared library scanner, so a scan returns unauthorized roles alongside retrieved credentials. An agent can then track those roles in the credentials manager keyed by credentials id, if needed for metrics. An unauthorized role leaves any credentials already held in place, and is cleared when credentials arrive for the id or the id is removed. The Docker-based agent in this repository only logs this info for debugging. It does not track the unauthorized task credentials in its credentials manager, since no metric is wired for emission. --- agent/imdscreds/refresher.go | 11 +- agent/imdscreds/refresher_integ_test.go | 3 + agent/imdscreds/refresher_test.go | 73 +++- .../credentials/imds/mocks/imds_mocks.go | 4 +- .../ecs-agent/credentials/imds/scanner.go | 106 +++-- .../imds/testutil/mock_imds_server.go | 45 +- .../ecs-agent/credentials/imds/types.go | 32 ++ .../ecs-agent/credentials/interface.go | 4 +- .../ecs-agent/credentials/manager.go | 36 +- .../credentials/mocks/credentials_mocks.go | 26 ++ .../credentials/imds/mocks/imds_mocks.go | 4 +- ecs-agent/credentials/imds/scanner.go | 106 +++-- ecs-agent/credentials/imds/scanner_test.go | 395 ++++++++++++++++-- .../imds/testutil/mock_imds_server.go | 45 +- ecs-agent/credentials/imds/types.go | 32 ++ ecs-agent/credentials/interface.go | 4 +- ecs-agent/credentials/manager.go | 36 +- ecs-agent/credentials/manager_test.go | 113 +++++ .../credentials/mocks/credentials_mocks.go | 26 ++ 19 files changed, 945 insertions(+), 156 deletions(-) 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()