Skip to content

Make OCP\Security stricter #7900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MorrisJobke merged 1 commit into from
Jan 17, 2018
Merged

Make OCP\Security stricter #7900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions lib/private/Security/Crypto.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
Expand Down Expand Up @@ -67,7 +68,7 @@ public function __construct(IConfig $config, ISecureRandom $random) {
* @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');
}
Expand All @@ -86,7 +87,7 @@ public function calculateHMAC($message, $password = '') {
* @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');
}
Expand Down Expand Up @@ -115,7 +116,7 @@ public function decrypt(string $authenticatedCiphertext, string $password = ''):
$this->cipher->setPassword($password);

$parts = explode('|', $authenticatedCiphertext);
if(count($parts) !== 3) {
if(\count($parts) !== 3) {
throw new \Exception('Authenticated ciphertext could not be decoded.');
}

Expand Down
17 changes: 9 additions & 8 deletions lib/private/Security/Hasher.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
Expand Down Expand Up @@ -63,7 +64,7 @@ public function __construct(IConfig $config) {
$this->config = $config;

$hashingCost = $this->config->getSystemValue('hashingCost', null);
if(!is_null($hashingCost)) {
if(!\is_null($hashingCost)) {
$this->options['cost'] = $hashingCost;
}
}
Expand All @@ -76,7 +77,7 @@ public function __construct(IConfig $config) {
* @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);
}

Expand All @@ -85,9 +86,9 @@ public function hash($message) {
* @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]);
}
Expand All @@ -103,13 +104,13 @@ protected function splitHash($prefixedHash) {
* @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);
Expand All @@ -126,7 +127,7 @@ protected function legacyHashVerify($message, $hash, &$newHash = null) {
* @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);
Expand All @@ -143,7 +144,7 @@ protected function verifyHashV1($message, $hash, &$newHash = null) {
* @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'])) {
Expand Down
5 changes: 3 additions & 2 deletions lib/public/Security/ICrypto.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
Expand Down Expand Up @@ -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)
Expand All @@ -51,7 +52,7 @@ public function calculateHMAC($message, $password = '');
* @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)
Expand Down
5 changes: 3 additions & 2 deletions lib/public/Security/IHasher.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
Expand Down Expand Up @@ -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
Expand All @@ -62,5 +63,5 @@ public function hash($message);
* @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 ;
}
3 changes: 2 additions & 1 deletion lib/public/Security/StringUtils.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
Expand Down Expand Up @@ -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);
}
}
4 changes: 0 additions & 4 deletions tests/lib/Security/HasherTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ public function versionHashProvider()
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),
Expand Down