Derek (freebsd lists)
2015-Feb-10 12:26 UTC
[patch] libcrypt & friends - modular crypt format support in /etc/login.conf
Hello! I've been working on this for a while, and I've produced a patch that does a few things with the base system: 1. allows modular crypt to be specified as passwd_format in /etc/login.conf - this allows setting the algorithm *and rounds*, i.e. $2b$10$ for users of varying classes. - this will allow any future algorithms and parameters supported by crypt(3) to be supported by the tools around login.conf 2. introduces a new api, crypt_makesalt which will generate an appropriate salt for any algorithm selected 3. updates userland to use this API, and removes totally the {crypt_set_format, login_setcryptfmt, login_getcryptfmt} APIs 4. switches crypt algorithms to use thread-local storage, so the good old global crypt buffer is thread-local 5. includes a bunch of new test vectors for libcrypt ATF tests There are references to previous discussions/patches/etc here: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=182518 http://docs.freebsd.org/cgi/getmsg.cgi?fetch=168499+0+/usr/local/www/db/text/2013/freebsd-current/20131006.freebsd-current http://docs.freebsd.org/cgi/getmsg.cgi?fetch=361757+0+/usr/local/www/db/text/2014/freebsd-current/20140112.freebsd-current And most recent discussion here: http://docs.freebsd.org/cgi/getmsg.cgi?fetch=1751919+0+archive/2014/freebsd-current/20140716.freebsd-current Anyways, I've put a bunch of work into this, and am anxious to actually get this accepted into -HEAD. What more can I do at this point? A patch against current is in the original PR/"bug": https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=182518 Thanks, Derek
John-Mark Gurney
2015-Feb-11 02:19 UTC
[CFR] Re: [patch] libcrypt & friends - modular crypt format support in /etc/login.conf
Derek (freebsd lists) wrote this message on Tue, Feb 10, 2015 at 07:26 -0500:> I've been working on this for a while, and I've produced a patch > that does a few things with the base system: > > 1. allows modular crypt to be specified as passwd_format in > /etc/login.conf > - this allows setting the algorithm *and rounds*, i.e. $2b$10$ > for users of varying classes. > - this will allow any future algorithms and parameters > supported by crypt(3) to be supported by the tools around login.conf > > 2. introduces a new api, crypt_makesalt which will generate an > appropriate salt for any algorithm selected > > 3. updates userland to use this API, and removes totally the > {crypt_set_format, login_setcryptfmt, login_getcryptfmt} APIs > > 4. switches crypt algorithms to use thread-local storage, so the > good old global crypt buffer is thread-local > > 5. includes a bunch of new test vectors for libcrypt ATF tests > > > There are references to previous discussions/patches/etc here: > > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=182518 > > http://docs.freebsd.org/cgi/getmsg.cgi?fetch=168499+0+/usr/local/www/db/text/2013/freebsd-current/20131006.freebsd-current > > http://docs.freebsd.org/cgi/getmsg.cgi?fetch=361757+0+/usr/local/www/db/text/2014/freebsd-current/20140112.freebsd-current > > > And most recent discussion here: > > http://docs.freebsd.org/cgi/getmsg.cgi?fetch=1751919+0+archive/2014/freebsd-current/20140716.freebsd-current > > > Anyways, I've put a bunch of work into this, and am anxious to > actually get this accepted into -HEAD. > > > > What more can I do at this point?I finally got around to reviewing this... For the tests, we should probably add an invalid password test for each format... We need man pages for the new function... I guess this new man page would be a good place to document all the modular formats in more detail.. what is in crypt(3) isn't that useful... Also, crypt(3) should have an xref to crypt_makesalt... Other than those, unless someone objects, I'll commit it... -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not."
Matthew D. Fuller
2015-Feb-14 17:33 UTC
[patch] libcrypt & friends - modular crypt format support in /etc/login.conf
On Tue, Feb 10, 2015 at 07:26:07AM -0500 I heard the voice of Derek (freebsd lists), and lo! it spake thus:> > 2. introduces a new api, crypt_makesalt which will generate an > appropriate salt for any algorithm selectedIt has been an endlessly-repeated source of pain to me that there isn't a standard API for this, and it's just been into the wound[0] that there isn't even a NON-standard one, and so I have to guess and re-implement any time I want to use crypt(3) for anything except /etc/passwd. Of course, I want it in non-C, but one problem at a time... If you accomplish nothing else with this, I'll happily fall at your feet just for this 8-} [0] By a hydraulic press, I think. -- Matthew Fuller (MF4839) | fullermd at over-yonder.net Systems/Network Administrator | http://www.over-yonder.net/~fullermd/ On the Internet, nobody can hear you scream.
Jilles Tjoelker
2015-Feb-14 23:17 UTC
[patch] libcrypt & friends - modular crypt format support in /etc/login.conf
On Tue, Feb 10, 2015 at 07:26:07AM -0500, Derek (freebsd lists) wrote:> I've been working on this for a while, and I've produced a patch > that does a few things with the base system:> 1. allows modular crypt to be specified as passwd_format in > /etc/login.conf > - this allows setting the algorithm *and rounds*, i.e. $2b$10$ > for users of varying classes. > - this will allow any future algorithms and parameters > supported by crypt(3) to be supported by the tools around login.confOK.> 2. introduces a new api, crypt_makesalt which will generate an > appropriate salt for any algorithm selectedI like the idea.> 3. updates userland to use this API, and removes totally the > {crypt_set_format, login_setcryptfmt, login_getcryptfmt} APIsRemoving API functions completely requires a SHLIB_MAJOR bump. I think this can be avoided by replacing the functions with a stub instead, so they would behave as if the default always applied and not allow changes to it.> 4. switches crypt algorithms to use thread-local storage, so the > good old global crypt buffer is thread-localThis uses quite a bit of memory for each thread created, even if it does not call crypt() at all. Fortunately, libcrypt is not commonly used. Given that crypt() has never been thread-safe, consider implementing crypt_r() as in glibc and leaving crypt() thread-unsafe. Thread-local storage via pthread_key_create() (one key for libcrypt) is still "magic" but reduces the memory waste for threads that do not call crypt().> 5. includes a bunch of new test vectors for libcrypt ATF testsOK. Some remarks about the code: lib/libcrypt/crypt.c crypt_makesalt()> b64_from_24bit((uint8_t) rand_buf[2], (uint8_t) rand_buf[1], (uint8_t) > rand_buf[0], diff, (int *) &(diff), &out);All these casts can be avoided by making the affected variables the proper type in the first place. The cast of &diff causes a strict-aliasing violation and is definitely wrong on 64-bit big-endian systems. rand_buf is a salt, not a secret, so clearing it afterwards is unnecessary. Consider memcpy() and adding '\0' afterward instead of strncpy(). It seems unnecessary to clear the buffer completely. -- Jilles Tjoelker