Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 15 Oct 2020 22:32:32 +0200
From: Solar Designer <solar@...nwall.com>
To: Tim van der Staaij <git@....vanderstaaij.email>
Cc: musl <musl@...ts.openwall.com>
Subject: Re: [PATCH] crypt: support $2b$ prefix for blowfish

Hi,

Wow, I didn't realize we forgot to get this into musl.

On Thu, Oct 15, 2020 at 09:56:56PM +0200, Tim van der Staaij wrote:
> 2b is functionally equivalent to 2y, i.e. no known bugs at this time.
> 
> openbsd, which created the original bcrypt implementation,
> and several other implementations use this prefix since 2014:
> https://marc.info/?l=openbsd-misc&m=139320023202696
> ---
>  src/crypt/crypt_blowfish.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

Your patch looks OK to me at first glance, but it'd benefit from
existing testing and perhaps it'd help further maintenance to merge my
upstream changes instead of making slightly different (even if smaller)
changes specific to musl.

https://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/glibc/crypt_blowfish/crypt_blowfish.c.diff?r1=1.30;r2=1.31

> -	    test_hash[buf.s[2] & 1],
> +	    test_hash[buf.s[2] != 'x'],

This is really subtle, but I recall deliberately avoiding the "!= 'x'"
comparison as it's more likely to result in a conditional branch, which
is an additional side-channel leak about the hash sub-type.  We accept
leaks through data cache access patterns, but we avoided branching on
hash sub-type so far (let alone on anything truly security-sensitive).

As I recall, this is why I had to move flags_by_subtype to global scope
in that source file.

Alexander

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.