Skip to content

Commit

Permalink
More comments and boyscouting.
Browse files Browse the repository at this point in the history
  • Loading branch information
@paragonie-security
paragonie-security committed Jun 9, 2017
1 parent e0b9119 commit b5477c5
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 17 deletions.
107 changes: 99 additions & 8 deletions src/Core/Util.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,21 @@ abstract class ParagonIE_Sodium_Core_Util
*
* @internal You should not use this directly from another application
*
* @param string $bin_string (raw binary)
* @param string $binaryString (raw binary)
* @return string
* @throws TypeError
*/
public static function bin2hex($bin_string)
public static function bin2hex($binaryString)
{
/* Type checks: */
if (!is_string($binaryString)) {
throw new TypeError('Argument 1 must be a string, ' . gettype($binaryString) . ' given.');
}

$hex = '';
$len = self::strlen($bin_string);
$len = self::strlen($binaryString);
for ($i = 0; $i < $len; ++$i) {
$chunk = unpack('C', self::substr($bin_string, $i, 2));
$chunk = unpack('C', self::substr($binaryString, $i, 2));
$c = $chunk[1] & 0xf;
$b = $chunk[1] >> 4;
$hex .= pack(
Expand Down Expand Up @@ -89,6 +95,10 @@ public static function bin2hexUpper($bin_string)
*/
public static function chrToInt($chr)
{
/* Type checks: */
if (!is_string($chr)) {
throw new TypeError('Argument 1 must be a string, ' . gettype($chr) . ' given.');
}
if (self::strlen($chr) !== 1) {
throw new Error('chrToInt() expects a string that is exactly 1 character long');
}
Expand Down Expand Up @@ -133,9 +143,18 @@ public static function compare($left, $right, $len = null)
* @param string $left
* @param string $right
* @return bool
* @throws TypeError
*/
public static function hashEquals($left, $right)
{
/* Type checks: */
if (!is_string($left)) {
throw new TypeError('Argument 1 must be a string, ' . gettype($left) . ' given.');
}
if (!is_string($right)) {
throw new TypeError('Argument 2 must be a string, ' . gettype($right) . ' given.');
}

if (is_callable('hash_equals')) {
return hash_equals($left, $right);
}
Expand All @@ -147,7 +166,9 @@ public static function hashEquals($left, $right)
for ($i = 0; $i < $len; ++$i) {
$d |= self::chrToInt($left[$i]) ^ self::chrToInt($right[$i]);
}
if ($d !== 0) {

/* branch-free variant of === 0 */
if (-(1 & (($d - 1) >> 8))) {
return false;
}
return $left === $right;
Expand All @@ -163,9 +184,15 @@ public static function hashEquals($left, $right)
* @param bool $strictPadding
* @return string (raw binary)
* @throws RangeException
* @throws TypeError
*/
public static function hex2bin($hexString, $strictPadding = false)
{
/* Type checks: */
if (!is_string($hexString)) {
throw new TypeError('Argument 1 must be a string, ' . gettype($hexString) . ' given.');
}

$hex_pos = 0;
$bin = '';
$c_acc = 0;
Expand Down Expand Up @@ -235,9 +262,6 @@ public static function intArrayToString(array $ints)
*/
public static function intToChr($int)
{
if (!is_int($int)) {
throw new TypeError('intToChr() expects an integer');
}
return pack('C', $int);
}

Expand All @@ -249,9 +273,16 @@ public static function intToChr($int)
* @param string $string
* @return int
* @throws RangeException
* @throws TypeError
*/
public static function load_3($string)
{
/* Type checks: */
if (!is_string($string)) {
throw new TypeError('Argument 1 must be a string, ' . gettype($string) . ' given.');
}

/* Input validation: */
if (self::strlen($string) < 3) {
throw new RangeException(
'String must be 3 bytes or more; ' . self::strlen($string) . ' given.'
Expand All @@ -271,9 +302,16 @@ public static function load_3($string)
* @param string $string
* @return int
* @throws RangeException
* @throws TypeError
*/
public static function load_4($string)
{
/* Type checks: */
if (!is_string($string)) {
throw new TypeError('Argument 1 must be a string, ' . gettype($string) . ' given.');
}

/* Input validation: */
if (self::strlen($string) < 4) {
throw new RangeException(
'String must be 4 bytes or more; ' . self::strlen($string) . ' given.'
Expand All @@ -294,9 +332,16 @@ public static function load_4($string)
* @param string $string
* @return int
* @throws RangeException
* @throws TypeError
*/
public static function load64_le($string)
{
/* Type checks: */
if (!is_string($string)) {
throw new TypeError('Argument 1 must be a string, ' . gettype($string) . ' given.');
}

/* Input validation: */
if (self::strlen($string) < 4) {
throw new RangeException(
'String must be 4 bytes or more; ' . self::strlen($string) . ' given.'
Expand Down Expand Up @@ -425,9 +470,19 @@ public static function numericTo64BitInteger($num)
*
* @param int $int
* @return string
* @throws TypeError
*/
public static function store_3($int)
{
/* Type checks: */
if (!is_int($int)) {
if (is_numeric($int)) {
$int = (int) $int;
} else {
throw new TypeError('Argument 1 must be an integer, ' . gettype($int) . ' given.');
}
}

return self::intToChr(($int >> 16) & 0xff) .
self::intToChr(($int >> 8) & 0xff) .
self::intToChr($int & 0xff);
Expand All @@ -440,9 +495,19 @@ public static function store_3($int)
*
* @param int $int
* @return string
* @throws TypeError
*/
public static function store32_le($int)
{
/* Type checks: */
if (!is_int($int)) {
if (is_numeric($int)) {
$int = (int) $int;
} else {
throw new TypeError('Argument 1 must be an integer, ' . gettype($int) . ' given.');
}
}

return self::intToChr($int & 0xff) .
self::intToChr(($int >> 8) & 0xff) .
self::intToChr(($int >> 16) & 0xff) .
Expand All @@ -456,9 +521,19 @@ public static function store32_le($int)
*
* @param int $int
* @return string
* @throws TypeError
*/
public static function store_4($int)
{
/* Type checks: */
if (!is_int($int)) {
if (is_numeric($int)) {
$int = (int) $int;
} else {
throw new TypeError('Argument 1 must be an integer, ' . gettype($int) . ' given.');
}
}

return self::intToChr(($int >> 24) & 0xff) .
self::intToChr(($int >> 16) & 0xff) .
self::intToChr(($int >> 8) & 0xff) .
Expand All @@ -472,9 +547,19 @@ public static function store_4($int)
*
* @param int $int
* @return string
* @throws TypeError
*/
public static function store64_le($int)
{
/* Type checks: */
if (!is_int($int)) {
if (is_numeric($int)) {
$int = (int) $int;
} else {
throw new TypeError('Argument 1 must be an integer, ' . gettype($int) . ' given.');
}
}

if (PHP_INT_SIZE === 8) {
return self::intToChr($int & 0xff) .
self::intToChr(($int >> 8) & 0xff) .
Expand Down Expand Up @@ -513,6 +598,7 @@ public static function store64_le($int)
*/
public static function strlen($str)
{
/* Type checks: */
if (!is_string($str)) {
throw new TypeError('String expected');
}
Expand Down Expand Up @@ -562,6 +648,7 @@ public static function stringToIntArray($string)
*/
public static function substr($str, $start = 0, $length = null)
{
/* Type checks: */
if (!is_string($str)) {
throw new TypeError('String expected');
}
Expand Down Expand Up @@ -598,6 +685,7 @@ public static function substr($str, $start = 0, $length = null)
*/
public static function verify_16($a, $b)
{
/* Type checks: */
if (!is_string($a)) {
throw new TypeError('String expected');
}
Expand All @@ -622,6 +710,7 @@ public static function verify_16($a, $b)
*/
public static function verify_32($a, $b)
{
/* Type checks: */
if (!is_string($a)) {
throw new TypeError('String expected');
}
Expand All @@ -646,12 +735,14 @@ public static function verify_32($a, $b)
*/
public static function xorStrings($a, $b)
{
/* Type checks: */
if (!is_string($a)) {
throw new TypeError('Argument 1 must be a string');
}
if (!is_string($b)) {
throw new TypeError('Argument 2 must be a string');
}

return $a ^ $b;
}

Expand Down
35 changes: 26 additions & 9 deletions src/Crypto.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public static function aead_chacha20poly1305_decrypt(
$key
);

/* Recalculate the Poly1305 authentication tag (MAC): */
$state = new ParagonIE_Sodium_Core_Poly1305_State($block0);
try {
ParagonIE_Sodium_Compat::memzero($block0);
Expand All @@ -111,6 +112,7 @@ public static function aead_chacha20poly1305_decrypt(
$state->update(ParagonIE_Sodium_Core_Util::store64_le($clen));
$computed_mac = $state->finish();

/* Compare the given MAC with the recalculated MAC: */
if (!ParagonIE_Sodium_Core_Util::verify_16($computed_mac, $mac)) {
throw new Error('Invalid MAC');
}
Expand Down Expand Up @@ -223,6 +225,7 @@ public static function aead_chacha20poly1305_ietf_decrypt(
$len - self::aead_chacha20poly1305_IETF_ABYTES
);

/* Recalculate the Poly1305 authentication tag (MAC): */
$state = new ParagonIE_Sodium_Core_Poly1305_State($block0);
try {
ParagonIE_Sodium_Compat::memzero($block0);
Expand All @@ -237,6 +240,7 @@ public static function aead_chacha20poly1305_ietf_decrypt(
$state->update(ParagonIE_Sodium_Core_Util::store64_le($clen));
$computed_mac = $state->finish();

/* Compare the given MAC with the recalculated MAC: */
if (!ParagonIE_Sodium_Core_Util::verify_16($computed_mac, $mac)) {
throw new Error('Invalid MAC');
}
Expand Down Expand Up @@ -775,13 +779,7 @@ public static function keyExchange($my_sk, $their_pk, $client_pk, $server_pk)
public static function scalarmult($sKey, $pKey)
{
$q = ParagonIE_Sodium_Core_X25519::crypto_scalarmult_curve25519_ref10($sKey, $pKey);
$d = 0;
for ($i = 0; $i < self::box_curve25519xsalsa20poly1305_SECRETKEYBYTES; ++$i) {
$d |= ParagonIE_Sodium_Core_Util::chrToInt($q[$i]);
}
if (-(1 & (($d - 1) >> 8))) {
throw new Error('Zero public key is not allowed');
}
self::scalarmult_throw_if_zero($q);
return $q;
}

Expand All @@ -797,14 +795,28 @@ public static function scalarmult($sKey, $pKey)
public static function scalarmult_base($secret)
{
$q = ParagonIE_Sodium_Core_X25519::crypto_scalarmult_curve25519_ref10_base($secret);
self::scalarmult_throw_if_zero($q);
return $q;
}

/**
* This throws an Error if a zero public key was passed to the function.
*
* @param string $q
* @return void
* @throws Error
*/
protected static function scalarmult_throw_if_zero($q)
{
$d = 0;
for ($i = 0; $i < self::box_curve25519xsalsa20poly1305_SECRETKEYBYTES; ++$i) {
$d |= ParagonIE_Sodium_Core_Util::chrToInt($q[$i]);
}

/* branch-free variant of === 0 */
if (-(1 & (($d - 1) >> 8))) {
throw new Error('Zero public key is not allowed');
}
return $q;
}

/**
Expand Down Expand Up @@ -918,7 +930,11 @@ public static function secretbox_open($ciphertext, $nonce, $key)
ParagonIE_Sodium_Core_Util::substr($nonce, 16, 8),
$subkey
);
$verified = ParagonIE_Sodium_Core_Poly1305::onetimeauth_verify($mac, $c, ParagonIE_Sodium_Core_Util::substr($block0, 0, 32));
$verified = ParagonIE_Sodium_Core_Poly1305::onetimeauth_verify(
$mac,
$c,
ParagonIE_Sodium_Core_Util::substr($block0, 0, 32)
);
if (!$verified) {
try {
ParagonIE_Sodium_Compat::memzero($subkey);
Expand Down Expand Up @@ -1068,6 +1084,7 @@ public static function secretbox_xchacha20poly1305_open($ciphertext, $nonce, $ke
$c,
ParagonIE_Sodium_Core_Util::substr($block0, 0, 32)
);

if (!$verified) {
try {
ParagonIE_Sodium_Compat::memzero($subkey);
Expand Down

0 comments on commit b5477c5

Please sign in to comment.