Skip to content

Commit

Permalink
Update test and codebase for signature verification and improvements (#…
Browse files Browse the repository at this point in the history
…84)

The patch introduces signature verification to test cases with RSA, ECDSA and EdDSA algorithms. It also refactors code by replacing 'mb_strlen', 'mb_substr' with 'strlen', 'substr' for better performance. Minor changes include renaming variables for better clarity and consistency. Several pieces of code have been modified to use more efficient methods, and some unused portions have been removed. Commit also includes a fix for configuration issue in 'deptrac.yaml'.
  • Loading branch information
Spomky authored Jul 18, 2024
1 parent c2802a4 commit 2166016
Show file tree
Hide file tree
Showing 11 changed files with 104 additions and 96 deletions.
9 changes: 4 additions & 5 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
"php": ">=8.1",
"ext-json": "*",
"ext-openssl": "*",
"ext-mbstring": "*",
"brick/math": "^0.9|^0.10|^0.11|^0.12",
"spomky-labs/pki-framework": "^1.0"
},
Expand All @@ -29,18 +28,18 @@
}
},
"require-dev": {
"infection/infection": "^0.27",
"infection/infection": "^0.29",
"phpstan/phpstan": "^1.7",
"phpstan/phpstan-deprecation-rules": "^1.0",
"phpstan/phpstan-phpunit": "^1.1",
"phpstan/phpstan-strict-rules": "^1.2",
"phpunit/phpunit": "^10.1",
"rector/rector": "^0.19",
"phpunit/phpunit": "^10.1|^11.0",
"rector/rector": "^1.0",
"symplify/easy-coding-standard": "^12.0",
"symfony/phpunit-bridge": "^6.4|^7.0",
"ekino/phpstan-banned-code": "^1.0",
"php-parallel-lint/php-parallel-lint": "^1.3",
"qossmic/deptrac-shim": "^1.0",
"qossmic/deptrac": "^2.0",
"phpstan/extension-installer": "^1.3"
},
"autoload-dev": {
Expand Down
10 changes: 5 additions & 5 deletions deptrac.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ parameters:
layers:
- name: 'CoseLib'
collectors:
- type: 'className'
regex: '^Cose\\'
- type: 'classLike'
value: '^Cose\\'
- name: 'Vendors'
collectors:
- { type: className, regex: '^CBOR\\' }
- { type: className, regex: '^Brick\\' }
- { type: className, regex: '^SpomkyLabs\\Pki\\' }
- { type: 'classLike', value: '^CBOR\\' }
- { type: 'classLike', value: '^Brick\\' }
- { type: 'classLike', value: '^SpomkyLabs\\Pki\\' }
ruleset:
CoseLib:
- Vendors
4 changes: 0 additions & 4 deletions ecs.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

declare(strict_types=1);

use PhpCsFixer\Fixer\Alias\MbStrFunctionsFixer;
use PhpCsFixer\Fixer\ArrayNotation\ArraySyntaxFixer;
use PhpCsFixer\Fixer\ClassNotation\ProtectedToPrivateFixer;
use PhpCsFixer\Fixer\Comment\HeaderCommentFixer;
Expand All @@ -25,7 +24,6 @@
use PhpCsFixer\Fixer\Strict\StrictComparisonFixer;
use PhpCsFixer\Fixer\Strict\StrictParamFixer;
use PhpCsFixer\Fixer\Whitespace\ArrayIndentationFixer;
use PhpCsFixer\Fixer\Whitespace\CompactNullableTypehintFixer;
use PhpCsFixer\Fixer\Whitespace\MethodChainingIndentationFixer;
use Symplify\EasyCodingStandard\Config\ECSConfig;
use Symplify\EasyCodingStandard\ValueObject\Set\SetList;
Expand Down Expand Up @@ -53,11 +51,9 @@
$config->rule(ProtectedToPrivateFixer::class);
$config->rule(DeclareStrictTypesFixer::class);
$config->rule(NativeConstantInvocationFixer::class);
$config->rule(MbStrFunctionsFixer::class);
$config->rule(LinebreakAfterOpeningTagFixer::class);
$config->rule(CombineConsecutiveIssetsFixer::class);
$config->rule(CombineConsecutiveUnsetsFixer::class);
$config->rule(CompactNullableTypehintFixer::class);
$config->rule(NoSuperfluousElseifFixer::class);
$config->rule(NoSuperfluousPhpdocTagsFixer::class);
$config->rule(PhpdocTrimConsecutiveBlankLineSeparationFixer::class);
Expand Down
2 changes: 1 addition & 1 deletion src/Algorithm/Mac/Hmac.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public function hash(string $data, Key $key): string
$this->checKey($key);
$signature = hash_hmac($this->getHashAlgorithm(), $data, (string) $key->get(SymmetricKey::DATA_K), true);

Check warning on line 19 in src/Algorithm/Mac/Hmac.php

View workflow job for this annotation

GitHub Actions / 5️⃣ Mutation Testing

Escaped Mutant for Mutator "CastString": @@ @@ public function hash(string $data, Key $key): string { $this->checKey($key); - $signature = hash_hmac($this->getHashAlgorithm(), $data, (string) $key->get(SymmetricKey::DATA_K), true); + $signature = hash_hmac($this->getHashAlgorithm(), $data, $key->get(SymmetricKey::DATA_K), true); return substr($signature, 0, intdiv($this->getSignatureLength(), 8)); } public function verify(string $data, Key $key, string $signature): bool

return mb_substr($signature, 0, intdiv($this->getSignatureLength(), 8), '8bit');
return substr($signature, 0, intdiv($this->getSignatureLength(), 8));
}

