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

U2F fails in batch mode #238

Closed
ad8-bdl opened this issue May 8, 2020 · 13 comments
Closed

U2F fails in batch mode #238

ad8-bdl opened this issue May 8, 2020 · 13 comments

Comments

@ad8-bdl
Copy link
Contributor

ad8-bdl commented May 8, 2020

When using batch mode with a 2FA Yubikey enabled account, keeper prompts for the U2F touch however immediately falls through to a 2FA code prompt, as if "Enter" had been pressed.

env:

% sw_vers 
ProductName:	Mac OS X
ProductVersion:	10.15.4
BuildVersion:	19E287
% keeper --version                                         
Keeper Commander, version 4.31
  • keeper installed via pip3, per readme

repro:

% rm config.json
% echo 'whoami' | keeper --user '[email protected]' -
Password:

Touch the flashing U2F device to authenticate or press Enter to resume with the primary two factor authentication...
Two-Factor Code:
  • same thing when using --batch-mode shell
  • fwiw, the yubikey begins flashing at the expected time; entering a valid code succeeds
@ad8-bdl
Copy link
Contributor Author

ad8-bdl commented May 8, 2020

Also, the message "Touch the flashing U2F device to authenticate or press Enter to resume with the primary two factor authentication..." is sent to stdout; should be stderr as per the "Two-Factor Code" prompt.

@sk-keeper
Copy link
Collaborator

As far as I can see this issue, when user interaction is required in a batch mode we can either

  1. interrupt batch mode and try to interact with user. In this case it's OK to use stdout as long as stdin is going to be used
  2. fail a batch
    Commander uses the first approach. It's subjective which way is better.

@ad8-bdl
Copy link
Contributor Author

ad8-bdl commented May 10, 2020

I'd have expected batch-mode to be intended in programmatic use cases, and thus stdout probably wouldn't be the user's terminal. e.g. You can't use stdout when you're directing it to a pipe or file. IMHO you should be able to do:

echo -e 'sync-down\nexport --format json\nlogout' | keeper --user "${keeperuser}" - | gpg --sign --encrypt --recipient "${keeperuser}" > keeper-$(date '+%Y-%m-%d').json.gpg

without the backup including prompts from the authentication process.

Also, the password prompt uses the user's terminal, not stdout, for unix (https://docs.python.org/3/library/getpass.html#getpass.getpass). It would be good to have the password auth and 2FA be consistent by 2FA prompts behaving the same as for getpass.

@ad8-bdl
Copy link
Contributor Author

ad8-bdl commented Oct 16, 2020

The fix for this issue, PR #241 , doesn't appear to have been merged yet. Any idea when this might be done?

@ad8-bdl
Copy link
Contributor Author

ad8-bdl commented Jan 5, 2021

The issue is still present with the new 'v3' authentication flow.

Expected behaviour is that running in batch mode should send all auth flow to the console, such that one can run commands like
echo 'whoami' | keeper - > file

@ad8-bdl
Copy link
Contributor Author

ad8-bdl commented Dec 5, 2022

The issue is still present as of release 16.8.4. The current Enterprise documentation implicitly requires a persistent login to work around having to provide authn via
--batch-mode.

Expected behaviour is that running in batch mode should send all auth flow to the console, such that one can run commands like

echo 'whoami' | keeper - > file

without requiring a persistent session.

That is, I want to reduce the window of valid authentication to just the specific invocation of the keeper client; not for some arbitrary period of time. (edit: I wrote earlier re. a concern that the persistent session could expire in the middle of a sequence of batch commands, however it appears that the timeout is an "idle timeout", not an absolute session timeout (undocumented); that is, the persistent session can effectively be extended indefinitely by way of a while echo keep-alive | keeper --batch-mode -; do sleep 60; done; whether this is better or worse depends on your perspective I guess.)

As a better workaround, it seems you can include the this-device persistent-login off command at the end of the batch to forcible expire the persistent session. For example

% keeper --user "${keeperuser}" this-device persistent-login on
... (interactive auth)
% echo -e 'whoami\nthis-device persistent-login off' | keeper --batch-mode -
... (output from whoami)
Successfully DISABLED Persistent Login on this device

@sk-keeper
Copy link
Collaborator

The only acceptable login user interaction while in the batch mode is asking for password.
So persistent login is not the only option though the most convenient.
2fa login step (and any other non-password steps) uses stdin/stdout. It breaks the batch mode.

The config.json file could be prepared for password only login if persistent login is not an option.

@ad8-bdl
Copy link
Contributor Author

ad8-bdl commented Dec 5, 2022

The only acceptable login user interaction while in the batch mode is asking for password.

Why's that? How is interacting for a password any different than interacting for a password + MFA? Consider a passwordless future - the "interaction" may not even be on the console, it could be via my phone.

Perhaps I should explain my use case better: my goal is to prepare a set of Keeper commands (batch commands) to be executed by a user. For example, a simple wrapper shell script that might look like this

#!/bin/sh

keeper --batch-mode - <<.
whoami
ls
.

The user does not need to know anything about the keeper CLI commands. All they need to do is run the script and enter their creds+MFA as prompted.

Now, as a workaround I can do

#!/bin/sh

keeper this-device persistent-login on
keeper --batch-mode - <<.
whoami
ls
this-device persistent-login off
.

edit: actually: that doesn't work; keeper this-device persistent-login loses the console when run via a script. Sigh.

Take 2:

#!/bin/sh

tty=$(tty)
keeper this-device persistent-login on > $tty < $tty
keeper --batch-mode - <<.
whoami
ls
this-device persistent-login off
.

(I'm sure there's a better way to force keeper to use the current tty; but really, should I have to?)

However the workaround is fragile - if the script or batch commands fail, the persistent-login will not be nuked, and we'll have a nice backdoor hanging around.

@sk-keeper
Copy link
Collaborator

sk-keeper commented Dec 5, 2022

Also the Commander's commands can be stored to some file then
keeper --user "${keeperuser}" <filename>
Commander will execute commands from the file.

login ${keeperuser}
whoami
ls

@sk-keeper
Copy link
Collaborator

password does not use stdin it uses direct tty access

@ad8-bdl
Copy link
Contributor Author

ad8-bdl commented Dec 5, 2022

Ah - thanks for the pointer re. keeper <filename> - turns out that seems to do exactly what I need!

No idea how I missed this when RTFM ;-)

@sk-keeper
Copy link
Collaborator

sk-keeper commented Dec 5, 2022

I'm not sure if it is documented but Commander is supposed to suppress command errors if commands start with @
For instance:

@mkdir "Folder"

does not fail if the folder exists

@ad8-bdl
Copy link
Contributor Author

ad8-bdl commented Dec 6, 2022

No idea how I missed this when RTFM

Now I see how I missed this in the doc: it's not really made obvious.

There are a couple of issues with that feature, ref. #903

@ad8-bdl ad8-bdl closed this as completed Dec 6, 2022
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