From 51d9ae0c01675ce0903e9296d99f513e82e4cac6 Mon Sep 17 00:00:00 2001 From: Scott Arciszewski Date: Sat, 22 Apr 2017 17:55:57 -0400 Subject: [PATCH 1/4] Begin Psalm integration. --- .travis.yml | 29 +++++++++++++++++++++-------- composer.json | 2 +- psalm.xml | 9 +++++++++ 3 files changed, 31 insertions(+), 9 deletions(-) create mode 100644 psalm.xml diff --git a/.travis.yml b/.travis.yml index d05dc37..28e1f86 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,17 +1,29 @@ language: php -php: - - 7.1 - - 7.0 - - 5.6 - - 5.5 - - 5.4 - sudo: false matrix: - fast_finish: true + fast_finish: true + include: + - php: "5.4" + env: USE_PSALM=0 + - php: "5.5" + env: USE_PSALM=0 + - php: "5.6" + env: USE_PSALM=1 + - php: "7.0" + env: USE_PSALM=1 + - php: "7.1" + env: USE_PSALM=1 + - php: "nightly" + env: USE_PSALM=1 + - php: "hhvm" + env: USE_PSALM=1 + allow_failures: + - php: "nightly" + - php: "hhvm" install: + - if [[ $USE_PSALM -eq 1 ]]; then composer require --dev "vimeo/psalm:dev-master"; fi - composer install - curl -LSs https://box-project.github.io/box2/installer.php | php - mkdir ~/box @@ -21,3 +33,4 @@ script: - ./test.sh - PATH=$PATH:~/box/ make -C dist/ build-phar - ./test.sh dist/defuse-crypto.phar + - if [[ $USE_PSALM -eq 1 ]]; then vendor/bin/psalm; fi diff --git a/composer.json b/composer.json index b02eb21..e0ff79c 100644 --- a/composer.json +++ b/composer.json @@ -26,6 +26,6 @@ "php": ">=5.4.0" }, "require-dev": { - "nikic/php-parser": "^2.0" + "nikic/php-parser": "^2.0|^3.0" } } diff --git a/psalm.xml b/psalm.xml new file mode 100644 index 0000000..b3f25ca --- /dev/null +++ b/psalm.xml @@ -0,0 +1,9 @@ + + + + + + From 55b2794986311c5d1cab6030675d918473ac020b Mon Sep 17 00:00:00 2001 From: Scott Arciszewski Date: Sun, 23 Apr 2017 09:42:35 -0400 Subject: [PATCH 2/4] defuse/php-encryption is now verifiably type-safe --- src/Core.php | 13 ++++--- src/Crypto.php | 45 ++++++++++++++++------- src/DerivedKeys.php | 17 +++++++-- src/Encoding.php | 10 +++--- src/File.php | 65 ++++++++++++++++++++++++---------- src/Key.php | 5 ++- src/KeyOrPassword.php | 18 ++++++++-- src/KeyProtectedByPassword.php | 5 ++- src/RuntimeTests.php | 5 +++ 9 files changed, 137 insertions(+), 46 deletions(-) diff --git a/src/Core.php b/src/Core.php index 38e9a40..3026801 100644 --- a/src/Core.php +++ b/src/Core.php @@ -131,7 +131,7 @@ public static function HKDF($hash, $ikm, $length, $info = '', $salt = null) { static $nativeHKDF = null; if ($nativeHKDF === null) { - $nativeHKDF = \function_exists('hash_hkdf'); + $nativeHKDF = \is_callable('\\hash_hkdf'); } if ($nativeHKDF) { return \hash_hkdf($hash, $ikm, $length, $info, $salt); @@ -180,8 +180,9 @@ public static function HKDF($hash, $ikm, $length, $info = '', $salt = null) } // ORM = first L octets of T + /** @var string $orm */ $orm = Core::ourSubstr($t, 0, $length); - if ($orm === false) { + if (!\is_string($orm)) { throw new Ex\EnvironmentIsBrokenException(); } return $orm; @@ -230,6 +231,7 @@ public static function hashEquals($expected, $given) * Throws an exception if the constant doesn't exist. * * @param string $name + * @return void * * @throws Ex\EnvironmentIsBrokenException */ @@ -244,6 +246,7 @@ public static function ensureConstantExists($name) * Throws an exception if the function doesn't exist. * * @param string $name + * @return void * * @throws Ex\EnvironmentIsBrokenException */ @@ -295,7 +298,7 @@ public static function ourStrlen($str) * * @throws Ex\EnvironmentIsBrokenException * - * @return string + * @return string|bool */ public static function ourSubstr($str, $start, $length = null) { @@ -434,9 +437,9 @@ public static function pbkdf2($algorithm, $password, $salt, $count, $key_length, } if ($raw_output) { - return Core::ourSubstr($output, 0, $key_length); + return (string) Core::ourSubstr($output, 0, $key_length); } else { - return Encoding::binToHex(Core::ourSubstr($output, 0, $key_length)); + return Encoding::binToHex((string) Core::ourSubstr($output, 0, $key_length)); } } } diff --git a/src/Crypto.php b/src/Crypto.php index e466a47..fdb411c 100644 --- a/src/Crypto.php +++ b/src/Crypto.php @@ -111,12 +111,18 @@ public static function legacyDecrypt($ciphertext, $key) 'Ciphertext is too short.' ); } + /** + * @var string + */ $hmac = Core::ourSubstr($ciphertext, 0, Core::LEGACY_MAC_BYTE_SIZE); - if ($hmac === false) { + if (!\is_string($hmac)) { throw new Ex\EnvironmentIsBrokenException(); } + /** + * @var string + */ $ciphertext = Core::ourSubstr($ciphertext, Core::LEGACY_MAC_BYTE_SIZE); - if ($ciphertext === false) { + if (!\is_string($ciphertext)) { throw new Ex\EnvironmentIsBrokenException(); } @@ -145,17 +151,24 @@ public static function legacyDecrypt($ciphertext, $key) 'Ciphertext is too short.' ); } + /** + * @var string + */ $iv = Core::ourSubstr($ciphertext, 0, Core::LEGACY_BLOCK_BYTE_SIZE); - if ($iv === false) { + if (!\is_string($iv)) { throw new Ex\EnvironmentIsBrokenException(); } - $ciphertext = Core::ourSubstr($ciphertext, Core::LEGACY_BLOCK_BYTE_SIZE); - if ($ciphertext === false) { + + /** + * @var string + */ + $actualCiphertext = Core::ourSubstr($ciphertext, Core::LEGACY_BLOCK_BYTE_SIZE); + if (!\is_string($actualCiphertext)) { throw new Ex\EnvironmentIsBrokenException(); } // Do the decryption. - $plaintext = self::plainDecrypt($ciphertext, $ekey, $iv, Core::LEGACY_CIPHER_METHOD); + $plaintext = self::plainDecrypt($actualCiphertext, $ekey, $iv, Core::LEGACY_CIPHER_METHOD); return $plaintext; } else { throw new Ex\WrongKeyOrModifiedCiphertextException( @@ -226,6 +239,7 @@ private static function decryptInternal($ciphertext, KeyOrPassword $secret, $raw } // Get and check the version header. + /** @var string $header */ $header = Core::ourSubstr($ciphertext, 0, Core::HEADER_VERSION_SIZE); if ($header !== Core::CURRENT_VERSION) { throw new Ex\WrongKeyOrModifiedCiphertextException( @@ -234,36 +248,40 @@ private static function decryptInternal($ciphertext, KeyOrPassword $secret, $raw } // Get the salt. + /** @var string $salt */ $salt = Core::ourSubstr( $ciphertext, Core::HEADER_VERSION_SIZE, Core::SALT_BYTE_SIZE ); - if ($salt === false) { + if (!\is_string($salt)) { throw new Ex\EnvironmentIsBrokenException(); } // Get the IV. + /** @var string $iv */ $iv = Core::ourSubstr( $ciphertext, Core::HEADER_VERSION_SIZE + Core::SALT_BYTE_SIZE, Core::BLOCK_BYTE_SIZE ); - if ($iv === false) { + if (!\is_string($iv)) { throw new Ex\EnvironmentIsBrokenException(); } // Get the HMAC. + /** @var string $hmac */ $hmac = Core::ourSubstr( $ciphertext, Core::ourStrlen($ciphertext) - Core::MAC_BYTE_SIZE, Core::MAC_BYTE_SIZE ); - if ($hmac === false) { + if (!\is_string($hmac)) { throw new Ex\EnvironmentIsBrokenException(); } // Get the actual encrypted ciphertext. + /** @var string $encrypted */ $encrypted = Core::ourSubstr( $ciphertext, Core::HEADER_VERSION_SIZE + Core::SALT_BYTE_SIZE + @@ -271,7 +289,7 @@ private static function decryptInternal($ciphertext, KeyOrPassword $secret, $raw Core::ourStrlen($ciphertext) - Core::MAC_BYTE_SIZE - Core::SALT_BYTE_SIZE - Core::BLOCK_BYTE_SIZE - Core::HEADER_VERSION_SIZE ); - if ($encrypted === false) { + if (!\is_string($encrypted)) { throw new Ex\EnvironmentIsBrokenException(); } @@ -304,6 +322,7 @@ protected static function plainEncrypt($plaintext, $key, $iv) { Core::ensureConstantExists('OPENSSL_RAW_DATA'); Core::ensureFunctionExists('openssl_encrypt'); + /** @var string $ciphertext */ $ciphertext = \openssl_encrypt( $plaintext, Core::CIPHER_METHOD, @@ -312,7 +331,7 @@ protected static function plainEncrypt($plaintext, $key, $iv) $iv ); - if ($ciphertext === false) { + if (!\is_string($ciphertext)) { throw new Ex\EnvironmentIsBrokenException( 'openssl_encrypt() failed.' ); @@ -337,6 +356,8 @@ protected static function plainDecrypt($ciphertext, $key, $iv, $cipherMethod) { Core::ensureConstantExists('OPENSSL_RAW_DATA'); Core::ensureFunctionExists('openssl_decrypt'); + + /** @var string $plaintext */ $plaintext = \openssl_decrypt( $ciphertext, $cipherMethod, @@ -344,7 +365,7 @@ protected static function plainDecrypt($ciphertext, $key, $iv, $cipherMethod) OPENSSL_RAW_DATA, $iv ); - if ($plaintext === false) { + if (!\is_string($plaintext)) { throw new Ex\EnvironmentIsBrokenException( 'openssl_decrypt() failed.' ); diff --git a/src/DerivedKeys.php b/src/DerivedKeys.php index fcfc043..86a48e5 100644 --- a/src/DerivedKeys.php +++ b/src/DerivedKeys.php @@ -2,13 +2,25 @@ namespace Defuse\Crypto; +/** + * Class DerivedKeys + * @package Defuse\Crypto + */ final class DerivedKeys { - private $akey = null; - private $ekey = null; + /** + * @var string + */ + private $akey = ''; + + /** + * @var string + */ + private $ekey = ''; /** * Returns the authentication key. + * @return string */ public function getAuthenticationKey() { @@ -17,6 +29,7 @@ public function getAuthenticationKey() /** * Returns the encryption key. + * @return string */ public function getEncryptionKey() { diff --git a/src/Encoding.php b/src/Encoding.php index 72d8f52..001fb6e 100644 --- a/src/Encoding.php +++ b/src/Encoding.php @@ -131,7 +131,7 @@ public static function trimTrailingWhitespace($string = '') $sub = (((0x1f - $chr) & ($chr - 0x21)) >> 8) & 1; $length -= $sub; } while ($prevLength !== $length && $length > 0); - return Core::ourSubstr($string, 0, $length); + return (string) Core::ourSubstr($string, 0, $length); } /* @@ -229,7 +229,7 @@ public static function loadBytesFromChecksummedAsciiSafeString($expected_header, } /* Grab the version header. */ - $actual_header = Core::ourSubstr($bytes, 0, self::SERIALIZE_HEADER_BYTES); + $actual_header = (string) Core::ourSubstr($bytes, 0, self::SERIALIZE_HEADER_BYTES); if ($actual_header !== $expected_header) { throw new Ex\BadFormatException( @@ -238,14 +238,14 @@ public static function loadBytesFromChecksummedAsciiSafeString($expected_header, } /* Grab the bytes that are part of the checksum. */ - $checked_bytes = Core::ourSubstr( + $checked_bytes = (string) Core::ourSubstr( $bytes, 0, Core::ourStrlen($bytes) - self::CHECKSUM_BYTE_SIZE ); /* Grab the included checksum. */ - $checksum_a = Core::ourSubstr( + $checksum_a = (string) Core::ourSubstr( $bytes, Core::ourStrlen($bytes) - self::CHECKSUM_BYTE_SIZE, self::CHECKSUM_BYTE_SIZE @@ -261,7 +261,7 @@ public static function loadBytesFromChecksummedAsciiSafeString($expected_header, ); } - return Core::ourSubstr( + return (string) Core::ourSubstr( $bytes, self::SERIALIZE_HEADER_BYTES, Core::ourStrlen($bytes) - self::SERIALIZE_HEADER_BYTES - self::CHECKSUM_BYTE_SIZE diff --git a/src/File.php b/src/File.php index d739077..78e39d6 100644 --- a/src/File.php +++ b/src/File.php @@ -12,6 +12,7 @@ final class File * @param string $inputFilename * @param string $outputFilename * @param Key $key + * @return void * * @throws Ex\EnvironmentIsBrokenException * @throws Ex\IOException @@ -32,6 +33,7 @@ public static function encryptFile($inputFilename, $outputFilename, Key $key) * @param string $inputFilename * @param string $outputFilename * @param string $password + * @return void * * @throws Ex\EnvironmentIsBrokenException * @throws Ex\IOException @@ -51,6 +53,7 @@ public static function encryptFileWithPassword($inputFilename, $outputFilename, * @param string $inputFilename * @param string $outputFilename * @param Key $key + * @return void * * @throws Ex\EnvironmentIsBrokenException * @throws Ex\IOException @@ -72,6 +75,7 @@ public static function decryptFile($inputFilename, $outputFilename, Key $key) * @param string $inputFilename * @param string $outputFilename * @param string $password + * @return void * * @throws Ex\EnvironmentIsBrokenException * @throws Ex\IOException @@ -93,6 +97,7 @@ public static function decryptFileWithPassword($inputFilename, $outputFilename, * @param resource $inputHandle * @param resource $outputHandle * @param Key $key + * @return void * * @throws Ex\EnvironmentIsBrokenException * @throws Ex\WrongKeyOrModifiedCiphertextException @@ -114,6 +119,7 @@ public static function encryptResource($inputHandle, $outputHandle, Key $key) * @param resource $inputHandle * @param resource $outputHandle * @param string $password + * @return void * * @throws Ex\EnvironmentIsBrokenException * @throws Ex\IOException @@ -135,6 +141,7 @@ public static function encryptResourceWithPassword($inputHandle, $outputHandle, * @param resource $inputHandle * @param resource $outputHandle * @param Key $key + * @return void * * @throws Ex\EnvironmentIsBrokenException * @throws Ex\IOException @@ -156,6 +163,7 @@ public static function decryptResource($inputHandle, $outputHandle, Key $key) * @param resource $inputHandle * @param resource $outputHandle * @param string $password + * @return void * * @throws Ex\EnvironmentIsBrokenException * @throws Ex\IOException @@ -176,6 +184,7 @@ public static function decryptResourceWithPassword($inputHandle, $outputHandle, * @param string $inputFilename * @param string $outputFilename * @param KeyOrPassword $secret + * @return void * * @throws Ex\CryptoException * @throws Ex\IOException @@ -240,6 +249,7 @@ private static function encryptFileInternal($inputFilename, $outputFilename, Key * @param string $inputFilename * @param string $outputFilename * @param KeyOrPassword $secret + * @return void * * @throws Ex\CryptoException * @throws Ex\IOException @@ -254,7 +264,7 @@ private static function decryptFileInternal($inputFilename, $outputFilename, Key self::getLastErrorMessage() ); } - + if (\is_callable('\\stream_set_read_buffer')) { /* This call can fail, but the only consequence is performance. */ \stream_set_read_buffer($if, 0); @@ -269,7 +279,7 @@ private static function decryptFileInternal($inputFilename, $outputFilename, Key self::getLastErrorMessage() ); } - + if (\is_callable('\\stream_set_write_buffer')) { /* This call can fail, but the only consequence is performance. */ \stream_set_write_buffer($of, 0); @@ -306,6 +316,7 @@ private static function decryptFileInternal($inputFilename, $outputFilename, Key * @param resource $inputHandle * @param resource $outputHandle * @param KeyOrPassword $secret + * @return void * * @throws Ex\EnvironmentIsBrokenException * @throws Ex\IOException @@ -335,8 +346,9 @@ private static function encryptResourceInternal($inputHandle, $outputHandle, Key $iv = Core::secureRandom($ivsize); /* Initialize a streaming HMAC state. */ + /** @var resource $hmac */ $hmac = \hash_init(Core::HASH_FUNCTION_NAME, HASH_HMAC, $akey); - if ($hmac === false) { + if (!\is_resource($hmac)) { throw new Ex\EnvironmentIsBrokenException( 'Cannot initialize a hash context' ); @@ -358,14 +370,15 @@ private static function encryptResourceInternal($inputHandle, $outputHandle, Key $thisIv = $iv; /* How many blocks do we encrypt at a time? We increment by this value. */ - $inc = Core::BUFFER_BYTE_SIZE / Core::BLOCK_BYTE_SIZE; + $inc = (int) (Core::BUFFER_BYTE_SIZE / Core::BLOCK_BYTE_SIZE); /* Loop until we reach the end of the input file. */ $at_file_end = false; while (! (\feof($inputHandle) || $at_file_end)) { /* Find out if we can read a full buffer, or only a partial one. */ + /** @var int */ $pos = \ftell($inputHandle); - if ($pos === false) { + if (!\is_int($pos)) { throw new Ex\IOException( 'Could not get current position in input file during encryption' ); @@ -385,6 +398,7 @@ private static function encryptResourceInternal($inputHandle, $outputHandle, Key } /* Encrypt this buffer. */ + /** @var string */ $encrypted = \openssl_encrypt( $read, Core::CIPHER_METHOD, @@ -393,7 +407,7 @@ private static function encryptResourceInternal($inputHandle, $outputHandle, Key $thisIv ); - if ($encrypted === false) { + if (!\is_string($encrypted)) { throw new Ex\EnvironmentIsBrokenException( 'OpenSSL encryption error' ); @@ -422,6 +436,7 @@ private static function encryptResourceInternal($inputHandle, $outputHandle, Key * @param resource $inputHandle * @param resource $outputHandle * @param KeyOrPassword $secret + * @return void * * @throws Ex\EnvironmentIsBrokenException * @throws Ex\IOException @@ -476,7 +491,7 @@ public static function decryptResourceInternal($inputHandle, $outputHandle, KeyO $thisIv = $iv; /* How many blocks do we encrypt at a time? We increment by this value. */ - $inc = Core::BUFFER_BYTE_SIZE / Core::BLOCK_BYTE_SIZE; + $inc = (int) (Core::BUFFER_BYTE_SIZE / Core::BLOCK_BYTE_SIZE); /* Get the HMAC. */ if (\fseek($inputHandle, (-1 * Core::MAC_BYTE_SIZE), SEEK_END) === false) { @@ -486,8 +501,9 @@ public static function decryptResourceInternal($inputHandle, $outputHandle, KeyO } /* Get the position of the last byte in the actual ciphertext. */ + /** @var int $cipher_end */ $cipher_end = \ftell($inputHandle); - if ($cipher_end === false) { + if (!\is_int($cipher_end)) { throw new Ex\IOException( 'Cannot read input file' ); @@ -496,11 +512,13 @@ public static function decryptResourceInternal($inputHandle, $outputHandle, KeyO --$cipher_end; /* Read the HMAC. */ + /** @var string $stored_mac */ $stored_mac = self::readBytes($inputHandle, Core::MAC_BYTE_SIZE); /* Initialize a streaming HMAC state. */ + /** @var resource $hmac */ $hmac = \hash_init(Core::HASH_FUNCTION_NAME, HASH_HMAC, $akey); - if ($hmac === false) { + if (!\is_resource($hmac)) { throw new Ex\EnvironmentIsBrokenException( 'Cannot initialize a hash context' ); @@ -525,12 +543,14 @@ public static function decryptResourceInternal($inputHandle, $outputHandle, KeyO \hash_update($hmac, $header); \hash_update($hmac, $file_salt); \hash_update($hmac, $iv); + /** @var resource $hmac2 */ $hmac2 = \hash_copy($hmac); $break = false; while (! $break) { + /** @var int $pos */ $pos = \ftell($inputHandle); - if ($pos === false) { + if (!\is_int($pos)) { throw new Ex\IOException( 'Could not get current position in input file during decryption' ); @@ -554,8 +574,9 @@ public static function decryptResourceInternal($inputHandle, $outputHandle, KeyO \hash_update($hmac, $read); /* Remember this buffer-sized chunk's HMAC. */ + /** @var resource $chunk_mac */ $chunk_mac = \hash_copy($hmac); - if ($chunk_mac === false) { + if (!\is_resource($chunk_mac)) { throw new Ex\EnvironmentIsBrokenException( 'Cannot duplicate a hash context' ); @@ -564,6 +585,7 @@ public static function decryptResourceInternal($inputHandle, $outputHandle, KeyO } /* Get the final HMAC, which should match the stored one. */ + /** @var string $final_mac */ $final_mac = \hash_final($hmac, true); /* Verify the HMAC. */ @@ -584,8 +606,9 @@ public static function decryptResourceInternal($inputHandle, $outputHandle, KeyO $at_file_end = false; while (! $at_file_end) { + /** @var int $pos */ $pos = \ftell($inputHandle); - if ($pos === false) { + if (!\is_int($pos)) { throw new Ex\IOException( 'Could not get current position in input file during decryption' ); @@ -609,8 +632,9 @@ public static function decryptResourceInternal($inputHandle, $outputHandle, KeyO * remembered from pass #1 to ensure attackers didn't change the * ciphertext after MAC verification. */ \hash_update($hmac2, $read); + /** @var resource $calc_mac */ $calc_mac = \hash_copy($hmac2); - if ($calc_mac === false) { + if (!\is_resource($calc_mac)) { throw new Ex\EnvironmentIsBrokenException( 'Cannot duplicate a hash context' ); @@ -628,6 +652,7 @@ public static function decryptResourceInternal($inputHandle, $outputHandle, KeyO } /* Decrypt this buffer-sized chunk. */ + /** @var string $decrypted */ $decrypted = \openssl_decrypt( $read, Core::CIPHER_METHOD, @@ -635,7 +660,7 @@ public static function decryptResourceInternal($inputHandle, $outputHandle, KeyO OPENSSL_RAW_DATA, $thisIv ); - if ($decrypted === false) { + if (!\is_string($decrypted)) { throw new Ex\EnvironmentIsBrokenException( 'OpenSSL decryption error' ); @@ -649,6 +674,7 @@ public static function decryptResourceInternal($inputHandle, $outputHandle, KeyO ); /* Increment the IV by the amount of blocks in a buffer. */ + /** @var string $thisIv */ $thisIv = Core::incrementCounter($thisIv, $inc); /* WARNING: Usually, unless the file is a multiple of the buffer * size, $thisIv will contain an incorrect value here on the last @@ -661,6 +687,7 @@ public static function decryptResourceInternal($inputHandle, $outputHandle, KeyO * * @param resource $stream * @param int $num_bytes + * @return string * * @throws Ex\IOException * @throws Ex\EnvironmentIsBrokenException @@ -679,9 +706,9 @@ public static function readBytes($stream, $num_bytes) $buf = ''; $remaining = $num_bytes; while ($remaining > 0 && ! \feof($stream)) { + /** @var string $read */ $read = \fread($stream, $remaining); - - if ($read === false) { + if (!\is_string($read)) { throw new Ex\IOException( 'Could not read from the file' ); @@ -703,6 +730,7 @@ public static function readBytes($stream, $num_bytes) * @param resource $stream * @param string $buf * @param int $num_bytes + * @return int * * @throws Ex\IOException * @@ -726,13 +754,14 @@ public static function writeBytes($stream, $buf, $num_bytes = null) } $remaining = $num_bytes; while ($remaining > 0) { + /** @var int $written */ $written = \fwrite($stream, $buf, $remaining); - if ($written === false) { + if (!\is_int($written)) { throw new Ex\IOException( 'Could not write to the file' ); } - $buf = Core::ourSubstr($buf, $written, null); + $buf = (string) Core::ourSubstr($buf, $written, null); $remaining -= $written; } return $num_bytes; diff --git a/src/Key.php b/src/Key.php index 8ce3951..fe4bf7d 100644 --- a/src/Key.php +++ b/src/Key.php @@ -9,7 +9,10 @@ final class Key const KEY_CURRENT_VERSION = "\xDE\xF0\x00\x00"; const KEY_BYTE_SIZE = 32; - private $key_bytes = null; + /** + * @var string + */ + private $key_bytes; /** * Creates new random key. diff --git a/src/KeyOrPassword.php b/src/KeyOrPassword.php index 2a46b71..4a810d3 100644 --- a/src/KeyOrPassword.php +++ b/src/KeyOrPassword.php @@ -10,8 +10,15 @@ final class KeyOrPassword const SECRET_TYPE_KEY = 1; const SECRET_TYPE_PASSWORD = 2; - private $secret_type = null; - private $secret = null; + /** + * @var int + */ + private $secret_type = 0; + + /** + * @var Key|string + */ + private $secret; /** * Initializes an instance of KeyOrPassword from a key. @@ -43,6 +50,7 @@ public static function createFromPassword($password) * * @param string $salt * + * @throws Ex\CryptoException * @throws Ex\EnvironmentIsBrokenException * * @return DerivedKeys @@ -54,6 +62,9 @@ public function deriveKeys($salt) } if ($this->secret_type === self::SECRET_TYPE_KEY) { + if (!($this->secret instanceof Key)) { + throw new Ex\CryptoException('Expected a Key object'); + } $akey = Core::HKDF( Core::HASH_FUNCTION_NAME, $this->secret->getRawBytes(), @@ -70,6 +81,9 @@ public function deriveKeys($salt) ); return new DerivedKeys($akey, $ekey); } elseif ($this->secret_type === self::SECRET_TYPE_PASSWORD) { + if (!\is_string($this->secret)) { + throw new Ex\CryptoException('Expected a string'); + } /* Our PBKDF2 polyfill is vulnerable to a DoS attack documented in * GitHub issue #230. The fix is to pre-hash the password to ensure * it is short. We do the prehashing here instead of in pbkdf2() so diff --git a/src/KeyProtectedByPassword.php b/src/KeyProtectedByPassword.php index b9a40f1..9d32e76 100644 --- a/src/KeyProtectedByPassword.php +++ b/src/KeyProtectedByPassword.php @@ -8,7 +8,10 @@ final class KeyProtectedByPassword { const PASSWORD_KEY_CURRENT_VERSION = "\xDE\xF1\x00\x00"; - private $encrypted_key = null; + /** + * @var string + */ + private $encrypted_key = ''; /** * Creates a random key protected by the provided password. diff --git a/src/RuntimeTests.php b/src/RuntimeTests.php index 9c0ba33..9f00a97 100644 --- a/src/RuntimeTests.php +++ b/src/RuntimeTests.php @@ -17,6 +17,7 @@ class RuntimeTests extends Crypto * Runs the runtime tests. * * @throws Ex\EnvironmentIsBrokenException + * @return void */ public static function runtimeTest() { @@ -79,6 +80,7 @@ public static function runtimeTest() * High-level tests of Crypto operations. * * @throws Ex\EnvironmentIsBrokenException + * @return void */ private static function testEncryptDecrypt() { @@ -148,6 +150,7 @@ private static function testEncryptDecrypt() * Test HKDF against test vectors. * * @throws Ex\EnvironmentIsBrokenException + * @return void */ private static function HKDFTestVector() { @@ -186,6 +189,7 @@ private static function HKDFTestVector() * Test HMAC against test vectors. * * @throws Ex\EnvironmentIsBrokenException + * @return void */ private static function HMACTestVector() { @@ -202,6 +206,7 @@ private static function HMACTestVector() * Test AES against test vectors. * * @throws Ex\EnvironmentIsBrokenException + * @return void */ private static function AESTestVector() { From 0ed93160cbb6691463d3001f1f943615935b85b2 Mon Sep 17 00:00:00 2001 From: Scott Arciszewski Date: Sun, 23 Apr 2017 10:06:01 -0400 Subject: [PATCH 3/4] Suppress a false positive. --- src/Core.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Core.php b/src/Core.php index 3026801..3f5ed52 100644 --- a/src/Core.php +++ b/src/Core.php @@ -124,6 +124,7 @@ public static function secureRandom($octets) * @param string $salt * * @throws Ex\EnvironmentIsBrokenException + * @psalm-suppress UndefinedFunction - We're checking if the function exists first. * * @return string */ From 083c3428cbb3a78779320b18b329d4d672fd2921 Mon Sep 17 00:00:00 2001 From: Scott Date: Mon, 15 May 2017 18:54:06 -0400 Subject: [PATCH 4/4] Add Psalm instructions to documentation --- docs/InternalDeveloperDocs.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/InternalDeveloperDocs.md b/docs/InternalDeveloperDocs.md index 372211c..4604cff 100644 --- a/docs/InternalDeveloperDocs.md +++ b/docs/InternalDeveloperDocs.md @@ -25,6 +25,21 @@ First do `composer install` and then you can run the tests by running `./test.sh`. This will download a PHPUnit PHAR, verify its cryptographic signatures, and then use it to run the tests in `test/unit`. +Getting and Using Psalm +----------------------- + +[Psalm](https://github.com/vimeo/psalm) is a static analysis suite for PHP projects. +We use Psalm to ensure type safety throughout our library. + +To install Psalm, you just need to run one command: + + composer require --dev "vimeo/psalm:dev-master" + +To verify that your code changes are still strictly type-safe, run the following +command: + + vendor/bin/psalm + Reporting Bugs ---------------