From 4f7074ea6336c6aa2621c31fb5cf4f9705943c0c Mon Sep 17 00:00:00 2001
From: Paragon Initiative Enterprises <security@paragonie.com>
Date: Wed, 19 Jan 2022 01:24:14 -0500
Subject: [PATCH] Boyscouting

---
 src/Asymmetric/Crypto.php              | 23 +++++++++++++--------
 src/Asymmetric/EncryptionSecretKey.php |  2 +-
 src/Asymmetric/SecretKey.php           |  4 ++--
 src/Asymmetric/SignatureSecretKey.php  |  2 +-
 src/Stream/MutableFile.php             | 16 +++++++++++----
 src/Stream/ReadOnlyFile.php            | 19 +++++++++++------
 src/Structure/MerkleTree.php           | 21 ++++++++++---------
 src/Structure/Node.php                 |  6 +++---
 src/Structure/TrimmedMerkleTree.php    |  5 ++++-
 src/Symmetric/AuthenticationKey.php    |  1 +
 src/Symmetric/Config.php               |  5 +++++
 src/Symmetric/Crypto.php               | 28 +++++++++++++++++++-------
 12 files changed, 89 insertions(+), 43 deletions(-)

diff --git a/src/Asymmetric/Crypto.php b/src/Asymmetric/Crypto.php
index 382b916..3f9362c 100644
--- a/src/Asymmetric/Crypto.php
+++ b/src/Asymmetric/Crypto.php
@@ -23,6 +23,9 @@
 use RangeException;
 use SodiumException;
 use TypeError;
+use const
+    SODIUM_CRYPTO_STREAM_KEYBYTES,
+    SODIUM_CRYPTO_SIGN_BYTES;
 use function
     is_string,
     sodium_crypto_box_keypair_from_secretkey_and_publickey,
