Newsgroups: php.doc,php.internals Path: news.php.net Xref: news.php.net php.doc:969386553 php.internals:98798 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 82845 invoked from network); 14 Apr 2017 09:46:17 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 14 Apr 2017 09:46:17 -0000 Authentication-Results: pb1.pair.com smtp.mail=info@pieterhordijk.com; spf=permerror; sender-id=unknown Authentication-Results: pb1.pair.com header.from=info@pieterhordijk.com; sender-id=unknown Received-SPF: error (pb1.pair.com: domain pieterhordijk.com from 185.78.96.68 cause and error) X-PHP-List-Original-Sender: info@pieterhordijk.com X-Host-Fingerprint: 185.78.96.68 mailsrv1.hostingfactory.nl Received: from [185.78.96.68] ([185.78.96.68:58789] helo=mailsrv1.hostingfactory.nl) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id CD/E4-31410-46A90F85 for ; Fri, 14 Apr 2017 05:46:14 -0400 Received: from localhost (localhost [127.0.0.1]) by mailsrv1.hostingfactory.nl (Postfix) with ESMTP id 3CE9010406CC; Fri, 14 Apr 2017 11:45:59 +0200 (CEST) Received: from mailsrv1.hostingfactory.nl ([127.0.0.1]) by localhost (mailsrv1.hostingfactory.nl [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id WRQJKp0RnuZV; Fri, 14 Apr 2017 11:45:49 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by mailsrv1.hostingfactory.nl (Postfix) with ESMTP id 2E5C510406D3; Fri, 14 Apr 2017 11:45:49 +0200 (CEST) X-Virus-Scanned: amavisd-new at mailsrv1.hostingfactory.nl Received: from mailsrv1.hostingfactory.nl ([127.0.0.1]) by localhost (mailsrv1.hostingfactory.nl [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id 1mJ6LUA8jptG; Fri, 14 Apr 2017 11:45:49 +0200 (CEST) Received: from mailsrv1.hostingfactory.nl (mailsrv1.hostingfactory.nl [185.78.96.68]) by mailsrv1.hostingfactory.nl (Postfix) with ESMTP id 0682710406CC; Fri, 14 Apr 2017 11:45:49 +0200 (CEST) Date: Fri, 14 Apr 2017 11:45:48 +0200 (CEST) To: yohgaki@ohgaki.net Cc: Joe Watkins , Andrey Andreev , internals , phpdoc , nikita.ppv@gmail.com Message-ID: <690015854.1384408.1492163148986.JavaMail.zimbra@pieterhordijk.com> In-Reply-To: References: <1924612862.1298112.1492071094545.JavaMail.zimbra@pieterhordijk.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Originating-IP: [185.78.96.68] X-Mailer: Zimbra 8.7.0_GA_1659 (ZimbraWebClient - GC57 (Win)/8.7.0_GA_1659) Thread-Topic: Improve hash_hkdf() parameter Thread-Index: d+mFQS+PA4gNm5zrjSug8XJuKcmoKQ== Subject: Re: [PHP-DEV] [RFC][VOTE] Improve hash_hkdf() parameter From: info@pieterhordijk.com (Pieter Hordijk) ----- Original Message ----- > From: "Nikita Popov" > To: "Yasuo Ohgaki" > Cc: "Pieter Hordijk" , "Joe Watkins" , "Andrey Andreev" > , "internals" , "phpdoc" > Sent: Friday, April 14, 2017 11:24:53 AM > Subject: Re: [PHP-DEV] [RFC][VOTE] Improve hash_hkdf() parameter > On Thu, Apr 13, 2017 at 11:22 PM, Yasuo Ohgaki < [ mailto:yohgaki@ohgaki.= net | > yohgaki@ohgaki.net ] > wrote: >> Hi Pieter and all, >> On Thu, Apr 13, 2017 at 5:11 PM, Pieter Hordijk < [ >> mailto:info@pieterhordijk.com | info@pieterhordijk.com ] > >> wrote: >> > Is this really something we need in our official docs instead of for >> > example >> > on a personal blog? >> I wrote draft doc patch. >> Please verify. >> Index: en/reference/hash/functions/hash-hkdf.xml >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- en/reference/hash/functions/hash-hkdf.xml (=E3=83=AA=E3=83=93=E3=82= =B8=E3=83=A7=E3=83=B3 342317) >> +++ en/reference/hash/functions/hash-hkdf.xml (=E4=BD=9C=E6=A5=AD=E3=82= =B3=E3=83=94=E3=83=BC) >> @@ -3,7 +3,7 @@ >> > http://docbook.org/ns/docbook ] " >> xmlns:xlink=3D" [ http://www.w3.org/1999/xlink | http://www.w3.org/1999/= xlink ] "> >> >> hash_hkdf >> - Generate a HKDF key derivation of a supplied key >> input >> + Derive secure new key from existing key by using >> HKDF >> >> >> &reftitle.description; >> @@ -16,6 +16,20 @@ >> > choice=3D"opt">stringsalt'' >> >> + >> + RFC 5869 defines HKDF (HMAC based Key Derivation Function) which >> + is general purpose KDF. HKDF could be useful for many PHP >> + applications that require temporary keys, such CSRF token, >> + pre-signed key for URI, password for password protected >> + URI, and so on. >> + >> + >> + >> + When info and length >> + is not required for your program, more efficient >> + hash_hmac could be used instead. >> + >> + >> >> >> &reftitle.parameters; >> @@ -25,7 +39,7 @@ >> algo >> >> >> - Name of selected hashing algorithm (i.e. "sha256", "sha512", >> "haval160,4", etc..) >> + Name of selected hashing algorithm (i.e. "sha3-256", "sha3-512", >> "sha256", "sha512", "haval160,4", etc..) >> See hash_algos for a list of supported >> algorithms. >> >> >> @@ -39,7 +53,7 @@ >> ikm >> >> >> - Input keying material (raw binary). Cannot be empty. >> + Input keying material. Cannot be empty. >> >> >> >> @@ -60,7 +74,8 @@ >> info >> >> >> - Application/context-specific info string. >> + Application/context-specific info string. Info is intended for >> + public information such as user ID, protocol version, etc. >> >> >> >> @@ -71,8 +86,32 @@ >> Salt to use during derivation. >> >> >> - While optional, adding random salt significantly improves the >> strength of HKDF. >> + While optional, adding random salt significantly improves the >> + strength of HKDF. Salt could be either secret or >> + non-secret. It is used as "Pre Shared Key" in many use cases. >> + Strong value is preferred. e.g. Use >> random_bytes. >> + Optimal salt size is size of used hash algorithm. >> >> + >> + >> + Although salt is the last optional parameter, salt is the >> + most important parameter for key security. Omitted salt is >> + indication of inappropriate design in most cases. Users must >> + set appropriate salt value whenever it is possible. Omit salt >> + only when it cannot be used. >> + >> + >> + Strong salt is mandatory and must be kept secret when input >> + key is weak, otherwise input key security will not be kept. >> + Even when input key is strong, providing strong salt is the >> + best practice for the best possible key security. >> + >> + >> + Salt must not be able to be controlled by users. i.e. User >> + must not be able to set salt value and get derived key. User >> + controlled salt allows input key analysis to attackers. >> + >> + >> >> >> >> @@ -101,6 +140,99 @@ >> &reftitle.examples; >> >> >> + URI specific CSRF token that supports expiration by >> <function>hash_hkdf</function> >> + >> +> +> +define('CSRF_TOKEN_EXPIRE', 180); // CSRF token expiration >> +define('CSRF_TOKENS', 5); // Last 5 CSRF tokens are valid >> + >> +/************************************** >> + * Implementation note >> + * >> + * It uses "counter" for CSRF expiration management. >> + * "counter" is very low entropy, but input key is strong and >> + * CSRF_TOKEN_SEED is short term key. It should be OK. >> + * >> + * This CSRF token implementation has pros and cons >> + * >> + * Pros >> + * - A CSRF token is valid only for specific URI. >> + * - No database is required for URI specific CSRF tokens. >> + * - Only CSRF token is required. i.e. No timestamp parameter. >> + * - When user is active, a CSRF token is valid upto CSRF_TOKEN_EXPIRE = * >> CSRF_TOKENS sec. >> + * - Even when user had long idle time, CSRF token is valid. >> + * - CSRF token will expire eventually. >> + * - Invalidating all active CSRF tokens could be done by >> unset($_SESSION['CSRF_TOKEN_SEED']). >> + * It is recommended to reset CSRF tokens by login/logout event at >> least. >> + * It may be good idea to invalidate all of older CSRF tokens when idle >> time is long. >> + * >> + * Cons >> + * - There could be no CSRF expiration time. >> + * >> + * Precise CSRF token expiration is easy. Just add timestamp parameter >> + * as "info" and check it. >> + **************************************/ >> + >> +session_start(); >> +if (empty($_SESSION['CSRF_TOKEN_SEED'])) { >> + $_SESSION['CSRF_TOKEN_SEED'] =3D random_bytes(32); >> + $_SESSION['CSRF_TOKEN_COUNT'] =3D 1; >> + $_SESSION['CSRF_TOKEN_EXPIRE'] =3D time(); >> +} >> + >> + >> +function csrf_get_token($uri) { >> + // Check expiration >> + if ($_SESSION['CSRF_TOKEN_EXPIRE'] + CSRF_TOKEN_EXPIRE < time()) { >> + $_SESSION['CSRF_TOKEN_COUNT']++; >> + $_SESSION['CSRF_TOKEN_EXPIRE'] =3D time(); >> + } >> + // Equivalent(NOT exactly the same) value by using hash_hmac() >> + // return hash_hmac('sha3-256', hash_hmac('sha3-256', >> $_SESSION['CSRF_TOKEN_SEED'], $_SESSION['CSRF_TOKEN_COUNT']), $uri); >> + return hash_hkdf('sha3-256', $_SESSION['CSRF_TOKEN_SEED'], 0, $uri, >> $_SESSION['CSRF_TOKEN_COUNT']); >> +} >> + >> +function csrf_validate_token($csrf_token, $uri) { >> + for($i =3D 0; $i < CSRF_TOKENS; $i++) { >> + // Equivalent(NOT exactly the same) value by using hash_hmac() >> + // $token =3D hash_hmac('sha3-256', hash_hmac('sha3-256', >> $_SESSION['CSRF_TOKEN_SEED'], $_SESSION['CSRF_TOKEN_COUNT'] - $i), $uri)= ; >> + $token =3D hash_hkdf('sha3-256', $_SESSION['CSRF_TOKEN_SEED'], 0, >> $uri, $_SESSION['CSRF_TOKEN_COUNT'] - $i); >> + if (hash_equals($csrf_token, $token)) { >> + return TRUE; >> + } >> + } >> + return FALSE; >> +} >> + >> + >> +//// Generating CSRF token //// >> +// $uri is target URI that browser POSTs form data >> +$uri =3D ' [ https://example.com/some_form/ | https://example.com/some_= form/ ] '; >> +$csrf_token =3D csrf_get_token($uri); >> +// embed $csrf_token to your form >> + >> +//// Validating CSRF token //// >> +$csrf_token =3D $_POST['csrf_token'] ?? ''; >> +if (!csrf_validate_token($csrf_token, $_SERVER['REQUEST_URI'])) { >> + // Invalid CSRF token >> + throw new Exception('CSRF token validation error'); >> +} >> +// valid request >> +?> >> +]]> >> + >> + >> + Common CSRF token uses the same token value for a session and all >> + URI. This example CSRF token expires and is specific to a >> + URI. i.e. CSRF token [ http://example.com/form_A/ | http://example.com= /form_A/ >> ] is not valid for >> + [ http://example.com/form_B/ | http://example.com/form_B/ ] Since toke= n value >> is computed, no >> + database is required. >> + >> + >> + >> + >> + >> <function>hash_hkdf</function> example >> >> > @@ -124,6 +256,30 @@ >> >> >> >> + >> + >> + <function>hash_hkdf</function> bad example >> + >> + Users must not simply extend input key material length. HKDF does >> + not add additional entropy automatically. Therefore, weak key >> + remains weak unless strong salt is supplied. Following is bad >> + example. >> + >> + >> +> +> +$inputKey =3D get_my_aes128_key(); // AES 128 bit key >> + >> +// Derive AES 256 key from AES 128 key >> +$encryptionKey =3D hash_hkdf('sha256', $inputKey, 32, 'aes-256-encrypti= on'); >> +// Users should not do this. $encryptionKey only has 128 bit >> +// entropy while it should have 256 bit entropy. >> +// To derive strong AES 256 key, strong enough salt is required. >> +?> >> +]]> >> + >> + >> + >> >> >> @@ -130,6 +286,7 @@ >> &reftitle.seealso; >> >> >> + hash_hmac >> hash_pbkdf2 >> RFC 5869 >> > xlink:href=3D"&url.git.hub;narfbg/hash_hkdf_compat">userland >> implementation As I already said yesterday and which you chose to ignore. The manual shoul= d describe the functions and the paramaters and that's it. Maybe add a warning / note where it is warranted, or even link to some external resourc= e (e.g. owasp) when it really is needed. However when reading the above changes I think: great blog post or nice OSS library shared on github for people to use. Imo all these examples and opinionated talk about things being mandatory have no place in our manu= al. Look at the page of crypt (http://php.net/manual/en/function.crypt.php). Th= ere is a short text in the intro about password hashing with password_hash and = a warning. Look at the page of openssl_encrypt (http://php.net/manual/en/function.open= ssl-encrypt.php). Nowhere on that page is being said it is mandatory to not use ECB mode. So as far as I'm concerned I personally don't agree with the above changes,= because it's trying to use the manual for something it's not meant for (describing = functions and their parameters). So please just keep it at that and write a blog post / open source library for everything else. I have the feeling you keep adding your own personal preferences to the man= ual. Also note I am not even talking about the actual technical implications as = Nikita done below. So a big -1 from me too. > Strong -1 on these docs changes. They are wrong and they will confuse use= rs > about when and how HKDF should be used. > I have no idea where you got the idea that HKDF is supposed to be used fo= r CSRF > token generation, but it isn't. I did not check whether your code is corr= ect > and secure, but CSRF token generation is certainly not a common or typica= l > application of HKDF and as such should not be present in the documentatio= n. > Your "bad example" is actually pretty much the textbook use-case for HKDF= . The > way you wrote it (get a AES-256 key from an AES-128 key) doesn't make muc= h > sense, but the general principle of extracting two keys (for encryption a= nd > authentication) from a single key is one of *the* use-cases of HKDF. It i= s > also, contrary to your statement in the documentation snippet, perfectly > cryptographically sound. A salt is not required for this case. A salt *ma= y* be > beneficial, but for entirely different reasons (as Scott pointed out, for= many > block cipher modes fixed encryption keys only have a lifetime of around 2= ^64 > encryptions, past which point IV collisions are to be expected -- a salt = in key > derivation could mitigate this.)