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 for importing AWS vault credentials from encrypted files #251

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

williamhpark
Copy link
Contributor

@williamhpark williamhpark commented Apr 21, 2023

Summary

Resolves: #248

Currently, if you store AWS credentials using AWS vault with an encrypted file as the backend, then try to import credentials into 1Password using the AWS vault importer, you get a long invalid memory address or nil pointer dereference error in the terminal and no import candidates are shown. We want to support importing credentials from encrypted files.

Also, while importing using a build of the AWS plugin that isn't a local build, several log messages display in the terminal. These messages are invoked in the AWS vault code - we want to silence these. Example:

2023/04/21 13:19:32 Loading config file /Users/williampark/.aws/config
2023/04/21 13:19:32 Parsing config file /Users/williampark/.aws/config
2023/04/21 13:19:32 Unrecognised ini file section: DEFAULT

How to Test

  1. Add a profile through aws-vault using an encrypted file as the backend: aws-vault add file-profile --backend file
  2. Run op plugin init aws (don't use a local build) > Import into 1Password...

Expected Behaviour

No unnecessary log messages should be displayed.

You should get a prompt in the terminal: Enter passphrase to unlock "~/.awsvault/keys/": . After inputting the correct password, Encrypted file (file-profile) should be displayed as an import candidate. You should be able to import the credential into 1Password successfully.

image

@williamhpark williamhpark self-assigned this Apr 21, 2023
@williamhpark williamhpark marked this pull request as ready for review April 21, 2023 20:22
@williamhpark
Copy link
Contributor Author

williamhpark commented Apr 21, 2023

There's currently a gap in this PR that I haven't been able to find a good solution for.

When a non-local build for the AWS plugin is used, everything behaves as desired (i.e. like in the PR description). However, when a local build is used, the Enter passphrase to unlock "~/.awsvault/keys/": prompt doesn't display in the terminal. This makes it seem like the terminal is just hanging while it waits for the user's password input:

image

I had a discussion with @AndyTitu and he mentioned that this is likely because of the use of RPC for local builds causing issues outputting to the terminal.

One simple-ish solution could be to detect if the AWS build is local or not in importers.go, and if it is local, simply skip encrypted files as a possible backend by removing it from the availableBackends list. If we don't want to do this, would anyone have any other suggestions on how to fix this issue?

@williamhpark williamhpark added the waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member label Apr 26, 2023
plugins/aws/importers.go Show resolved Hide resolved
}

fmt.Fprintf(os.Stderr, "%s: ", prompt)
b, err := term.ReadPassword(int(os.Stdin.Fd()))
Copy link
Contributor

Choose a reason for hiding this comment

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

I wasn't able to test this because I cannot get encrypted files to work correctly as a backend to my aws-vault.
Screenshot 2023-04-26 at 15 39 49
Is this working as expected for you @williamhpark ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @AndyTitu, the issue is that you need to specify --backend file when you run aws exec as well (I know kind of weird). So running aws-vault exec file-profile --backend file -- aws sts get-caller-identity should work for you. It works for me.

return password, nil
}

fmt.Fprintf(os.Stderr, "%s: ", prompt)
Copy link
Contributor

@AndyTitu AndyTitu Apr 26, 2023

Choose a reason for hiding this comment

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

This doesn't show up when running a local profile through RPC. However, the line underneath for the Stdin works as expected. @jpcoenen do you know why that is, and how could we overcome that? This is the only aspect that blocks this PR.

However, when a local build is used, the Enter passphrase to unlock "~/.awsvault/keys/": prompt doesn't display in the terminal. This makes it seem like the terminal is just hanging while it waits for the user's password input:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we printing to Stderr?

Copy link
Member

Choose a reason for hiding this comment

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

Apologies; I missed this comment originally. I think there is a way to read the output a plugin writes to stderr, but I am not sure whether that is the preferred way to go here.

Might be interesting to see if there are any Terraform Providers that prompt the users. If so, we could see how they tackle that.

@AndyTitu AndyTitu added in-progress this PR is being worked on/comments are in the process of being addressed by the contributor and removed waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member labels May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-progress this PR is being worked on/comments are in the process of being addressed by the contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AWS Vault importer failing when using encrypted file as backend
4 participants