-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Initial support for using Kerberos. #1261
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
Conversation
Oh, sorry, I didn't mean to start this right away. (I thought I was pulling against our forked repo.) Anyway, does this look like the right approach? What sort of tests would you guys want here, if any. (I am just passing the various kerberos parameters from the env into paramiko.) |
Testing kerberos is actually super difficult as far as I can tell (we were unable to add meaningful tests to Paramiko's support) so I think we'd have to settle for just "are we passing the right parameters to the Paramiko level" perhaps. Re: approach, it looks like an okay direction for now, this is basically what I was expecting (see #195 (comment)). Thanks! Please comment again when you're ready for actual review (also note that I have a huge backlog D:) |
@bitprophet Thanks. I was going to bug the other people here to see if they'd do anything differently. I'm using this now and it seems to be working fine. (If that fills you with much confidence. Hah!) |
@funkaoshi So you would consider this ready for merger/review now? If so I think I’ll give it a try. |
@kyrias no one at the office has complained! It's a very small change. |
(Right now for simplicity you can't pick a different host/port.) |
Eager to hear feedback from @kyrias but given how simple it is I'm gonna guess it works fine & any issues will be at the Paramiko level. Added to next feature release milestone. |
Should I rebase this on top of master? (Or one of the release branches?) Adding gss_host as a parameter might be tricky, because you can run commands against multiple hosts at the same time. (I think that's why I didn't include it originally.) |
Yep, seems to work just fine for me. |
@funkaoshi Feature releases come off of master, so no need to move it to a release branch. Rebase to latest master if you want but the diff is pretty small & github isn't identifying any merge conflicts, so I doubt there's a problem at this point. @kyrias Thanks! Good to know. Keeping it in that release milestone, not sure when I'll get to it, probably post Pycon or during sprints. |
Sorry, one more thing. To use Kerberos tickets you need to install python-gssapi. Should this become a dependency of Fabric? I don't feel like it should, because how many people actually use Kerberos. On the other hand using |
Also i'll be at PyCon too. What! |
This PR changed my life! A few years back ssh-keygen and I had an altercation which left me hospitalized for a month. Ever since then I've been unable to use keypair authentication and assumed my life was over as it was (password auth? HAH, here let me just package my identity up for you to use at will). But now with GSSAPI support I can fab deploy a new life without worry or fear ever again But in all seriousness +1 on this |
Explicitly erroring out/printing a warning when trying to use do GSS-API stuff without python-gssapi might be good, though the paramiko error, while less nice maybe, is sufficient I guess. |
TBH we should just make the paramiko error better, it's historically been kinda bluh at useful errors and it is one area I want to tidy up as I now own it 😩 |
@bitprophet Agreed. Better to do things at the source, usually. Are you guys (still) at PyCon? |
Will someone else write the docs? I'm not sure where to start with that. |
@funkaoshi I'm willing to write 'em up, don't think I can modify your PR though. Seems like the docs update should be bundled with the merge. |
@dahlke You can send a PR to the sdelements/fabric basic-kerberos branch though. |
Oh man it's been months. What do I need to do? Or how can I help you @dahlke? You could do a PR on top of my PR, yeah. |
Barf, yea it's been forever. We even semi-fixed-up GSSAPI after it broke again, in Paramiko. There's still some annoying confusion because of the issue documented in paramiko/paramiko#584 but AFAIK if one has If this basic "pass the vars through" approach still works with Paramiko 1.15/1.16 + |
To be clear, what this means is I'd like @funkaoshi or somebody to:
Then I will merge & tweak our install docs & it'll be ready for next Fab feature release (or I will just make one because big releases are dumb and a very bad habit). Thanks! |
Thanks. Let me rebase on top of master and see what's the what. |
Support 3 additional parameters when connecting to hosts: gss-auth, gss-deleg, gss-kex. These correspond to the equivalent in paramiko. It does all the hard work.
27459ff
to
07b0e41
Compare
Will test things locally here and report back. |
And based on my (non-exhaustive) testing it looks like this continues to work (assuming you have python-gssapi installed as well). We use this fork internally for some DevOps stuff, so I can ask them to update and see if anything breaks for them. |
It would be really nice to have this in a release, is there anything I can do to help get this merged? |
@buckett see #1261 (comment) - tl;dr additional "I tested this with a recent Paramiko release and a live Kerberos install and it worked for me" notes would be helpful. That said, @funkaoshi gave a thumbs up so I mostly just need to find time to sprint through the 1.11 milestone that this is part of! |
Noticed one big problem with this, which is that it blows up on any Paramiko<1.15 (which is when Kerberos support landed). I'm fixing it up so it only submits those kwargs when they're set to non- This should mean enabling them works as before, but if one doesn't touch them, they're not submitted, and Paramiko e.g. 1.13 won't explode. |
Allows use of Paramiko<1.15. Re #1261
Ah, thanks for making the change. Looks good. |
Support 3 additional parameters when connecting to hosts: gss-auth,
gss-deleg, gss-kex. These correspond to the equivalent in paramiko. It
does all the hard work.