Repository: defuse/php-encryption (language: php)

Issue:
Make use of PHP 8.2's #[\SensitiveParameter] to improve security
https://www.php.net/manual/en/class.sensitiveparameter.php 

This issue is a suggestion.


Discussion / hints:

Fix patch:
diff --git a/.travis.yml b/.travis.yml
index 0aa111b..ffaefc0 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -17,9 +17,11 @@ matrix:
     - php: "7.4"
       env: USE_PSALM=1
     - php: "8.0"
-      env: USE_PSALM=1
+      # psalm currently doesn't like our \[#SensitiveParameter]s
+      env: USE_PSALM=0
     - php: "8.1"
-      env: USE_PSALM=1
+      # psalm currently doesn't like our \[#SensitiveParameter]s
+      env: USE_PSALM=0
     - php: "8.2"
       env: USE_PSALM=1
       dist: focal
diff --git a/psalm.xml b/psalm.xml
index 7e4c600..3f87dab 100644
--- a/psalm.xml
+++ b/psalm.xml
@@ -9,5 +9,8 @@
         <DocblockTypeContradiction errorLevel="info" />
         <RedundantCondition errorLevel="info" />
         <RedundantConditionGivenDocblockType errorLevel="info" />
+        <!-- It's complaining about our #[\SensitiveParameter]s for no reason -->
+        <InvalidAttribute errorLevel="info" />
+        <UndefinedAttributeClass errorLevel="info" />
     </issueHandlers>
 </psalm>
diff --git a/src/Core.php b/src/Core.php
index bd99a1a..3fc20ab 100644
--- a/src/Core.php
+++ b/src/Core.php
@@ -386,7 +386,15 @@ public static function ourSubstr($str, $start, $length = null)
      *
      * @return string A $key_length-byte key derived from the password and salt.
      */