public function verify(string $data, Key $key, string $signature): bool
Expand Down
26 changes: 13 additions & 13 deletions src/Algorithm/Signature/ECDSA/ECSignature.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
use function dechex;
use function hex2bin;
use function hexdec;
use function mb_strlen;
use function mb_substr;
use function str_pad;
use function strlen;
use function substr;
use const STR_PAD_LEFT;

/**
Expand Down Expand Up @@ -41,8 +41,8 @@ public static function toAsn1(string $signature, int $length): string
throw new InvalidArgumentException('Invalid signature length.');
}

$pointR = self::preparePositiveInteger(mb_substr($signature, 0, $length, '8bit'));
$pointS = self::preparePositiveInteger(mb_substr($signature, $length, null, '8bit'));
$pointR = self::preparePositiveInteger(substr($signature, 0, $length));
$pointS = self::preparePositiveInteger(substr($signature, $length, null));

$lengthR = self::octetLength($pointR);
$lengthS = self::octetLength($pointS);
Expand Down Expand Up @@ -80,28 +80,28 @@ public static function fromAsn1(string $signature, int $length): string

private static function octetLength(string $data): int
{
return intdiv(mb_strlen($data, '8bit'), self::BYTE_SIZE);
return intdiv(strlen($data), self::BYTE_SIZE);
}

private static function preparePositiveInteger(string $data): string
{
if (mb_substr($data, 0, self::BYTE_SIZE, '8bit') > self::ASN1_BIG_INTEGER_LIMIT) {
if (substr($data, 0, self::BYTE_SIZE) > self::ASN1_BIG_INTEGER_LIMIT) {

Check warning on line 88 in src/Algorithm/Signature/ECDSA/ECSignature.php

View workflow job for this annotation

GitHub Actions / 5️⃣ Mutation Testing

Escaped Mutant for Mutator "UnwrapSubstr": @@ @@ } private static function preparePositiveInteger(string $data): string { - if (substr($data, 0, self::BYTE_SIZE) > self::ASN1_BIG_INTEGER_LIMIT) { + if ($data > self::ASN1_BIG_INTEGER_LIMIT) { return self::ASN1_NEGATIVE_INTEGER . $data; } while (str_starts_with($data, self::ASN1_NEGATIVE_INTEGER) && substr($data, 2, self::BYTE_SIZE) <= self::ASN1_BIG_INTEGER_LIMIT) {
return self::ASN1_NEGATIVE_INTEGER . $data;
}

while (
mb_strpos($data, self::ASN1_NEGATIVE_INTEGER, 0, '8bit') === 0
&& mb_substr($data, 2, self::BYTE_SIZE, '8bit') <= self::ASN1_BIG_INTEGER_LIMIT
str_starts_with($data, self::ASN1_NEGATIVE_INTEGER)
&& substr($data, 2, self::BYTE_SIZE) <= self::ASN1_BIG_INTEGER_LIMIT

Check warning on line 94 in src/Algorithm/Signature/ECDSA/ECSignature.php

View workflow job for this annotation

GitHub Actions / 5️⃣ Mutation Testing

Escaped Mutant for Mutator "LessThanOrEqualTo": @@ @@ if (substr($data, 0, self::BYTE_SIZE) > self::ASN1_BIG_INTEGER_LIMIT) { return self::ASN1_NEGATIVE_INTEGER . $data; } - while (str_starts_with($data, self::ASN1_NEGATIVE_INTEGER) && substr($data, 2, self::BYTE_SIZE) <= self::ASN1_BIG_INTEGER_LIMIT) { + while (str_starts_with($data, self::ASN1_NEGATIVE_INTEGER) && substr($data, 2, self::BYTE_SIZE) < self::ASN1_BIG_INTEGER_LIMIT) { $data = substr($data, 2, null); } return $data;
) {
$data = mb_substr($data, 2, null, '8bit');
$data = substr($data, 2, null);
}

return $data;
}

private static function readAsn1Content(string $message, int &$position, int $length): string
{
$content = mb_substr($message, $position, $length, '8bit');
$content = substr($message, $position, $length);
$position += $length;

return $content;
Expand All @@ -121,10 +121,10 @@ private static function readAsn1Integer(string $message, int &$position): string
private static function retrievePositiveInteger(string $data): string
{
while (
mb_strpos($data, self::ASN1_NEGATIVE_INTEGER, 0, '8bit') === 0
&& mb_substr($data, 2, self::BYTE_SIZE, '8bit') > self::ASN1_BIG_INTEGER_LIMIT
str_starts_with($data, self::ASN1_NEGATIVE_INTEGER)
&& substr($data, 2, self::BYTE_SIZE) > self::ASN1_BIG_INTEGER_LIMIT

Check warning on line 125 in src/Algorithm/Signature/ECDSA/ECSignature.php

View workflow job for this annotation

GitHub Actions / 5️⃣ Mutation Testing

Escaped Mutant for Mutator "GreaterThan": @@ @@ } private static function retrievePositiveInteger(string $data): string { - while (str_starts_with($data, self::ASN1_NEGATIVE_INTEGER) && substr($data, 2, self::BYTE_SIZE) > self::ASN1_BIG_INTEGER_LIMIT) { + while (str_starts_with($data, self::ASN1_NEGATIVE_INTEGER) && substr($data, 2, self::BYTE_SIZE) >= self::ASN1_BIG_INTEGER_LIMIT) { $data = substr($data, 2, null); } return $data; } }
) {
$data = mb_substr($data, 2, null, '8bit');
$data = substr($data, 2, null);
}

return $data;
Expand Down
23 changes: 11 additions & 12 deletions src/Algorithm/Signature/RSA/PSSRSA.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,12 @@
use function ceil;
use function chr;
use function hash_equals;
use function mb_strlen;
use function mb_substr;
use function ord;
use function pack;
use function random_bytes;
use function str_pad;
use function str_repeat;
use function strlen;
use const STR_PAD_LEFT;

/**
Expand All @@ -31,7 +30,7 @@ abstract class PSSRSA implements Signature
public function sign(string $data, Key $key): string
{
$key = $this->handleKey($key);
$modulusLength = mb_strlen($key->n(), '8bit');
$modulusLength = strlen($key->n());

$em = $this->encodeEMSAPSS($data, 8 * $modulusLength - 1, $this->getHashAlgorithm());
$message = BigInteger::createFromBinaryString($em);
Expand All @@ -43,8 +42,8 @@ public function sign(string $data, Key $key): string
public function verify(string $data, Key $key, string $signature): bool
{
$key = $this->handleKey($key);
$modulusLength = mb_strlen($key->n(), '8bit');
if (mb_strlen($signature, '8bit') !== $modulusLength) {
$modulusLength = strlen($key->n());
if (strlen($signature) !== $modulusLength) {
throw new InvalidArgumentException('Invalid modulus length');
}
$s2 = BigInteger::createFromBinaryString($signature);
Expand Down Expand Up @@ -99,7 +98,7 @@ private function handleKey(Key $key): RsaKey
private function convertIntegerToOctetString(BigInteger $x, int $xLen): string
{
$xB = $x->toBytes();
if (mb_strlen($xB, '8bit') > $xLen) {
if (strlen($xB) > $xLen) {
throw new RuntimeException('Unable to convert the integer');
}

Expand All @@ -118,7 +117,7 @@ private function getMGF1(string $mgfSeed, int $maskLen, Hash $mgfHash): string
$t .= $mgfHash->hash($mgfSeed . $c);
}

return mb_substr($t, 0, $maskLen, '8bit');
return substr($t, 0, $maskLen);
}

/**
Expand Down Expand Up @@ -155,11 +154,11 @@ private function verifyEMSAPSS(string $m, string $em, int $emBits, Hash $hash):
if ($emLen < $hash->getLength() + $sLen + 2) {
throw new InvalidArgumentException();
}
if ($em[mb_strlen($em, '8bit') - 1] !== chr(0xBC)) {
if ($em[strlen($em) - 1] !== chr(0xBC)) {
throw new InvalidArgumentException();
}
$maskedDB = mb_substr($em, 0, -$hash->getLength() - 1, '8bit');
$h = mb_substr($em, -$hash->getLength() - 1, $hash->getLength(), '8bit');
$maskedDB = substr($em, 0, -$hash->getLength() - 1);
$h = substr($em, -$hash->getLength() - 1, $hash->getLength());
$temp = chr(0xFF << ($emBits & 7));
if ((~$maskedDB[0] & $temp) !== $temp) {
throw new InvalidArgumentException();
Expand All @@ -168,13 +167,13 @@ private function verifyEMSAPSS(string $m, string $em, int $emBits, Hash $hash):
$db = $maskedDB ^ $dbMask;
$db[0] = ~chr(0xFF << ($emBits & 7)) & $db[0];
$temp = $emLen - $hash->getLength() - $sLen - 2;
if (mb_strpos($db, str_repeat(chr(0), $temp), 0, '8bit') !== 0) {
if (! str_starts_with($db, str_repeat(chr(0), $temp))) {
throw new InvalidArgumentException();
}
if (ord($db[$temp]) !== 1) {
throw new InvalidArgumentException();
}
$salt = mb_substr($db, $temp + 1, null, '8bit'); // should be $sLen long
$salt = substr($db, $temp + 1, null); // should be $sLen long
$m2 = "\0\0\0\0\0\0\0\0" . $mHash . $salt;
$h2 = $hash->hash($m2);

Expand Down
3 changes: 2 additions & 1 deletion src/BigInteger.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Brick\Math\BigInteger as BrickBigInteger;
use function chr;
use function hex2bin;
use function strlen;
use function unpack;

/**
Expand Down Expand Up @@ -42,7 +43,7 @@ public function toBytes(): string
}

$temp = $this->value->toBase(16);
$temp = 0 !== (mb_strlen($temp, '8bit') & 1) ? '0' . $temp : $temp;
$temp = 0 !== (strlen($temp) & 1) ? '0' . $temp : $temp;
$temp = hex2bin($temp);

return ltrim($temp, chr(0));
Expand Down
9 changes: 5 additions & 4 deletions src/Key/Ec2Key.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use function array_key_exists;
use function in_array;
use function is_int;
use function strlen;

/**
* @final
Expand Down Expand Up @@ -93,10 +94,10 @@ public function __construct(array $data)
if (! isset($data[self::DATA_CURVE], $data[self::DATA_X], $data[self::DATA_Y])) {
throw new InvalidArgumentException('Invalid EC2 key. The curve or the "x/y" coordinates are missing');
}
if (mb_strlen((string) $data[self::DATA_X], '8bit') !== self::CURVE_KEY_LENGTH[$data[self::DATA_CURVE]]) {
if (strlen((string) $data[self::DATA_X]) !== self::CURVE_KEY_LENGTH[$data[self::DATA_CURVE]]) {
throw new InvalidArgumentException('Invalid length for x coordinate');
}
if (mb_strlen((string) $data[self::DATA_Y], '8bit') !== self::CURVE_KEY_LENGTH[$data[self::DATA_CURVE]]) {
if (strlen((string) $data[self::DATA_Y]) !== self::CURVE_KEY_LENGTH[$data[self::DATA_CURVE]]) {
throw new InvalidArgumentException('Invalid length for y coordinate');
}
if (is_int($data[self::DATA_CURVE])) {
Expand Down Expand Up @@ -188,8 +189,8 @@ private function getCurveOid(): string

private function pem(string $type, string $der): string
{
return sprintf("-----BEGIN %s-----\n", mb_strtoupper($type)) .
return sprintf("-----BEGIN %s-----\n", strtoupper($type)) .
chunk_split(base64_encode($der), 64, "\n") .
sprintf("-----END %s-----\n", mb_strtoupper($type));
sprintf("-----END %s-----\n", strtoupper($type));
}
}
Loading

0 comments on commit 2166016

Please sign in to comment.