feature(security): Adds API to create and validate HMAC tokens

This also has getHmac() use a binary encoding of the site key instead
of the Base64 or hex encodings, and improves the docs of the site secret
component.

Fixes Elgg#7824

docs/guides/actions.rst

@@ -425,3 +425,35 @@ You can also access the tokens from javascript:
 These are refreshed periodically so should always be up-to-date.
 
 
+Security Tokens
+===============
+On occasion we need to pass data through an untrusted party or generate an "unguessable token" based on some data.
+The industry-standard `HMAC <http://security.stackexchange.com/a/20301/4982>`_ algorithm is the right tool for this.
+It allows us to verify that received data were generated by our site, and were not tampered with. Note that even
+strong hash functions like SHA-2 should *not* be used directly for these tasks.
+
+Elgg provides ``elgg_generate_mac()`` and ``elgg_validate_mac()`` to generate and validate HMAC message authentication
+codes that are unguessable without the site's private key.
+
+.. code:: php
+
+    // generate a querystring such that $a and $b can't be altered
+    $a = 1234;
+    $b = "hello";
+    $query = http_build_query([
+        'a' => $a,
+        'b' => $b,
+        'mac' => elgg_generate_mac("$a|$b"),
+    ]);
+    $url = "action/foo?$query";
+
+
+    // validate the querystring
+    $a = get_input('a', '', false);
+    $b = get_input('b', '', false);
+    $mac = get_input('mac', '', false);
+
+    if (elgg_validate_mac($mac, "$a|$b")) {
+        // $a and $b have not been altered
+    }
+

engine/classes/Elgg/ActionsService.php

