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

SIGSEGV when trying to use current head version with multi-profile and mfa #196

Closed
deinspanjer opened this issue Feb 28, 2018 · 12 comments
Closed

Comments

@deinspanjer
Copy link
Contributor

I've had a working setup for a while like this:
~/.aws/credentials
[default]
key_id/access with read-only privs
[me_mfa]
key_id/access for my aws role which requires mfa

~/.aws/config
[profile me]
source_profile = me_mfa
role_arn = ..Admin..
mfa_serial .../me

I did a go get -u today and the default profile was working fine, but when I tried to do anything with my me account, either by passing -p me or by using awl switch, it immediately returned a SIGSEGV:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1d61b71]

goroutine 1 [running]:
github.com/wallix/awless/commands.hasEmbeddedRegionInSharedConfigForProfile(0xc4203f5198, 0x3, 0x7fff5fbff932, 0x3, 0x0, 0x0, 0x2e7b680)
	/Users/me/go/src/github.com/wallix/awless/commands/hooks.go:250 +0xc1
github.com/wallix/awless/commands.applyRegionAndProfilePrecedence(0x0, 0x0)
	/Users/me/go/src/github.com/wallix/awless/commands/hooks.go:70 +0xb9
github.com/wallix/awless/commands.initAwlessEnvHook(0x2ebf5c0, 0xc4203fb2c0, 0x0, 0x2, 0x22c0e48, 0x117f509)
	/Users/me/go/src/github.com/wallix/awless/commands/hooks.go:50 +0xa7
github.com/wallix/awless/commands.applyHooks.func1(0x2ebf5c0, 0xc4203fb2c0, 0x0, 0x2)
	/Users/me/go/src/github.com/wallix/awless/commands/hooks.go:38 +0x88
github.com/wallix/awless/vendor/github.com/spf13/cobra.(*Command).execute(0x2ebf5c0, 0xc4203fb260, 0x2, 0x2, 0x2ebf5c0, 0xc4203fb260)
	/Users/me/go/src/github.com/wallix/awless/vendor/github.com/spf13/cobra/command.go:730 +0x237
github.com/wallix/awless/vendor/github.com/spf13/cobra.(*Command).ExecuteC(0x2ec0a00, 0x0, 0x42, 0xc420036d70)
	/Users/me/go/src/github.com/wallix/awless/vendor/github.com/spf13/cobra/command.go:831 +0x2e4
github.com/wallix/awless/vendor/github.com/spf13/cobra.(*Command).Execute(0x2ec0a00, 0xc4201cbf78, 0xc4200aa058)
	/Users/me/go/src/github.com/wallix/awless/vendor/github.com/spf13/cobra/command.go:784 +0x2b
main.main()
	/Users/me/go/src/github.com/wallix/awless/main.go:22 +0x2d

I tried reverting back to the latest release installed view Homebrew and it works fine.

Please let me know what other info you might want to track this down.

@simcap
Copy link
Contributor

simcap commented Mar 26, 2018

@deinspanjer Thanks for reporting and sorry for the delay!! (I got resourced on another project).

Also I do not know which 2 versions (commit sha) you ran, I am pretty sure that it has to do with this commit.

I shall be fixing that today.

@deinspanjer
Copy link
Contributor Author

Okay, happy to test it out. If necessary, I can probably go back and figure out the specific commits I was working against.

simcap added a commit that referenced this issue Mar 26, 2018
@simcap
Copy link
Contributor

simcap commented Mar 26, 2018

@deinspanjer Thanks for helping. It would be better if you give it a try with the latest master and show me the output. (Partial fix above should avoid nil reference and print what is wrong with the config I hope).

Can you do that?

@deinspanjer
Copy link
Contributor Author

Hmm.. well, I tried a simple brew install --HEAD awless, and it fails with Error: No such file or directory - awless.
I'll clean that up and try doing a pure go install instead.

➜  ~ brew install --HEAD awless
==> Installing awless from wallix/awless
==> Cloning https://github.com/wallix/awless.git
Updating /Users/dre/Library/Caches/Homebrew/awless--git
==> Checking out branch master
==> go run release.go -tag vHEAD-9fba342 -brew -arch amd64 -os darwin
Error: No such file or directory - awless

@deinspanjer
Copy link
Contributor Author

Okay, installing via go worked fine. Here is the (sanitized) output I get from exercising the two profiles:

