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

Add more flexible support for two factor authentication. #467

Merged
merged 1 commit into from Nov 4, 2015

Conversation

Projects
None yet
8 participants
@perryjrandall
Contributor

perryjrandall commented Dec 30, 2014

Allow paramiko to partially authenticate and continue by
echo'ing the prompt on the remote end and responding.

Add more flexible support for two factor authentication.
Allow paramiko to partially authenticate and continue by
echo'ing the prompt on the remote end and responding.
@perryjrandall

This comment has been minimized.

Contributor

perryjrandall commented Dec 30, 2014

Last PR, sorry for spamming with the updates / closes I just learned how to update a PR XD

Again addresses auth for OTP or other interactive 2fac auth tokens / keys

@perryjrandall

This comment has been minimized.

Contributor

perryjrandall commented Dec 30, 2014

In regards to the test failing on CI... I cannot repro that behavior on my machine

>>> python -V
Python 3.4.2
>>> pip -V
pip 1.5.6 from /usr/lib/python3/dist-packages (python 3.4)
>>> inv test --coverage
...
test_3_multiple_key_files (test_client.SSHClientTest) ... ok
...
----------------------------------------------------------------------
Ran 147 tests in 20.953s

OK

I'm running Debian Testing

@lndbrg

This comment has been minimized.

Contributor

lndbrg commented Dec 30, 2014

I re-triggered the ci run (it timed out the first time) and it is green now.

@perryjrandall

This comment has been minimized.

Contributor

perryjrandall commented Dec 31, 2014

awesome, thanks

@perryjrandall

This comment has been minimized.

Contributor

perryjrandall commented Jan 2, 2015

Bumping :) I know it's still kinda the holidays but I'd greatly appreciate this getting pulled in! Thanks

@perryjrandall

This comment has been minimized.

Contributor

perryjrandall commented Jan 9, 2015

Hey @bitprophet what specifically do you want to see in the way of docs / changelog?

@perryjrandall

This comment has been minimized.

Contributor

perryjrandall commented Jan 27, 2015

Bumping again, hey @bitprophet I'd love to get this committed, what needs to be done?

@chantra

This comment has been minimized.

chantra commented Feb 13, 2015

Is there anything we can help with to get this into master?

@bitprophet bitprophet added this to the 2.0 milestone Feb 27, 2015

@bitprophet

This comment has been minimized.

Member

bitprophet commented Feb 27, 2015

Nope, just need time to check it over and test it on my end (just dug out of some start-of-year dayjob responsibilities, entering a pre-pycon open source sprint phase).

For now I am assuming the notes in #412 about reproducing/verifying, will apply to this implementation so that's what I'll do first.

@cbaenziger

This comment has been minimized.

cbaenziger commented Mar 7, 2015

For reference, I was able to use client.get_transport().auth_interactive_dumb(username=user) successfully for a proprietary two-factor authentication system where a simple client.connect(host, user, static_password) failed with:

paramiko.ssh_exception.BadAuthenticationType: ('Bad authentication type', [u'keyboard-interactive']) (allowed_types=[u'keyboard-interactive'])

However, I am new to Paramiko, so I do not know what could have been silently incorrect or wonky. Certainly all my password-y input was echoed back for better or worse (and I was just running in the Python REPL).

Either way, thanks for the awesome work!

@cbaenziger

This comment has been minimized.

cbaenziger commented Mar 7, 2015

My apologies I may be too naive to be of use. I also used Paramiko version 1.10 as provided with Ubuntu 12.04 and ran the following to the same two-factor authentication system successfully:

from getpass import getpass
def my_handler(title, instructions, prompt_list):
   print title
   print instructions
   return [echo and raw_input(prompt) or getpass(prompt) for (prompt, echo) in prompt_list]

try:
  client.connect(host, username=user)
except paramiko.ssh_exception.SSHException:
  pass

client.get_transport().auth_interactive(username=user, handler=my_handler)

Also, just to correct my last post the handler is required to not echo the password; nothing wrong with auth_interactive_dumb.

@adimania

This comment has been minimized.

adimania commented Sep 16, 2015

bumping this. The checks have passed. Can we merge this sometime soon please?

@mattrobenolt

This comment has been minimized.

mattrobenolt commented Oct 27, 2015

I tested this patch out, and it almost works.

Requires this extra patch to work in our case:

