More test coverage. Ensure no unsafe X25519 public keys are accepted.

paragonie · Jan 26, 2017 · 35031fa · 35031fa
1 parent a1ead67
commit 35031fa
Show file tree

Hide file tree

Showing 4 changed files with 384 additions and 20 deletions.
diff --git a/src/Compat.php b/src/Compat.php
@@ -916,15 +916,7 @@ public static function crypto_scalarmult($secretKey, $publicKey)
         if (ParagonIE_Sodium_Core_Util::hashEquals($publicKey, str_repeat("\0", self::CRYPTO_BOX_PUBLICKEYBYTES))) {
             throw new Error('Zero public key is not allowed');
         }
-        $q = ParagonIE_Sodium_Crypto::scalarmult($secretKey, $publicKey);
-        $d = 0;
-        for ($i = 0; $i < self::CRYPTO_SCALARMULT_BYTES; ++$i) {
-            $d |= ParagonIE_Sodium_Core_Util::chrToInt($q[$i]);
-        }
-        if (-(1 & (($d - 1) >> 8))) {
-            throw new Error('Zero public key is not allowed');
-        }
-        return $q;
+        return ParagonIE_Sodium_Crypto::scalarmult($secretKey, $publicKey);
     }
 
     /**
@@ -949,15 +941,7 @@ public static function crypto_scalarmult_base($secretKey)
         if (ParagonIE_Sodium_Core_Util::hashEquals($secretKey, str_repeat("\0", self::CRYPTO_BOX_SECRETKEYBYTES))) {
             throw new Error('Zero secret key is not allowed');
         }
-        $q = ParagonIE_Sodium_Crypto::scalarmult_base($secretKey);
-        $d = 0;
-        for ($i = 0; $i < self::CRYPTO_SCALARMULT_BYTES; ++$i) {
-            $d |= ParagonIE_Sodium_Core_Util::chrToInt($q[$i]);
-        }
-        if (-(1 & (($d - 1) >> 8))) {
-            throw new Error('Zero public key returned. This is not safe to use.');
-        }
-        return $q;
+        return ParagonIE_Sodium_Crypto::scalarmult_base($secretKey);
     }
 
     /**

diff --git a/src/Crypto.php b/src/Crypto.php
@@ -633,10 +633,20 @@ public static function keyExchange($my_sk, $their_pk, $client_pk, $server_pk)
      * @param string $sKey
      * @param string $pKey
      * @return string
+     *
+     * @throws Error
      */
     public static function scalarmult($sKey, $pKey)
     {
-        return ParagonIE_Sodium_Core_X25519::crypto_scalarmult_curve25519_ref10($sKey, $pKey);
+        $q =  ParagonIE_Sodium_Core_X25519::crypto_scalarmult_curve25519_ref10($sKey, $pKey);
+        $d = 0;
+        for ($i = 0; $i < self::box_curve25519xsalsa20poly1305_SECRETKEYBYTES; ++$i) {
+            $d |= ParagonIE_Sodium_Core_Util::chrToInt($q[$i]);
+        }
+        if (-(1 & (($d - 1) >> 8))) {
+            throw new Error('Zero public key is not allowed');
+        }
+        return $q;
     }
 
     /**
@@ -645,10 +655,20 @@ public static function scalarmult($sKey, $pKey)
      *
      * @param string $secret
      * @return string
+     *
+     * @throws Error
      */
     public static function scalarmult_base($secret)
     {
-        return ParagonIE_Sodium_Core_X25519::crypto_scalarmult_curve25519_ref10_base($secret);
+        $q = ParagonIE_Sodium_Core_X25519::crypto_scalarmult_curve25519_ref10_base($secret);
+        $d = 0;
+        for ($i = 0; $i < self::box_curve25519xsalsa20poly1305_SECRETKEYBYTES; ++$i) {
+            $d |= ParagonIE_Sodium_Core_Util::chrToInt($q[$i]);
+        }
+        if (-(1 & (($d - 1) >> 8))) {
+            throw new Error('Zero public key is not allowed');
+        }
+        return $q;
     }
 
     /**

diff --git a/tests/unit/CryptoTest.php b/tests/unit/CryptoTest.php
@@ -7,6 +7,26 @@ public function setUp()
         ParagonIE_Sodium_Compat::$disableFallbackForUnitTests = true;
     }
 
+    /**
+     * @covers ParagonIE_Sodium_Compat::crypto_auth()
+     * @covers ParagonIE_Sodium_Compat::crypto_auth_verify()
+     */
+    public function testCryptoAuth()
+    {
+        $key = random_bytes(ParagonIE_Sodium_Compat::CRYPTO_AUTH_KEYBYTES);
+        $message = "Lorem ipsum dolor sit amet, consectetur adipiscing elit.";
+        $message .= random_bytes(64);
+
+        $mac = ParagonIE_Sodium_Compat::crypto_auth($message, $key);
+        $this->assertTrue(
+            ParagonIE_Sodium_Compat::crypto_auth_verify($mac, $message, $key)
+        );
+        $message .= 'wrong';
+        $this->assertFalse(
+            ParagonIE_Sodium_Compat::crypto_auth_verify($mac, $message, $key)
+        );
+    }
+
     /**
      * @covers ParagonIE_Sodium_Compat::crypto_aead_chacha20poly1305_decrypt()
      * @covers ParagonIE_Sodium_Compat::crypto_aead_chacha20poly1305_encrypt()