Skip to content
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.

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

Constant-time alternative to strtok() #176

Merged
merged 3 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 3 additions & 9 deletions src/Keys/Version4/AsymmetricPublicKey.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -88,21 +88,15 @@ public static function importPem(string $pem, ?ProtocolInterface $protocol = nul
{
$formattedKey = str_replace('-----BEGIN PUBLIC KEY-----', '', $pem);
$formattedKey = str_replace('-----END PUBLIC KEY-----', '', $formattedKey);

/**
* @psalm-suppress DocblockTypeContradiction
* PHP 8.4 updated the docblock return for str_replace, which makes this check required
*/
if (!is_string($formattedKey)) {
throw new PasetoException('Invalid PEM format', ExceptionCode::UNSPECIFIED_CRYPTOGRAPHIC_ERROR);
}

$tokenizedKey = strtok($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)));
Expand Down
11 changes: 3 additions & 8 deletions src/Keys/Version4/AsymmetricSecretKey.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -109,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 = strtok($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)));
Expand Down
41 changes: 41 additions & 0 deletions src/Util.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
preg_match_all,
sodium_memzero,
str_replace;
use ParagonIE_Sodium_Core_Util as SodiumUtil;

/**
* Class Util
Expand Down Expand Up @@ -173,6 +174,46 @@ public static function removeFooter(string $payload): string
return $payload;
}

/**
* Strip all newlines (CR, LF) characters from a string.
*
* @param string $input
* @return string
*/
public static function stripNewlines(string $input): string
{
$bytes = SodiumUtil::stringToIntArray($input);
$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;

// Now let's strip:
for ($i = 0; $i < $length; ++$i) {
$char = ($bytes[$i]);

// Determine if we're stripping this character or not?
$isCR = ((($char ^ 0x0d) - 1) >> 8) & 1;
$isLF = ((($char ^ 0x0a) - 1) >> 8) & 1;
$isNewline = $isCR | $isLF;

// 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;
paragonie-security marked this conversation as resolved.
Show resolved Hide resolved

// We only advance $j if we didn't encounter a newline:
$j += 1 - $isNewline;
}
return SodiumUtil::intArrayToString(array_slice($return, 1, $j - 1));
}

/**
* If a footer was included with the message, first verify that
* it's equivalent to the one we expect, then remove it from the
Expand Down
18 changes: 18 additions & 0 deletions tests/UtilTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
};
use ParagonIE\Paseto\Exception\EncodingException;
use ParagonIE\Paseto\Util;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;
use TypeError;

Expand Down Expand Up @@ -123,6 +124,23 @@ public function testPreAuthEncode()
);
}

public static function newlineProvider(): array
{
return [
["abc\ndef", 'abcdef'],
["abc\r\ndef", 'abcdef'],
["abc \r\ndef", 'abc def'],
["\n", ''],
["\r\n", ''],
];
}

#[DataProvider('newlineProvider')]
public function testStripNewliens(string $input, string $output): void
{
$this->assertSame($output, Util::stripNewlines($input));
}

/**
* @covers Util::validateAndRemoveFooter()
*/
Expand Down