Skip to content

Commit d88768a

Browse files
authored
Merge pull request #739 from projectdiscovery/improve-aws-verification
fix(aws): improve verify - check all accounts and improve role assumption reliability
2 parents c53a8ef + 1d7e604 commit d88768a

File tree

1 file changed

+51
-22
lines changed

1 file changed

+51
-22
lines changed

pkg/providers/aws/aws.go

Lines changed: 51 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -249,16 +249,21 @@ func New(block schema.OptionBlock) (*Provider, error) {
249249

250250
if err != nil && options.AssumeRoleName != "" && len(options.AccountIds) > 0 {
251251
// Base user doesn't have DescribeRegions permission, try with assumed role
252-
tempSession, err := createAssumedRoleSession(options, sess, config)
253-
if err != nil {
254-
return nil, errors.Wrap(err, "could not create assumed role session")
252+
var regionErr error
253+
for _, accountId := range options.AccountIds {
254+
tempSession, err := createAssumedRoleSession(options, sess, config, accountId)
255+
if err != nil {
256+
regionErr = err
257+
continue
258+
}
259+
tempRC := ec2.New(tempSession)
260+
regions, regionErr = tempRC.DescribeRegions(&ec2.DescribeRegionsInput{})
261+
if regionErr == nil {
262+
break
263+
}
255264
}
256-
257-
// Use assumed role session for DescribeRegions
258-
tempRC := ec2.New(tempSession)
259-
regions, err = tempRC.DescribeRegions(&ec2.DescribeRegionsInput{})
260-
if err != nil {
261-
return nil, errors.Wrap(err, "could not get list of regions even with assumed role")
265+
if regionErr != nil {
266+
return nil, errors.Wrap(regionErr, "could not get list of regions with any account")
262267
}
263268
} else if err != nil {
264269
return nil, errors.Wrap(err, "could not get list of regions")
@@ -270,12 +275,9 @@ func New(block schema.OptionBlock) (*Provider, error) {
270275
return provider, nil
271276
}
272277

273-
func createAssumedRoleSession(options *ProviderOptions, sess *session.Session, config *aws.Config) (*session.Session, error) {
274-
if len(options.AccountIds) == 0 {
275-
return nil, errors.New("no account IDs provided for assume role")
276-
}
278+
func createAssumedRoleSession(options *ProviderOptions, sess *session.Session, config *aws.Config, accountId string) (*session.Session, error) {
277279
stsClient := sts.New(sess)
278-
roleArn := fmt.Sprintf("arn:aws:iam::%s:role/%s", options.AccountIds[0], options.AssumeRoleName)
280+
roleArn := fmt.Sprintf("arn:aws:iam::%s:role/%s", accountId, options.AssumeRoleName)
279281

280282
roleInput := &sts.AssumeRoleInput{
281283
RoleArn: aws.String(roleArn),
@@ -524,14 +526,41 @@ func (p *Provider) Verify(ctx context.Context) error {
524526
}
525527

526528
if p.options.AssumeRoleName != "" && len(p.options.AccountIds) > 0 {
527-
tempSession, err := createAssumedRoleSession(p.options, p.session, p.session.Config)
528-
if err != nil {
529-
return err
530-
}
531-
p.initServices(tempSession)
532-
err = p.verify()
533-
if err != nil {
534-
return err
529+
var mu sync.Mutex
530+
var failedAccounts []string
531+
var wg sync.WaitGroup
532+
sem := make(chan struct{}, 200)
533+
534+
for _, accountId := range p.options.AccountIds {
535+
wg.Add(1)
536+
sem <- struct{}{}
537+
go func(id string) {
538+
defer wg.Done()
539+
defer func() { <-sem }()
540+
tempSession, err := createAssumedRoleSession(p.options, p.session, p.session.Config, id)
541+
if err != nil {
542+
mu.Lock()
543+
failedAccounts = append(failedAccounts, id)
544+
mu.Unlock()
545+
return
546+
}
547+
tempProvider := &Provider{options: p.options, session: tempSession}
548+
tempProvider.initServices(tempSession)
549+
if err := tempProvider.verify(); err != nil {
550+
mu.Lock()
551+
failedAccounts = append(failedAccounts, id)
552+
mu.Unlock()
553+
}
554+
}(accountId)
555+
}
556+
wg.Wait()
557+
558+
if len(failedAccounts) > 0 {
559+
msg := fmt.Sprintf("failed to assume role %s in accounts: %s", p.options.AssumeRoleName, strings.Join(failedAccounts, ", "))
560+
if p.options.OrgDiscoveryRoleArn != "" {
561+
msg += ". Add these to exclude_account_ids if they should not be part of discovery"
562+
}
563+
return errors.New(msg)
535564
}
536565
return nil
537566
}

0 commit comments

Comments
 (0)