Skip to content

Commit dbc4db9

Browse files
defusedefuse
authored
Merge pull request #513 from defuse/sensitive-parameter-rebase
Added SensitiveParameter attribute to all places accepting a password or a key in a string form.
2 parents 1da53bd + 915ffc1 commit dbc4db9

10 files changed

Lines changed: 125 additions & 24 deletions

File tree

‎.travis.yml‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@ matrix:
1717
- php: "7.4"
1818
env: USE_PSALM=1
1919
- php: "8.0"
20-
env: USE_PSALM=1
20+
# psalm currently doesn't like our \[#SensitiveParameter]s
21+
env: USE_PSALM=0
2122
- php: "8.1"
22-
env: USE_PSALM=1
23+
# psalm currently doesn't like our \[#SensitiveParameter]s
24+
env: USE_PSALM=0
2325
- php: "8.2"
2426
env: USE_PSALM=1
2527
dist: focal

‎psalm.xml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,8 @@
99
<DocblockTypeContradiction errorLevel="info" />
1010
<RedundantCondition errorLevel="info" />
1111
<RedundantConditionGivenDocblockType errorLevel="info" />
12+
<!-- It's complaining about our #[\SensitiveParameter]s for no reason -->
13+
<InvalidAttribute errorLevel="info" />
14+
<UndefinedAttributeClass errorLevel="info" />
1215
</issueHandlers>
1316
</psalm>

