Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:114094 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 63443 invoked from network); 22 Apr 2021 08:03:16 -0000 Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5) by pb1.pair.com with SMTP; 22 Apr 2021 08:03:16 -0000 Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 95FC41804DB for ; Thu, 22 Apr 2021 01:06:13 -0700 (PDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,HTML_MESSAGE, RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.2 X-Spam-Virus: No X-Envelope-From: Received: from mail-lj1-f178.google.com (mail-lj1-f178.google.com [209.85.208.178]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Thu, 22 Apr 2021 01:06:13 -0700 (PDT) Received: by mail-lj1-f178.google.com with SMTP id a36so39954464ljq.8 for ; Thu, 22 Apr 2021 01:06:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=khoL3KCqAN46wnu12FjRSS+sJSqfiIZKbrYfh2VP0NE=; b=pgg7eG9EEcSXOk0RnlrBcsxftWxNTZXvaSt/o3JsmZakyHi/8tSNaGf3gbGottRaG3 VxapfooQqk7c6vTG7T2NXOaEfAwoSCupDDz66IPtqELgEef6Ys9epRbB84pB9UT7g7HM b0/ru0MEYwhNtE99B+D7zz8eZTQz9h76xfJJXsNUGFjT6U7mbOSiwW/oiEw3alv+JrpT Rbtbarr9auNc4dcbHreuwuvwblzcCTsXpcaOJOUMNS71iJRaosapiFH6AwNMORRPoUT+ KZqMn3SNq2oYypvehMPuKdAtAaUpm2jSqSYxbrjaweixPfzt7ptbH+HFBYwl55kwFYFQ lvPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=khoL3KCqAN46wnu12FjRSS+sJSqfiIZKbrYfh2VP0NE=; b=i+9Z6fn0GXYDNZ9NTXBKKb8rXqktEN6Vu/ZVee5RXrs7PZnxJln3l4b5rONLZNai2r 8wqea4oWDHmGOSa8sZJ2uB2enUFGvAaP7ok0B8Aarx//PupcJ/1B5T8MMIkDXvDGx3XW gHNYhkgPKyF+c+vmYEkbghhu1Xdv5HAF0djQwMUAdoPmFerXHq+rogM1oYVuGlkqAntY oKncUu9kll8TML8O3XnepG5yzXJTnk3z99EYiIi0dbmQhVedIGbxQ1KtVM8NhgIZhsGW +nUmZgK4s4WWZGohZposGRk5qNiz37lWs/UNzTHleUkcnO0JPU5d8P9zXpRdj39AWZMi iGag== X-Gm-Message-State: AOAM532kKbKZE5UL38s0YLeyaINiPgz77Svlg6FjBnES/TabU3e4oAkg WrYcIE1cgn3iJ6UwlmP9R4ABrAHItJ4aZil4EMU= X-Google-Smtp-Source: ABdhPJzKWwxMCfVELAfNAz2nc54hNry1Vj4RBpDSilNrCvEBC+kS1xcaA/RNmJqlk7oLamozpl2aSNKTCcfgfsePnhE= X-Received: by 2002:a2e:7819:: with SMTP id t25mr1652128ljc.93.1619078769885; Thu, 22 Apr 2021 01:06:09 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: Date: Thu, 22 Apr 2021 10:05:53 +0200 Message-ID: To: Sara Golemon Cc: PHP internals , Anthony Ferrara Content-Type: multipart/alternative; boundary="000000000000f1436a05c08b26be" Subject: Re: [PHP-DEV] Binary (un)safety of password_hash() used with PASSWORD_BCRYPT From: nikita.ppv@gmail.com (Nikita Popov) --000000000000f1436a05c08b26be Content-Type: text/plain; charset="UTF-8" On Thu, Apr 22, 2021 at 3:48 AM Sara Golemon wrote: > I have this notion that we've discussed this before, I'm certain I knew > that bcrypt wasn't binary safe, but someone reminded me that > password_hash() could be called with null bytes in the password itself and > that is just SCREAMING to have some safety-rails put on IMO. > > So I've thrown together https://github.com/php/php-src/pull/6897 to test > that argon2 algos behave well (they do!), and modified the bcrypt algo to > throw an exception if you try to hash a new password containing a null, but > only warns if you've got a null when running password_verify(). My > reasoning for the latter is because anyone trying to auth with a null > character that succeeds does definitely know enough of the password to pass > by simply not passing the null, so we shouldn't break existing users who > already have hashes. This only aims to break users who would otherwise be > able to include a null, because they would have a false sense of security > having their password truncated and can remedy that in their password > choosing. > > Since this does introduce a (small) break, I'm putting it to the list for > opinions. > If nobody objects, I'll merge this (8.1 only) at the end of the month. > I don't think this is a good idea. This adds an error condition that is based on the input string, which is generally user-provided. As there is nothing in the default stack preventing null bytes from passing in via HTTP (correct me if I'm wrong), this means the error can be trivially triggered, probably on about any PHP application using password_hash(). I don't think this is acceptable, for much the same reason that having file_exists() throw on null bytes is a bad idea (something we recently changed). I'm not sure what problem you're actually trying to solve here. Ultimately, all this allows is a user to shoot themselves in the foot if they want to -- and they do need to try actively, it's not like you can end up with a null byte in a password by accident. Regards, Nikita --000000000000f1436a05c08b26be--