From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 24421 invoked from network); 15 Oct 2020 20:32:56 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 15 Oct 2020 20:32:56 -0000 Received: (qmail 9925 invoked by uid 550); 15 Oct 2020 20:32:53 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 9813 invoked from network); 15 Oct 2020 20:32:37 -0000 Date: Thu, 15 Oct 2020 22:32:32 +0200 From: Solar Designer To: Tim van der Staaij Cc: musl Message-ID: <20201015203232.GA7129@openwall.com> References: <1752dd63ca0.f45c706c130962.3068699904194055827@tim.vanderstaaij.email> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1752dd63ca0.f45c706c130962.3068699904194055827@tim.vanderstaaij.email> User-Agent: Mutt/1.4.2.3i Subject: Re: [musl] [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