@@ -86,7 +89,7 @@ public static function encrypt(
         EncryptionPublicKey $theirPublicKey,
         string|bool $encoding = Halite::ENCODE_BASE64URLSAFE
     ): string {
-        return self::encryptWithAd(
+        return self::encryptWithAD(
             $plaintext,
             $ourPrivateKey,
             $theirPublicKey,
@@ -114,7 +117,7 @@ public static function encrypt(
      * @throws SodiumException
      * @throws TypeError
      */
-    public static function encryptWithAd(
+    public static function encryptWithAD(
         HiddenString $plaintext,
         EncryptionSecretKey $ourPrivateKey,
         EncryptionPublicKey $theirPublicKey,
@@ -129,7 +132,7 @@ public static function encryptWithAd(
             self::getAsymmetricConfig(Halite::HALITE_VERSION, true)
         );
         $sharedSecretKey = new EncryptionKey($ss);
-        $ciphertext = SymmetricCrypto::encryptWithAd(
+        $ciphertext = SymmetricCrypto::encryptWithAD(
             $plaintext,
             $sharedSecretKey,
             $additionalData,
@@ -164,7 +167,7 @@ public static function decrypt(
         EncryptionPublicKey $theirPublicKey,
         string|bool $encoding = Halite::ENCODE_BASE64URLSAFE
     ): HiddenString {
-        return self::decryptWithAd(
+        return self::decryptWithAD(
             $ciphertext,
             $ourPrivateKey,
             $theirPublicKey,
@@ -193,7 +196,7 @@ public static function decrypt(
      * @throws SodiumException
      * @throws TypeError
      */
-    public static function decryptWithAd(
+    public static function decryptWithAD(
         string $ciphertext,
         EncryptionSecretKey $ourPrivateKey,
         EncryptionPublicKey $theirPublicKey,
@@ -208,7 +211,7 @@ public static function decryptWithAd(
             self::getAsymmetricConfig($ciphertext, $encoding)
         );
         $sharedSecretKey = new EncryptionKey($ss);
-        $plaintext = SymmetricCrypto::decryptWithAd(
+        $plaintext = SymmetricCrypto::decryptWithAD(
             $ciphertext,
             $sharedSecretKey,
             $additionalData,
@@ -228,8 +231,11 @@ public static function decryptWithAd(
      * @param EncryptionPublicKey $publicKey  Public key (theirs)
      * @param bool $get_as_object             Get as a Key object?
      * @param ?Config $config                 Asymmetric Config
+     *
      * @return HiddenString|Key
      *
+     * @throws CannotPerformOperation
+     * @throws InvalidDigestLength
      * @throws InvalidKey
      * @throws SodiumException
      * @throws TypeError
@@ -248,7 +254,7 @@ public static function getSharedSecret(
                             $privateKey->getRawKeyMaterial(),
                             $publicKey->getRawKeyMaterial()
                         ),
-                        32,
+                        SODIUM_CRYPTO_STREAM_KEYBYTES,
                         (string) $config->HASH_DOMAIN_SEPARATION
                     )
                 );
@@ -484,6 +490,7 @@ public static function verify(
      * @param SignaturePublicKey $senderPublicKey  Private signing key
      * @param SecretKey $givenSecretKey            Public encryption key
      * @param string|bool $encoding                Which encoding scheme to use?
+     *
      * @return HiddenString
      *
      * @throws CannotPerformOperation
@@ -552,6 +559,6 @@ public static function getAsymmetricConfig(
             0,
             Halite::VERSION_TAG_LEN
         );
-        return Config::getConfig($version, 'encrypt');
+        return Config::getConfig($version);
     }
 }
diff --git a/src/Asymmetric/EncryptionSecretKey.php b/src/Asymmetric/EncryptionSecretKey.php
index 20c1b63..ede9f8b 100644
--- a/src/Asymmetric/EncryptionSecretKey.php
+++ b/src/Asymmetric/EncryptionSecretKey.php
@@ -50,7 +50,7 @@ public function __construct(HiddenString $keyMaterial, ?HiddenString $pk = null)
      * @throws TypeError
      * @throws SodiumException
      */
-    public function derivePublicKey()
+    public function derivePublicKey(): EncryptionPublicKey
     {
         if (is_null($this->cachedPublicKey)) {
             $this->cachedPublicKey = sodium_crypto_box_publickey_from_secretkey(
diff --git a/src/Asymmetric/SecretKey.php b/src/Asymmetric/SecretKey.php
index f85e2f8..6da48fa 100644
--- a/src/Asymmetric/SecretKey.php
+++ b/src/Asymmetric/SecretKey.php
@@ -36,10 +36,10 @@ public function __construct(HiddenString $keyMaterial, ?HiddenString $pk = null)
     /**
      * See the appropriate derived class.
      * @throws CannotPerformOperation
-     * @return mixed
+     * @return PublicKey
      * @codeCoverageIgnore
      */
-    public function derivePublicKey()
+    public function derivePublicKey(): PublicKey
     {
         throw new CannotPerformOperation(
             'This is not implemented in the base class'
diff --git a/src/Asymmetric/SignatureSecretKey.php b/src/Asymmetric/SignatureSecretKey.php
index 38d858e..f834f6c 100644
--- a/src/Asymmetric/SignatureSecretKey.php
+++ b/src/Asymmetric/SignatureSecretKey.php
@@ -55,7 +55,7 @@ public function __construct(HiddenString $keyMaterial, ?HiddenString $pk = null)
      * @throws SodiumException
      * @throws TypeError
      */
-    public function derivePublicKey()
+    public function derivePublicKey(): SignaturePublicKey
     {
         if (is_null($this->cachedPublicKey)) {
             $this->cachedPublicKey = sodium_crypto_sign_publickey_from_secretkey(
diff --git a/src/Stream/MutableFile.php b/src/Stream/MutableFile.php
index ba6299e..d1cb7a5 100644
--- a/src/Stream/MutableFile.php
+++ b/src/Stream/MutableFile.php
@@ -74,6 +74,7 @@ class MutableFile implements StreamInterface
     /**
      * MutableFile constructor.
      * @param string|resource $file
+     *
      * @throws InvalidType
      * @throws FileAccessDenied
      * @psalm-suppress RedundantConditionGivenDocblockType
@@ -132,6 +133,8 @@ public function __construct($file)
     /**
      * Close the file handle.
      *
+     * @return void
+     *
      * @psalm-suppress InvalidPropertyAssignmentValue
      */
     public function close(): void
@@ -187,7 +190,9 @@ public function getStreamMetadata(): array
      *
      * @param int $num
      * @param bool $skipTests
+     *
      * @return string
+     *
      * @throws CannotPerformOperation
      * @throws FileAccessDenied
      */
@@ -250,15 +255,17 @@ public function remainingBytes(): int
     /**
      * Set the current cursor position to the desired location
      * 
-     * @param int $i
+     * @param int $position
+     *
      * @return bool
+     *
      * @throws CannotPerformOperation
      * @codeCoverageIgnore
      */
-    public function reset(int $i = 0): bool
+    public function reset(int $position = 0): bool
     {
-        $this->pos = $i;
-        if (fseek($this->fp, $i, SEEK_SET) === 0) {
+        $this->pos = $position;
+        if (fseek($this->fp, $position, SEEK_SET) === 0) {
             return true;
         }
         throw new CannotPerformOperation(
@@ -271,6 +278,7 @@ public function reset(int $i = 0): bool
      *
      * @param string $buf
      * @param ?int $num (number of bytes)
+     *
      * @return int
      *
      * @throws CannotPerformOperation
diff --git a/src/Stream/ReadOnlyFile.php b/src/Stream/ReadOnlyFile.php
index 5506f3c..a2bac8d 100644
--- a/src/Stream/ReadOnlyFile.php
+++ b/src/Stream/ReadOnlyFile.php
@@ -151,7 +151,8 @@ public function close(): void
      * Calculate a BLAKE2b hash of a file
      *
      * @return string
-     * @throws \SodiumException
+     *
+     * @throws SodiumException
      * @throws FileModified
      * @throws FileError
      */
@@ -224,10 +225,12 @@ public function getStreamMetadata(): array
      * decision to make lightly!)
      *
      * @param int $num
-     * @param bool $skipTests Only set this to TRUE if you're absolutely sure
-     *                           that you don't want to defend against TOCTOU /
-     *                           race condition attacks on the filesystem!
+     * @param bool $skipTests  Only set this to TRUE if you're absolutely sure
+     *                         that you don't want to defend against TOCTOU /
+     *                         race condition attacks on the filesystem!
+     *
      * @return string
+     *
      * @throws CannotPerformOperation
      * @throws FileAccessDenied
      * @throws FileModified
@@ -255,7 +258,6 @@ public function readBytes(int $num, bool $skipTests = false): string
                 break;
             }
             // @codeCoverageIgnoreEnd
-            /** @var string|bool $read */
             $read = fread($this->fp, $remaining);
             if (!is_string($read)) {
                 // @codeCoverageIgnoreStart
@@ -290,7 +292,9 @@ public function remainingBytes(): int
      * Set the current cursor position to the desired location
      *
      * @param int $position
+     *
      * @return bool
+     *
      * @throws CannotPerformOperation
      */
     public function reset(int $position = 0): bool
@@ -311,8 +315,9 @@ public function reset(int $position = 0): bool
      * verifying that the hash matches and the current cursor position/file
      * size matches their values when the file was first opened.
      *
-     * @throws FileModified
      * @return void
+     *
+     * @throws FileModified
      */
     public function toctouTest(): void
     {
@@ -336,7 +341,9 @@ public function toctouTest(): void
      *
      * @param string $buf
      * @param ?int $num (number of bytes)
+     *
      * @return int
+     *
      * @throws FileAccessDenied
      */
     public function writeBytes(string $buf, ?int $num = null): int
diff --git a/src/Structure/MerkleTree.php b/src/Structure/MerkleTree.php
index 502eef2..2a59fbb 100644
--- a/src/Structure/MerkleTree.php
+++ b/src/Structure/MerkleTree.php
@@ -47,15 +47,7 @@ class MerkleTree
      * @var Node[]
      */
     protected array $nodes = [];
-
-    /**
-     * @var string
-     */
     protected string $personalization = '';
-    
-    /**
-     * @var int
-     */
     protected int $outputSize = SODIUM_CRYPTO_GENERICHASH_BYTES;
     
     /**
@@ -74,6 +66,7 @@ public function __construct(Node ...$nodes)
      * @param bool $raw - Do we want a raw string instead of a hex string?
      *
      * @return string
+     *
      * @throws CannotPerformOperation
      * @throws TypeError
      * @throws SodiumException
@@ -92,7 +85,9 @@ public function getRoot(bool $raw = false): string
      * Merkle Trees are immutable. Return a replacement with extra nodes.
      *
      * @param array<int, Node> $nodes
+     *
      * @return MerkleTree
+     *
      * @throws InvalidDigestLength
      */
     public function getExpandedTree(Node ...$nodes): MerkleTree
@@ -110,7 +105,9 @@ public function getExpandedTree(Node ...$nodes): MerkleTree
      * Set the hash output size.
      *
      * @param int $size
+     *
      * @return self
+     *
      * @throws InvalidDigestLength
      */
     public function setHashSize(int $size): self
@@ -142,6 +139,7 @@ public function setHashSize(int $size): self
      * Sets the personalization string for the Merkle root calculation
      *
      * @param string $str
+     *
      * @return self
      */
     public function setPersonalizationString(string $str = ''): self
@@ -157,9 +155,10 @@ public function setPersonalizationString(string $str = ''): self
      * Explicitly recalculate the Merkle root
      *
      * @return self
+     *
      * @throws CannotPerformOperation
-     * @throws \TypeError
-     * @throws \SodiumException
+     * @throws TypeError
+     * @throws SodiumException
      * @codeCoverageIgnore
      */
     public function triggerRootCalculation(): self
@@ -174,6 +173,7 @@ public function triggerRootCalculation(): self
      * to protect against second-preimage attacks
      *
      * @return string
+     *
      * @throws CannotPerformOperation
      * @throws TypeError
      * @throws SodiumException
@@ -250,6 +250,7 @@ protected function calculateRoot(): string
      * Let's go ahead and round up to the nearest multiple of 2
      *
      * @param int $inputSize
+     *
      * @return int
      */
     public static function getSizeRoundedUp(int $inputSize): int
diff --git a/src/Structure/Node.php b/src/Structure/Node.php
index 622cd1d..8bc083d 100644
--- a/src/Structure/Node.php
+++ b/src/Structure/Node.php
@@ -24,13 +24,11 @@
  */
 class Node
 {
-    /**
-     * @var string
-     */
     private string $data;
 
     /**
      * Node constructor.
+     *
      * @param string $data
      */
     public function __construct(string $data)
@@ -58,6 +56,7 @@ public function getData(): string
      * @param string $personalization
      *
      * @return string
+     *
      * @throws CannotPerformOperation
      * @throws TypeError
      * @throws SodiumException
@@ -83,6 +82,7 @@ public function getHash(
      * Nodes are immutable, but you can create one with extra data.
      *
      * @param string $concat
+     *
      * @return Node
      */
     public function getExpandedNode(string $concat): Node
diff --git a/src/Structure/TrimmedMerkleTree.php b/src/Structure/TrimmedMerkleTree.php
index 398c4de..8a9a29d 100644
--- a/src/Structure/TrimmedMerkleTree.php
+++ b/src/Structure/TrimmedMerkleTree.php
@@ -39,6 +39,7 @@ class TrimmedMerkleTree extends MerkleTree
      * to protect against second-preimage attacks
      *
      * @return string
+     *
      * @throws CannotPerformOperation
      * @throws SodiumException
      * @throws TypeError
@@ -95,10 +96,12 @@ protected function calculateRoot(): string
      * Merkle Trees are immutable. Return a replacement with extra nodes.
      *
      * @param array<int, Node> $nodes
+     *
      * @return TrimmedMerkleTree
+     *
      * @throws InvalidDigestLength
      */
-    public function getExpandedTree(Node ...$nodes): MerkleTree
+    public function getExpandedTree(Node ...$nodes): TrimmedMerkleTree
     {
         $thisTree = $this->nodes;
         foreach ($nodes as $node) {
diff --git a/src/Symmetric/AuthenticationKey.php b/src/Symmetric/AuthenticationKey.php
index afef733..3ef8f51 100644
--- a/src/Symmetric/AuthenticationKey.php
+++ b/src/Symmetric/AuthenticationKey.php
@@ -21,6 +21,7 @@ final class AuthenticationKey extends SecretKey
 {
     /**
      * AuthenticationKey constructor.
+     *
      * @param HiddenString $keyMaterial - The actual key data
      *
      * @throws InvalidKey
diff --git a/src/Symmetric/Config.php b/src/Symmetric/Config.php
index db55d9c..3f19877 100644
--- a/src/Symmetric/Config.php
+++ b/src/Symmetric/Config.php
@@ -35,6 +35,7 @@ final class Config extends BaseConfig
      *
      * @param string $header
      * @param string $mode
+     *
      * @return self
      *
      * @throws InvalidMessage
@@ -74,7 +75,9 @@ public static function getConfig(
      * 
      * @param int $major
      * @param int $minor
+     *
      * @return array
+     *
      * @throws InvalidMessage
      */
     public static function getConfigEncrypt(int $major, int $minor): array
@@ -123,7 +126,9 @@ public static function getConfigEncrypt(int $major, int $minor): array
      * 
      * @param int $major
      * @param int $minor
+     *
      * @return array
+     *
      * @throws InvalidMessage
      */
     public static function getConfigAuth(int $major, int $minor): array
diff --git a/src/Symmetric/Crypto.php b/src/Symmetric/Crypto.php
index 2e3617e..c62266a 100644
--- a/src/Symmetric/Crypto.php
+++ b/src/Symmetric/Crypto.php
@@ -121,7 +121,7 @@ public static function decrypt(
         EncryptionKey $secretKey,
         bool|string $encoding = Halite::ENCODE_BASE64URLSAFE
     ): HiddenString {
-        return self::decryptWithAd(
+        return self::decryptWithAD(
             $ciphertext,
             $secretKey,
             '',
@@ -132,6 +132,14 @@ public static function decrypt(
     /**
      * Decrypt a message using the Halite encryption protocol
      *
+     * Verifies the MAC before decryption
+     * - Halite 5+ verifies the BLAKE2b-MAC before decrypting with XChaCha20
+     * - Halite 4 and below verifies the BLAKE2b-MAC before decrypting with XSalsa20
+     *
+     * You don't need to worry about chosen-ciphertext attacks.
+     * You don't need to worry about Invisible Salamanders.
+     * You don't need to worry about timing attacks on MAC validation.
+     *
      * @param string $ciphertext
      * @param EncryptionKey $secretKey
      * @param string $additionalData
@@ -147,7 +155,7 @@ public static function decrypt(
      * @throws SodiumException
      * @throws TypeError
      */
-    public static function decryptWithAd(
+    public static function decryptWithAD(
         string $ciphertext,
         EncryptionKey $secretKey,
         string $additionalData = '',
@@ -245,9 +253,6 @@ public static function decryptWithAd(
     /**
      * Encrypt a message using the Halite encryption protocol
      *
-     * (Encrypt then MAC -- xsalsa20 then keyed-Blake2b)
-     * You don't need to worry about chosen-ciphertext attacks.
-     *
      * @param HiddenString $plaintext
      * @param EncryptionKey $secretKey
      * @param string|bool $encoding
@@ -266,7 +271,7 @@ public static function encrypt(
         EncryptionKey $secretKey,
         bool|string $encoding = Halite::ENCODE_BASE64URLSAFE
     ): string {
-        return self::encryptWithAd(
+        return self::encryptWithAD(
             $plaintext,
             $secretKey,
             '',
@@ -275,6 +280,15 @@ public static function encrypt(
     }
 
     /**
+     * Encrypt a message using the Halite encryption protocol
+     *
+     * Encrypt then MAC.
+     * - Halite 5+ uses XChaCha20 then BLAKE2b-MAC
+     * - Halite 4 and below use XSalsa20 then BLAKE2b-MAC
+     *
+     * You don't need to worry about chosen-ciphertext attacks.
+     * You don't need to worry about Invisible Salamanders.
+     *
      * @param HiddenString $plaintext
      * @param EncryptionKey $secretKey
      * @param string $additionalData
@@ -289,7 +303,7 @@ public static function encrypt(
      * @throws SodiumException
      * @throws TypeError
      */
-    public static function encryptWithAd(
+    public static function encryptWithAD(
         HiddenString $plaintext,
         EncryptionKey $secretKey,
         string $additionalData = '',