@@ -144,7 +144,7 @@ public function validateActionToken($visible_errors = true, $token = null, $ts =
 
 		if (($token) && ($ts) && ($session_id)) {
 			// generate token, check with input and forward if invalid
-			$required_token = generate_action_token($ts);
+			$required_token = $this->generateActionToken($ts);
 
 			// Validate token
 			$token_matches = _elgg_services()->crypto->areEqual($token, $required_token);
@@ -248,7 +248,8 @@ public function gatekeeper($action) {
 			}
 
 			// let the validator send an appropriate msg
-			validate_action_token();
+			$this->validateActionToken();
+
 		} else if ($this->validateActionToken()) {
 			return true;
 		}
@@ -261,16 +262,14 @@ public function gatekeeper($action) {
 	 * @access private
 	 */
 	public function generateActionToken($timestamp) {
-		$site_secret = _elgg_services()->siteSecret->get();
 		$session_id = _elgg_services()->session->getId();
-		// Session token
-		$st = _elgg_services()->session->get('__elgg_session');
-
-		if ($session_id && $site_secret) {
-			return _elgg_services()->crypto->getHmac($timestamp . $session_id . $st, $site_secret, 'md5');
+		if (!$session_id) {
+			return false;
 		}
-
-		return false;
+
+		$session_token = _elgg_services()->session->get('__elgg_session');
+
+		return _elgg_services()->crypto->getHmac("$timestamp|$session_id|$session_token", '', 'md5');
 	}
 
 	/**

engine/classes/Elgg/Database/SiteSecret.php

@@ -2,7 +2,16 @@
 namespace Elgg\Database;
 
 /**
- * WARNING: API IN FLUX. DO NOT USE DIRECTLY.
+ * Manages a site-specific secret key, encoded as a 32 byte string "secret"
+ *
+ * The key can have two formats:
+ *   - Since 1.8.17 all keys generated are Base64URL-encoded with the 1st character set to "z" so that
+ *     the format can be recognized. With one character lost, this makes the keys effectively 186 bits.
+ *   - Before 1.8.17 keys were hex-encoded (128 bits) but created from insufficiently random sources.
+ *
+ * The hex keys were created with rand() as the only decent source of entropy (the site's creation time
+ * is not too difficult to find). As such, systems with a low getrandmax() value created particularly
+ * weak keys. You can check key string using getStrength().
  *
  * @access private
  *
@@ -11,6 +20,7 @@
  * @since      1.10.0
  */
 class SiteSecret {
+
 	/**
 	 * Initialise the site secret (32 bytes: "z" to indicate format + 186-bit key in Base64 URL).
 	 *
@@ -23,34 +33,55 @@ class SiteSecret {
 	 */
 	function init() {
 		$secret = 'z' . _elgg_services()->crypto->getRandomString(31);
-	
+
 		if (_elgg_services()->datalist->set('__site_secret__', $secret)) {
 			return $secret;
 		}
-	
+
 		return false;
 	}
-	
+
 	/**
 	 * Returns the site secret.
 	 *
 	 * Used to generate difficult to guess hashes for sessions and action tokens.
 	 *
+	 * @param bool $raw If true, a binary key will be returned
+	 *
 	 * @return string Site secret.
 	 * @access private
 	 */
-	function get() {
+	function get($raw = false) {
 		$secret = _elgg_services()->datalist->get('__site_secret__');
 		if (!$secret) {
 			$secret = init_site_secret();
 		}
-
+
+		if ($raw) {
+			// try to return binary key
+			if ($secret[0] === 'z') {
+				// new keys are "z" + base64URL
+				$base64 = strtr(substr($secret, 1), '-_', '+/');
+				$key = base64_decode($base64);
+				if ($key !== false) {
+					// on failure, at least return string key :/
+					return $key;
+				}
+			} else {
+				// old keys are hex
+				return hex2bin($secret);
+			}
+		}
+
 		return $secret;
 	}
-	
+
 	/**
 	 * Get the strength of the site secret
 	 *
+	 * If "weak" or "moderate" is returned, this assumes we're running on the same system that created
+	 * the key.
+	 *
 	 * @return string "strong", "moderate", or "weak"
 	 * @access private
 	 */
@@ -67,5 +98,5 @@ function getStrength() {
 		}
 		return 'strong';
 	}
-		
-}
+
+}

engine/classes/Elgg/Database/UsersTable.php

@@ -446,7 +446,7 @@ function register($username, $password, $name, $email, $allow_multiple_emails =
 	 */
 	function generateInviteCode($username) {
 		$time = time();
-		return "{$time}." . _elgg_services()->crypto->getHmac($time . $username);
+		return "{$time}." . _elgg_services()->crypto->getHmac("$time|$username");
 	}
 
 	/**
@@ -467,7 +467,7 @@ function validateInviteCode($username, $code) {
 		$mac = $m[2];
 
 		$crypto = _elgg_services()->crypto;
-		return $crypto->areEqual($mac, $crypto->getHmac($time . $username));
+		return $crypto->areEqual($mac, $crypto->getHmac("$time|$username"));
 	}
 
 	/**

engine/classes/ElggCrypto.php

@@ -165,9 +165,10 @@ public function getRandomBytes($length) {
 	 * @param string $algo HMAC hash algorithm
 	 * @return string
 	 */
-	function getHmac($data, $key = '', $algo = 'sha256') {
+	public function getHmac($data, $key = '', $algo = 'sha256') {
 		if (!$key) {
-			$key = _elgg_services()->siteSecret->get();
+			// Prefer binary key for a couple good reasons: http://crypto.stackexchange.com/a/8662/7197
+			$key = _elgg_services()->siteSecret->get(true);
 		}
 		$bytes = hash_hmac($algo, $data, $key, true);
 		return strtr(rtrim(base64_encode($bytes), '='), '+/', '-_');

engine/lib/actions.php

@@ -97,6 +97,34 @@ function elgg_unregister_action($action) {
 	return _elgg_services()->actions->unregister($action);
 }
 
+/**
+ * Generate an authentication code/token in Base64URL encoding, using the site secret as key.
+ *
+ * @param string $data Data passed through an untrusted channel
+ *
+ * @return string
+ * @since 1.11
+ * @see elgg_validate_mac
+ */
+function elgg_generate_mac($data) {
+	return _elgg_services()->crypto->getHmac($data);
+}
+
+/**
+ * Validate an authentication code generated via elgg_generate_mac().
+ *
+ * @param string $mac  MAC given in input, in Base64URL format
+ * @param string $data Data ostensibly unaltered by the user
+ *
+ * @return bool
+ * @since 1.10
+ * @see elgg_generate_mac
+ */
+function elgg_validate_mac($mac, $data) {
+	$crypto = _elgg_services()->crypto;
+	return $crypto->areEqual($mac, $crypto->getHmac($data));
+}
+
 /**
  * Validate an action token.
  *

engine/tests/phpunit/ElggCryptoTest.php

@@ -7,6 +7,10 @@ class ElggCryptoTest extends \PHPUnit_Framework_TestCase {
 	 */
 	protected $stub;
 
+	/**
+	 * @see ElggCrypto
+	 * @see ElggCrypto::getRandomBytes
+	 */
 	protected function setUp() {
 		$this->stub = $this->getMockBuilder('\ElggCrypto')
 			->setMethods(array('getRandomBytes'))
@@ -41,4 +45,36 @@ function provider() {
 	function testGetRandomString($length, $chars, $expected) {
 		$this->assertSame($expected, $this->stub->getRandomString($length, $chars));
 	}
+
+	function testGeneratesMacInBase64Url() {
+		$crypto = new ElggCrypto();
+		$key = 'a very bad key';
+		$data = '1';
+		$expected = 'nL0lgXrVWgGK0Cmr9_PjqQcR2_PzuAHH114AsPZk-AM';
+		$algo = 'sha256';
+
+		$this->assertEquals($expected, $crypto->getHmac($data, $key, $algo));
+	}
+
+	function testStringCastDoesntAffectMacs() {
+		$crypto = new ElggCrypto();
+		$key = 'a very bad key';
+
+		$this->assertEquals($crypto->getHmac(1234, $key), $crypto->getHmac('1234', $key));
+	}
+
+	function testMacAlteredByVaryingData() {
+		$crypto = new ElggCrypto();
+		$key = 'a very bad key';
+
+		$this->assertNotEquals($crypto->getHmac('1234', $key), $crypto->getHmac('1235', $key));
+	}
+
+	function testMacAlteredByVaryingKey() {
+		$crypto = new ElggCrypto();
+		$key1 = 'a very bad key';
+		$key2 = 'b very bad key';
+
+		$this->assertNotEquals($crypto->getHmac('1234', $key1), $crypto->getHmac('1234', $key2));
+	}
 }

mod/uservalidationbyemail/lib/functions.php

@@ -16,7 +16,7 @@
 function uservalidationbyemail_generate_code($user_guid, $email_address) {
 	// Note: binding to site URL for multisite.
 	$site_url = elgg_get_site_url();
-	return _elgg_services()->crypto->getHmac($user_guid . $email_address . $site_url);
+	return elgg_generate_mac("$user_guid|$email_address|$site_url");
 }
 
 /**
@@ -79,12 +79,13 @@ function uservalidationbyemail_request_validation($user_guid, $admin_requested =
  */
 function uservalidationbyemail_validate_email($user_guid, $code) {
 	$user = get_entity($user_guid);
+	$site_url = elgg_get_site_url();
 
-	if ($code == uservalidationbyemail_generate_code($user_guid, $user->email)) {
-		return elgg_set_user_validation_status($user_guid, true, 'email');
+	if (!elgg_validate_mac($code, "$user_guid|{$user->email}|$site_url")) {
+		return false;
 	}
 
-	return false;
+	return elgg_set_user_validation_status($user_guid, true, 'email');
 }
 
 /**