Merge pull request #7900 from nextcloud/strict_security

Make OCP\Security stricter

lib/private/Security/Crypto.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -67,7 +68,7 @@ class Crypto implements ICrypto {
 	 * @param string $password Password to use (defaults to `secret` in config.php)
 	 * @return string Calculated HMAC
 	 */
-	public function calculateHMAC($message, $password = '') {
+	public function calculateHMAC(string $message, string $password = ''): string {
 		if($password === '') {
 			$password = $this->config->getSystemValue('secret');
 		}
@@ -86,7 +87,7 @@ class Crypto implements ICrypto {
 	 * @param string $password Password to encrypt, if not specified the secret from config.php will be taken
 	 * @return string Authenticated ciphertext
 	 */
-	public function encrypt($plaintext, $password = '') {
+	public function encrypt(string $plaintext, string $password = ''): string {
 		if($password === '') {
 			$password = $this->config->getSystemValue('secret');
 		}
@@ -115,7 +116,7 @@ class Crypto implements ICrypto {
 		$this->cipher->setPassword($password);
 
 		$parts = explode('|', $authenticatedCiphertext);
-		if(count($parts) !== 3) {
+		if(\count($parts) !== 3) {
 			throw new \Exception('Authenticated ciphertext could not be decoded.');
 		}
 

lib/private/Security/Hasher.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -63,7 +64,7 @@ class Hasher implements IHasher {
 		$this->config = $config;
 
 		$hashingCost = $this->config->getSystemValue('hashingCost', null);
-		if(!is_null($hashingCost)) {
+		if(!\is_null($hashingCost)) {
 			$this->options['cost'] = $hashingCost;
 		}
 	}
@@ -76,7 +77,7 @@ class Hasher implements IHasher {
 	 * @param string $message Message to generate hash from
 	 * @return string Hash of the message with appended version parameter
 	 */
-	public function hash($message) {
+	public function hash(string $message): string {
 		return $this->currentVersion . '|' . password_hash($message, PASSWORD_DEFAULT, $this->options);
 	}
 
@@ -85,9 +86,9 @@ class Hasher implements IHasher {
 	 * @param string $prefixedHash
 	 * @return null|array Null if the hash is not prefixed, otherwise array('version' => 1, 'hash' => 'foo')
 	 */
-	protected function splitHash($prefixedHash) {
+	protected function splitHash(string $prefixedHash) {
 		$explodedString = explode('|', $prefixedHash, 2);
-		if(count($explodedString) === 2) {
+		if(\count($explodedString) === 2) {
 			if((int)$explodedString[0] > 0) {
 				return array('version' => (int)$explodedString[0], 'hash' => $explodedString[1]);
 			}
@@ -103,13 +104,13 @@ class Hasher implements IHasher {
 	 * @param null|string &$newHash Reference will contain the updated hash
 	 * @return bool Whether $hash is a valid hash of $message
 	 */
-	protected function legacyHashVerify($message, $hash, &$newHash = null) {
+	protected function legacyHashVerify($message, $hash, &$newHash = null): bool {
 		if(empty($this->legacySalt)) {
 			$this->legacySalt = $this->config->getSystemValue('passwordsalt', '');
 		}
 
 		// Verify whether it matches a legacy PHPass or SHA1 string
-		$hashLength = strlen($hash);
+		$hashLength = \strlen($hash);
 		if($hashLength === 60 && password_verify($message.$this->legacySalt, $hash) ||
 			$hashLength === 40 && hash_equals($hash, sha1($message))) {
 			$newHash = $this->hash($message);
@@ -126,7 +127,7 @@ class Hasher implements IHasher {
 	 * @param null|string &$newHash Reference will contain the updated hash if necessary. Update the existing hash with this one.
 	 * @return bool Whether $hash is a valid hash of $message
 	 */
-	protected function verifyHashV1($message, $hash, &$newHash = null) {
+	protected function verifyHashV1(string $message, string $hash, &$newHash = null): bool {
 		if(password_verify($message, $hash)) {
 			if(password_needs_rehash($hash, PASSWORD_DEFAULT, $this->options)) {
 				$newHash = $this->hash($message);
@@ -143,7 +144,7 @@ class Hasher implements IHasher {
 	 * @param null|string &$newHash Reference will contain the updated hash if necessary. Update the existing hash with this one.
 	 * @return bool Whether $hash is a valid hash of $message
 	 */
-	public function verify($message, $hash, &$newHash = null) {
+	public function verify(string $message, string $hash, &$newHash = null): bool {
 		$splittedHash = $this->splitHash($hash);
 
 		if(isset($splittedHash['version'])) {

lib/public/Security/ICrypto.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -42,7 +43,7 @@ interface ICrypto {
 	 * @return string Calculated HMAC
 	 * @since 8.0.0
 	 */
-	public function calculateHMAC($message, $password = '');
+	public function calculateHMAC(string $message, string $password = ''): string;
 
 	/**
 	 * Encrypts a value and adds an HMAC (Encrypt-Then-MAC)
@@ -51,7 +52,7 @@ interface ICrypto {
 	 * @return string Authenticated ciphertext
 	 * @since 8.0.0
 	 */
-	public function encrypt($plaintext, $password = '');
+	public function encrypt(string $plaintext, string $password = ''): string;
 
 	/**
 	 * Decrypts a value and verifies the HMAC (Encrypt-Then-Mac)

lib/public/Security/IHasher.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -53,7 +54,7 @@ interface IHasher {
 	 * @return string Hash of the message with appended version parameter
 	 * @since 8.0.0
 	 */
-	public function hash($message);
+	public function hash(string $message): string;
 
 	/**
 	 * @param string $message Message to verify
@@ -62,5 +63,5 @@ interface IHasher {
 	 * @return bool Whether $hash is a valid hash of $message
 	 * @since 8.0.0
 	 */
-	public function verify($message, $hash, &$newHash = null);
+	public function verify(string $message, string $hash, &$newHash = null): bool ;
 }

lib/public/Security/StringUtils.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -42,7 +43,7 @@ class StringUtils {
 	 * @since 8.0.0
 	 * @deprecated 9.0.0 Use hash_equals
 	 */
-	public static function equals($expected, $input) {
+	public static function equals(string $expected, string $input): bool {
 		return hash_equals($expected, $input);
 	}
 }

tests/lib/Security/HasherTest.php

@@ -35,10 +35,6 @@ class HasherTest extends \Test\TestCase {
 	public function allHashProviders()
 	{
 		return array(
-			// Bogus values
-			array(null, 'asf32äà$$a.|3', false),
-			array(null, false, false),
-
 			// Valid SHA1 strings
 			array('password', '5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8', true),
 			array('owncloud.com', '27a4643e43046c3569e33b68c1a4b15d31306d29', true),