]> granicus.if.org Git - php/commitdiff
Add hash_equals() to perform string comparisons that are not vulnerable to timing...
authorRouven Weßling <me@rouvenwessling.de>
Sun, 10 Nov 2013 21:37:10 +0000 (22:37 +0100)
committerRouven Weßling <me@rouvenwessling.de>
Mon, 17 Mar 2014 08:37:28 +0000 (09:37 +0100)
NEWS
UPGRADING
ext/hash/hash.c
ext/hash/php_hash.h
ext/hash/tests/hash_equals.phpt [new file with mode: 0644]

diff --git a/NEWS b/NEWS
index c5fbb6cd9964a4ab58c427ced057c1fb3c1aeece..a2f563b9605b9b456d049bf1980ca2d30edd06f7 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -30,6 +30,8 @@ PHP                                                                        NEWS
 - Hash:
   . Fixed bug #66698 (Missing FNV1a32 and FNV1a64 hash functions).
     (Michael M Slusarz).
+  . Implemented timing attack safe string comparison function
+    (RFC: https://wiki.php.net/rfc/timing_attack). (Rouven Weßling)
 
 - Intl:
   . Fixed bug #66873 (A reproductible crash in UConverter when given invalid
index 9cfc0a642d3f520ef2bb4ed23022e02ecc6b310e..8b20f512f23d0910d7c305240a900342d57a2974 100755 (executable)
--- a/UPGRADING
+++ b/UPGRADING
@@ -71,6 +71,9 @@ PHP 5.6 UPGRADE NOTES
 - Added use function and use const.
   (https://wiki.php.net/rfc/use_function)
 
+- Added a function for timing attack safe string comparison
+  (https://wiki.php.net/rfc/timing_attack)
+
 - Added gost-crypto (CryptoPro S-box) hash algorithm.
 
 - Stream wrappers verify peer certificates and host names by default in
@@ -182,6 +185,9 @@ PHP 5.6 UPGRADE NOTES
 5. New Functions
 ========================================
 
+- Hash
+  Added hash_equals($known_string, $user_string)
+
 - GMP:
   Added gmp_root($a, $nth) and gmp_rootrem($a, $nth) for calculating nth roots.
 
index 41c7a70d2b097215cfeedecf35b14f93e780e03d..f14437d96fa7008a5d410a277c7df55b7f0fb13c 100644 (file)
@@ -728,6 +728,46 @@ PHP_FUNCTION(hash_pbkdf2)
 }
 /* }}} */
 
+/* {{{ proto bool hash_equals(string known_string, string user_string)
+   Compares two strings using the same time whether they're equal or not.
+   A difference in length will leak */
+PHP_FUNCTION(hash_equals)
+{
+       zval *known_zval, *user_zval;
+       char *known_str, *user_str;
+       int result = 0, j;
+
+       if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "zz", &known_zval, &user_zval) == FAILURE) {
+               return;
+       }
+
+       /* We only allow comparing string to prevent unexpected results. */
+       if (Z_TYPE_P(known_zval) != IS_STRING) {
+               php_error_docref(NULL TSRMLS_CC, E_WARNING, "Expected known_string to be a string, %s given", zend_zval_type_name(known_zval));
+               RETURN_FALSE;
+       }
+
+       if (Z_TYPE_P(user_zval) != IS_STRING) {
+               php_error_docref(NULL TSRMLS_CC, E_WARNING, "Expected user_string to be a string, %s given", zend_zval_type_name(user_zval));
+               RETURN_FALSE;
+       }
+
+       if (Z_STRLEN_P(known_zval) != Z_STRLEN_P(user_zval)) {
+               RETURN_FALSE;
+       }
+
+       known_str = Z_STRVAL_P(known_zval);
+       user_str = Z_STRVAL_P(user_zval);
+
+       /* This is security sensitive code. Do not optimize this for speed. */
+       for (j = 0; j < Z_STRLEN_P(known_zval); j++) {
+               result |= known_str[j] ^ user_str[j];
+       }
+
+       RETURN_BOOL(0 == result);
+}
+/* }}} */
+
 /* Module Housekeeping */
 
 static void php_hash_dtor(zend_rsrc_list_entry *rsrc TSRMLS_DC) /* {{{ */
@@ -1152,6 +1192,11 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_hash_pbkdf2, 0, 0, 4)
        ZEND_ARG_INFO(0, raw_output)
 ZEND_END_ARG_INFO()
 
+ZEND_BEGIN_ARG_INFO(arginfo_hash_equals, 0)
+       ZEND_ARG_INFO(0, known_string)
+       ZEND_ARG_INFO(0, user_string)
+ZEND_END_ARG_INFO()
+
 /* BC Land */
 #ifdef PHP_MHASH_BC
 ZEND_BEGIN_ARG_INFO(arginfo_mhash_get_block_size, 0)
@@ -1199,6 +1244,7 @@ const zend_function_entry hash_functions[] = {
 
        PHP_FE(hash_algos,                                                              arginfo_hash_algos)
        PHP_FE(hash_pbkdf2,                                                             arginfo_hash_pbkdf2)
+       PHP_FE(hash_equals,                                                             arginfo_hash_equals)
 
        /* BC Land */
 #ifdef PHP_HASH_MD5_NOT_IN_CORE
index c5cb6604db7752adf88d57883eb005d5a06cb655..c75b93093646ef4533c1ec334f3fb9ca9a55d822 100644 (file)
@@ -136,6 +136,7 @@ PHP_FUNCTION(hash_update_file);
 PHP_FUNCTION(hash_final);
 PHP_FUNCTION(hash_algos);
 PHP_FUNCTION(hash_pbkdf2);
+PHP_FUNCTION(hash_equals);
 
 PHP_HASH_API const php_hash_ops *php_hash_fetch_ops(const char *algo, int algo_len);
 PHP_HASH_API void php_hash_register_algo(const char *algo, const php_hash_ops *ops);
diff --git a/ext/hash/tests/hash_equals.phpt b/ext/hash/tests/hash_equals.phpt
new file mode 100644 (file)
index 0000000..8f87985
--- /dev/null
@@ -0,0 +1,43 @@
+--TEST--
+hash_equals() function
+--FILE--
+<?php
+var_dump(hash_equals("same", "same"));
+var_dump(hash_equals("not1same", "not2same"));
+var_dump(hash_equals("short", "longer"));
+var_dump(hash_equals("longer", "short"));
+var_dump(hash_equals("", "notempty"));
+var_dump(hash_equals("notempty", ""));
+var_dump(hash_equals("", ""));
+var_dump(hash_equals(123, "NaN"));
+var_dump(hash_equals("NaN", 123));
+var_dump(hash_equals(123, 123));
+var_dump(hash_equals(null, ""));
+var_dump(hash_equals(null, 123));
+var_dump(hash_equals(null, null));
+--EXPECTF--
+bool(true)
+bool(false)
+bool(false)
+bool(false)
+bool(false)
+bool(false)
+bool(true)
+
+Warning: hash_equals(): Expected known_string to be a string, integer given in %s on line %d
+bool(false)
+
+Warning: hash_equals(): Expected user_string to be a string, integer given in %s on line %d
+bool(false)
+
+Warning: hash_equals(): Expected known_string to be a string, integer given in %s on line %d
+bool(false)
+
+Warning: hash_equals(): Expected known_string to be a string, null given in %s on line %d
+bool(false)
+
+Warning: hash_equals(): Expected known_string to be a string, null given in %s on line %d
+bool(false)
+
+Warning: hash_equals(): Expected known_string to be a string, null given in %s on line %d
+bool(false)