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

bound_iam_role_arn is not bound to a role ARN but rather an instance-profile ARN #1769

Closed
skippy opened this issue Aug 23, 2016 · 9 comments
Closed
Assignees
Milestone

Comments

@skippy
Copy link
Contributor

skippy commented Aug 23, 2016

Perhaps I'm missing something, but when I read the directions, I thought this was a role arn, such as: arn:aws:iam::[acct_id]:role/proxy, and that it would look for that role from the aws instance profile. However, I received this error msg from vault:

Error writing data to auth/aws-ec2/login: Error making API request.

URL: PUT https://active.vault.service.consul:8200/v1/auth/aws-ec2/login
Code: 400. Errors:

* IAM Role ARN arn:aws:iam::[acct_id]:instance-profile/proxy_profile does not belong to role proxy

The proxy role is actually a part of this instance-profile, but that doesn't seem to be what the code is expecting.

when I set bound_iam_role_arn to the instance profile, such as bound_iam_role_arn=arn:aws:iam::[acct_id]:instance-profile/proxy_profile it works. That is fine, but it seems from the feature name and description that it is looking for a role arn and not an instance-profile arn.

BUT, this is a nice bound_ and it fits a nice middle ground between ami and account bounds; thank you for adding it!

@jefferai
Copy link
Member

@vishalnayak Thoughts on this? Maybe we need to switch the behavior for 0.6.2 and support both of these? (In the meantime we could update the docs.)

@skippy Any reason to support one over the other, or are both useful?

@jefferai jefferai added this to the 0.6.2 milestone Aug 23, 2016
@skippy
Copy link
Contributor Author

skippy commented Aug 23, 2016

@skippy Any reason to support one over the other, or are both useful?

A role ARN is a bit more generic (I think) as it tied to a set of credentials (whether from the ec2 instance or passed in as separate iam access keys) vs an instance-profile, which is only available if the credentials are gathered from the local ec2 meta provider. Another way to say this is if I passed in an access_key and secret directly into auth/aws-ec2/config/client, bound_iam_role_arn (which contains the instance-profile arn) probably wouldn't work as the access_key and secret can only have a role arn attached to them (I think).

(sorry for the I Think qualifications; IAM is large enough that the edge-case behaviors can be a bit of a surprise unless you actually have code that figure out what the real behavior is...and this is one edge case I have not actually explored; AWS hides the interactions between instance-profile and role-arns, and goes so far as to not show instance-profiles in the console and if you happen to create a role arn in the console it will automatically create an instance-profile for you... odd stuff; tis another reason to use CF/terraform :) )

@jefferai
Copy link
Member

jefferai commented Aug 30, 2016

@vishalnayak @skippy I've been thinking about this, and here's my thoughts -- let me know if they sound reasonable.

The current behavior is a bug. Changing that behavior will unfortunately undoubtedly make some people unhappy, but the parameter clearly says one thing but actually uses another, and the documentation also says the same thing (If set, defines a constraint on the EC2 instances that they should be using the IAM Role ARN specified by this parameter.) but is actually not correct either.

I think we should change the behavior to actually use the IAM Role ARN as documented, and add an additional bind against the instance profile ARN.

We could potentially be backwards-compatible by detecting :instance-profile/ in the ARN and swapping that value over to the new bind; this might get tricky with locking in the backend but if we can sort that out, we should probably do so.

@vishalnayak
Copy link
Contributor

@jefferai @skippy I agree that both should be supported, and that we should also try to be backwards-compatible.

@skippy
Copy link
Contributor Author

skippy commented Aug 31, 2016

@jefferai one thought on backwards compatibility; the other option besides what you suggested is to add a new bind bound_iam_instance_profile_arn, and for bound_iam_role_arn have a branch that says:

  if (bound_iam_role_arn =~ /:instance_profile\//) {
    log.warn "DEPRECATED: ...."
    return old_behavior
  }
  new_behavior

that way if someone has a bound_iam_instance_profile_arn and bound_iam_role_arn with an old instance_profile, the behavior will be more explicit (this situation may occur during an upgrade cycle).

thanks folks

@jefferai
Copy link
Member

@skippy That may be what we need to do if the locking situation is too difficult, but ideally we'll just migrate the value over. That way it will be properly set in the right place, and will additionally show up correctly on a read of the role.

@skippy skippy changed the title bound_iam_role_arn is not really bound to a role ARN but rather an instance-profile ARN bound_iam_role_arn is not bound to a role ARN but rather an instance-profile ARN Sep 7, 2016
@vishalnayak
Copy link
Contributor

@jefferai I could not find a way to retrieve the "IAM Role ARN" set on the instance. So, I am simply fixing the issue of wrong nomenclature of the field and handling the upgrade and also the deprecation warnings.

I did not add another bound restriction to the role for the role ARN.
Please let me know if you happen to bump across a method to retrieve the role ARN of an EC2 instance.

@vishalnayak
Copy link
Contributor

After discussing internally, we did figure out a way to support both bound_iam_instance_profile_arn and `bound_iam_role_arn.

@skippy
Copy link
Contributor Author

skippy commented Sep 28, 2016

thanks!

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

No branches or pull requests

3 participants