Skip to content

Commit

Permalink
Added SensitiveParameter attribute to all places accepting a password…
Browse files Browse the repository at this point in the history
…, or a key in a string form.

Fixes #501
  • Loading branch information
boenrobot authored and defuse committed Jun 17, 2023
1 parent 1da53bd commit 91e5f54
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 21 deletions.
10 changes: 9 additions & 1 deletion src/Core.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
44 changes: 38 additions & 6 deletions src/Crypto.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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);
Expand Down
12 changes: 10 additions & 2 deletions src/Encoding.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
28 changes: 24 additions & 4 deletions src/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 9 additions & 2 deletions src/Key.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 9 additions & 2 deletions src/KeyOrPassword.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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) {
Expand Down
22 changes: 18 additions & 4 deletions src/KeyProtectedByPassword.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 91e5f54

Please sign in to comment.