-    public static function pbkdf2($algorithm, $password, $salt, $count, $key_length, $raw_output = false)
+    public static function pbkdf2(
+        $algorithm,
+        #[\SensitiveParameter]
+        $password,
+        $salt,
+        $count,
+        $key_length,
+        $raw_output = false
+    )
     {
         // Type checks:
         if (! \is_string($algorithm)) {
diff --git a/src/Crypto.php b/src/Crypto.php
index 86eb204..f148db4 100644
--- a/src/Crypto.php
+++ b/src/Crypto.php
@@ -55,7 +55,12 @@ public static function encrypt($plaintext, $key, $raw_binary = false)
      *
      * @return string
      */
-    public static function encryptWithPassword($plaintext, $password, $raw_binary = false)
+    public static function encryptWithPassword(
+        $plaintext,
+        #[\SensitiveParameter]
+        $password,
+        $raw_binary = false
+    )
     {
         if (!\is_string($plaintext)) {
             throw new \TypeError(
@@ -130,7 +135,12 @@ public static function decrypt($ciphertext, $key, $raw_binary = false)
      *
      * @return string
      */
-    public static function decryptWithPassword($ciphertext, $password, $raw_binary = false)
+    public static function decryptWithPassword(
+        $ciphertext,
+        #[\SensitiveParameter]
+        $password,
+        $raw_binary = false
+    )
     {
         if (!\is_string($ciphertext)) {
             throw new \TypeError(
@@ -166,7 +176,11 @@ public static function decryptWithPassword($ciphertext, $password, $raw_binary =
      *
      * @return string
      */
-    public static function legacyDecrypt($ciphertext, $key)
+    public static function legacyDecrypt(
+        $ciphertext,
+        #[\SensitiveParameter]
+        $key
+    )
     {
         if (!\is_string($ciphertext)) {
             throw new \TypeError(
@@ -378,7 +392,13 @@ private static function decryptInternal($ciphertext, KeyOrPassword $secret, $raw
      *
      * @return string
      */
-    protected static function plainEncrypt($plaintext, $key, $iv)
+    protected static function plainEncrypt(
+        $plaintext,
+        #[\SensitiveParameter]
+        $key,
+        #[\SensitiveParameter]
+        $iv
+    )
     {
         Core::ensureConstantExists('OPENSSL_RAW_DATA');
         Core::ensureFunctionExists('openssl_encrypt');
@@ -408,7 +428,14 @@ protected static function plainEncrypt($plaintext, $key, $iv)
      *
      * @return string
      */
-    protected static function plainDecrypt($ciphertext, $key, $iv, $cipherMethod)
+    protected static function plainDecrypt(
+        $ciphertext,
+        #[\SensitiveParameter]
+        $key,
+        #[\SensitiveParameter]
+        $iv,
+        $cipherMethod
+    )
     {
         Core::ensureConstantExists('OPENSSL_RAW_DATA');
         Core::ensureFunctionExists('openssl_decrypt');
@@ -437,7 +464,12 @@ protected static function plainDecrypt($ciphertext, $key, $iv, $cipherMethod)
      *
      * @return bool
      */
-    protected static function verifyHMAC($expected_hmac, $message, $key)
+    protected static function verifyHMAC(
+        $expected_hmac,
+        $message,
+        #[\SensitiveParameter]
+        $key
+    )
     {
         $message_hmac = \hash_hmac(Core::HASH_FUNCTION_NAME, $message, $key, true);
         return Core::hashEquals($message_hmac, $expected_hmac);
diff --git a/src/Encoding.php b/src/Encoding.php
index 30a60a8..b14044f 100644
--- a/src/Encoding.php
+++ b/src/Encoding.php
@@ -175,7 +175,11 @@ public static function trimTrailingWhitespace($string = '')
      *
      * @return string
      */
-    public static function saveBytesToChecksummedAsciiSafeString($header, $bytes)
+    public static function saveBytesToChecksummedAsciiSafeString(
+        $header,
+        #[\SensitiveParameter]
+        $bytes
+    )
     {
         // Headers must be a constant length to prevent one type's header from
         // being a prefix of another type's header, leading to ambiguity.
@@ -207,7 +211,11 @@ public static function saveBytesToChecksummedAsciiSafeString($header, $bytes)
      *
      * @return string
      */
-    public static function loadBytesFromChecksummedAsciiSafeString($expected_header, $string)
+    public static function loadBytesFromChecksummedAsciiSafeString(
+        $expected_header,
+        #[\SensitiveParameter]
+        $string
+    )
     {
         // Headers must be a constant length to prevent one type's header from
         // being a prefix of another type's header, leading to ambiguity.
diff --git a/src/File.php b/src/File.php
index ad2eac1..caf32ee 100644
--- a/src/File.php
+++ b/src/File.php
@@ -38,7 +38,12 @@ public static function encryptFile($inputFilename, $outputFilename, Key $key)
      * @throws Ex\EnvironmentIsBrokenException
      * @throws Ex\IOException
      */
-    public static function encryptFileWithPassword($inputFilename, $outputFilename, $password)
+    public static function encryptFileWithPassword(
+        $inputFilename,
+        $outputFilename,
+        #[\SensitiveParameter]
+        $password
+    )
     {
         self::encryptFileInternal(
             $inputFilename,
@@ -81,7 +86,12 @@ public static function decryptFile($inputFilename, $outputFilename, Key $key)
      * @throws Ex\IOException
      * @throws Ex\WrongKeyOrModifiedCiphertextException
      */
-    public static function decryptFileWithPassword($inputFilename, $outputFilename, $password)
+    public static function decryptFileWithPassword(
+        $inputFilename,
+        $outputFilename,
+        #[\SensitiveParameter]
+        $password
+    )
     {
         self::decryptFileInternal(
             $inputFilename,
@@ -125,7 +135,12 @@ public static function encryptResource($inputHandle, $outputHandle, Key $key)
      * @throws Ex\IOException
      * @throws Ex\WrongKeyOrModifiedCiphertextException
      */
-    public static function encryptResourceWithPassword($inputHandle, $outputHandle, $password)
+    public static function encryptResourceWithPassword(
+        $inputHandle,
+        $outputHandle,
+        #[\SensitiveParameter]
+        $password
+    )
     {
         self::encryptResourceInternal(
             $inputHandle,
@@ -169,7 +184,12 @@ public static function decryptResource($inputHandle, $outputHandle, Key $key)
      * @throws Ex\IOException
      * @throws Ex\WrongKeyOrModifiedCiphertextException
      */
-    public static function decryptResourceWithPassword($inputHandle, $outputHandle, $password)
+    public static function decryptResourceWithPassword(
+        $inputHandle,
+        $outputHandle,
+        #[\SensitiveParameter]
+        $password
+    )
     {
         self::decryptResourceInternal(
             $inputHandle,
diff --git a/src/Key.php b/src/Key.php
index 27b919f..1767d1f 100644
--- a/src/Key.php
+++ b/src/Key.php
@@ -41,7 +41,11 @@ public static function createNewRandomKey()
      *
      * @return Key
      */
-    public static function loadFromAsciiSafeString($saved_key_string, $do_not_trim = false)
+    public static function loadFromAsciiSafeString(
+        #[\SensitiveParameter]
+        $saved_key_string,
+        $do_not_trim = false
+    )
     {
         if (!$do_not_trim) {
             $saved_key_string = Encoding::trimTrailingWhitespace($saved_key_string);
@@ -82,7 +86,10 @@ public function getRawBytes()
      *
      * @throws Ex\EnvironmentIsBrokenException
      */
-    private function __construct($bytes)
+    private function __construct(
+        #[\SensitiveParameter]
+        $bytes
+    )
     {
         Core::ensureTrue(
             Core::ourStrlen($bytes) === self::KEY_BYTE_SIZE,
diff --git a/src/KeyOrPassword.php b/src/KeyOrPassword.php
index 890b2c2..3850655 100644
--- a/src/KeyOrPassword.php
+++ b/src/KeyOrPassword.php
@@ -39,7 +39,10 @@ public static function createFromKey(Key $key)
      *
      * @return KeyOrPassword
      */
-    public static function createFromPassword($password)
+    public static function createFromPassword(
+        #[\SensitiveParameter]
+        $password
+    )
     {
         return new KeyOrPassword(self::SECRET_TYPE_PASSWORD, $password);
     }
@@ -133,7 +136,11 @@ public function deriveKeys($salt)
      * @param int   $secret_type
      * @param mixed $secret      (either a Key or a password string)
      */
-    private function __construct($secret_type, $secret)
+    private function __construct(
+        $secret_type,
+        #[\SensitiveParameter]
+        $secret
+    )
     {
         // The constructor is private, so these should never throw.
         if ($secret_type === self::SECRET_TYPE_KEY) {
diff --git a/src/KeyProtectedByPassword.php b/src/KeyProtectedByPassword.php
index 347bdd9..7d90ed1 100644
--- a/src/KeyProtectedByPassword.php
+++ b/src/KeyProtectedByPassword.php
@@ -22,7 +22,10 @@ final class KeyProtectedByPassword
      *
      * @return KeyProtectedByPassword
      */
-    public static function createRandomPasswordProtectedKey($password)
+    public static function createRandomPasswordProtectedKey(
+        #[\SensitiveParameter]
+        $password
+    )
     {
         $inner_key = Key::createNewRandomKey();
         /* The password is hashed as a form of poor-man's domain separation
@@ -47,7 +50,10 @@ public static function createRandomPasswordProtectedKey($password)
      *
      * @return KeyProtectedByPassword
      */
-    public static function loadFromAsciiSafeString($saved_key_string)
+    public static function loadFromAsciiSafeString(
+        #[\SensitiveParameter]
+        $saved_key_string
+    )
     {
         $encrypted_key = Encoding::loadBytesFromChecksummedAsciiSafeString(
             self::PASSWORD_KEY_CURRENT_VERSION,
@@ -82,7 +88,10 @@ public function saveToAsciiSafeString()
      * @param string $password
      * @return Key
      */
-    public function unlockKey($password)
+    public function unlockKey(
+        #[\SensitiveParameter]
+        $password
+    )
     {
         try {
             $inner_key_encoded = Crypto::decryptWithPassword(
@@ -115,7 +124,12 @@ public function unlockKey($password)
      *
      * @return KeyProtectedByPassword
      */
-    public function changePassword($current_password, $new_password)
+    public function changePassword(
+        #[\SensitiveParameter]
+        $current_password,
+        #[\SensitiveParameter]
+        $new_password
+    )
     {
         $inner_key = $this->unlockKey($current_password);
         /* The password is hashed as a form of poor-man's domain separation


Test patch:
diff --git a/test/phpunit-8.xml b/test/phpunit-8.xml
index f76c419..03d8998 100644
--- a/test/phpunit-8.xml
+++ b/test/phpunit-8.xml
@@ -1,4 +1,4 @@
-<phpunit>
+<phpunit beStrictAboutTestsThatDoNotTestAnything="false">
     <filter>
         <whitelist processUncoveredFilesFromWhitelist="true">
             <directory suffix=".php">../src</directory>