Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support paths in AWS instance profiles for AWS IID node attestation #2825

Merged
merged 1 commit into from Mar 7, 2022

Conversation

appian-ashugarts
Copy link
Contributor

Pull Request check list

  • Commit conforms to CONTRIBUTING.md?
  • Proper tests/regressions included?
  • Documentation updated?
    • Shouldn't require documentation update, but can add if requested

Affected functionality

Server AWS IID node attestor plugin

Description of change

Referencing issue #2796, when an instance profile has a path in the
ARN (say
arn:aws:iam::123412341234:instance-profile/some/path/profile-name)
the AWS IID node attestor attempts to get the instance profile
information from AWS passing in both the path and the
name (some/path/profile-name). AWS, however, considers only the
name (profile-name) to be relevant and returns a ValidationError if
the path is included as the forward slashes in the path are considered
invalid. AWS IAM documentation indicates that profile names are simply
EC2 specific versions of role names which are guaranteed to be unique
regardless of path. The fix for the node attestor is to pull out only
the name of the instance profile from the string that was previously
being passed in.

Which issue this PR fixes

Fixes #2796

@azdagron
Copy link
Member

azdagron commented Mar 7, 2022

Thanks for fixing this @appian-ashugarts!

AWS IAM documentation indicates that profile names are simply
EC2 specific versions of role names which are guaranteed to be unique
regardless of path.

For my own edification, do you have a link to this part of the documentation?

@appian-ashugarts
Copy link
Contributor Author

Thanks for fixing this @appian-ashugarts!

AWS IAM documentation indicates that profile names are simply
EC2 specific versions of role names which are guaranteed to be unique
regardless of path.

For my own edification, do you have a link to this part of the documentation?

Sure @azdagron! Here you go: https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_use_switch-role-ec2.html and https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_use_switch-role-ec2_instance-profiles.html. I admit you have to squint a little to get the simplified summary I wrote though 😄

azdagron
azdagron previously approved these changes Mar 7, 2022
Copy link
Member

@azdagron azdagron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reading, I am satisfied with your explanation :) And again, thanks for the contribution!

@azdagron
Copy link
Member

azdagron commented Mar 7, 2022

Would you mind either 1) rebasing this on latest main or 2) giving us permission to add commits to this PR (there is a checkbox) so we can pull it up?

Referencing issue spiffe#2796, when an instance profile has a path in the
ARN (say
`arn:aws:iam::123412341234:instance-profile/some/path/profile-name`)
the AWS IID node attestor attempts to get the instance profile
information from AWS passing in both the path and the
name (`some/path/profile-name`). AWS, however, considers only the
name (`profile-name`) to be relevant and returns a ValidationError if
the path is included as the forward slashes in the path are considered
invalid. AWS IAM documentation indicates that profile names are simply
EC2 specific versions of role names which are guaranteed to be unique
regardless of path. The fix for the node attestor is to pull out only
the name of the instance profile from the string that was previously
being passed in.

Signed-off-by: Andrew Shugarts <andrew.shugarts@appian.com>
@appian-ashugarts
Copy link
Contributor Author

Should be rebased, I'm sure you'll let me know if I've borked it though haha

Copy link
Member

@azdagron azdagron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

@azdagron azdagron added this to the 1.2.1 milestone Mar 7, 2022
@azdagron azdagron merged commit 93e650d into spiffe:main Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error Using aws_iid NodeAttestor Plugin When Instance Profile Has a Path
2 participants