Skip to content

Commit

Permalink
PHP's order of operations need to be reined in.
Browse files Browse the repository at this point in the history
  • Loading branch information
paragonie-scott committed Oct 15, 2015
1 parent 6f0131c commit b202f42
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
4 changes: 2 additions & 2 deletions src/Key.php
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,11 @@ public static function deriveFromPassword(
new ASecretKey($secret_key, $signing), // Secret key
new APublicKey($public_key, $signing) // Public key
];
} elseif ($type & self::SECRET_KEY !== 0) {
} elseif (($type & self::SECRET_KEY) !== 0) {
/**
* Are we doing encryption or authentication?
*/
if ($type & self::SIGNATURE !== 0) {
if (($type & self::SIGNATURE) !== 0) {
$signing = true;
$secret_key = \Sodium\crypto_pwhash_scryptsalsa208sha256(
\Sodium\CRYPTO_AUTH_KEYBYTES,
Expand Down
14 changes: 13 additions & 1 deletion test/unit/KeyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
use \ParagonIE\Halite\Asymmetric\Crypto as Asymmetric;
use \ParagonIE\Halite\Asymmetric\SecretKey as ASecretKey;
use \ParagonIE\Halite\Asymmetric\PublicKey as APublicKey;
use \ParagonIE\Halite\Symmetric\SecretKey as SecretKey;

/**
* @backupGlobals disabled
Expand All @@ -23,6 +22,19 @@ public function testDerive()
"\x36\xa6\xc2\xb9\x6a\x65\x0d\x80\xbf\x7e\x02\x5e\x0f\x58\xf3\xd6".
"\x36\x33\x95\x75\xde\xfb\x37\x08\x01\xa5\x42\x13\xbd\x54\x58\x2d"
);
$salt = \Sodium\hex2bin(
'762ce4cabd543065172236de1027536ad52ec4c9133ced3766ff319f10301888'
);

// Issue #10
$enc_secret = Key::deriveFromPassword(
'correct horse battery staple',
$salt,
Key::ENCRYPTION | Key::SECRET_KEY
);
$this->assertTrue(
$enc_secret->isEncryptionKey()
);
}

public function testDeriveSigningKey()
Expand Down

4 comments on commit b202f42

@shadowhand
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🖕

@paragonie-scott
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

???

@shadowhand
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was expressing my assumption of your frustration at discovering this bug. 😉

@paragonie-scott
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, gotcha. Aye, that was frustrating, but I was more explicit elsewhere so I can only blame myself for being inconsistent here.

Instead, I thought I burned a 0day you were cooking up or something by fixing it. 😆

Please sign in to comment.