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

Server should not request user's password for GetAddSSHKey #36

Open
frangarciam opened this issue Mar 30, 2015 · 4 comments
Open

Server should not request user's password for GetAddSSHKey #36

frangarciam opened this issue Mar 30, 2015 · 4 comments

Comments

@frangarciam
Copy link
Contributor

When adding a new SSH Key through hologram-authorize, the server actually requests to the ldap server the user's password so it can compare its hash with the one sent by the user. Hologram should not request ldap for the user's password (among other reasons because implementations like AD won't actually return it) and should instead create a new connection to the ldap server and try to bind to it with the user-provided username/password combination.

( See https://github.com/AdRoll/hologram/blob/master/server/server.go#L152 )

@copumpkin
Copy link
Contributor

I support this, but LDAP authentication makes me kind of sad all around 😦 In one scenario (the one you currently use), the LDAP server reveals user passwords (and thus has to store them reversibly!) to privileged services. In the other, services have to accept the raw user password and attempt to bind under those credentials. Kerberos is the "nicer" way to do it but that's a whole PITA of its own.

@frangarciam
Copy link
Contributor Author

I fully agree with you, and would like to have a nicer solution to this as well, but in the meantime doing the bind might be the lesser evil for several reasons:

  • Requiring something like Kerberos to use hologram raises the barrier of entry significantly (I even think that currently requiring ldap as a backend is too much already)
  • This system is needed to allow users to add their own keys. If you have an administrator or other kind of privileged process adding those keys then you can happily skip this, but you'll still need a nice way of getting the keys from user to admin and that's going to require some kind of authentication (emailing your public key to your sysadmin only scales up to a certain point).
  • This is only supposed to be a one-time thing (once your key is in ldap, you don't need to do it again), so I'm not overly concerned

@copumpkin
Copy link
Contributor

Agreed, I was mostly just whining :)

One thing that could make this a little cleaner: the hologram server
doesn't need to be an actor in this at all. When I mentioned I was using a
different tool, I just have a small script that talks to the LDAP server
directly and adds the key. hologram-authorize could do the same thing
unless you expect the hologram server to be accessible from places the LDAP
server isn't.

On Monday, March 30, 2015, Fran Garcia notifications@github.com wrote:

I fully agree with you, and would like to have a nicer solution to this as
well, but in the meantime doing the bind might be the lesser evil for
several reasons:

  • Requiring something like Kerberos to use hologram raises the barrier
    of entry significantly (I even think that currently requiring ldap as a
    backend is too much already)
  • This system is needed to allow users to add their own keys. If you
    have an administrator or other kind of privileged process adding those keys
    then you can happily skip this, but you'll still need a nice way of getting
    the keys from user to admin and that's going to require some kind of
    authentication (emailing your public key to your sysadmin only scales up to
    a certain point).
  • This is only supposed to be a one-time thing (once your key is in
    ldap, you don't need to do it again), so I'm not overly concerned


Reply to this email directly or view it on GitHub
#36 (comment).

@frangarciam
Copy link
Contributor Author

That could work, but would require configuration allowing that from the ldap side, which is always a tradeoff. I'm going to bounce that idea with a couple of people here to see what kind of feedback I get.

Thanks for the suggestions!

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