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

Change split on instance profile name #2802

Merged
merged 1 commit into from
Jun 5, 2017
Merged

Change split on instance profile name #2802

merged 1 commit into from
Jun 5, 2017

Conversation

pwae
Copy link
Contributor

@pwae pwae commented Jun 4, 2017

Previously this would split on the Instance profile ARN on :instance_profile/
This changed the split to /, so that the line below the change, can pull only the short name of the Instance Profile instead of the instance profile name with path.

e.g. for the ARN: arn:aws:iam::111111111111:instance-profile/some/name

Previously:
iam:GetInstanceProfile would be sent "some/name"

Now:

iam:GetInstanceProfile is sent "name".

relates to #2729

This now splits on the /, so we only get the last component of the instance profile name (ignoring paths)
@jefferai
Copy link
Member

jefferai commented Jun 4, 2017

@joelthompson from earlier discussions I feel like this might help some and break others. Thoughts?

@jefferai jefferai added this to the 0.7.3 milestone Jun 4, 2017
@joelthompson
Copy link
Contributor

@jefferai -- tl;dr: I'm pretty confident this particular change is safe.

For the change we were discussing in #2729 relates to the IAM auth method only and how users are inputting the bound IAM principal ARN. The only time this code here gets called is using either the ec2 auth method or inferring an EC2 instance, and specifying a bound _iam_role_arn. Vault reads the IAM instance profile ARN out of the the EC2 API then parses that for the sole purpose of looking up the IAM role that is attached to the instance profile.

Currently, Vault is improperly parsing the IAM instance profile arn, and when there is a path component to the profile (e.g., for an ARN of arn:aws:iam::123456789012:instance-profile/path/ProfileName the path would be "/path/"), Vault includes the path in the actual name, resulting in an instance profile "name" that looks like "path/ProfileName". That is an invalid name; the API docs for iam:GetInstanceProfile have a regex that doesn't allow "/" in the name, and it results in an error for the client every time. In fact, the path component isn't even a parameter in any of the instance profile API methods except iam:CreateInstanceProfile and iam:ListInstanceProfiles.

The only way I can think that this would break a user is if that user has an instance profile name with a "/" in it, but that's not allowed by AWS and breaks every time (and, e.g., the iam:CreateInstanceProfile docs have the same regex for allowed names, so there's no way to even create such an instance profile in the first place), so I can't think of a way this would break any existing users.

@jefferai
Copy link
Member

jefferai commented Jun 5, 2017

@joelthompson many thanks for digging in and looking!

@jefferai jefferai merged commit f6ba66c into hashicorp:master Jun 5, 2017
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.

3 participants