grinnellplans / grinnellplans-php

Automatically exported from code.google.com/p/grinnellplans
Other
7 stars 7 forks source link

Passwords should be stored in a more secure fashion #97

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Plans is currently salt-crypting passwords to store them in the database. 

We should use some other cryptographic one-way function such as MD5 or SHA1.

Performance is key.

Original issue reported on code.google.com by thatha7777 on 31 Jul 2009 at 2:20

GoogleCodeExporter commented 9 years ago
If nothing else, we should just remove the salt parameter from crypt(), at which
point it should default to md5 and generate a different salt for each password, 
which
is good.

Original comment by ian.gree...@gmail.com on 31 Jul 2009 at 4:21

GoogleCodeExporter commented 9 years ago
Basically any hash function will be faster than crypt(), which was designed to 
be 
slow as hell.

MD5 is way, way deprecated. SHA1 loses NIST certification in 2010, and it's 
already 
unrecommended for use. 

re: comment 1- if crypt() is autogenerating salts, how do you retain them?

Original comment by saul.stj...@gmail.com on 31 Jul 2009 at 4:42

GoogleCodeExporter commented 9 years ago
The autogenerated salts are stored right with the password.

# php -r 'echo crypt("mypassword");'
$1$LRNhCO0j$Z4H2ybhGkO2Dks/Ss6Sp01

The $1$ indicates the type of hash, and everything from there to the next $ is 
the
salt. This is a standard feature and is a far bigger win than any differences 
between
MD5, SHA, AES, or Blowfish would be.

Original comment by ian.gree...@gmail.com on 31 Jul 2009 at 5:31

GoogleCodeExporter commented 9 years ago
Oh, that's fun. crypt() still does thousands of rounds of hashing in MD5 mode, 
though. Why not use SHA?

Original comment by saul.stj...@gmail.com on 31 Jul 2009 at 10:06

GoogleCodeExporter commented 9 years ago
SHA1 is indeed deprecated. Although I still have warm fuzzy feelings for 
properly
salted MD5 (via crypt, for example) it is indeed losing cred by the day.

Do you think SHA256 of the SHA2 family would offer a compromise between 
performance
and security? All passwords padded with a random 128-bit salt?

Original comment by thatha7777 on 31 Jul 2009 at 11:24

GoogleCodeExporter commented 9 years ago
Back up. Ask yourself: "why encrypt passwords in the database anyway?" They're 
being 
HTTP POSTed in plain text! Unless you're going to change the login page to use 
a 
challenge-response type deal to authenticate users (or open up port 443), 
there's 
not much of a point to hashing the passwords at all.

If the concern is that someone will walk away with the database (and everyone's 
password), use TDE or whatever the mysql equivalent is, or keep the database on 
a 
loopback crypto system.

If the concern is that admins will be able to read passwords out of the 
database-- 
passwords that people shouldn't be using for multiple sites, but clearly are-- 
I 
dunno. ROT13? How much do you trust people with DB access?

We're not talking about financial transactions; this is Plans, for god's sake. 

Original comment by saul.stj...@gmail.com on 1 Aug 2009 at 12:14

GoogleCodeExporter commented 9 years ago
Sniffing someone's connection is very expensive in terms of time. The reward is 
one
password. The ROI is very low.

Hacking the server is equally expensive. In the past, Plans servers have been 
hacked,
in fact. However, the reward is 3000 passwords, with email addresses and, on
occasion, full name. The ROI is VERY high.

TDE or a loopback crypto system require custom deployments. If, in the future, 
one
only has an account on a box (and not root, as the case is with virtual hosting
solutions) it would be impossible to customize the environment of the website 
so deeply.

In re the admins, I believe this is equivalent with case (1) where someone 
steals the
database.

I am confused: do you believe Plans should be very secure (see thread in re
chromahash), or do you believe we shouldn't care (see this thread)?

Original comment by thatha7777 on 1 Aug 2009 at 12:23

GoogleCodeExporter commented 9 years ago
Connection sniffing is trivial if you're sitting at a router (or, if you're the 
NSA).

But I'm more confused than you. Plans doesn't seem like a very lucrative target 
unless you quantify the lucracy of target in LOLz. What, xactly, is the benefit 
of 
obtaining a list of 3000 plans passwords, email addresses, and (occasionally) 
full 
names? Now, instead of one plan to post meaningless shit on, someone suddenly 
has 
3000? Or, I guess you could try logging into each email account with their 
Plans 
password. It would probably work, but those people deserve what happens then.

I think Plans should be consistant in terms of security. What I'm observing is 
anything but. You're fine leaking information about user passwords, and you 
don't 
see a privacy violation in leaking geolocation data about end-users, and you're 
fine 
with sending passwords in plaintext over the Internet, but you're concerned 
that 
someone is going to penetrate the server hosting Plans and walk off with the DB?

What's less expensive than trying to exploit a Unix server? Sitting in front of 
it 
and reading passwords off the wire. Please don't pretend to be super-concerned 
with 
the security of the DB itself when it all the information going into and coming 
out 
of it is traveling in plaintext over a medium that, as CS101 taught us all, 
must be 
considered hostile.

So, I put it to you: how secure do you believe Plans should be?

Original comment by saul.stj...@gmail.com on 1 Aug 2009 at 1:10

