NIS backend doesn't support MD5 passwords

Review Request #647 — Created Nov. 28, 2008 and submitted


Review Board SVN (deprecated)


The current NIS login code only supports DES passwords.  This change adds support for MD5 passwords.
(Our NIS server automatically uses MD5 once a password reaches a certain length..)

  1. Just to be sure, are we absolutely positive this doesn't break existing NIS support?
  2. /trunk/reviewboard/accounts/ (Diff revision 1)
    No need for parens. Also, you can just use [:3].
  2. /trunk/reviewboard/accounts/ (Diff revision 2)
    Looking at this again, I think we're doing too much work. Wouldn't this make more sense:
    if original_crypted.startswith("$1$"):
        salt = original_crypted
        salt = original_crypted[:2]
    1. That's cool.  I didn't know crypt would work if you passed the entire encrypted password as the salt.  In my diff, for an MD5 encrypted password of:
      I passing in a salt of:
      instead of your suggestion (which works too) of:
      Since we can just pass in the entire password, how about the following change for line 18 (remove 4 characters)?
        new_crypted = crypt.crypt(password, original_crypted)
      I tested the above change with original DES and MD5 password users and it works for me.  All the suggested changes work, though, so I'm happy with any of them.  Let me know what patch you prefer..
    2. If this works, then that's great. Less good is preferable :) Let's take the latter patch, and if things break (sure hope not) we can always go back and use the more conservative approach.
Review request changed
Change Summary:
Incorporated feedback from diff version #2.
  1. Looks great. Nice and simple. Thanks!
    Committed as r1634.