‎src/Core.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,15 @@ public static function ourSubstr($str, $start, $length = null)
386386
*
387387
* @return string A $key_length-byte key derived from the password and salt.
388388
*/
389-
public static function pbkdf2($algorithm, $password, $salt, $count, $key_length, $raw_output = false)
389+
public static function pbkdf2(
390+
$algorithm,
391+
#[\SensitiveParameter]
392+
$password,
393+
$salt,
394+
$count,
395+
$key_length,
396+
$raw_output = false
397+
)
390398
{
391399
// Type checks:
392400
if (! \is_string($algorithm)) {

‎src/Crypto.php‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,12 @@ public static function encrypt($plaintext, $key, $raw_binary = false)
5555
*
5656
* @return string
5757
*/
58-
public static function encryptWithPassword($plaintext, $password, $raw_binary = false)
58+
public static function encryptWithPassword(
59+
$plaintext,
60+
#[\SensitiveParameter]
61+
$password,
62+
$raw_binary = false
63+
)
5964
{
6065
if (!\is_string($plaintext)) {
6166
throw new \TypeError(
@@ -130,7 +135,12 @@ public static function decrypt($ciphertext, $key, $raw_binary = false)
130135
*
131136
* @return string
132137
*/
133-
public static function decryptWithPassword($ciphertext, $password, $raw_binary = false)
138+
public static function decryptWithPassword(
139+
$ciphertext,
140+
#[\SensitiveParameter]
141+
$password,
142+
$raw_binary = false
143+
)
134144
{
135145
if (!\is_string($ciphertext)) {
136146
throw new \TypeError(
@@ -166,7 +176,11 @@ public static function decryptWithPassword($ciphertext, $password, $raw_binary =
166176
*
167177
* @return string
168178
*/
169-
public static function legacyDecrypt($ciphertext, $key)
179+
public static function legacyDecrypt(
180+
$ciphertext,
181+
#[\SensitiveParameter]
182+
$key
183+
)
170184
{
171185
if (!\is_string($ciphertext)) {
172186
throw new \TypeError(
@@ -378,7 +392,13 @@ private static function decryptInternal($ciphertext, KeyOrPassword $secret, $raw
378392
*
379393
* @return string
380394
*/
381-
protected static function plainEncrypt($plaintext, $key, $iv)
395+
protected static function plainEncrypt(
396+
$plaintext,
397+
#[\SensitiveParameter]
398+
$key,
399+
#[\SensitiveParameter]
400+
$iv
401+
)
382402
{
383403
Core::ensureConstantExists('OPENSSL_RAW_DATA');
384404
Core::ensureFunctionExists('openssl_encrypt');
@@ -408,7 +428,14 @@ protected static function plainEncrypt($plaintext, $key, $iv)
408428
*
409429
* @return string
410430
*/
411-
protected static function plainDecrypt($ciphertext, $key, $iv, $cipherMethod)
431+
protected static function plainDecrypt(
432+
$ciphertext,
433+
#[\SensitiveParameter]
434+
$key,
435+
#[\SensitiveParameter]
436+
$iv,
437+
$cipherMethod
438+
)
412439
{
413440
Core::ensureConstantExists('OPENSSL_RAW_DATA');
414441
Core::ensureFunctionExists('openssl_decrypt');
@@ -437,7 +464,12 @@ protected static function plainDecrypt($ciphertext, $key, $iv, $cipherMethod)
437464
*
438465
* @return bool
439466
*/
440-
protected static function verifyHMAC($expected_hmac, $message, $key)
467+
protected static function verifyHMAC(
468+
$expected_hmac,
469+
$message,
470+
#[\SensitiveParameter]
471+
$key
472+
)
441473
{
442474
$message_hmac = \hash_hmac(Core::HASH_FUNCTION_NAME, $message, $key, true);
443475
return Core::hashEquals($message_hmac, $expected_hmac);

‎src/Encoding.php‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,11 @@ public static function trimTrailingWhitespace($string = '')
175175
*
176176
* @return string
177177
*/
178-
public static function saveBytesToChecksummedAsciiSafeString($header, $bytes)
178+
public static function saveBytesToChecksummedAsciiSafeString(
179+
$header,
180+
#[\SensitiveParameter]
181+
$bytes
182+
)
179183
{
180184
// Headers must be a constant length to prevent one type's header from
181185
// being a prefix of another type's header, leading to ambiguity.
@@ -207,7 +211,11 @@ public static function saveBytesToChecksummedAsciiSafeString($header, $bytes)
207211
*
208212
* @return string
209213
*/
210-
public static function loadBytesFromChecksummedAsciiSafeString($expected_header, $string)
214+
public static function loadBytesFromChecksummedAsciiSafeString(
215+
$expected_header,
216+
#[\SensitiveParameter]
217+
$string
218+
)
211219
{
212220
// Headers must be a constant length to prevent one type's header from
213221
// being a prefix of another type's header, leading to ambiguity.

‎src/File.php‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,12 @@ public static function encryptFile($inputFilename, $outputFilename, Key $key)
3838
* @throws Ex\EnvironmentIsBrokenException
3939
* @throws Ex\IOException
4040
*/
41-
public static function encryptFileWithPassword($inputFilename, $outputFilename, $password)
41+
public static function encryptFileWithPassword(
42+
$inputFilename,
43+
$outputFilename,
44+
#[\SensitiveParameter]
45+
$password
46+
)
4247
{
4348
self::encryptFileInternal(
4449
$inputFilename,
@@ -81,7 +86,12 @@ public static function decryptFile($inputFilename, $outputFilename, Key $key)
8186
* @throws Ex\IOException
8287
* @throws Ex\WrongKeyOrModifiedCiphertextException
8388
*/
84-
public static function decryptFileWithPassword($inputFilename, $outputFilename, $password)
89+
public static function decryptFileWithPassword(
90+
$inputFilename,
91+
$outputFilename,
92+
#[\SensitiveParameter]
93+
$password
94+
)
8595
{
8696
self::decryptFileInternal(
8797
$inputFilename,
@@ -125,7 +135,12 @@ public static function encryptResource($inputHandle, $outputHandle, Key $key)
125135
* @throws Ex\IOException
126136
* @throws Ex\WrongKeyOrModifiedCiphertextException
127137
*/
128-
public static function encryptResourceWithPassword($inputHandle, $outputHandle, $password)
138+
public static function encryptResourceWithPassword(
139+
$inputHandle,
140+
$outputHandle,
141+
#[\SensitiveParameter]
142+
$password
143+
)
129144
{
130145
self::encryptResourceInternal(
131146
$inputHandle,
@@ -169,7 +184,12 @@ public static function decryptResource($inputHandle, $outputHandle, Key $key)
169184
* @throws Ex\IOException
170185
* @throws Ex\WrongKeyOrModifiedCiphertextException
171186
*/
172-
public static function decryptResourceWithPassword($inputHandle, $outputHandle, $password)
187+
public static function decryptResourceWithPassword(
188+
$inputHandle,
189+
$outputHandle,
190+
#[\SensitiveParameter]
191+
$password
192+
)
173193
{
174194
self::decryptResourceInternal(
175195
$inputHandle,

‎src/Key.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,11 @@ public static function createNewRandomKey()
4141
*
4242
* @return Key
4343
*/
44-
public static function loadFromAsciiSafeString($saved_key_string, $do_not_trim = false)
44+
public static function loadFromAsciiSafeString(
45+
#[\SensitiveParameter]
46+
$saved_key_string,
47+
$do_not_trim = false
48+
)
4549
{
4650
if (!$do_not_trim) {
4751
$saved_key_string = Encoding::trimTrailingWhitespace($saved_key_string);
@@ -82,7 +86,10 @@ public function getRawBytes()
8286
*
8387
* @throws Ex\EnvironmentIsBrokenException
8488
*/
85-
private function __construct($bytes)
89+
private function __construct(
90+
#[\SensitiveParameter]
91+
$bytes
92+
)
8693
{
8794
Core::ensureTrue(
8895
Core::ourStrlen($bytes) === self::KEY_BYTE_SIZE,

‎src/KeyOrPassword.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@ public static function createFromKey(Key $key)
3939
*
4040
* @return KeyOrPassword
4141
*/
42-
public static function createFromPassword($password)
42+
public static function createFromPassword(
43+
#[\SensitiveParameter]
44+
$password
45+
)
4346
{
4447
return new KeyOrPassword(self::SECRET_TYPE_PASSWORD, $password);
4548
}
@@ -133,7 +136,11 @@ public function deriveKeys($salt)
133136
* @param int $secret_type
134137
* @param mixed $secret (either a Key or a password string)
135138
*/
136-
private function __construct($secret_type, $secret)
139+
private function __construct(
140+
$secret_type,
141+
#[\SensitiveParameter]
142+
$secret
143+
)
137144
{
138145
// The constructor is private, so these should never throw.
139146
if ($secret_type === self::SECRET_TYPE_KEY) {

‎src/KeyProtectedByPassword.php‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ final class KeyProtectedByPassword
2222
*
2323
* @return KeyProtectedByPassword
2424
*/
25-
public static function createRandomPasswordProtectedKey($password)
25+
public static function createRandomPasswordProtectedKey(
26+
#[\SensitiveParameter]
27+
$password
28+
)
2629
{
2730
$inner_key = Key::createNewRandomKey();
2831
/* The password is hashed as a form of poor-man's domain separation
@@ -47,7 +50,10 @@ public static function createRandomPasswordProtectedKey($password)
4750
*
4851
* @return KeyProtectedByPassword
4952
*/
50-
public static function loadFromAsciiSafeString($saved_key_string)
53+
public static function loadFromAsciiSafeString(
54+
#[\SensitiveParameter]
55+
$saved_key_string
56+
)
5157
{
5258
$encrypted_key = Encoding::loadBytesFromChecksummedAsciiSafeString(
5359
self::PASSWORD_KEY_CURRENT_VERSION,
@@ -82,7 +88,10 @@ public function saveToAsciiSafeString()
8288
* @param string $password
8389
* @return Key
8490
*/
85-
public function unlockKey($password)
91+
public function unlockKey(
92+
#[\SensitiveParameter]
93+
$password
94+
)
8695
{
8796
try {
8897
$inner_key_encoded = Crypto::decryptWithPassword(
@@ -115,7 +124,12 @@ public function unlockKey($password)
115124
*
116125
* @return KeyProtectedByPassword
117126
*/
118-
public function changePassword($current_password, $new_password)
127+
public function changePassword(
128+
#[\SensitiveParameter]
129+
$current_password,
130+
#[\SensitiveParameter]
131+
$new_password
132+
)
119133
{
120134
$inner_key = $this->unlockKey($current_password);
121135
/* The password is hashed as a form of poor-man's domain separation

‎test/phpunit-8.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<phpunit>
1+
<phpunit beStrictAboutTestsThatDoNotTestAnything="false">
22
<filter>
33
<whitelist processUncoveredFilesFromWhitelist="true">
44
<directory suffix=".php">../src</directory>

0 commit comments

Comments
 (0)