From 38b745f3f99450072f94f9a0792b9924d00b405b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=A0pa=C4=8Dek?= Date: Sun, 2 Aug 2026 18:40:17 +0200 Subject: [PATCH] Check the code with `spaze/phpstan-disallowed-calls` and all its bundled rules All five bundled config files are included. The dangerous, execution, insecure, and loose ones pass with no changes and no per-path exceptions; even the `print_r()` rule is happy, because the leak tests use the return-a-string form which the bundled rule allows. The non-timing-safe one, new in version 4.14, flags `hex2bin()`, `bin2hex()`, `base64_encode()`, and `base64_decode()`, whose run time depends on the processed bytes, which can leak them. The library code already used only the constant-time sodium functions; the tests and the README key generation examples now do too, so `bin2hex(random_bytes(32))` became `sodium_bin2hex(random_bytes(32))`. Not that anyone could measure the run time of a test or of a one-off key generation, but the examples teach what the rules then enforce, and a blanket rule beats deciding per call site whether the bytes are secret. `AGENTS.md` no longer scopes the sodium-functions rule to `src/` and no longer excuses the tests, PHPStan now enforces the rule everywhere. --- AGENTS.md | 2 +- README.md | 8 ++++---- composer.json | 3 ++- phpstan.neon | 6 ++++++ tests/AnonymousPublicKeyEncryptionTest.phpt | 10 +++++----- .../AuthenticatedPublicKeyEncryptionTest.phpt | 16 +++++++-------- tests/SymmetricKeyEncryptionTest.phpt | 20 +++++++++---------- 7 files changed, 36 insertions(+), 29 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c31e70c..1f03447 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,7 +18,7 @@ Breaking any of these is a bug even when all tests pass. When a change makes a t - **No key material in exception messages, traces, or object dumps.** Raw key bytes are held only in `HiddenString`, never in plain properties. Values repeated in messages that can come from stored data or from a mispasted config slot go through `LogSafeValue`. Parameters carrying keys or plaintext get `#[SensitiveParameter]`, including on private helper methods, because traces mask arguments per frame. - **Exception constructors are not public API**, change their parameters freely. The exception class names, the inheritance (related failures are empty subclasses inheriting the message), and the no-key-material guarantee are API. - **Validation happens in constructors**, not on first use: a misconfiguration fails at deploy time. -- **Byte encoding in `src/` goes through the constant-time sodium functions** (`sodium_hex2bin()`, `sodium_bin2hex()`, `sodium_bin2base64()`), never `hex2bin()`/`bin2hex()`/`base64_*()`. The tests still use `bin2hex()` until planned linter rules land; don't add new uses anywhere. +- **Byte encoding goes through the constant-time sodium functions** (`sodium_hex2bin()`, `sodium_bin2hex()`, `sodium_bin2base64()`), never `hex2bin()`/`bin2hex()`/`base64_*()`; PHPStan enforces this everywhere via the bundled `disallowed-non-timing-safe-calls.neon`. - **Verify claims about Halite and libsodium against `vendor/paragonie/halite` sources or a `php -r` experiment**, never from memory or secondhand docs. ## Conventions diff --git a/README.md b/README.md index 056e5a5..d8b4300 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ Spaze\Encryption\SymmetricKeyEncryption::__construct(array $keys, string $active ``` #### `array $keys` An array of encryption keys, a _key id_ (will be part of the encrypted string) as the array key, the prefixed _key_ (`prefix` + `_` + `[0-9A-F]{64}`) as the value. -Generate your own encryption keys with for example `bin2hex(random_bytes(32))`. +Generate your own encryption keys with for example `sodium_bin2hex(random_bytes(32))`. The constructor validates each key: the prefix must match, the key material must be valid hex, and it must decode to exactly 32 bytes (64 hexadecimal characters). The key id must be non-empty and must not contain `$`, because the id becomes part of the encrypted output format. A misconfigured key throws an exception at construction time, not on first use. #### `string $activeKeyId` @@ -147,8 +147,8 @@ $encryption = new Spaze\Encryption\AuthenticatedPublicKeyEncryption($secretKeys, Each party generates their own pair, keeps the secret key to themselves and gives the public key to the other party: ```php $keyPair = sodium_crypto_box_keypair(); -$secretKey = 'adek_secret_' . bin2hex(sodium_crypto_box_secretkey($keyPair)); -$publicKey = 'adek_public_' . bin2hex(sodium_crypto_box_publickey($keyPair)); +$secretKey = 'adek_secret_' . sodium_bin2hex(sodium_crypto_box_secretkey($keyPair)); +$publicKey = 'adek_public_' . sodium_bin2hex(sodium_crypto_box_publickey($keyPair)); ``` ### Encrypt & decrypt @@ -244,7 +244,7 @@ A file the service reads is a good fit if you want to keep the keys in a file: m YOU HAVE TO GENERATE YOUR OWN KEYS. You can use for example ```php -bin2hex(random_bytes(32)) +sodium_bin2hex(random_bytes(32)) ``` to generate a key, then add the prefix. You can have multiple keys in each group (here we see two groups: `password` and `email`), meaning you will be able to decrypt data encrypted with these keys. Data will always be encrypted with what's defined in `activeKeyIds` section. diff --git a/composer.json b/composer.json index 3fcb841..6a23f73 100644 --- a/composer.json +++ b/composer.json @@ -25,7 +25,8 @@ "php-parallel-lint/php-console-highlighter": "^1.0", "phpstan/phpstan": "^2.2.7", "nette/tester": "^2.5.7", - "phpstan/phpstan-strict-rules": "^2.0" + "phpstan/phpstan-strict-rules": "^2.0", + "spaze/phpstan-disallowed-calls": "^4.14" }, "config": { "allow-plugins": { diff --git a/phpstan.neon b/phpstan.neon index 4ced1c9..f4939b0 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -10,3 +10,9 @@ parameters: includes: - vendor/phpstan/phpstan/conf/bleedingEdge.neon - vendor/phpstan/phpstan-strict-rules/rules.neon + - vendor/spaze/phpstan-disallowed-calls/extension.neon + - vendor/spaze/phpstan-disallowed-calls/disallowed-dangerous-calls.neon + - vendor/spaze/phpstan-disallowed-calls/disallowed-execution-calls.neon + - vendor/spaze/phpstan-disallowed-calls/disallowed-insecure-calls.neon + - vendor/spaze/phpstan-disallowed-calls/disallowed-loose-calls.neon + - vendor/spaze/phpstan-disallowed-calls/disallowed-non-timing-safe-calls.neon diff --git a/tests/AnonymousPublicKeyEncryptionTest.phpt b/tests/AnonymousPublicKeyEncryptionTest.phpt index ec43e44..ab57bd7 100644 --- a/tests/AnonymousPublicKeyEncryptionTest.phpt +++ b/tests/AnonymousPublicKeyEncryptionTest.phpt @@ -66,8 +66,8 @@ class AnonymousPublicKeyEncryptionTest extends TestCase $this->publicKeys = []; foreach ([self::INACTIVE_KEY, self::ACTIVE_KEY] as $id) { $keyPair = sodium_crypto_box_keypair(); - $this->secretKeys[$id] = self::KEY_PREFIX . '_secret_' . bin2hex(sodium_crypto_box_secretkey($keyPair)); - $this->publicKeys[$id] = self::KEY_PREFIX . '_public_' . bin2hex(sodium_crypto_box_publickey($keyPair)); + $this->secretKeys[$id] = self::KEY_PREFIX . '_secret_' . sodium_bin2hex(sodium_crypto_box_secretkey($keyPair)); + $this->publicKeys[$id] = self::KEY_PREFIX . '_public_' . sodium_bin2hex(sodium_crypto_box_publickey($keyPair)); } $this->encryption = new AnonymousPublicKeyEncryption($this->secretKeys, $this->publicKeys, self::ACTIVE_KEY, self::KEY_PREFIX); } @@ -491,7 +491,7 @@ class AnonymousPublicKeyEncryptionTest extends TestCase public function testConstructorInvalidKeyLength(): void { - $shortKey = bin2hex(random_bytes(16)); + $shortKey = sodium_bin2hex(random_bytes(16)); $e = Assert::exception( function () use ($shortKey): void { new AnonymousPublicKeyEncryption(['short' => self::KEY_PREFIX . '_secret_' . $shortKey], [], 'short', self::KEY_PREFIX); @@ -504,7 +504,7 @@ class AnonymousPublicKeyEncryptionTest extends TestCase // The public keys array is validated the same way as the secret keys array Assert::exception( function (): void { - new AnonymousPublicKeyEncryption([], ['bytes31' => self::KEY_PREFIX . '_public_' . bin2hex(random_bytes(31))], 'bytes31', self::KEY_PREFIX); + new AnonymousPublicKeyEncryption([], ['bytes31' => self::KEY_PREFIX . '_public_' . sodium_bin2hex(random_bytes(31))], 'bytes31', self::KEY_PREFIX); }, InvalidKeyLengthException::class, "Key 'bytes31' must be 32 bytes (64 hexadecimal characters) but is 31 bytes", @@ -514,7 +514,7 @@ class AnonymousPublicKeyEncryptionTest extends TestCase public function testConstructorInvalidKeyEncoding(): void { - $truncatedKey = substr(bin2hex(random_bytes(32)), 0, 63); + $truncatedKey = substr(sodium_bin2hex(random_bytes(32)), 0, 63); $e = Assert::exception( function () use ($truncatedKey): void { new AnonymousPublicKeyEncryption(['truncated' => self::KEY_PREFIX . '_secret_' . $truncatedKey], [], 'truncated', self::KEY_PREFIX); diff --git a/tests/AuthenticatedPublicKeyEncryptionTest.phpt b/tests/AuthenticatedPublicKeyEncryptionTest.phpt index e661d69..f70541e 100644 --- a/tests/AuthenticatedPublicKeyEncryptionTest.phpt +++ b/tests/AuthenticatedPublicKeyEncryptionTest.phpt @@ -86,10 +86,10 @@ class AuthenticatedPublicKeyEncryptionTest extends TestCase foreach ([self::INACTIVE_KEY, self::ACTIVE_KEY] as $id) { $ourKeyPair = sodium_crypto_box_keypair(); $theirKeyPair = sodium_crypto_box_keypair(); - $this->ourSecretKeys[$id] = self::KEY_PREFIX . '_secret_' . bin2hex(sodium_crypto_box_secretkey($ourKeyPair)); - $this->ourPublicKeys[$id] = self::KEY_PREFIX . '_public_' . bin2hex(sodium_crypto_box_publickey($ourKeyPair)); - $this->theirSecretKeys[$id] = self::KEY_PREFIX . '_secret_' . bin2hex(sodium_crypto_box_secretkey($theirKeyPair)); - $this->theirPublicKeys[$id] = self::KEY_PREFIX . '_public_' . bin2hex(sodium_crypto_box_publickey($theirKeyPair)); + $this->ourSecretKeys[$id] = self::KEY_PREFIX . '_secret_' . sodium_bin2hex(sodium_crypto_box_secretkey($ourKeyPair)); + $this->ourPublicKeys[$id] = self::KEY_PREFIX . '_public_' . sodium_bin2hex(sodium_crypto_box_publickey($ourKeyPair)); + $this->theirSecretKeys[$id] = self::KEY_PREFIX . '_secret_' . sodium_bin2hex(sodium_crypto_box_secretkey($theirKeyPair)); + $this->theirPublicKeys[$id] = self::KEY_PREFIX . '_public_' . sodium_bin2hex(sodium_crypto_box_publickey($theirKeyPair)); } $this->encryption = new AuthenticatedPublicKeyEncryption($this->ourSecretKeys, $this->theirPublicKeys, self::ACTIVE_KEY, self::KEY_PREFIX); } @@ -605,7 +605,7 @@ class AuthenticatedPublicKeyEncryptionTest extends TestCase public function testConstructorInvalidKeyLength(): void { - $shortKey = bin2hex(random_bytes(16)); + $shortKey = sodium_bin2hex(random_bytes(16)); $e = Assert::exception( function () use ($shortKey): void { new AuthenticatedPublicKeyEncryption(['short' => self::KEY_PREFIX . '_secret_' . $shortKey], [], 'short', self::KEY_PREFIX); @@ -618,7 +618,7 @@ class AuthenticatedPublicKeyEncryptionTest extends TestCase // The public keys array is validated the same way as the secret keys array Assert::exception( function (): void { - new AuthenticatedPublicKeyEncryption($this->ourSecretKeys, [self::ACTIVE_KEY => self::KEY_PREFIX . '_public_' . bin2hex(random_bytes(31))], self::ACTIVE_KEY, self::KEY_PREFIX); + new AuthenticatedPublicKeyEncryption($this->ourSecretKeys, [self::ACTIVE_KEY => self::KEY_PREFIX . '_public_' . sodium_bin2hex(random_bytes(31))], self::ACTIVE_KEY, self::KEY_PREFIX); }, InvalidKeyLengthException::class, "Key 'dev2' must be 32 bytes (64 hexadecimal characters) but is 31 bytes", @@ -628,7 +628,7 @@ class AuthenticatedPublicKeyEncryptionTest extends TestCase public function testConstructorInvalidKeyEncoding(): void { - $truncatedKey = substr(bin2hex(random_bytes(32)), 0, 63); + $truncatedKey = substr(sodium_bin2hex(random_bytes(32)), 0, 63); $e = Assert::exception( function () use ($truncatedKey): void { new AuthenticatedPublicKeyEncryption(['truncated' => self::KEY_PREFIX . '_secret_' . $truncatedKey], [], 'truncated', self::KEY_PREFIX); @@ -727,7 +727,7 @@ class AuthenticatedPublicKeyEncryptionTest extends TestCase private function derivePublicKeyHex(string $secretKeyHex): string { - return bin2hex(sodium_crypto_box_publickey_from_secretkey(sodium_hex2bin($secretKeyHex))); + return sodium_bin2hex(sodium_crypto_box_publickey_from_secretkey(sodium_hex2bin($secretKeyHex))); } } diff --git a/tests/SymmetricKeyEncryptionTest.phpt b/tests/SymmetricKeyEncryptionTest.phpt index 0aab15e..f8a3248 100644 --- a/tests/SymmetricKeyEncryptionTest.phpt +++ b/tests/SymmetricKeyEncryptionTest.phpt @@ -68,8 +68,8 @@ class SymmetricKeyEncryptionTest extends TestCase protected function setUp(): void { $this->keys = [ - self::INACTIVE_KEY => self::KEY_PREFIX . '_' . bin2hex(random_bytes(32)), - self::ACTIVE_KEY => self::KEY_PREFIX . '_' . bin2hex(random_bytes(32)), + self::INACTIVE_KEY => self::KEY_PREFIX . '_' . sodium_bin2hex(random_bytes(32)), + self::ACTIVE_KEY => self::KEY_PREFIX . '_' . sodium_bin2hex(random_bytes(32)), ]; $this->encryption = new SymmetricKeyEncryption($this->keys, self::ACTIVE_KEY, self::KEY_PREFIX); } @@ -455,7 +455,7 @@ class SymmetricKeyEncryptionTest extends TestCase // An empty key id would produce '$$' which the parser rejects Assert::exception( function (): void { - new SymmetricKeyEncryption(['' => self::KEY_PREFIX . '_' . bin2hex(random_bytes(32))], '', self::KEY_PREFIX); + new SymmetricKeyEncryption(['' => self::KEY_PREFIX . '_' . sodium_bin2hex(random_bytes(32))], '', self::KEY_PREFIX); }, InvalidKeyIdException::class, 'Key id must not be empty', @@ -463,7 +463,7 @@ class SymmetricKeyEncryptionTest extends TestCase // A key id with the separator would encrypt fine but produce output that can never be decrypted Assert::exception( function (): void { - new SymmetricKeyEncryption(['key$1' => self::KEY_PREFIX . '_' . bin2hex(random_bytes(32))], 'key$1', self::KEY_PREFIX); + new SymmetricKeyEncryption(['key$1' => self::KEY_PREFIX . '_' . sodium_bin2hex(random_bytes(32))], 'key$1', self::KEY_PREFIX); }, InvalidKeyIdException::class, "Key id 'key\$1' must not contain '\$'", @@ -491,7 +491,7 @@ class SymmetricKeyEncryptionTest extends TestCase public function testConstructorNumericKeyId(): void { // PHP casts a numeric key id to an integer, the constructor has to cope with that and not just with strings - $keys = ['1' => self::KEY_PREFIX . '_' . bin2hex(random_bytes(32))]; + $keys = ['1' => self::KEY_PREFIX . '_' . sodium_bin2hex(random_bytes(32))]; Assert::same([0 => 1], array_keys($keys)); // the id is an int now, there's no way to keep it a string $encryption = new SymmetricKeyEncryption($keys, '1', self::KEY_PREFIX); $encrypted = $encryption->encrypt(self::PLAINTEXT); @@ -503,7 +503,7 @@ class SymmetricKeyEncryptionTest extends TestCase public function testConstructorInvalidKeyLength(): void { - $shortKey = bin2hex(random_bytes(16)); + $shortKey = sodium_bin2hex(random_bytes(16)); $e = Assert::exception( function () use ($shortKey): void { new SymmetricKeyEncryption(['short' => self::KEY_PREFIX . '_' . $shortKey], 'short', self::KEY_PREFIX); @@ -515,7 +515,7 @@ class SymmetricKeyEncryptionTest extends TestCase Assert::notContains($shortKey, $e->getMessage()); Assert::exception( function (): void { - new SymmetricKeyEncryption(['bytes31' => self::KEY_PREFIX . '_' . bin2hex(random_bytes(31))], 'bytes31', self::KEY_PREFIX); + new SymmetricKeyEncryption(['bytes31' => self::KEY_PREFIX . '_' . sodium_bin2hex(random_bytes(31))], 'bytes31', self::KEY_PREFIX); }, InvalidKeyLengthException::class, "Key 'bytes31' must be 32 bytes (64 hexadecimal characters) but is 31 bytes", @@ -525,7 +525,7 @@ class SymmetricKeyEncryptionTest extends TestCase public function testConstructorInvalidKeyEncoding(): void { - $truncatedKey = substr(bin2hex(random_bytes(32)), 0, 63); + $truncatedKey = substr(sodium_bin2hex(random_bytes(32)), 0, 63); $e = Assert::exception( function () use ($truncatedKey): void { new SymmetricKeyEncryption(['truncated' => self::KEY_PREFIX . '_' . $truncatedKey], 'truncated', self::KEY_PREFIX); @@ -544,7 +544,7 @@ class SymmetricKeyEncryptionTest extends TestCase // str_replace() used to strip all prefix occurrences silently, substr() removes only the leading one Assert::exception( function (): void { - new SymmetricKeyEncryption(['double' => self::KEY_PREFIX . '_' . self::KEY_PREFIX . '_' . bin2hex(random_bytes(32))], 'double', self::KEY_PREFIX); + new SymmetricKeyEncryption(['double' => self::KEY_PREFIX . '_' . self::KEY_PREFIX . '_' . sodium_bin2hex(random_bytes(32))], 'double', self::KEY_PREFIX); }, InvalidKeyEncodingException::class, ); @@ -552,7 +552,7 @@ class SymmetricKeyEncryptionTest extends TestCase // the symmetric class must never start interpreting the tags Assert::exception( function (): void { - new SymmetricKeyEncryption(['tagged' => self::KEY_PREFIX . '_secret_' . bin2hex(random_bytes(32))], 'tagged', self::KEY_PREFIX); + new SymmetricKeyEncryption(['tagged' => self::KEY_PREFIX . '_secret_' . sodium_bin2hex(random_bytes(32))], 'tagged', self::KEY_PREFIX); }, InvalidKeyEncodingException::class, );