➜  ~ go get -u github.com/wallix/awless
# (Git commit installed: 9fba342)
➜  ~ ln -s ~/go/bin/awless /usr/local/bin
➜  ~ awl whoami
cannot check profile 'dre' has embedded region in shared config file: AssumeRoleTokenProviderNotSetError: assume role with MFA enabled, but AssumeRoleTokenProvider session option not set.
Assume Role MFA token code: ######
ResourceType: assumed-role, Resource: AdminRole/###, Id: XXX:YYY, Account: ZZZ
➜  ~ awl switch default
cannot check profile 'dre' has embedded region in shared config file: AssumeRoleTokenProviderNotSetError: assume role with MFA enabled, but AssumeRoleTokenProvider session option not set.
[info]    region precedence: 'us-east-1' loaded through profile 'default' (see AWS config files $HOME/.aws/{credentials,config})
➜  ~ awl whoami
Username: read_only, Id: XXX, Account: ZZZ

Attached policies (i.e. managed):
	- ReadOnlyAccess

Inlined policies: none
➜  ~ awl -p dre whoami
cannot check profile 'dre' has embedded region in shared config file: AssumeRoleTokenProviderNotSetError: assume role with MFA enabled, but AssumeRoleTokenProvider session option not set.
ResourceType: assumed-role, Resource: AdminRole/###, Id: XXX:YYY, Account: ZZZ
➜  ~

@simcap
Copy link
Contributor

simcap commented Mar 27, 2018

Thanks!

To clarify, the check that we make is only to detect if a region is in the running embedded profile as shown here.

The code for this check is legit but I am guessing some loading options are missing in the session.NewSessionWithOptions for more advanced configuration (failing in our case with the error AssumeRoleTokenProviderNotSetError).

@deinspanjer Can you confirm your configuration in ~/.aws/{credentials, config} is valid? (i.e. it works correctly with the AWS CLI). I am guessing yes ;)

If yes, on my side I will try some options to not fail on loading configuration such as yours.

@deinspanjer
Copy link
Contributor Author

Yes, the config definitely works fine with core aws cli as well as boto3 and other libraries/utils.

Also note that awless actually successfully performs whatever operations are requested, it just gives that error first in the current version, and in the version released in brew on March 1, it gave no error at all.

@deinspanjer
Copy link
Contributor Author

deinspanjer commented Mar 28, 2018

Here is the .aws/config and .aws.credentials file structures I'm running with:

config:

[default]
region = us-east-1
output = text

[profile dre]
source_profile = dre_mfa
role_arn = arn:aws:iam::xxx:role/AdminRole
mfa_serial = arn:aws:iam::xxx:mfa/dre
region = us-east-1
output = json

[profile gfw_xxx]
source_profile = gfw_ecr_xxx
role_arn = arn:aws:iam::xxx:role/AdminRole

[profile gfw_ecr_xxx]
region = us-east-1

[profile kops]
region = us-east-1
output = json
[profile ecr]
region = us-east-1

credentials:

[default]
aws_access_key_id = xxx
aws_secret_access_key = yyy

[gfw_ecr_dev]
aws_access_key_id = xxx
aws_secret_access_key = yyy
aws_session_token = zzz==

[dre_mfa]
aws_access_key_id = xxx
aws_secret_access_key = yyy

[ecr]
aws_access_key_id = xxx
aws_secret_access_key = yyy

@deinspanjer
Copy link
Contributor Author

deinspanjer commented Mar 28, 2018

I found an issue in the k8s github that mentions an AssumeRoleTokenProvider option added a few months back: kubernetes/kops#226 (comment)

I'll try that out real quick and see if it has any effect.

Okay, I don't know the full ramifications of the change, but adding that option does definitely eliminate the error:
#199

@simcap
Copy link
Contributor

simcap commented Mar 29, 2018

Thanks for the input! I will have a look at that. In the meantime, using the current master will only print out the error. Everything should work fine expect when you have an embedded region in that config profile: in that case awless will not pick up the region. That is what need to be fixed.

@simcap
Copy link
Contributor

simcap commented Mar 29, 2018

This is the right fix concerning awless. We use this option in our official way to load the AWS session as shown here (compared to the hook quick session loading to see if a region is embedded).

When I implemented the code to look for an embedded region I overlook this option!

So your PR is valid and I will merge it. I will also revert back to returning an error instead of printing it (i.e. used for debugging that issue)

@simcap
Copy link
Contributor

simcap commented Mar 29, 2018

It got closed automatically!

@deinspanjer Anyway you can pull master that contains a commit and your PR. Any issues let me know. And thanks for the work.

Hopefully the version 0.1.10 should be out soon.

simcap added a commit that referenced this issue Apr 10, 2018
simcap pushed a commit that referenced this issue Apr 10, 2018
…der' to hasEmbeddedRegionInSharedConfigForProfile (#199)

Adding option 'assumeRoleTokenProvider: stscreds.StdinTokenProvider' to hasEmbeddedRegionInSharedConfigForProfile. Fixes #196
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

2 participants