[go: up one dir, main page]

oss-sec mailing list archives

OpenBSD bcrypt error return


From: Solar Designer <solar () openwall com>
Date: Tue, 15 Nov 2011 07:14:17 +0400

Hi,

The bcrypt implementation from OpenBSD, now also found in FreeBSD and
NetBSD, returns a constant string on error.

http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/crypt/
http://www.freebsd.org/cgi/cvsweb.cgi/src/secure/lib/libcrypt/
http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libcrypt/

static char    error[] = ":";

Clearly, ":" can't match a field value in an /etc/passwd-like file,
which is great, but what happens if one of those errors occurs when
setting a new password?  Luckily, the specific errors being checked for
have to do with unsupported or invalid salt strings, so they can't
happen on a properly configured system.  Nevertheless, this may be a
disaster waiting to happen - e.g., if a new "$2" prefix is introduced
(like I did when dealing with the crypt_blowfish bug), support for it is
added to a password-changing program, but an appropriate update to libc
or libcrypt is not yet deployed on a system.

Thus, to avoid this disaster, this poor way of handling errors may also
get in the way of adding support for such extra "$2" prefixes on *BSD,
unfortunately.  This is something I forgot about when deciding on those
this summer, even though I was aware of this issue in OpenBSD since 1998
or so.

Yes, I did report this issue to OpenBSD folks at least twice - last time
this summer, after it was independently discovered by Zefram.

Maybe FreeBSD and/or NetBSD will want to patch it, or at least to be
aware of the risk - hence the posting in here.

The fix may be to reuse the approach from crypt_blowfish:

int _crypt_output_magic(const char *setting, char *output, int size)
{
        if (size < 3)
                return -1;

        output[0] = '*';
        output[1] = '0';
        output[2] = '\0';

        if (setting[0] == '*' && setting[1] == '0')
                output[1] = '1';

        return 0;
}

This may be done in bcrypt.c or in wrapper code common for all crypt(3)
hash types.

Proactive security, anyone?

Alexander


Current thread: