Skip to content

Commit

Permalink
Add and use Core::ensureTrue(, )
Browse files Browse the repository at this point in the history
  • Loading branch information
defuse committed Apr 23, 2018
1 parent 39fba0c commit ed55664
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 182 deletions.
119 changes: 53 additions & 66 deletions src/Core.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,29 +50,25 @@ final class Core
*/
public static function incrementCounter($ctr, $inc)
{
if (Core::ourStrlen($ctr) !== Core::BLOCK_BYTE_SIZE) {
throw new Ex\EnvironmentIsBrokenException(
'Trying to increment a nonce of the wrong size.'
);
}

if (! \is_int($inc)) {
throw new Ex\EnvironmentIsBrokenException(
'Trying to increment nonce by a non-integer.'
);
}

if ($inc < 0) {
throw new Ex\EnvironmentIsBrokenException(
'Trying to increment nonce by a negative amount.'
);
}

if ($inc > PHP_INT_MAX - 255) {
throw new Ex\EnvironmentIsBrokenException(
'Integer overflow may occur.'
);
}
Core::ensureTrue(
Core::ourStrlen($ctr) === Core::BLOCK_BYTE_SIZE,
'Trying to increment a nonce of the wrong size.'
);

Core::ensureTrue(
\is_int($inc),
'Trying to increment nonce by a non-integer.'
);

Core::ensureTrue(
$inc >= 0,
'Trying to increment a nonce by a nonpositive amount'
);

Core::ensureTrue(
$inc <= PHP_INT_MAX - 255,
'Integer overflow may occur'
);

/*
* We start at the rightmost byte (big-endian)
Expand All @@ -82,11 +78,7 @@ public static function incrementCounter($ctr, $inc)
$sum = \ord($ctr[$i]) + $inc;

/* Detect integer overflow and fail. */
if (! \is_int($sum)) {
throw new Ex\EnvironmentIsBrokenException(
'Integer overflow in CTR mode nonce increment.'
);
}
Core::ensureTrue(\is_int($sum), 'Integer overflow in CTR mode nonce increment');

$ctr[$i] = \pack('C', $sum & 0xFF);
$inc = $sum >> 8;
Expand Down Expand Up @@ -146,12 +138,10 @@ public static function HKDF($hash, $ikm, $length, $info = '', $salt = null)
$digest_length = Core::ourStrlen(\hash_hmac($hash, '', '', true));

// Sanity-check the desired output length.
if (empty($length) || ! \is_int($length) ||
$length < 0 || $length > 255 * $digest_length) {
throw new Ex\EnvironmentIsBrokenException(
'Bad output length requested of HKDF.'
);
}
Core::ensureTrue(
!empty($length) && \is_int($length) && $length >= 0 && $length <= 255 * $digest_length,
'Bad output length requested of HDKF.'
);

// "if [salt] not provided, is set to a string of HashLen zeroes."
if (\is_null($salt)) {
Expand All @@ -166,9 +156,7 @@ public static function HKDF($hash, $ikm, $length, $info = '', $salt = null)
// HKDF-Expand:

// This check is useless, but it serves as a reminder to the spec.
if (Core::ourStrlen($prk) < $digest_length) {
throw new Ex\EnvironmentIsBrokenException();
}
Core::ensureTrue(Core::ourStrlen($prk) >= $digest_length);

// T(0) = ''
$t = '';
Expand All @@ -188,9 +176,7 @@ 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 (!\is_string($orm)) {
throw new Ex\EnvironmentIsBrokenException();
}
Core::ensureTrue(\is_string($orm));
return $orm;
}

Expand Down Expand Up @@ -224,9 +210,7 @@ public static function hashEquals($expected, $given)
// We're not attempting to make variable-length string comparison
// secure, as that's very difficult. Make sure the strings are the same
// length.
if (Core::ourStrlen($expected) !== Core::ourStrlen($given)) {
throw new Ex\EnvironmentIsBrokenException();
}
Core::ensureTrue( Core::ourStrlen($expected) === Core::ourStrlen($given));

$blind = Core::secureRandom(32);
$message_compare = \hash_hmac(Core::HASH_FUNCTION_NAME, $given, $blind);
Expand All @@ -243,9 +227,7 @@ public static function hashEquals($expected, $given)
*/
public static function ensureConstantExists($name)
{
if (! \defined($name)) {
throw new Ex\EnvironmentIsBrokenException();
}
Core::ensureTrue(\defined($name));
}

/**
Expand All @@ -258,8 +240,21 @@ public static function ensureConstantExists($name)
*/
public static function ensureFunctionExists($name)
{
if (! \function_exists($name)) {
throw new Ex\EnvironmentIsBrokenException();
Core::ensureTrue(\function_exists($name));
}

/**
* Throws an exception if the condition is false.
*
* @param bool $condition
* @return void
*
* @throws Ex\EnvironmentIsBrokenException
*/
public static function ensureTrue($condition, $message = '')
{
if (!$condition) {
throw new Ex\EnvironmentIsBrokenException($message);
}
}

Expand All @@ -286,9 +281,7 @@ public static function ourStrlen($str)
}
if ($exists) {
$length = \mb_strlen($str, '8bit');
if ($length === false) {
throw new Ex\EnvironmentIsBrokenException();
}
Core::ensureTrue($length !== false);
return $length;
} else {
return \strlen($str);
Expand Down Expand Up @@ -403,28 +396,22 @@ public static function pbkdf2($algorithm, $password, $salt, $count, $key_length,
$key_length += 0;

$algorithm = \strtolower($algorithm);
if (! \in_array($algorithm, \hash_algos(), true)) {
throw new Ex\EnvironmentIsBrokenException(
'Invalid or unsupported hash algorithm.'
);
}
Core::ensureTrue(
\in_array($algorithm, \hash_algos(), true),
'Invalid or unsupported hash algorithm.'
);

// Whitelist, or we could end up with people using CRC32.
$ok_algorithms = [
'sha1', 'sha224', 'sha256', 'sha384', 'sha512',
'ripemd160', 'ripemd256', 'ripemd320', 'whirlpool',
];
if (! \in_array($algorithm, $ok_algorithms, true)) {
throw new Ex\EnvironmentIsBrokenException(
'Algorithm is not a secure cryptographic hash function.'
);
}
Core::ensureTrue(
\in_array($algorithm, $ok_algorithms, true),
'Algorithm is not a secure cryptographic hash function.'
);

if ($count <= 0 || $key_length <= 0) {
throw new Ex\EnvironmentIsBrokenException(
'Invalid PBKDF2 parameters.'
);
}
Core::ensureTrue($count > 0 && $key_length > 0, 'Invalid PBKDF2 parameters.');

if (\function_exists('hash_pbkdf2')) {
// The output length is in NIBBLES (4-bits) if $raw_output is false!
Expand Down
44 changes: 10 additions & 34 deletions src/Crypto.php
Original file line number Diff line number Diff line change
Expand Up @@ -181,16 +181,12 @@ public static function legacyDecrypt($ciphertext, $key)
* @var string
*/
$hmac = Core::ourSubstr($ciphertext, 0, Core::LEGACY_MAC_BYTE_SIZE);
if (!\is_string($hmac)) {
throw new Ex\EnvironmentIsBrokenException();
}
Core::ensureTrue(\is_string($hmac));
/**
* @var string
*/
$messageCiphertext = Core::ourSubstr($ciphertext, Core::LEGACY_MAC_BYTE_SIZE);
if (!\is_string($messageCiphertext)) {
throw new Ex\EnvironmentIsBrokenException();
}
Core::ensureTrue(\is_string($messageCiphertext));

// Regenerate the same authentication sub-key.
$akey = Core::HKDF(
Expand Down Expand Up @@ -221,17 +217,13 @@ public static function legacyDecrypt($ciphertext, $key)
* @var string
*/
$iv = Core::ourSubstr($messageCiphertext, 0, Core::LEGACY_BLOCK_BYTE_SIZE);
if (!\is_string($iv)) {
throw new Ex\EnvironmentIsBrokenException();
}
Core::ensureTrue(\is_string($iv));

/**
* @var string
*/
$actualCiphertext = Core::ourSubstr($messageCiphertext, Core::LEGACY_BLOCK_BYTE_SIZE);
if (!\is_string($actualCiphertext)) {
throw new Ex\EnvironmentIsBrokenException();
}
Core::ensureTrue(\is_string($actualCiphertext));

// Do the decryption.
$plaintext = self::plainDecrypt($actualCiphertext, $ekey, $iv, Core::LEGACY_CIPHER_METHOD);
Expand Down Expand Up @@ -320,9 +312,7 @@ private static function decryptInternal($ciphertext, KeyOrPassword $secret, $raw
Core::HEADER_VERSION_SIZE,
Core::SALT_BYTE_SIZE
);
if (!\is_string($salt)) {
throw new Ex\EnvironmentIsBrokenException();
}
Core::ensureTrue(\is_string($salt));

// Get the IV.
/** @var string $iv */
Expand All @@ -331,9 +321,7 @@ private static function decryptInternal($ciphertext, KeyOrPassword $secret, $raw
Core::HEADER_VERSION_SIZE + Core::SALT_BYTE_SIZE,
Core::BLOCK_BYTE_SIZE
);
if (!\is_string($iv)) {
throw new Ex\EnvironmentIsBrokenException();
}
Core::ensureTrue(\is_string($iv));

// Get the HMAC.
/** @var string $hmac */
Expand All @@ -342,9 +330,7 @@ private static function decryptInternal($ciphertext, KeyOrPassword $secret, $raw
Core::ourStrlen($ciphertext) - Core::MAC_BYTE_SIZE,
Core::MAC_BYTE_SIZE
);
if (!\is_string($hmac)) {
throw new Ex\EnvironmentIsBrokenException();
}
Core::ensureTrue(\is_string($hmac));

// Get the actual encrypted ciphertext.
/** @var string $encrypted */
Expand All @@ -355,9 +341,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 (!\is_string($encrypted)) {
throw new Ex\EnvironmentIsBrokenException();
}
Core::ensureTrue(\is_string($encrypted));

// Derive the separate encryption and authentication keys from the key
// or password, whichever it is.
Expand Down Expand Up @@ -397,11 +381,7 @@ protected static function plainEncrypt($plaintext, $key, $iv)
$iv
);

if (!\is_string($ciphertext)) {
throw new Ex\EnvironmentIsBrokenException(
'openssl_encrypt() failed.'
);
}
Core::ensureTrue(\is_string($ciphertext), 'openssl_encrypt() failed');

return $ciphertext;
}
Expand Down Expand Up @@ -431,11 +411,7 @@ protected static function plainDecrypt($ciphertext, $key, $iv, $cipherMethod)
OPENSSL_RAW_DATA,
$iv
);
if (!\is_string($plaintext)) {
throw new Ex\EnvironmentIsBrokenException(
'openssl_decrypt() failed.'
);
}
Core::ensureTrue(\is_string($plaintext), 'openssl_decrypt() failed.');

return $plaintext;
}
Expand Down
18 changes: 8 additions & 10 deletions src/Encoding.php
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,10 @@ public static function saveBytesToChecksummedAsciiSafeString($header, $bytes)
{
// Headers must be a constant length to prevent one type's header from
// being a prefix of another type's header, leading to ambiguity.
if (Core::ourStrlen($header) !== self::SERIALIZE_HEADER_BYTES) {
throw new Ex\EnvironmentIsBrokenException(
'Header must be ' . self::SERIALIZE_HEADER_BYTES . ' bytes.'
);
}
Core::ensureTrue(
Core::ourStrlen($header) === self::SERIALIZE_HEADER_BYTES,
'Header must be ' . self::SERIALIZE_HEADER_BYTES . ' bytes.'
);

return Encoding::binToHex(
$header .
Expand Down Expand Up @@ -211,11 +210,10 @@ public static function loadBytesFromChecksummedAsciiSafeString($expected_header,
{
// Headers must be a constant length to prevent one type's header from
// being a prefix of another type's header, leading to ambiguity.
if (Core::ourStrlen($expected_header) !== self::SERIALIZE_HEADER_BYTES) {
throw new Ex\EnvironmentIsBrokenException(
'Header must be 4 bytes.'
);
}
Core::ensureTrue(
Core::ourStrlen($expected_header) === self::SERIALIZE_HEADER_BYTES,
'Header must be 4 bytes.'
);

/* If you get an exception here when attempting to load from a file, first pass your
key to Encoding::trimTrailingWhitespace() to remove newline characters, etc. */
Expand Down
Loading

0 comments on commit ed55664

Please sign in to comment.