GoogleCodeExporter commented 9 years ago
Geographic location based on an IP address is not a violation of anyone's 
privacy. An
IP address is a public identifier. Data derived from this is not private.

I am not fine with sending passwords in plaintext. Which is it happens only on 
login.
Afterwards all cookies are encrypted. I am making a strategic compromise 
because I do
not want to pay for a SSL certificate. Such a thing would complicate the team
dynamics by a lot.

In the past, the server has been penetrated and people did download stuff. If 
you
recall there was an announcement by Laiu for everyone to make sure they changed 
their
passwords.

I do not believe that people with less knowledge "deserve what happens then" to 
them. 

Plans ought to be as secure as possible under the constraints of our operations.

Original comment by thatha7777 on 1 Aug 2009 at 1:15

GoogleCodeExporter commented 9 years ago
"Geographic location based on an IP address is not a violation of anyone's 
privacy."

There's a couple thousand jailed Iranian Twitterers who would, I bet, disagree 
with 
that.

"I am not fine with sending passwords in plaintext."

Then don't do it. SSL isn't the only way to secure login information, 
especially if 
you're ALREADY hashing the password client side. Issue a challenge salt, hash 
the 
password client side, concatinate the challenge salt with the hashed password, 
and 
hash that again. Salt and hash the hashed password server-side similarly. If 
the 
client responds with the right 2nd-order hash, you know they know the password!

"In the past, the server has been penetrated and people did download stuff."

How?

"Plans ought to be as secure as possible under the constraints of our 
operations."

That's a laudible sentiment. Your developmental choices aren't reflective of 
it. 
ChromaHash is just eye-candy UI at the expense of a modicum of cryptographic 
security. Publically available maps of the geolocated IP addresses of Plans 
users is 
a similar trade off.

Plans doesn't have to be Fort Knox. If flashy UI is more important than 
security, 
that's a valid choice. Be honest about it, though.

BTW, that TOS needs updating, and I've been waiting for years to hear about 
Plans's 
data retention policies. IMO, that's much more of a priority than cryptographic 
penis contests.

(Mine's bigger, anyways.)

Original comment by saul.stj...@gmail.com on 1 Aug 2009 at 1:44

GoogleCodeExporter commented 9 years ago
"There's a couple thousand jailed Iranian Twitterers who would, I bet, disagree 
with
that."

That is geolocation based on ISP databases, not the ones Plans uses.

Please do not litter the issue tracker with irrelevant information. 

Original comment by thatha7777 on 1 Aug 2009 at 1:47

GoogleCodeExporter commented 9 years ago
"I am confused: do you believe Plans should be very secure (see thread in re
chromahash), or do you believe we shouldn't care (see this thread)?"
"Please do not litter the issue tracker with irrelevant information."

That's entrapment.

Original comment by saul.stj...@gmail.com on 1 Aug 2009 at 1:50

GoogleCodeExporter commented 9 years ago

Original comment by thatha7777 on 1 Aug 2009 at 1:50

GoogleCodeExporter commented 9 years ago

Original comment by thatha7777 on 1 Aug 2009 at 5:15

GoogleCodeExporter commented 9 years ago
I think SHA256 would be more than sufficient. Unfortunately, it looks like 
standard
PHP libs don't come with anything stronger than MD5 - need the mcrypt 
extension. If
that extension poses any major problems to install, I really do feel that 
properly
salted MD5 would be good enough for our purposes. Sure, MD5 is broken, but it's 
still
reasonably effective, especially for an application like this where collisions 
are
not a big concern.

Original comment by ian.gree...@gmail.com on 3 Aug 2009 at 5:40

GoogleCodeExporter commented 9 years ago
I managed to dig up the article that explains why, if we want to be truly 
secure,
*not* performance is key. The URL is
<http://www.matasano.com/log/958/enough-with-the-rainbow-tables-what-you-need-to
-know-about-secure-password-schemes/>,
but the site is currently having difficulty, so here's the Google cache:
<http://74.125.95.132/search?q=cache:uH3EwCGaZMYJ:www.matasano.com/log/958/>.

I'm still fine with my previous recommendations, as I don't see Plans as an 
extremely
high-security environment, but it's worth considering using something like 
phpass
<http://www.openwall.com/phpass/> to handle the passwords. After all, the fewer
security decisions we have to make, the better, because crypto is _hard_.

Original comment by ian.gree...@gmail.com on 3 Aug 2009 at 7:17

GoogleCodeExporter commented 9 years ago
Any idea why crypt($password, 'ab') is trimming passwords at 8 characters right 
now?

Original comment by thatha7777 on 3 Aug 2009 at 7:20

GoogleCodeExporter commented 9 years ago
I believe that's because of the outdated DES encryption scheme that is being
triggered, which only uses the first 8 characters of input for hashing.

Original comment by ian.gree...@gmail.com on 3 Aug 2009 at 8:15

GoogleCodeExporter commented 9 years ago
r444 updates the password stuff to use MD5 with random salts for all new/changed
passwords. If in the future we want to install mcrypt or use the Blowfish that 
is
built into PHP 5.3.x, we can do so and it will continue to be backwards 
compatible.

Original comment by ian.gree...@gmail.com on 9 Aug 2009 at 8:37

GoogleCodeExporter commented 9 years ago
Server is now updated.

Original comment by thatha7777 on 16 Dec 2009 at 7:38