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

fixes #9551 - checking the encryption format so osx doesn't fail #2206

Closed
wants to merge 1 commit into from

Conversation

unorthodoxgeek
Copy link
Member

No description provided.

@lzap
Copy link
Member

lzap commented Feb 25, 2015

Before I even start digging - I don't like this at all :-)

Can you paste the actual error you encounter and some description what the problem is? I believe MacOS engineers created "better" crypt function which has different behavior.

@unorthodoxgeek
Copy link
Member Author

@lzap - this is actually the second time we encounter this. see http://apidock.com/ruby/String/crypt
because the #crypt method relies on the native libc library, stuff can work differently on different machines.
*nix have an output which looks sort of like "$1$default$hCkak1kaJPQILNmYbUXhD0"
Darwin's libc has to be different, because apple, right? so their output will resemble "$fgbhdfgheirgre"

we can either check the output or stop relying on the output, we can't ignore the fact that #crypt will work different on different machines.

@lzap
Copy link
Member

lzap commented Feb 25, 2015

Can't we simply change the regexp to ^\$\d ? That will match against Linux and MacOS.

One could possibly have a password starting with $1mypass but that is very fragile password in general.

Alternatively let's detect OS properly and in case of MacOS change the regexp. Not following introduction of is_actually_encrypted.

@unorthodoxgeek
Copy link
Member Author

actually, the variable is the right thing to do here anyway, as it prevents code duplication :)

@dLobatog
Copy link
Member

[test]

@lzap
Copy link
Member

lzap commented Feb 26, 2015

Ok, can we get rid of

PasswordCrypt.passw_crypt("test_this").match('^\$\d+\$.+\$.+')

and do this test during application load (class variable or constant in the utility lib/ module). Platforms do not change during runtime :-)

@unorthodoxgeek
Copy link
Member Author

@lzap - done

@@ -9,4 +9,8 @@ def self.passw_crypt(passwd, hash_alg = 'MD5')
def self.grub2_passw_crypt(passw)
self.passw_crypt(passw, 'MD5')
end

def self.nix?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do more transparent method name? I would suggest crypt_unix_compatible or if we want to be precise then crypt_mcf_compatible (Modular Crypt Format defacto-standard: http://pythonhosted.org/passlib/modular_crypt_format.html). Definitely not "*nix" because MacOS is also unix-like OS.

@lzap
Copy link
Member

lzap commented Feb 26, 2015

Looks fine! Thanks.

@lzap
Copy link
Member

lzap commented Feb 26, 2015

Merged as 526e02f thanks!

@lzap lzap closed this Feb 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants