RFC: md5 and sha1 support for HTPasswdAuth #447

Closed
wants to merge 2 commits into
from

Projects

None yet

4 participants

@jiuka
Contributor
jiuka commented Jun 8, 2012

Implement apr1/md5 and sha1 password hashing and checking for HTPasswdAuth.
This enable more modern hashing the DES in htaccess.

However the cryptMD5 function is somehow derivative of apr, as it implements the "Apache-specific algorithm" for md5 password hashes.

As I am not a copyright advocate i request for comment if this is fine for inclusion in buildbot.

http://httpd.apache.org/docs/2.2/misc/password_encryptions.html
http://svn.apache.org/viewvc/apr/apr-util/branches/1.5.x/crypto/apr_md5.c?revision=1346872&view=markup

@jiuka jiuka md5 and sha1 support for HTPasswdAuth
Implement apr�1/md5 and sha1 password hashing and checking.
171a47e
Owner

I am wary of including security sensitive code like this in buildbot. I'd be much happier reusing existing code to handle this (something like twisted.cred).

@tomprince tomprince commented on the diff Jun 8, 2012
master/buildbot/status/web/auth.py
+ phash = ''
+ itoa64 = './0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz'
+ for a, b, c in ((0,6,12), (1,7,13), (2,8,14), (3,9,15), (4,10,5)):
+ l = ord(final[a]) << 16 | ord(final[b]) << 8 | ord(final[c])
+ for i in range(4):
+ phash += itoa64[l & 0x3f];
+ l >>= 6
+ l = ord(final[11])
+ for i in range(2):
+ phash += itoa64[l & 0x3f];
+ l >>= 6
+
+ return "$%s$%s$%s" % (magic, salt, phash)
+
+ def cryptSHA1(self, password, salt):
+ """Simple unsalted sha1 from hashlib."""
tomprince
tomprince Jun 8, 2012 Owner

"unsalted" here, for example, worries me.

jiuka
jiuka Jun 8, 2012 Contributor

I could remove cryptSHA1, its just there to support all but plain of the basic-authentication type apache understands. To just support crypt (max 8 characters) is not nice too. With md5 for mod_svn it's possible to use the same htpasswd for svn and buildbot.

Contributor
jiuka commented Jun 8, 2012

I did not found a backend for the htpasswd files in twisted.cred. If there is one it would of corse be the bether solution to use it. However the HTPasswdAuth without md5 dont' make much sense for me.

Contributor
jiuka commented Jun 8, 2012

With ctypes and libaprutil it would be possible to implement the same if the only concern are about 'including security sensitive code'.

Contributor
jiuka commented Jun 9, 2012

If provided with a hash function which can hash apr1/md5 yes. The hash function itself needs to be implemented anyway.
In a longer term to use twisted.cred for all authentication stuff would may be nice.
For both HTPasswdAuth and FilePasswordDB some libapr based password check like https://gist.github.com/2900161 could be an option, as libapr is needed to create the htpasswd file anyway.

Contributor
maruel commented Jun 13, 2012

FTR,

Storing password hash as md5 is equivalent to storing them in clear text. Salting doesn't help anymore.
Storing an unsalted password hash with sha1 is not secure at all. Try searching for [linked in sha1] for a recent real world example.

Owner

between license worries, security concerns, and including complex, security sensitive code in Buildbot, this has me worried. If there are external libs we can use that address the second concern, that'd be best.

Owner

@jiuka - is there a way forward on this pull request, or should I close it pending a new one?

Contributor
jiuka commented Jun 24, 2012

@djmitche - please close it. I will open a new one for https://github.com/jiuka/buildbot/commits/htpasswd_apr

@djmitche djmitche closed this Jun 24, 2012
@mzdaniel mzdaniel pushed a commit to mzdaniel/buildbot that referenced this pull request Aug 2, 2013
@tianon tianon Improve mkimage-debian script to also tag using the release version n…
…umber of the final image (6.0.7, 7.0, etc.)

This is as discussed on #447.
4b3354a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment