From 1b5faf903532439a910bfc56fd545541058f8207 Mon Sep 17 00:00:00 2001 From: Paragon Initiative Enterprises Date: Wed, 29 Jan 2025 05:41:30 -0500 Subject: [PATCH 1/3] Constant-time alternative to strtok() --- src/Keys/Version4/AsymmetricPublicKey.php | 4 +- src/Keys/Version4/AsymmetricSecretKey.php | 4 +- src/Util.php | 51 +++++++++++++++++++++++ tests/UtilTest.php | 26 ++++++++++++ 4 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/Keys/Version4/AsymmetricPublicKey.php b/src/Keys/Version4/AsymmetricPublicKey.php index 93cc6f1..b992fdb 100644 --- a/src/Keys/Version4/AsymmetricPublicKey.php +++ b/src/Keys/Version4/AsymmetricPublicKey.php @@ -14,7 +14,7 @@ use ParagonIE\Paseto\ProtocolInterface; use ParagonIE\Paseto\Util; use TypeError; -use function str_replace, strlen, strtok, substr; +use function str_replace, strlen, substr; /** * Class AsymmetricPublicKey @@ -97,7 +97,7 @@ public static function importPem(string $pem, ?ProtocolInterface $protocol = nul throw new PasetoException('Invalid PEM format', ExceptionCode::UNSPECIFIED_CRYPTOGRAPHIC_ERROR); } - $tokenizedKey = strtok($formattedKey, "\n"); + $tokenizedKey = Util::stopAtDelimiter($formattedKey, "\n"); if ($tokenizedKey === false) { throw new PasetoException('Invalid PEM format', ExceptionCode::UNSPECIFIED_CRYPTOGRAPHIC_ERROR); } diff --git a/src/Keys/Version4/AsymmetricSecretKey.php b/src/Keys/Version4/AsymmetricSecretKey.php index 6ff2994..2c19b8e 100644 --- a/src/Keys/Version4/AsymmetricSecretKey.php +++ b/src/Keys/Version4/AsymmetricSecretKey.php @@ -14,7 +14,7 @@ use ParagonIE\Paseto\ProtocolInterface; use ParagonIE\Paseto\Util; use TypeError; -use function str_replace, strlen, strtok, substr; +use function str_replace, strlen, substr; /** * Class AsymmetricSecretKey @@ -110,7 +110,7 @@ public static function importPem(string $pem, ?ProtocolInterface $protocol = nul throw new PasetoException('Invalid PEM format', ExceptionCode::UNSPECIFIED_CRYPTOGRAPHIC_ERROR); } - $tokenizedKey = strtok($formattedKey, "\n"); + $tokenizedKey = Util::stopAtDelimiter($formattedKey, "\n"); if ($tokenizedKey === false) { throw new PasetoException('Invalid PEM format', ExceptionCode::UNSPECIFIED_CRYPTOGRAPHIC_ERROR); } diff --git a/src/Util.php b/src/Util.php index 88a7473..892738c 100644 --- a/src/Util.php +++ b/src/Util.php @@ -24,6 +24,7 @@ preg_match_all, sodium_memzero, str_replace; +use ParagonIE_Sodium_Core_Util as SodiumUtil; /** * Class Util @@ -173,6 +174,56 @@ public static function removeFooter(string $payload): string return $payload; } + /** + * Return a substring of $input that terminates before the first instance of $delim. + * + * Unlike the built-in C function, our algorithm is constant-time. + * + * @param string $input + * @param string $delim + * @return string|false + */ + public static function stopAtDelimiter(string $input, string $delim): string|false + { + $inLength = SodiumUtil::strlen($input); + $dLen = SodiumUtil::strlen($delim); + if ($inLength < $dLen) { + return false; + } + + // Final value for this string + $terminate = 0; + // We want to work with bytes: + $bytes = SodiumUtil::stringToIntArray($input); + $d = SodiumUtil::stringToIntArray($delim); + $max = $inLength - $dLen; + $found = 0; + + // Iterate over all of $bytes, compare window, + for ($i = 0; $i < $max; ++$i) { + $cmp = 0; + for ($j = 0; $j < $dLen; ++$j) { + $cmp |= ($bytes[$i + $j] ^ $d[$j]); + } + // 1 if $cmp === 0, 0x00 otherwise + $result = (($cmp - 1) >> 8) & 1; + /* + * $found === 0 + * $terminate = $terminate + * $result === 1, $found === 0 + * $terminate = $i + * $result === 0, $found === 0 + * $terminate = $terminate + */ + $terminate = ($terminate & $found) ^ (~$found & (((-$result) & $i))); + $found |= -$result; + } + $terminate = ($terminate & $found) ^ (($inLength) & ~$found); + return SodiumUtil::intArrayToString( + array_slice($bytes, 0, $terminate) + ); + } + /** * If a footer was included with the message, first verify that * it's equivalent to the one we expect, then remove it from the diff --git a/tests/UtilTest.php b/tests/UtilTest.php index ebe2b52..b8d8a58 100644 --- a/tests/UtilTest.php +++ b/tests/UtilTest.php @@ -8,6 +8,7 @@ }; use ParagonIE\Paseto\Exception\EncodingException; use ParagonIE\Paseto\Util; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use TypeError; @@ -123,6 +124,31 @@ public function testPreAuthEncode() ); } + public static function strtokProvider(): array + { + $r = Base64UrlSafe::encodeUnpadded(random_bytes(32)); + return [ + ['Paragon Initiative Enterprises', ' ', 'Paragon'], + ['Initiative Enterprises', ' ', 'Initiative'], + ['Enterprises', ' ', 'Enterprises'], + ['', ' ', false], + [ + $r . "\n" . Base64UrlSafe::encodeUnpadded(random_bytes(16)), + "\n", + $r + ], + ]; + } + + #[DataProvider('strtokProvider')] + public function testStopAtDelimiter(string $input, string $delim, string|false $output): void + { + $result = Util::stopAtDelimiter($input, $delim); + $this->assertSame($output, $result, 'Expected result was not returned'); + $built_in = strtok($input, $delim); + $this->assertSame($built_in, $result, 'Regression with built-in strtok() behavior'); + } + /** * @covers Util::validateAndRemoveFooter() */ From b34a9d73ab07e1250931395dfaf6d7b81c98c41b Mon Sep 17 00:00:00 2001 From: Paragon Initiative Enterprises Date: Wed, 29 Jan 2025 05:50:33 -0500 Subject: [PATCH 2/3] Remove newlines --- src/Keys/Version4/AsymmetricPublicKey.php | 1 + src/Keys/Version4/AsymmetricSecretKey.php | 1 + src/Util.php | 6 +++++- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Keys/Version4/AsymmetricPublicKey.php b/src/Keys/Version4/AsymmetricPublicKey.php index b992fdb..baf6886 100644 --- a/src/Keys/Version4/AsymmetricPublicKey.php +++ b/src/Keys/Version4/AsymmetricPublicKey.php @@ -88,6 +88,7 @@ public static function importPem(string $pem, ?ProtocolInterface $protocol = nul { $formattedKey = str_replace('-----BEGIN PUBLIC KEY-----', '', $pem); $formattedKey = str_replace('-----END PUBLIC KEY-----', '', $formattedKey); + $formattedKey = str_replace(["\r", "\n"], '', $formattedKey); /** * @psalm-suppress DocblockTypeContradiction diff --git a/src/Keys/Version4/AsymmetricSecretKey.php b/src/Keys/Version4/AsymmetricSecretKey.php index 2c19b8e..b9bf146 100644 --- a/src/Keys/Version4/AsymmetricSecretKey.php +++ b/src/Keys/Version4/AsymmetricSecretKey.php @@ -101,6 +101,7 @@ public static function importPem(string $pem, ?ProtocolInterface $protocol = nul { $formattedKey = str_replace('-----BEGIN EC PRIVATE KEY-----', '', $pem); $formattedKey = str_replace('-----END EC PRIVATE KEY-----', '', $formattedKey); + $formattedKey = str_replace(["\r", "\n"], '', $formattedKey); /** * @psalm-suppress DocblockTypeContradiction diff --git a/src/Util.php b/src/Util.php index 892738c..3d5d4c9 100644 --- a/src/Util.php +++ b/src/Util.php @@ -196,9 +196,13 @@ public static function stopAtDelimiter(string $input, string $delim): string|fal // We want to work with bytes: $bytes = SodiumUtil::stringToIntArray($input); $d = SodiumUtil::stringToIntArray($delim); - $max = $inLength - $dLen; + + // This will be set to -1 once a value has been found: $found = 0; + // The maximum bounds for the loop: + $max = $inLength - $dLen; + // Iterate over all of $bytes, compare window, for ($i = 0; $i < $max; ++$i) { $cmp = 0; From 740e06b3e4d29418172959a8cfbc8388266db2c5 Mon Sep 17 00:00:00 2001 From: Paragon Initiative Enterprises Date: Wed, 29 Jan 2025 06:10:53 -0500 Subject: [PATCH 3/3] Actually, let's just strip newlines --- src/Keys/Version4/AsymmetricPublicKey.php | 11 +--- src/Keys/Version4/AsymmetricSecretKey.php | 10 +--- src/Util.php | 70 +++++++++-------------- tests/UtilTest.php | 26 +++------ 4 files changed, 41 insertions(+), 76 deletions(-) diff --git a/src/Keys/Version4/AsymmetricPublicKey.php b/src/Keys/Version4/AsymmetricPublicKey.php index baf6886..e292726 100644 --- a/src/Keys/Version4/AsymmetricPublicKey.php +++ b/src/Keys/Version4/AsymmetricPublicKey.php @@ -88,8 +88,6 @@ public static function importPem(string $pem, ?ProtocolInterface $protocol = nul { $formattedKey = str_replace('-----BEGIN PUBLIC KEY-----', '', $pem); $formattedKey = str_replace('-----END PUBLIC KEY-----', '', $formattedKey); - $formattedKey = str_replace(["\r", "\n"], '', $formattedKey); - /** * @psalm-suppress DocblockTypeContradiction * PHP 8.4 updated the docblock return for str_replace, which makes this check required @@ -97,13 +95,8 @@ public static function importPem(string $pem, ?ProtocolInterface $protocol = nul if (!is_string($formattedKey)) { throw new PasetoException('Invalid PEM format', ExceptionCode::UNSPECIFIED_CRYPTOGRAPHIC_ERROR); } - - $tokenizedKey = Util::stopAtDelimiter($formattedKey, "\n"); - if ($tokenizedKey === false) { - throw new PasetoException('Invalid PEM format', ExceptionCode::UNSPECIFIED_CRYPTOGRAPHIC_ERROR); - } - - $key = Base64::decode($tokenizedKey); + $formattedKey = Util::stripNewlines($formattedKey); + $key = Base64::decode($formattedKey); $prefix = Hex::decode(self::PEM_ENCODE_PREFIX); return new self(substr($key, strlen($prefix))); diff --git a/src/Keys/Version4/AsymmetricSecretKey.php b/src/Keys/Version4/AsymmetricSecretKey.php index b9bf146..d0d5490 100644 --- a/src/Keys/Version4/AsymmetricSecretKey.php +++ b/src/Keys/Version4/AsymmetricSecretKey.php @@ -101,7 +101,6 @@ public static function importPem(string $pem, ?ProtocolInterface $protocol = nul { $formattedKey = str_replace('-----BEGIN EC PRIVATE KEY-----', '', $pem); $formattedKey = str_replace('-----END EC PRIVATE KEY-----', '', $formattedKey); - $formattedKey = str_replace(["\r", "\n"], '', $formattedKey); /** * @psalm-suppress DocblockTypeContradiction @@ -110,13 +109,8 @@ public static function importPem(string $pem, ?ProtocolInterface $protocol = nul if (!is_string($formattedKey)) { throw new PasetoException('Invalid PEM format', ExceptionCode::UNSPECIFIED_CRYPTOGRAPHIC_ERROR); } - - $tokenizedKey = Util::stopAtDelimiter($formattedKey, "\n"); - if ($tokenizedKey === false) { - throw new PasetoException('Invalid PEM format', ExceptionCode::UNSPECIFIED_CRYPTOGRAPHIC_ERROR); - } - - $key = Base64::decode($tokenizedKey); + $formattedKey = Util::stripNewlines($formattedKey); + $key = Base64::decode($formattedKey); $prefix = Hex::decode(self::PEM_ENCODE_PREFIX); return new self(substr($key, strlen($prefix))); diff --git a/src/Util.php b/src/Util.php index 3d5d4c9..a4e7409 100644 --- a/src/Util.php +++ b/src/Util.php @@ -175,57 +175,43 @@ public static function removeFooter(string $payload): string } /** - * Return a substring of $input that terminates before the first instance of $delim. - * - * Unlike the built-in C function, our algorithm is constant-time. + * Strip all newlines (CR, LF) characters from a string. * * @param string $input - * @param string $delim - * @return string|false + * @return string */ - public static function stopAtDelimiter(string $input, string $delim): string|false + public static function stripNewlines(string $input): string { - $inLength = SodiumUtil::strlen($input); - $dLen = SodiumUtil::strlen($delim); - if ($inLength < $dLen) { - return false; - } - - // Final value for this string - $terminate = 0; - // We want to work with bytes: $bytes = SodiumUtil::stringToIntArray($input); - $d = SodiumUtil::stringToIntArray($delim); + $length = count($bytes); + + // First value is a dummy value, to overwrite it in constant-time + $return = array_fill(0, $length + 1, 0); + // Output index: + $j = 1; - // This will be set to -1 once a value has been found: - $found = 0; + // Now let's strip: + for ($i = 0; $i < $length; ++$i) { + $char = ($bytes[$i]); - // The maximum bounds for the loop: - $max = $inLength - $dLen; + // Determine if we're stripping this character or not? + $isCR = ((($char ^ 0x0d) - 1) >> 8) & 1; + $isLF = ((($char ^ 0x0a) - 1) >> 8) & 1; + $isNewline = $isCR | $isLF; - // Iterate over all of $bytes, compare window, - for ($i = 0; $i < $max; ++$i) { - $cmp = 0; - for ($j = 0; $j < $dLen; ++$j) { - $cmp |= ($bytes[$i + $j] ^ $d[$j]); - } - // 1 if $cmp === 0, 0x00 otherwise - $result = (($cmp - 1) >> 8) & 1; - /* - * $found === 0 - * $terminate = $terminate - * $result === 1, $found === 0 - * $terminate = $i - * $result === 0, $found === 0 - * $terminate = $terminate - */ - $terminate = ($terminate & $found) ^ (~$found & (((-$result) & $i))); - $found |= -$result; + // Set destination index: 0 if $isNewLine, $j otherwise + $swap = -$isNewline; + + // if ($isNewLine), $dest === 0, else $dest === $j + $dest = (~$swap & $j) ^ $swap; + + // Now let's overwrite the index (0 or $j) with $char: + $return[$dest] = $char; + + // We only advance $j if we didn't encounter a newline: + $j += 1 - $isNewline; } - $terminate = ($terminate & $found) ^ (($inLength) & ~$found); - return SodiumUtil::intArrayToString( - array_slice($bytes, 0, $terminate) - ); + return SodiumUtil::intArrayToString(array_slice($return, 1, $j - 1)); } /** diff --git a/tests/UtilTest.php b/tests/UtilTest.php index b8d8a58..1a926af 100644 --- a/tests/UtilTest.php +++ b/tests/UtilTest.php @@ -124,29 +124,21 @@ public function testPreAuthEncode() ); } - public static function strtokProvider(): array + public static function newlineProvider(): array { - $r = Base64UrlSafe::encodeUnpadded(random_bytes(32)); return [ - ['Paragon Initiative Enterprises', ' ', 'Paragon'], - ['Initiative Enterprises', ' ', 'Initiative'], - ['Enterprises', ' ', 'Enterprises'], - ['', ' ', false], - [ - $r . "\n" . Base64UrlSafe::encodeUnpadded(random_bytes(16)), - "\n", - $r - ], + ["abc\ndef", 'abcdef'], + ["abc\r\ndef", 'abcdef'], + ["abc \r\ndef", 'abc def'], + ["\n", ''], + ["\r\n", ''], ]; } - #[DataProvider('strtokProvider')] - public function testStopAtDelimiter(string $input, string $delim, string|false $output): void + #[DataProvider('newlineProvider')] + public function testStripNewliens(string $input, string $output): void { - $result = Util::stopAtDelimiter($input, $delim); - $this->assertSame($output, $result, 'Expected result was not returned'); - $built_in = strtok($input, $delim); - $this->assertSame($built_in, $result, 'Regression with built-in strtok() behavior'); + $this->assertSame($output, Util::stripNewlines($input)); } /**