99designs / aws-vault

A vault for securely storing and accessing AWS credentials in development environments
MIT License
8.52k stars 820 forks source link

Fix hang on invisible tty when using 2FA where prompt=terminal in some edge cases #1184

Closed onnos closed 1 year ago

onnos commented 1 year ago

This implements the fix suggested by @ngyuki in https://github.com/99designs/aws-vault/issues/1054. When using prompt=terminal (which we use to support both Linux and Mac using the same workflow and config, without osascript involvement), we used to have a workaround in .aws/config that would redirect the terminal prompt to stdout in most cases by adding 2> $(tty) to the end of the aws-vault credential_process call, like this:

credential_process=sh -c 'aws-vault --prompt terminal export base --duration 12h --format=json 2> $(tty)'

With this patch we can remove that redirect and the prompt always shows up, even when called through the SDK via aws --profile somerole s3 ls, for example. Also removes the creation of spurious files named not a tty when prompt=terminal is forced.

This expands on https://github.com/99designs/aws-vault/pull/1149 a bit, where instead of just preferring a non-terminal prompt without an active tty it should show up in all cases anyway when specifically requested.

mtibben commented 1 year ago

Are there are any weird edge cases to this? Is it possible to have a terminal without a TTY these days? Does this work on Windows and other OSs?

onnos commented 1 year ago

Are there are any weird edge cases to this?

I am not able to find any, in fact this only fixes an edge case. This particular path is only prompted when prompt=terminal is forced by greybeards like myself who do not want to leave the confines of the terminal to go through 2FA. In the normal flow the "workaround" of popup GUI prompts are used. While the GUI popups are good UI for many people, I'd argue that allowing a full-terminal flow when specifically requested is a nice thing to do. With v7, this was made possible by the changes in https://github.com/99designs/aws-vault/pull/1149, however one edge case still remains - specifically when credential_process is called in the background and prompt-terminal is specified, the aws-vault process has no access to any foreground ttys and still swallows the prompt.

Specifically: with ~.aws/config having the following sample configuration:

[profile base]
mfa_serial=arn:aws:iam::121212121212:mfa/username

[profile base-session]
credential_process=aws-vault7 --prompt terminal export base --duration 12h --format=json

[profile role]
role_arn=arn:aws:iam::242424242424:role/cross-account-role
source_profile=base-session

Running aws s3 ls --profile role will appear to hang. When you hit enter, you will get Error when retrieving credentials from custom-process: because the prompt was invisible. The version with this patch will show the prompt. Any other flow remains untouched.

Is it possible to have a terminal without a TTY these days?

I don't think so? I might be misunderstanding the question, but to my belief anything without a TTY cannot be considered a terminal. TTY's are essential for I/O and signalling. As The TTY demystified says though, "the TTY subsystem — while quite functional from a user's point of view — is a twisty little mess of special cases."

Does this work on Windows and other OSs?

I don't have a Windows machine to test on. That said, this works on Macos and Linux machines and I can't imagine it not working in any terminal emulator. Again though, this only targets one particular flow where prompt=terminal is forced - when that is not enabled the normal prompt flows will still trigger and work as they do currently.

I've rebased my branch and fixed the linter error that I saw show up in the Github Actions. Thanks for your quick review so far!