diff --git a/paramiko/client.py b/paramiko/client.py
index 0c8f6bd..2ecd487 100644
--- a/paramiko/client.py
+++ b/paramiko/client.py
@@ -503,7 +503,7 @@ def _auth(self, username, password, pkey, key_filenames, allow_agent,
                     try:
                         key = pkey_class.from_private_key_file(key_filename, password)
                         self._log(DEBUG, 'Trying key %s from %s' % (hexlify(key.get_fingerprint()), key_filename))
-                        self._transport.auth_publickey(username, key)
+                        allowed_types = set(self._transport.auth_publickey(username, key))
                         two_factor = (allowed_types & two_factor_types)
                         if not two_factor:
                             return

mattrobenolt@75b64b8

@bitprophet bitprophet modified the milestones: 1.16, 2.0 Oct 27, 2015

@bitprophet

This comment has been minimized.

Member

bitprophet commented Nov 4, 2015

Heyyyyy #467 what's good~

Prep work:

  • Dropping a link to #387 so that when I handle that crap, I remember that this is a thing & needs to not break during the rewrite (but hopefully that will be soon...) - that's if I don't throw my hands up and declare this as blocking on that.
    • Which I really don't want to do, but this part of Paramiko is just a shitpile and this PR doesn't make it any cleaner :D
  • I followed the tips from #412 to get google-based 2FA working via OpenSSH's client targeting a VM.
    • Both as the sole login method and as a literal "second" auth factor on top of password auth, depending on whether or not I @include common-auth in /etc/pam.d/sshd.
      • Which made me realize there's probably 2 related-but-distinct things to support here, a) support for multiple simultaneous authentication methods, and b) support for interactive non-password OTP type schemes, singularly or as a second factor.
    • I have to forcibly disable key auth on my client's end, since it seems to obviate any use of the Google Authenticator plugin. Might dig more if I care harder later.
    • The prompts OpenSSH client provides are useful (duh): says Password: for system password, Verification code: for google-auth.
  • Confirmed that hitting the VM with Paramiko master (via Fabric) with PAM configured to only use google-auth, works, since it's basically just a different password mechanism.
    • However, since Fabric has to write the prompt itself, it always says 'password' not 'verification code'.
  • Confirmed that once you tell PAM to want both password and google-auth, things don't work (only one prompt is presented and then failure occurs).

Trying to get the PR to function with the above setup fails with AuthenticationException after submitting a password, however, I suspect because the author & those testing it like @mattrobenolt have all convinced their sshd to do pubkey+google, whereas I am trying to test password+google. So I have to:

  • Figure out what's missing from the PR to get password+google working (having looked, I think not super hard) or identify what else is causing my failure;
  • Get pubkey+google working on my test target, then test that angle.
@mattrobenolt

This comment has been minimized.

mattrobenolt commented Nov 4, 2015

I suspect because the author & those testing it like @mattrobenolt have all convinced their sshd to do pubkey+google, whereas I am trying to test password+google

We're specifically using pubkey + Duo Unix with their PAM module.

And specifically using AuthenticationMethods publickey,keyboard-interactive in our sshd_config (following Duo's directions verbatim).

I'm happy to share any other information if you need it.

@mattrobenolt

This comment has been minimized.

mattrobenolt commented Nov 4, 2015

Also, for more context on our use case if it's relevant:

We use Fabric to command our deploys, which happen through some automated magic. This means the 2FA needs to be bypassed in our case, which is a thing that Duo supports. Our build use doesn't need to prompt for the second auth and Duo just noops their case.

Not sure if this changes the behavior here, but for both of the auth cases, there are no prompts. keyboard-interactive just ends up passing through without needing any input since the PAM module allows it.

@bitprophet

This comment has been minimized.

Member

bitprophet commented Nov 4, 2015

Notes to self...

Notable difference is that the userauth request method is keyboard-interactive on openssh's part but is password on Paramiko/Fabric's part. I assume this is why the failure scenario doesn't appear to respond as expected re: password, then auth code.

EDIT: yea, a bit of the ol' Google shows that keyboard-interactive basically supersedes password, and maps to telling OpenSSH's sshd ChallengeResponseAuthentication yes (which was required for the google plugin, and presumably any interesting PAM stuff, to function).

So when Paramiko's (horrible, terrible, no good, very bad) Client._auth falls down to the end and calls self.transport.auth_password, this triggers code in AuthHandler that explicitly submits a password-method auth request (as we saw in the logs).

That's supposed to trigger a BadAuthenticationType exception internally and fall back to auth_interactive. But this isn't happening in my case, probably because my sshd is actually supporting password so it's a different failure scenario from the server's perspective.

I think this can be considered a very niche case and I'm not likely to try fixing it right now, if I can get pubkey based auth working instead. Will try to confirm by telling the server to stop supporting password.

@bitprophet

This comment has been minimized.

Member

bitprophet commented Nov 4, 2015

@mattrobenolt thanks for the deets (& the missed-spot patch you commented earlier), that (so far) seems to map to what I'm seeing. I'm guessing your Duo Unix setup behaves closely enough like Google Auth that it won't matter on my end. We'll see...

@bitprophet

This comment has been minimized.

Member

bitprophet commented Nov 4, 2015

  • Confirmed that when I explicitly state PasswordAuthentication no on the sshd, openssh client is still happy in both 2FA and "just password" situations, and Fabric/Paramiko are still happy in the "just password" situation, proving that said fallback code works / is happening (also verified this in paramiko's log).
  • Paramiko still being a jerk in the "password + 2FA" scenario, trying to figure out why exactly - even when I update the logic at the bottom of Client._auth to not assume password and 2FA are mutually exclusive, the initial password step is failing during the fallback to keyboard-interactive.
@bitprophet

This comment has been minimized.

Member

bitprophet commented Nov 4, 2015

Aside from the failure log saying attempts 1 failures 1 (again, assuming that is because of the first password attempt) vs a success saying attempts 1 failures 0, the two logs look the same until the failure log is all sshd(pam_google_authenticator)[2101]: Invalid verification code.

Probably need to peep the packets being sent by openssh client vs paramiko cuz that still doesn't make sense to me, unless sshd is literally treating attempt 1 as the passwordy login and attempt 2 as the google-auth login.

I'm gonna tinker real quick to try and short circuit around the password login attempt to see if that's it, and if not, will skip ahead to key auth because ugh time.

@bitprophet

This comment has been minimized.

Member

bitprophet commented Nov 4, 2015

Aha, some more reading/logging shows that auth_interactive is being fallen back to via a handler callback, which yields the password that was to be used for the password-type message. (I.e. it's auth_password trying to non-interactively execute auth_interactive.)

So what's happening is this handler is called twice, once for each prompt, which is why the google-auth pam plugin is all mad - it really is being given input, and the wrong input.

This is apparently the documented behavior of Transport.auth_interactive, to wit, The handler may be called more than once if the server continues to ask questions. Specifically it's implemented inside AuthHandler as "store the handler, respond with it when you get a new INFO request of the expected type", and clearly the client gets two of those in a row from the server.

This was unexpected in part because all these auth handlers are implemented/documented as "in the rare case of multi-step auth, this returns other acceptable auth types" - but this specific "password + 2FA" setup is not actually multi-step auth in that sense because it's two keyboard-interactive "questions" instead of e.g. one privkey and one keyboard-interactive.

tl;dr this really is the fault of me trying to get it working without a key, so I am definitely doing that next and putting this aside for now.

@bitprophet

This comment has been minimized.

Member

bitprophet commented Nov 4, 2015

FTR I needed to:

  • Explicitly configure AuthenticationMethods in sshd config (to pubkey,keyboard-interactive, though there's no actual default apparently, just "any one thing will do", thus the behavior I was seeing)
  • Remove inclusion of PAM's general account module in the sshd module, since otherwise I need to give my pubkey, the 2FA token and the system user's password, which is a bit much.
    • Though post-#387 we'll hopefully be able to handle that as well...it's not worth futzing with before then IMHO.

This gets me a working pubkey+2FA setup which presumably matches the other folks' configurations.

With master Paramiko, things just hang after logging WARNING:paramiko.transport:Oops, unhandled type 3.

With my branch based on this PR, I am successfully prompted for the verification code, enter it, and successfully connect. Notes:

  • No changes needed on Fabric's end, so I can probably close fabric/fabric#1202
  • The prompt echoes my input, unlike vanilla OpenSSH which hides my input.
    • This is non-ideal, but not a deal-breaker given the nature of 2FA tokens. Suspect it too can wait until after #387 rearchitects auth handling.

Think this means we can merge and move on, finally.

@bitprophet bitprophet merged commit dae916f into paramiko:master Nov 4, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

bitprophet added a commit that referenced this pull request Nov 4, 2015

@perryjrandall

This comment has been minimized.

Contributor

perryjrandall commented Nov 4, 2015

Woo hoo! Glad to see this merged, thanks @bitprophet

@Symmetric

This comment has been minimized.

Symmetric commented Jan 15, 2016

It wasn't immediately obvious that this would be the case from skimming this thread, but this also works with sftp servers using the slightly odd configuration

AuthorizationMethods publickey,password

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment