From 482235dbe05f734bc2e8d50a9da694eeec7dbe57 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Wed, 12 Apr 2023 07:24:53 +0000 Subject: [PATCH 01/19] Add initial Encryption methods. --- mu-plugins/encryption/class-hiddenstring.php | 164 +++++++++++++++++++ mu-plugins/encryption/exports.php | 56 +++++++ mu-plugins/encryption/index.php | 160 ++++++++++++++++++ mu-plugins/loader.php | 1 + 4 files changed, 381 insertions(+) create mode 100644 mu-plugins/encryption/class-hiddenstring.php create mode 100644 mu-plugins/encryption/exports.php create mode 100644 mu-plugins/encryption/index.php diff --git a/mu-plugins/encryption/class-hiddenstring.php b/mu-plugins/encryption/class-hiddenstring.php new file mode 100644 index 000000000..4cd5c9cb3 --- /dev/null +++ b/mu-plugins/encryption/class-hiddenstring.php @@ -0,0 +1,164 @@ +internalStringValue = self::safeStrcpy($value); + $this->disallowInline = $disallowInline; + $this->disallowSerialization = $disallowSerialization; + } + + /** + * @param HiddenString $other + * @return bool + * @throws \TypeError + */ + public function equals(HiddenString $other) + { + return \hash_equals( + $this->getString(), + $other->getString() + ); + } + + /** + * Hide its internal state from var_dump() + * + * @return array + */ + public function __debugInfo() + { + return [ + 'internalStringValue' => + '*', + 'attention' => + 'If you need the value of a HiddenString, ' . + 'invoke getString() instead of dumping it.' + ]; + } + + /** + * Wipe it from memory after it's been used. + * @return void + */ + public function __destruct() + { + if (\is_callable('\sodium_memzero')) { + try { + \sodium_memzero($this->internalStringValue); + return; + } catch (\Throwable $ex) { + } + } + } + + /** + * Explicit invocation -- get the raw string value + * + * @return string + * @throws \TypeError + */ + public function getString(): string + { + return self::safeStrcpy($this->internalStringValue); + } + + /** + * Returns a copy of the string's internal value, which should be zeroed. + * Optionally, it can return an empty string. + * + * @return string + * @throws \TypeError + */ + public function __toString(): string + { + if (!$this->disallowInline) { + return self::safeStrcpy($this->internalStringValue); + } + return ''; + } + + /** + * @return array + */ + public function __sleep(): array + { + if (!$this->disallowSerialization) { + return [ + 'internalStringValue', + 'disallowInline', + 'disallowSerialization' + ]; + } + return []; + } + + /** + * PHP 7 uses interned strings. We don't want altering this one to alter + * the original string. + * + * @param string $string + * @return string + * @throws \TypeError + */ + public static function safeStrcpy(string $string): string + { + $length = mb_strlen($string, '8bit'); + $return = ''; + /** @var int $chunk */ + $chunk = $length >> 1; + if ($chunk < 1) { + $chunk = 1; + } + for ($i = 0; $i < $length; $i += $chunk) { + $return .= mb_substr($string, $i, $chunk, '8bit'); + } + return $return; + } +} \ No newline at end of file diff --git a/mu-plugins/encryption/exports.php b/mu-plugins/encryption/exports.php new file mode 100644 index 000000000..8684dc7fe --- /dev/null +++ b/mu-plugins/encryption/exports.php @@ -0,0 +1,56 @@ +getString(), false ); + } catch ( Exception $e ) { + return false; + } +} + +/** + * Decrypt a value. + * + * Unlike the Encryption plugin, this function simply returns false for any errors, and + * HiddenStrings that can be cast to string as needed. + * + * @param string $value The encrypted value. + * @param string $key The key to use for decryption. Optional. + * @return string|false The decrypted value, or false on error. + */ +function wporg_decrypt( string $value, string $key = '' ) { + try { + $value = \WordPressdotorg\MU_Plugins\Encryption\decrypt( $value, '', $key ); + + return new HiddenString( $value->getString(), false ); + } catch ( Exception $e ) { + return false; + } +} + +/** + * Determine if a value is encrypted. + * + * @param string $value The value to check. + * @param string $key Check if it uses this key. Optional. If not specified, it only validates that it's encrypted. + * @return bool True if the value is encrypted, false otherwise. + */ +function wporg_is_encrypted( string $value, string $key = '' ) : bool { + return \WordPressdotorg\MU_Plugins\Encryption\is_encrypted( $value, $key ); +} \ No newline at end of file diff --git a/mu-plugins/encryption/index.php b/mu-plugins/encryption/index.php new file mode 100644 index 000000000..bb5b65926 --- /dev/null +++ b/mu-plugins/encryption/index.php @@ -0,0 +1,160 @@ + v1 (RFC 6238, encrypted with XChaCha20-Poly1305, with a key derived from HMAC-SHA256 + * of the defined key. + * + * @var string + */ +const PREFIX = '$t1$'; + +/** + * The length of the keys. + * + * @var int + */ +const KEY_LENGTH = SODIUM_CRYPTO_AEAD_XCHACHA20POLY1305_IETF_KEYBYTES; +const NONCE_LENGTH = SODIUM_CRYPTO_AEAD_XCHACHA20POLY1305_IETF_NPUBBYTES; + +/** + * Encrypt a value. + * + * @param string $value Value to encrypt. + * @param string $additional_data Additional data to include in the encryption. Optional. + * @param string $key Key to use for encryption. Optional. + * @return string Encrypted value, exceptions thrown on error. + */ +function encrypt( $value, string $additional_data = '', string $key = '' ) { + $key = get_encryption_key( $key ); + $nonce = random_bytes( NONCE_LENGTH ); + if ( ! $key || ! $nonce ) { + throw new Exception( 'Unable to create a nonce.' ); + } + + if ( $value instanceOf HiddenString ) { + $value = $value->getString(); + } + + $encrypted = sodium_crypto_aead_xchacha20poly1305_ietf_encrypt( $value, $additional_data, $nonce, $key->getString() ); + + sodium_memzero( $value ); + + return new HiddenString( PREFIX . sodium_bin2hex( $nonce . $encrypted ) ); +} + +/** + * Decrypt a value. + * + * @param string $value Value to decrypt. + * @param string $additional_data Additional data to include in the encryption. Optional. + * @param string $key Key to use for decryption. Optional. + * @return string Decrypted value. + */ +function decrypt( $value, string $additional_data = '', string $key = '' ) : HiddenString { + // Fetch the encryption key. + $key = get_encryption_key( $key ); + if ( ! $key ) { + throw new Exception( 'Unable to get the encryption key.' ); + } + + if ( $value instanceOf HiddenString ) { + $value = $value->getString(); + } + + if ( ! is_encrypted( $value ) ) { + throw new Exception( 'Value is not encrypted.' ); + } + + // Remove the prefix, and convert back to binary. + $value = mb_substr( $value, mb_strlen( PREFIX, '8bit' ), null, '8bit' ); + $value = sodium_hex2bin( $value ); + + if ( mb_strlen( $value, '8bit' ) < NONCE_LENGTH ) { + throw new Exception( 'Invalid cipher text' ); + } + + $nonce = mb_substr( $value, 0, NONCE_LENGTH, '8bit' ); + $value = mb_substr( $value, NONCE_LENGTH, null, '8bit' ); + $plaintext = sodium_crypto_aead_xchacha20poly1305_ietf_decrypt( $value, $additional_data, $nonce, $key->getString() ); + + sodium_memzero( $nonce ); + sodium_memzero( $value ); + + if ( false === $plaintext ) { + throw new Exception( 'Invalid cipher text' ); + } + + return new HiddenString( $plaintext ); +} + +/** + * Check if a value is encrypted. + * + * @param string $value Value to check. + * @param string $key Key to use for decryption. Optional. + * @return bool True if the value is encrypted, false otherwise. + */ +function is_encrypted( $value, string $key = '' ) { + if ( $value instanceOf HiddenString ) { + $value = $value->getString(); + } + + if ( ! str_starts_with( $value, PREFIX ) ) { + return false; + } + + if ( mb_strlen( $value, '8bit' ) < NONCE_LENGTH + strlen( PREFIX ) ) { + return false; + } + + // Check if that's the key. + if ( $key ) { + try { + if ( ! decrypt( $value, $key ) ) { + return false; + } + } catch ( Exception $e ) { + return false; + } + } + + return true; +} + +/** + * Get the encryption key. + * + * @param string $key The key to use for decryption. + * @return string The encryption key. + */ +function get_encryption_key( $key = '' ) { + $constant = 'WPORG_ENCRYPTION_KEY'; + + if ( $key ) { + $constant = 'WPORG_' . str_replace( '-', '_', strtoupper( $key ) ) . '_ENCRYPTION_KEY'; + } + + if ( ! defined( $constant ) ) { + throw new Exception( sprintf( 'Encryption key "%s" not defined.', $constant ) ); + } + + return new HiddenString( sodium_hex2bin( constant( $constant ) ) ); +} + +/** + * Generate a random encryption key. + * + * @return string The encryption key. + */ +function generate_encryption_key() { + return sodium_bin2hex( random_bytes( KEY_LENGTH ) ); +} \ No newline at end of file diff --git a/mu-plugins/loader.php b/mu-plugins/loader.php index e9ad6ee4c..c92cb8d0c 100644 --- a/mu-plugins/loader.php +++ b/mu-plugins/loader.php @@ -29,3 +29,4 @@ require_once __DIR__ . '/rest-api/index.php'; require_once __DIR__ . '/skip-to/skip-to.php'; require_once __DIR__ . '/db-user-sessions/index.php'; +require_once __DIR__ . '/encryption/index.php'; From 65cd2e69f6bff1ca2ee5fa177feb884bfff6e4c8 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 13 Apr 2023 01:27:52 +0000 Subject: [PATCH 02/19] Clarify what '$additional_data' is. --- mu-plugins/encryption/index.php | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/mu-plugins/encryption/index.php b/mu-plugins/encryption/index.php index bb5b65926..9b99bfc92 100644 --- a/mu-plugins/encryption/index.php +++ b/mu-plugins/encryption/index.php @@ -23,13 +23,19 @@ * @var int */ const KEY_LENGTH = SODIUM_CRYPTO_AEAD_XCHACHA20POLY1305_IETF_KEYBYTES; + +/** + * The length of the per-encrypted-item nonce. + * + * @var int + */ const NONCE_LENGTH = SODIUM_CRYPTO_AEAD_XCHACHA20POLY1305_IETF_NPUBBYTES; /** * Encrypt a value. * * @param string $value Value to encrypt. - * @param string $additional_data Additional data to include in the encryption. Optional. + * @param string $additional_data Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. Optional. * @param string $key Key to use for encryption. Optional. * @return string Encrypted value, exceptions thrown on error. */ @@ -55,7 +61,7 @@ function encrypt( $value, string $additional_data = '', string $key = '' ) { * Decrypt a value. * * @param string $value Value to decrypt. - * @param string $additional_data Additional data to include in the encryption. Optional. + * @param string $additional_data Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. Optional. * @param string $key Key to use for decryption. Optional. * @return string Decrypted value. */ @@ -136,7 +142,7 @@ function is_encrypted( $value, string $key = '' ) { * @param string $key The key to use for decryption. * @return string The encryption key. */ -function get_encryption_key( $key = '' ) { +function get_encryption_key( string $key = '' ) { $constant = 'WPORG_ENCRYPTION_KEY'; if ( $key ) { @@ -157,4 +163,4 @@ function get_encryption_key( $key = '' ) { */ function generate_encryption_key() { return sodium_bin2hex( random_bytes( KEY_LENGTH ) ); -} \ No newline at end of file +} From e887fb8b7567cb0f2c2253d980d6e252350d85e6 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 13 Apr 2023 01:35:17 +0000 Subject: [PATCH 03/19] Add wporg_authenticated_encrypt() and wporg_authenticated_decrypt() wrappers. --- mu-plugins/encryption/exports.php | 45 +++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/mu-plugins/encryption/exports.php b/mu-plugins/encryption/exports.php index 8684dc7fe..69a6330a7 100644 --- a/mu-plugins/encryption/exports.php +++ b/mu-plugins/encryption/exports.php @@ -2,6 +2,9 @@ use WordPressdotorg\MU_Plugins\Encryption\HiddenString; /** * This file contains globally-exported function names for the Encryption plugin. + * + * It provides a wrapper around the libsodium's Authenticated Encryption with + * Additional Data ciphers (AEAD with XChaCha20-Poly1305) */ /** @@ -24,6 +27,27 @@ function wporg_encrypt( string $value, string $key = '' ) { } } +/** + * Encrypt a value, with authentication. + * + * Unlike the Encryption plugin, this function simply returns false for any errors, and + * HiddenStrings that can be cast to string as needed. + * + * @param string $value The plaintext value. + * @param string $additional_data Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. Optional. + * @param string $key The key to use for encryption. Optional. + * @return string|false The encrypted value, or false on error. + */ +function wporg_authenticated_encrypt( string $value, string $additional_data = '',string $key = '' ) { + try { + $value = \WordPressdotorg\MU_Plugins\Encryption\encrypt( $value, $additional_data, $key ); + + return new HiddenString( $value->getString(), false ); + } catch ( Exception $e ) { + return false; + } +} + /** * Decrypt a value. * @@ -44,6 +68,27 @@ function wporg_decrypt( string $value, string $key = '' ) { } } +/** + * Decrypt a value, with authentication. + * + * Unlike the Encryption plugin, this function simply returns false for any errors, and + * HiddenStrings that can be cast to string as needed. + * + * @param string $value The encrypted value. + * @param string $additional_data Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. Optional. + * @param string $key The key to use for decryption. Optional. + * @return string|false The decrypted value, or false on error. + */ +function wporg_authenticated_decrypt( string $value, string $additional_data = '', string $key = '' ) { + try { + $value = \WordPressdotorg\MU_Plugins\Encryption\decrypt( $value, $additional_data, $key ); + + return new HiddenString( $value->getString(), false ); + } catch ( Exception $e ) { + return false; + } +} + /** * Determine if a value is encrypted. * From dc879db0c55a61f37fb7df68426068aefa03d824 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 13 Apr 2023 01:36:27 +0000 Subject: [PATCH 04/19] Remove the $key parameter from is_encrypted(). It's not an ideal scenario that should be needed. --- mu-plugins/encryption/exports.php | 5 ++--- mu-plugins/encryption/index.php | 14 +------------- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/mu-plugins/encryption/exports.php b/mu-plugins/encryption/exports.php index 69a6330a7..b521917d8 100644 --- a/mu-plugins/encryption/exports.php +++ b/mu-plugins/encryption/exports.php @@ -93,9 +93,8 @@ function wporg_authenticated_decrypt( string $value, string $additional_data = ' * Determine if a value is encrypted. * * @param string $value The value to check. - * @param string $key Check if it uses this key. Optional. If not specified, it only validates that it's encrypted. * @return bool True if the value is encrypted, false otherwise. */ -function wporg_is_encrypted( string $value, string $key = '' ) : bool { - return \WordPressdotorg\MU_Plugins\Encryption\is_encrypted( $value, $key ); +function wporg_is_encrypted( string $value ) : bool { + return \WordPressdotorg\MU_Plugins\Encryption\is_encrypted( $value ); } \ No newline at end of file diff --git a/mu-plugins/encryption/index.php b/mu-plugins/encryption/index.php index 9b99bfc92..c0da8e02e 100644 --- a/mu-plugins/encryption/index.php +++ b/mu-plugins/encryption/index.php @@ -106,10 +106,9 @@ function decrypt( $value, string $additional_data = '', string $key = '' ) : Hid * Check if a value is encrypted. * * @param string $value Value to check. - * @param string $key Key to use for decryption. Optional. * @return bool True if the value is encrypted, false otherwise. */ -function is_encrypted( $value, string $key = '' ) { +function is_encrypted( $value ) { if ( $value instanceOf HiddenString ) { $value = $value->getString(); } @@ -122,17 +121,6 @@ function is_encrypted( $value, string $key = '' ) { return false; } - // Check if that's the key. - if ( $key ) { - try { - if ( ! decrypt( $value, $key ) ) { - return false; - } - } catch ( Exception $e ) { - return false; - } - } - return true; } From 542fe9f794b87ada0f4aa9fcdf7bddefdab18a3d Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 13 Apr 2023 12:49:30 +1000 Subject: [PATCH 05/19] Always use mb_strlen() for consistentness. --- mu-plugins/encryption/index.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mu-plugins/encryption/index.php b/mu-plugins/encryption/index.php index c0da8e02e..e686272c7 100644 --- a/mu-plugins/encryption/index.php +++ b/mu-plugins/encryption/index.php @@ -117,7 +117,7 @@ function is_encrypted( $value ) { return false; } - if ( mb_strlen( $value, '8bit' ) < NONCE_LENGTH + strlen( PREFIX ) ) { + if ( mb_strlen( $value, '8bit' ) < NONCE_LENGTH + mb_strlen( PREFIX, '8bit' ) ) { return false; } From 636e7920e1000df742c609a2ccd4c4e2ee4cab8f Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 13 Apr 2023 13:06:04 +1000 Subject: [PATCH 06/19] Tests: Add tests for the Encryption methods. --- mu-plugins/encryption/exports.php | 2 +- phpunit/test-encryption.php | 163 ++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 phpunit/test-encryption.php diff --git a/mu-plugins/encryption/exports.php b/mu-plugins/encryption/exports.php index b521917d8..500c17f2e 100644 --- a/mu-plugins/encryption/exports.php +++ b/mu-plugins/encryption/exports.php @@ -38,7 +38,7 @@ function wporg_encrypt( string $value, string $key = '' ) { * @param string $key The key to use for encryption. Optional. * @return string|false The encrypted value, or false on error. */ -function wporg_authenticated_encrypt( string $value, string $additional_data = '',string $key = '' ) { +function wporg_authenticated_encrypt( string $value, string $additional_data = '', string $key = '' ) { try { $value = \WordPressdotorg\MU_Plugins\Encryption\encrypt( $value, $additional_data, $key ); diff --git a/phpunit/test-encryption.php b/phpunit/test-encryption.php new file mode 100644 index 000000000..d670da2d8 --- /dev/null +++ b/phpunit/test-encryption.php @@ -0,0 +1,163 @@ +assertNotEquals( $input, $encrypted ); + + $decrypted = decrypt( $encrypted ); + + $this->assertTrue( $decrypted instanceOf HiddenString ); + + $this->assertNotEquals( $input, $decrypted ); + $this->assertEquals( $input, $decrypted->getString() ); + $this->assertNotEquals( $input, (string) $decrypted ); + } + + public function test_authenticated_encrypt_decrypt() { + $input = 'This is a plaintext string. It contains no sensitive data.'; + $additional_data = 'USER1'; + + $encrypted = encrypt( $input, $additional_data ); + + $this->assertNotEquals( $input, $encrypted ); + $this->assertStringNotContainsString( $additional_data, $encrypted ); + + $this->expectException( Exception::class ); + $decrypt_without_data = decrypt( $encrypted ); + + $this->expectException( Exception::class ); + $decrypt_with_wrong_data = decrypt( $encrypted, 'USER2' ); + + $this->expectException( Exception::class ); + $decrypt_with_wrong_key = decrypt( $encrypted, $additional_data, 'secondary' ); + + $decrypted = decrypt( $encrypted, $additional_data ); + + $this->assertTrue( $decrypted instanceOf HiddenString ); + + $this->assertNotEquals( $input, $decrypted ); + $this->assertEquals( $input, $decrypted->getString() ); + } + + public function test_is_encrypted() { + $this->assertFalse( is_encrypted( 'TEST STRING' ) ); + $this->assertFalse( is_encrypted( PREFIX ) ); + $this->assertFalse( is_encrypted( PREFIX . 'TEST STRING' ) ); + + $string_prefix_length = str_repeat( '.', mb_strlen( PREFIX, '8bit' ) ); + $string_nonce_length = str_repeat( '.', NONCE_LENGTH ); + + $this->assertFalse( is_encrypted( $string_prefix_length . $string_nonce_length ) ); + $this->assertFalse( is_encrypted( $string_prefix_length . $string_nonce_length . 'TEST STRING' ) ); + + $this->assertTrue( is_encrypted( PREFIX . $string_nonce_length ) ); + $this->assertTrue( is_encrypted( PREFIX . $string_nonce_length . 'TEST STRING' ) ); + + $test_string = 'This is a plaintext string. It contains no sensitive data.'; + $this->assertTrue( is_encrypted( encrypt( $test_string ) ) ); + } + + public function test_generate_key_different() { + $one_key = generate_encryption_key(); + + $length = mb_strlen( sodium_hex2bin( $one_key ), '8bit' ); + $this->assertEquals( KEY_LENGTH, $length ); + + $two_key = generate_encryption_key(); + $this->assertNotEquals( $one_key, $two_key ); + } + + public function test_get_encryption_key() { + $this->assertSame( sodium_hex2bin( WPORG_ENCRYPTION_KEY ), get_encryption_key()->getString() ); + $this->assertSame( sodium_hex2bin( WPORG_ENCRYPTION_KEY ), get_encryption_key( '' )->getString() ); + $this->assertSame( sodium_hex2bin( WPORG_ENCRYPTION_KEY ), get_encryption_key( false )->getString() ); + + $this->assertSame( sodium_hex2bin( WPORG_SECONDARY_ENCRYPTION_KEY ), get_encryption_key( 'secondary' )->getString() ); + + $this->expectException( Exception::class ); + get_encryption_key( '404-key' ); + } + + public function test_can_encrypt_hiddenstring() { + $hidden_string = new HiddenString( "TEST STRING" ); + + $encrypted = encrypt( $hidden_string ); + + $this->assertTrue( is_encrypted( $encrypted ) ); + + $this->assertSame( $hidden_string->getString(), decrypt( $encrypted )->getString() ); + } + + public function test_encrypt_decrypt_invalid_inputs() { + $this->expectException( Exception::class ); + encrypt( "TEST STRING", '', '404-key' ); + + $this->expectException( Exception::class ); + decrypt( "TEST STRING", '', '404-key' ); + + $this->expectException( Exception::class ); + decrypt( "TEST STRING" ); + } + + public function test_exported_functions() { + // This only tests the behavioural functions, not the encryption/decryption. + + $input = 'This is a plaintext string. It contains no sensitive data.'; + + $encrypted = wporg_encrypt( $input ); + + $this->assertNotEquals( $input, $encrypted ); + + $decrypted = wporg_decrypt( $encrypted ); + + $this->assertTrue( $decrypted instanceOf HiddenString ); + + $this->assertNotSame( $input, $decrypted ); + $this->assertEquals( $input, $decrypted->getString() ); + $this->assertEquals( $input, (string) $decrypted ); + + $this->assertFalse( wporg_encrypt( '', '404-key' ) ); + $this->assertFalse( wporg_decrypt( '', '404-key' ) ); + $this->assertFalse( wporg_decrypt( 'TEST STRING' ) ); + } + + public function test_exported_authenticated_functions() { + // This only tests the behavioural functions, not the encryption/decryption. + + $input = 'This is a plaintext string. It contains no sensitive data.'; + $additional_data = 'USER1'; + + $encrypted = wporg_authenticated_encrypt( $input, $additional_data ); + + $this->assertNotEquals( $input, $encrypted ); + + $decrypted = wporg_authenticated_decrypt( $encrypted, $additional_data ); + + $this->assertTrue( $decrypted instanceOf HiddenString ); + + $this->assertNotSame( $input, $decrypted ); + $this->assertEquals( $input, $decrypted->getString() ); + $this->assertEquals( $input, (string) $decrypted ); + + $this->assertFalse( wporg_authenticated_encrypt( '', '', '404-key' ) ); + $this->assertFalse( wporg_authenticated_decrypt( '', '', '404-key' ) ); + $this->assertFalse( wporg_authenticated_decrypt( 'TEST STRING' ) ); + } +} From 0b6a97981f4aa13e1b2f23f306091d6fa1c66cf7 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Fri, 14 Apr 2023 04:20:26 +0000 Subject: [PATCH 07/19] Whitespace and zero'ing of strings. --- mu-plugins/encryption/index.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/mu-plugins/encryption/index.php b/mu-plugins/encryption/index.php index e686272c7..71885ded4 100644 --- a/mu-plugins/encryption/index.php +++ b/mu-plugins/encryption/index.php @@ -22,7 +22,7 @@ * * @var int */ -const KEY_LENGTH = SODIUM_CRYPTO_AEAD_XCHACHA20POLY1305_IETF_KEYBYTES; +const KEY_LENGTH = SODIUM_CRYPTO_AEAD_XCHACHA20POLY1305_IETF_KEYBYTES; /** * The length of the per-encrypted-item nonce. @@ -40,8 +40,8 @@ * @return string Encrypted value, exceptions thrown on error. */ function encrypt( $value, string $additional_data = '', string $key = '' ) { - $key = get_encryption_key( $key ); - $nonce = random_bytes( NONCE_LENGTH ); + $key = get_encryption_key( $key ); + $nonce = random_bytes( NONCE_LENGTH ); if ( ! $key || ! $nonce ) { throw new Exception( 'Unable to create a nonce.' ); } @@ -121,6 +121,8 @@ function is_encrypted( $value ) { return false; } + sodium_memzero( $value ); + return true; } From 52133b16c8d285a96546221624c0290b7433f7eb Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Fri, 14 Apr 2023 04:44:43 +0000 Subject: [PATCH 08/19] Standardise on Exceptions with a trailing full-stop. --- mu-plugins/encryption/index.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mu-plugins/encryption/index.php b/mu-plugins/encryption/index.php index 71885ded4..6b3ed1c87 100644 --- a/mu-plugins/encryption/index.php +++ b/mu-plugins/encryption/index.php @@ -85,7 +85,7 @@ function decrypt( $value, string $additional_data = '', string $key = '' ) : Hid $value = sodium_hex2bin( $value ); if ( mb_strlen( $value, '8bit' ) < NONCE_LENGTH ) { - throw new Exception( 'Invalid cipher text' ); + throw new Exception( 'Invalid cipher text.' ); } $nonce = mb_substr( $value, 0, NONCE_LENGTH, '8bit' ); @@ -96,7 +96,7 @@ function decrypt( $value, string $additional_data = '', string $key = '' ) : Hid sodium_memzero( $value ); if ( false === $plaintext ) { - throw new Exception( 'Invalid cipher text' ); + throw new Exception( 'Invalid cipher text.' ); } return new HiddenString( $plaintext ); From e50fd6336dfbca0353be642e5a23240ac5002235 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Fri, 14 Apr 2023 04:47:35 +0000 Subject: [PATCH 09/19] get_encryption_key() throws an exception, there's no need to check it's return value as a result. --- mu-plugins/encryption/index.php | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/mu-plugins/encryption/index.php b/mu-plugins/encryption/index.php index 6b3ed1c87..faeaab891 100644 --- a/mu-plugins/encryption/index.php +++ b/mu-plugins/encryption/index.php @@ -40,9 +40,8 @@ * @return string Encrypted value, exceptions thrown on error. */ function encrypt( $value, string $additional_data = '', string $key = '' ) { - $key = get_encryption_key( $key ); $nonce = random_bytes( NONCE_LENGTH ); - if ( ! $key || ! $nonce ) { + if ( ! $nonce ) { throw new Exception( 'Unable to create a nonce.' ); } @@ -50,6 +49,7 @@ function encrypt( $value, string $additional_data = '', string $key = '' ) { $value = $value->getString(); } + $key = get_encryption_key( $key ); $encrypted = sodium_crypto_aead_xchacha20poly1305_ietf_encrypt( $value, $additional_data, $nonce, $key->getString() ); sodium_memzero( $value ); @@ -66,12 +66,6 @@ function encrypt( $value, string $additional_data = '', string $key = '' ) { * @return string Decrypted value. */ function decrypt( $value, string $additional_data = '', string $key = '' ) : HiddenString { - // Fetch the encryption key. - $key = get_encryption_key( $key ); - if ( ! $key ) { - throw new Exception( 'Unable to get the encryption key.' ); - } - if ( $value instanceOf HiddenString ) { $value = $value->getString(); } @@ -88,6 +82,7 @@ function decrypt( $value, string $additional_data = '', string $key = '' ) : Hid throw new Exception( 'Invalid cipher text.' ); } + $key = get_encryption_key( $key ); $nonce = mb_substr( $value, 0, NONCE_LENGTH, '8bit' ); $value = mb_substr( $value, NONCE_LENGTH, null, '8bit' ); $plaintext = sodium_crypto_aead_xchacha20poly1305_ietf_decrypt( $value, $additional_data, $nonce, $key->getString() ); From 0651e3b0eb174e4721ce78959988d2c57e286c51 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Fri, 14 Apr 2023 14:56:04 +1000 Subject: [PATCH 10/19] Tests: Don't use `expectException()` and instead test the exceptions thrown specifically. Ensures that test functions throwing unexpected Errors are caught. --- phpunit/test-encryption.php | 117 ++++++++++++++++++++++++++++++------ 1 file changed, 99 insertions(+), 18 deletions(-) diff --git a/phpunit/test-encryption.php b/phpunit/test-encryption.php index d670da2d8..0edebe93e 100644 --- a/phpunit/test-encryption.php +++ b/phpunit/test-encryption.php @@ -39,14 +39,45 @@ public function test_authenticated_encrypt_decrypt() { $this->assertNotEquals( $input, $encrypted ); $this->assertStringNotContainsString( $additional_data, $encrypted ); - $this->expectException( Exception::class ); - $decrypt_without_data = decrypt( $encrypted ); + // Decrypt without $additional_data. + try { + decrypt( $encrypted ); + } catch( Exception $e ) { + $this->assertEquals( 'Invalid cipher text.', $e->getMessage() ); + } finally { + $this->assertNotEmpty( $e, 'No Exception thrown?' ); + unset( $e ); + } - $this->expectException( Exception::class ); - $decrypt_with_wrong_data = decrypt( $encrypted, 'USER2' ); + // Decrypt with incorrect $additional_data. + try { + decrypt( $encrypted, 'USER2' ); + } catch( Exception $e ) { + $this->assertEquals( 'Invalid cipher text.', $e->getMessage() ); + } finally { + $this->assertNotEmpty( $e, 'No Exception thrown?' ); + unset( $e ); + } - $this->expectException( Exception::class ); - $decrypt_with_wrong_key = decrypt( $encrypted, $additional_data, 'secondary' ); + // Decrypt with incorrect key specified. + try { + decrypt( $encrypted, $additional_data, 'secondary' ); + } catch( Exception $e ) { + $this->assertEquals( 'Invalid cipher text.', $e->getMessage() ); + } finally { + $this->assertNotEmpty( $e, 'No Exception thrown?' ); + unset( $e ); + } + + // Decrypt with unknown key specified. + try { + decrypt( $encrypted, $additional_data, 'unknown-key' ); + } catch( Exception $e ) { + $this->assertEquals( 'Encryption key "WPORG_UNKNOWN_KEY_ENCRYPTION_KEY" not defined.', $e->getMessage() ); + } finally { + $this->assertNotEmpty( $e, 'No Exception thrown?' ); + unset( $e ); + } $decrypted = decrypt( $encrypted, $additional_data ); @@ -91,8 +122,15 @@ public function test_get_encryption_key() { $this->assertSame( sodium_hex2bin( WPORG_SECONDARY_ENCRYPTION_KEY ), get_encryption_key( 'secondary' )->getString() ); - $this->expectException( Exception::class ); - get_encryption_key( '404-key' ); + // Get an unknown key. + try { + get_encryption_key( 'unknown-key' ); + } catch( Exception $e ) { + $this->assertEquals( 'Encryption key "WPORG_UNKNOWN_KEY_ENCRYPTION_KEY" not defined.', $e->getMessage() ); + } finally { + $this->assertNotEmpty( $e, 'No Exception thrown?' ); + unset( $e ); + } } public function test_can_encrypt_hiddenstring() { @@ -106,14 +144,57 @@ public function test_can_encrypt_hiddenstring() { } public function test_encrypt_decrypt_invalid_inputs() { - $this->expectException( Exception::class ); - encrypt( "TEST STRING", '', '404-key' ); + // Invalid key specified. + try { + encrypt( 'TEST STRING', '', 'unknown-key' ); + } catch( Exception $e ) { + $this->assertEquals( 'Encryption key "WPORG_UNKNOWN_KEY_ENCRYPTION_KEY" not defined.', $e->getMessage() ); + } finally { + $this->assertNotEmpty( $e, 'No Exception thrown?' ); + unset( $e ); + } - $this->expectException( Exception::class ); - decrypt( "TEST STRING", '', '404-key' ); + // Not-encrypted Invalid data that. + try { + decrypt( PREFIX . 'TESTSTRINGTESTSTRINGTESTSTRINGTESTSTRINGTESTSTRINGTESTSTRING' ); + } catch( Exception $e ) { + // This is thrown by sodium_hex2bin(). + $this->assertEquals( 'invalid hex string', $e->getMessage() ); + } finally { + $this->assertNotEmpty( $e, 'No Exception thrown?' ); + unset( $e ); + } + + // Not-encrypted Possibly-valid data. + try { + decrypt( PREFIX . '012345678901234567890123456789012345678901234567890123456789' ); + } catch( Exception $e ) { + $this->assertEquals( 'Invalid cipher text.', $e->getMessage() ); + } finally { + $this->assertNotEmpty( $e, 'No Exception thrown?' ); + unset( $e ); + } + + // Invalid key specified, not-encrypted data that's not long enough. + try { + decrypt( 'TEST STRING', '', 'unknown-key' ); + } catch( Exception $e ) { + $this->assertEquals( 'Value is not encrypted.', $e->getMessage() ); + } finally { + $this->assertNotEmpty( $e, 'No Exception thrown?' ); + unset( $e ); + } + + // Not-encrypted data that's not long enough. + try { + decrypt( 'TEST STRING' ); + } catch( Exception $e ) { + $this->assertEquals( 'Value is not encrypted.', $e->getMessage() ); + } finally { + $this->assertNotEmpty( $e, 'No Exception thrown?' ); + unset( $e ); + } - $this->expectException( Exception::class ); - decrypt( "TEST STRING" ); } public function test_exported_functions() { @@ -133,8 +214,8 @@ public function test_exported_functions() { $this->assertEquals( $input, $decrypted->getString() ); $this->assertEquals( $input, (string) $decrypted ); - $this->assertFalse( wporg_encrypt( '', '404-key' ) ); - $this->assertFalse( wporg_decrypt( '', '404-key' ) ); + $this->assertFalse( wporg_encrypt( '', 'unknown-key' ) ); + $this->assertFalse( wporg_decrypt( '', 'unknown-key' ) ); $this->assertFalse( wporg_decrypt( 'TEST STRING' ) ); } @@ -156,8 +237,8 @@ public function test_exported_authenticated_functions() { $this->assertEquals( $input, $decrypted->getString() ); $this->assertEquals( $input, (string) $decrypted ); - $this->assertFalse( wporg_authenticated_encrypt( '', '', '404-key' ) ); - $this->assertFalse( wporg_authenticated_decrypt( '', '', '404-key' ) ); + $this->assertFalse( wporg_authenticated_encrypt( '', '', 'unknown-key' ) ); + $this->assertFalse( wporg_authenticated_decrypt( '', '', 'unknown-key' ) ); $this->assertFalse( wporg_authenticated_decrypt( 'TEST STRING' ) ); } } From 6c9d58adc00f5dc4509e759a0859a5a88558ae0c Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Tue, 18 Apr 2023 12:19:38 +1000 Subject: [PATCH 11/19] Use #[\SensitiveParameter] within HiddenString. --- mu-plugins/encryption/class-hiddenstring.php | 1 + 1 file changed, 1 insertion(+) diff --git a/mu-plugins/encryption/class-hiddenstring.php b/mu-plugins/encryption/class-hiddenstring.php index 4cd5c9cb3..a28241a99 100644 --- a/mu-plugins/encryption/class-hiddenstring.php +++ b/mu-plugins/encryption/class-hiddenstring.php @@ -45,6 +45,7 @@ final class HiddenString * @throws \TypeError */ public function __construct( + #[\SensitiveParameter] string $value, bool $disallowInline = true, bool $disallowSerialization = true From 3d5b2f6fdc39a80a6419971e4b3bc9973c348276 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Tue, 18 Apr 2023 12:29:35 +1000 Subject: [PATCH 12/19] Switch from storing keys in constants, and instead within the return value from a function. This allows them to be stored as HiddenString's. --- mu-plugins/encryption/index.php | 18 +++++++++------ phpunit/test-encryption.php | 41 ++++++++++++++++++++++----------- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/mu-plugins/encryption/index.php b/mu-plugins/encryption/index.php index faeaab891..f324f9a5a 100644 --- a/mu-plugins/encryption/index.php +++ b/mu-plugins/encryption/index.php @@ -128,17 +128,21 @@ function is_encrypted( $value ) { * @return string The encryption key. */ function get_encryption_key( string $key = '' ) { - $constant = 'WPORG_ENCRYPTION_KEY'; - if ( $key ) { - $constant = 'WPORG_' . str_replace( '-', '_', strtoupper( $key ) ) . '_ENCRYPTION_KEY'; + $keys = []; + if ( function_exists( 'wporg_encryption_keys' ) ) { + $keys = wporg_encryption_keys(); } - if ( ! defined( $constant ) ) { - throw new Exception( sprintf( 'Encryption key "%s" not defined.', $constant ) ); + if ( ! $key ) { + $key = 'default'; } - return new HiddenString( sodium_hex2bin( constant( $constant ) ) ); + if ( ! isset( $keys[ $key ] ) ) { + throw new Exception( sprintf( 'Encryption key "%s" not defined.', $key ) ); + } + + return $keys[ $key ]; } /** @@ -147,5 +151,5 @@ function get_encryption_key( string $key = '' ) { * @return string The encryption key. */ function generate_encryption_key() { - return sodium_bin2hex( random_bytes( KEY_LENGTH ) ); + return new HiddenString( random_bytes( KEY_LENGTH ) ); } diff --git a/phpunit/test-encryption.php b/phpunit/test-encryption.php index 0edebe93e..db157fe66 100644 --- a/phpunit/test-encryption.php +++ b/phpunit/test-encryption.php @@ -6,11 +6,25 @@ class Test_WPORG_Encryption extends WP_UnitTestCase { public function wpSetUpBeforeClass() { - if ( ! defined( 'WPORG_ENCRYPTION_KEY' ) ) { - define( 'WPORG_ENCRYPTION_KEY', generate_encryption_key() ); + self::_wporg_encryption_keys(); + } + + public static function _wporg_encryption_keys() { + if ( function_exists( 'wporg_encryption_keys' ) ) { + return; } - if ( ! defined( 'WPORG_SECONDARY_ENCRYPTION_KEY' ) ) { - define( 'WPORG_SECONDARY_ENCRYPTION_KEY', generate_encryption_key() ); + + function wporg_encryption_keys() { + static $keys = false; + + if ( ! $keys ) { + $keys = [ + 'default' => generate_encryption_key(), + 'secondary' => generate_encryption_key(), + ]; + } + + return $keys; } } @@ -73,7 +87,7 @@ public function test_authenticated_encrypt_decrypt() { try { decrypt( $encrypted, $additional_data, 'unknown-key' ); } catch( Exception $e ) { - $this->assertEquals( 'Encryption key "WPORG_UNKNOWN_KEY_ENCRYPTION_KEY" not defined.', $e->getMessage() ); + $this->assertEquals( 'Encryption key "unknown-key" not defined.', $e->getMessage() ); } finally { $this->assertNotEmpty( $e, 'No Exception thrown?' ); unset( $e ); @@ -108,25 +122,25 @@ public function test_is_encrypted() { public function test_generate_key_different() { $one_key = generate_encryption_key(); - $length = mb_strlen( sodium_hex2bin( $one_key ), '8bit' ); + $length = mb_strlen( $one_key->getString(), '8bit' ); $this->assertEquals( KEY_LENGTH, $length ); $two_key = generate_encryption_key(); - $this->assertNotEquals( $one_key, $two_key ); + $this->assertNotEquals( $one_key->getString(), $two_key->getString() ); } public function test_get_encryption_key() { - $this->assertSame( sodium_hex2bin( WPORG_ENCRYPTION_KEY ), get_encryption_key()->getString() ); - $this->assertSame( sodium_hex2bin( WPORG_ENCRYPTION_KEY ), get_encryption_key( '' )->getString() ); - $this->assertSame( sodium_hex2bin( WPORG_ENCRYPTION_KEY ), get_encryption_key( false )->getString() ); + $this->assertSame( wporg_encryption_keys()['default']->getString(), get_encryption_key()->getString() ); + $this->assertSame( wporg_encryption_keys()['default']->getString(), get_encryption_key( '' )->getString() ); + $this->assertSame( wporg_encryption_keys()['default']->getString(), get_encryption_key( false )->getString() ); - $this->assertSame( sodium_hex2bin( WPORG_SECONDARY_ENCRYPTION_KEY ), get_encryption_key( 'secondary' )->getString() ); + $this->assertSame( wporg_encryption_keys()['secondary']->getString(), get_encryption_key( 'secondary' )->getString() ); // Get an unknown key. try { get_encryption_key( 'unknown-key' ); } catch( Exception $e ) { - $this->assertEquals( 'Encryption key "WPORG_UNKNOWN_KEY_ENCRYPTION_KEY" not defined.', $e->getMessage() ); + $this->assertEquals( 'Encryption key "unknown-key" not defined.', $e->getMessage() ); } finally { $this->assertNotEmpty( $e, 'No Exception thrown?' ); unset( $e ); @@ -148,7 +162,7 @@ public function test_encrypt_decrypt_invalid_inputs() { try { encrypt( 'TEST STRING', '', 'unknown-key' ); } catch( Exception $e ) { - $this->assertEquals( 'Encryption key "WPORG_UNKNOWN_KEY_ENCRYPTION_KEY" not defined.', $e->getMessage() ); + $this->assertEquals( 'Encryption key "unknown-key" not defined.', $e->getMessage() ); } finally { $this->assertNotEmpty( $e, 'No Exception thrown?' ); unset( $e ); @@ -241,4 +255,5 @@ public function test_exported_authenticated_functions() { $this->assertFalse( wporg_authenticated_decrypt( '', '', 'unknown-key' ) ); $this->assertFalse( wporg_authenticated_decrypt( 'TEST STRING' ) ); } + } From 3caed5629abf96edb798faba3bf8a03ce198c7a7 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Tue, 18 Apr 2023 12:32:17 +1000 Subject: [PATCH 13/19] Encrypted values don't need to return HiddenString. --- mu-plugins/encryption/exports.php | 14 ++++---------- mu-plugins/encryption/index.php | 2 +- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/mu-plugins/encryption/exports.php b/mu-plugins/encryption/exports.php index 500c17f2e..d164c9ab0 100644 --- a/mu-plugins/encryption/exports.php +++ b/mu-plugins/encryption/exports.php @@ -10,8 +10,7 @@ /** * Encrypt a value. * - * Unlike the Encryption plugin, this function simply returns false for any errors, and - * HiddenStrings that can be cast to string as needed. + * Unlike the Encryption plugin, this function simply returns false for any errors. * * @param string $value The plaintext value. * @param string $key The key to use for encryption. Optional. @@ -19,9 +18,7 @@ */ function wporg_encrypt( string $value, string $key = '' ) { try { - $value = \WordPressdotorg\MU_Plugins\Encryption\encrypt( $value, '', $key ); - - return new HiddenString( $value->getString(), false ); + return \WordPressdotorg\MU_Plugins\Encryption\encrypt( $value, '', $key ); } catch ( Exception $e ) { return false; } @@ -30,8 +27,7 @@ function wporg_encrypt( string $value, string $key = '' ) { /** * Encrypt a value, with authentication. * - * Unlike the Encryption plugin, this function simply returns false for any errors, and - * HiddenStrings that can be cast to string as needed. + * Unlike the Encryption plugin, this function simply returns false for any errors. * * @param string $value The plaintext value. * @param string $additional_data Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. Optional. @@ -40,9 +36,7 @@ function wporg_encrypt( string $value, string $key = '' ) { */ function wporg_authenticated_encrypt( string $value, string $additional_data = '', string $key = '' ) { try { - $value = \WordPressdotorg\MU_Plugins\Encryption\encrypt( $value, $additional_data, $key ); - - return new HiddenString( $value->getString(), false ); + return \WordPressdotorg\MU_Plugins\Encryption\encrypt( $value, $additional_data, $key ); } catch ( Exception $e ) { return false; } diff --git a/mu-plugins/encryption/index.php b/mu-plugins/encryption/index.php index f324f9a5a..1549f7091 100644 --- a/mu-plugins/encryption/index.php +++ b/mu-plugins/encryption/index.php @@ -54,7 +54,7 @@ function encrypt( $value, string $additional_data = '', string $key = '' ) { sodium_memzero( $value ); - return new HiddenString( PREFIX . sodium_bin2hex( $nonce . $encrypted ) ); + return PREFIX . sodium_bin2hex( $nonce . $encrypted ); } /** From 6f8d488765aab108b4c4197a25ba5ff0d4e7b7ef Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Tue, 18 Apr 2023 12:40:34 +1000 Subject: [PATCH 14/19] Remove the non-auth encryption methods, and require $additional_data be specified. --- mu-plugins/encryption/exports.php | 41 +------------------ mu-plugins/encryption/index.php | 10 +++-- phpunit/test-encryption.php | 68 +++++++------------------------ 3 files changed, 24 insertions(+), 95 deletions(-) diff --git a/mu-plugins/encryption/exports.php b/mu-plugins/encryption/exports.php index d164c9ab0..af8ec517b 100644 --- a/mu-plugins/encryption/exports.php +++ b/mu-plugins/encryption/exports.php @@ -7,23 +7,6 @@ * Additional Data ciphers (AEAD with XChaCha20-Poly1305) */ -/** - * Encrypt a value. - * - * Unlike the Encryption plugin, this function simply returns false for any errors. - * - * @param string $value The plaintext value. - * @param string $key The key to use for encryption. Optional. - * @return string|false The encrypted value, or false on error. - */ -function wporg_encrypt( string $value, string $key = '' ) { - try { - return \WordPressdotorg\MU_Plugins\Encryption\encrypt( $value, '', $key ); - } catch ( Exception $e ) { - return false; - } -} - /** * Encrypt a value, with authentication. * @@ -34,7 +17,7 @@ function wporg_encrypt( string $value, string $key = '' ) { * @param string $key The key to use for encryption. Optional. * @return string|false The encrypted value, or false on error. */ -function wporg_authenticated_encrypt( string $value, string $additional_data = '', string $key = '' ) { +function wporg_encrypt( $value, string $additional_data, string $key = '' ) { try { return \WordPressdotorg\MU_Plugins\Encryption\encrypt( $value, $additional_data, $key ); } catch ( Exception $e ) { @@ -42,26 +25,6 @@ function wporg_authenticated_encrypt( string $value, string $additional_data = ' } } -/** - * Decrypt a value. - * - * Unlike the Encryption plugin, this function simply returns false for any errors, and - * HiddenStrings that can be cast to string as needed. - * - * @param string $value The encrypted value. - * @param string $key The key to use for decryption. Optional. - * @return string|false The decrypted value, or false on error. - */ -function wporg_decrypt( string $value, string $key = '' ) { - try { - $value = \WordPressdotorg\MU_Plugins\Encryption\decrypt( $value, '', $key ); - - return new HiddenString( $value->getString(), false ); - } catch ( Exception $e ) { - return false; - } -} - /** * Decrypt a value, with authentication. * @@ -73,7 +36,7 @@ function wporg_decrypt( string $value, string $key = '' ) { * @param string $key The key to use for decryption. Optional. * @return string|false The decrypted value, or false on error. */ -function wporg_authenticated_decrypt( string $value, string $additional_data = '', string $key = '' ) { +function wporg_decrypt( string $value, string $additional_data, string $key = '' ) { try { $value = \WordPressdotorg\MU_Plugins\Encryption\decrypt( $value, $additional_data, $key ); diff --git a/mu-plugins/encryption/index.php b/mu-plugins/encryption/index.php index 1549f7091..5033ef731 100644 --- a/mu-plugins/encryption/index.php +++ b/mu-plugins/encryption/index.php @@ -39,12 +39,16 @@ * @param string $key Key to use for encryption. Optional. * @return string Encrypted value, exceptions thrown on error. */ -function encrypt( $value, string $additional_data = '', string $key = '' ) { +function encrypt( $value, string $additional_data, string $key = '' ) { $nonce = random_bytes( NONCE_LENGTH ); if ( ! $nonce ) { throw new Exception( 'Unable to create a nonce.' ); } + if ( empty( $additional_data ) ) { + throw new Exception( '$additional_data cannot be empty.' ); + } + if ( $value instanceOf HiddenString ) { $value = $value->getString(); } @@ -63,9 +67,9 @@ function encrypt( $value, string $additional_data = '', string $key = '' ) { * @param string $value Value to decrypt. * @param string $additional_data Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. Optional. * @param string $key Key to use for decryption. Optional. - * @return string Decrypted value. + * @return HiddenString Decrypted value. */ -function decrypt( $value, string $additional_data = '', string $key = '' ) : HiddenString { +function decrypt( $value, string $additional_data, string $key = '' ) : HiddenString { if ( $value instanceOf HiddenString ) { $value = $value->getString(); } diff --git a/phpunit/test-encryption.php b/phpunit/test-encryption.php index db157fe66..41d5e8691 100644 --- a/phpunit/test-encryption.php +++ b/phpunit/test-encryption.php @@ -30,22 +30,6 @@ function wporg_encryption_keys() { public function test_encrypt_decrypt() { $input = 'This is a plaintext string. It contains no sensitive data.'; - - $encrypted = encrypt( $input ); - - $this->assertNotEquals( $input, $encrypted ); - - $decrypted = decrypt( $encrypted ); - - $this->assertTrue( $decrypted instanceOf HiddenString ); - - $this->assertNotEquals( $input, $decrypted ); - $this->assertEquals( $input, $decrypted->getString() ); - $this->assertNotEquals( $input, (string) $decrypted ); - } - - public function test_authenticated_encrypt_decrypt() { - $input = 'This is a plaintext string. It contains no sensitive data.'; $additional_data = 'USER1'; $encrypted = encrypt( $input, $additional_data ); @@ -55,7 +39,7 @@ public function test_authenticated_encrypt_decrypt() { // Decrypt without $additional_data. try { - decrypt( $encrypted ); + decrypt( $encrypted, '' ); } catch( Exception $e ) { $this->assertEquals( 'Invalid cipher text.', $e->getMessage() ); } finally { @@ -116,7 +100,7 @@ public function test_is_encrypted() { $this->assertTrue( is_encrypted( PREFIX . $string_nonce_length . 'TEST STRING' ) ); $test_string = 'This is a plaintext string. It contains no sensitive data.'; - $this->assertTrue( is_encrypted( encrypt( $test_string ) ) ); + $this->assertTrue( is_encrypted( encrypt( $test_string, 'additional_data' ) ) ); } public function test_generate_key_different() { @@ -150,17 +134,17 @@ public function test_get_encryption_key() { public function test_can_encrypt_hiddenstring() { $hidden_string = new HiddenString( "TEST STRING" ); - $encrypted = encrypt( $hidden_string ); + $encrypted = encrypt( $hidden_string, 'additional_data' ); $this->assertTrue( is_encrypted( $encrypted ) ); - $this->assertSame( $hidden_string->getString(), decrypt( $encrypted )->getString() ); + $this->assertSame( $hidden_string->getString(), decrypt( $encrypted, 'additional_data' )->getString() ); } public function test_encrypt_decrypt_invalid_inputs() { // Invalid key specified. try { - encrypt( 'TEST STRING', '', 'unknown-key' ); + encrypt( 'TEST STRING', 'additional_data', 'unknown-key' ); } catch( Exception $e ) { $this->assertEquals( 'Encryption key "unknown-key" not defined.', $e->getMessage() ); } finally { @@ -170,7 +154,7 @@ public function test_encrypt_decrypt_invalid_inputs() { // Not-encrypted Invalid data that. try { - decrypt( PREFIX . 'TESTSTRINGTESTSTRINGTESTSTRINGTESTSTRINGTESTSTRINGTESTSTRING' ); + decrypt( PREFIX . 'TESTSTRINGTESTSTRINGTESTSTRINGTESTSTRINGTESTSTRINGTESTSTRING', 'additional_data' ); } catch( Exception $e ) { // This is thrown by sodium_hex2bin(). $this->assertEquals( 'invalid hex string', $e->getMessage() ); @@ -181,7 +165,7 @@ public function test_encrypt_decrypt_invalid_inputs() { // Not-encrypted Possibly-valid data. try { - decrypt( PREFIX . '012345678901234567890123456789012345678901234567890123456789' ); + decrypt( PREFIX . '012345678901234567890123456789012345678901234567890123456789', 'additional_data' ); } catch( Exception $e ) { $this->assertEquals( 'Invalid cipher text.', $e->getMessage() ); } finally { @@ -191,7 +175,7 @@ public function test_encrypt_decrypt_invalid_inputs() { // Invalid key specified, not-encrypted data that's not long enough. try { - decrypt( 'TEST STRING', '', 'unknown-key' ); + decrypt( 'TEST STRING', 'additional_data', 'unknown-key' ); } catch( Exception $e ) { $this->assertEquals( 'Value is not encrypted.', $e->getMessage() ); } finally { @@ -201,7 +185,7 @@ public function test_encrypt_decrypt_invalid_inputs() { // Not-encrypted data that's not long enough. try { - decrypt( 'TEST STRING' ); + decrypt( 'TEST STRING', 'additional_data' ); } catch( Exception $e ) { $this->assertEquals( 'Value is not encrypted.', $e->getMessage() ); } finally { @@ -214,36 +198,14 @@ public function test_encrypt_decrypt_invalid_inputs() { public function test_exported_functions() { // This only tests the behavioural functions, not the encryption/decryption. - $input = 'This is a plaintext string. It contains no sensitive data.'; - - $encrypted = wporg_encrypt( $input ); - - $this->assertNotEquals( $input, $encrypted ); - - $decrypted = wporg_decrypt( $encrypted ); - - $this->assertTrue( $decrypted instanceOf HiddenString ); - - $this->assertNotSame( $input, $decrypted ); - $this->assertEquals( $input, $decrypted->getString() ); - $this->assertEquals( $input, (string) $decrypted ); - - $this->assertFalse( wporg_encrypt( '', 'unknown-key' ) ); - $this->assertFalse( wporg_decrypt( '', 'unknown-key' ) ); - $this->assertFalse( wporg_decrypt( 'TEST STRING' ) ); - } - - public function test_exported_authenticated_functions() { - // This only tests the behavioural functions, not the encryption/decryption. - $input = 'This is a plaintext string. It contains no sensitive data.'; - $additional_data = 'USER1'; + $additional_data = 'additional_data'; - $encrypted = wporg_authenticated_encrypt( $input, $additional_data ); + $encrypted = wporg_encrypt( $input, $additional_data ); $this->assertNotEquals( $input, $encrypted ); - $decrypted = wporg_authenticated_decrypt( $encrypted, $additional_data ); + $decrypted = wporg_decrypt( $encrypted, $additional_data ); $this->assertTrue( $decrypted instanceOf HiddenString ); @@ -251,9 +213,9 @@ public function test_exported_authenticated_functions() { $this->assertEquals( $input, $decrypted->getString() ); $this->assertEquals( $input, (string) $decrypted ); - $this->assertFalse( wporg_authenticated_encrypt( '', '', 'unknown-key' ) ); - $this->assertFalse( wporg_authenticated_decrypt( '', '', 'unknown-key' ) ); - $this->assertFalse( wporg_authenticated_decrypt( 'TEST STRING' ) ); + $this->assertFalse( wporg_encrypt( '', $additional_data, 'unknown-key' ) ); + $this->assertFalse( wporg_decrypt( '', $additional_data, 'unknown-key' ) ); + $this->assertFalse( wporg_decrypt( 'TEST STRING', $additional_data ) ); } } From a48403266bce68345f434c9bbc2cbcfa765d1ef4 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Tue, 18 Apr 2023 12:46:48 +1000 Subject: [PATCH 15/19] Rename $additional_data to $context for easier reading. --- mu-plugins/encryption/exports.php | 23 ++++++++------ mu-plugins/encryption/index.php | 24 +++++++------- phpunit/test-encryption.php | 52 ++++++++++++++++--------------- 3 files changed, 52 insertions(+), 47 deletions(-) diff --git a/mu-plugins/encryption/exports.php b/mu-plugins/encryption/exports.php index af8ec517b..b443b4058 100644 --- a/mu-plugins/encryption/exports.php +++ b/mu-plugins/encryption/exports.php @@ -5,6 +5,9 @@ * * It provides a wrapper around the libsodium's Authenticated Encryption with * Additional Data ciphers (AEAD with XChaCha20-Poly1305) + * + * NOTE: $context should always be passed, and should either be set to the stringy User ID, or a unique-per-item string. + * The context is not stored within the Encrypted data, but is used to validate that the value is being decrypted in the same context. */ /** @@ -12,14 +15,14 @@ * * Unlike the Encryption plugin, this function simply returns false for any errors. * - * @param string $value The plaintext value. - * @param string $additional_data Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. Optional. - * @param string $key The key to use for encryption. Optional. + * @param string $value The plaintext value. + * @param string $context Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. + * @param string $key The key to use for encryption. Optional. * @return string|false The encrypted value, or false on error. */ -function wporg_encrypt( $value, string $additional_data, string $key = '' ) { +function wporg_encrypt( $value, string $context, string $key = '' ) { try { - return \WordPressdotorg\MU_Plugins\Encryption\encrypt( $value, $additional_data, $key ); + return \WordPressdotorg\MU_Plugins\Encryption\encrypt( $value, $context, $key ); } catch ( Exception $e ) { return false; } @@ -31,14 +34,14 @@ function wporg_encrypt( $value, string $additional_data, string $key = '' ) { * Unlike the Encryption plugin, this function simply returns false for any errors, and * HiddenStrings that can be cast to string as needed. * - * @param string $value The encrypted value. - * @param string $additional_data Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. Optional. - * @param string $key The key to use for decryption. Optional. + * @param string $value The encrypted value. + * @param string $context Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. + * @param string $key The key to use for decryption. Optional. * @return string|false The decrypted value, or false on error. */ -function wporg_decrypt( string $value, string $additional_data, string $key = '' ) { +function wporg_decrypt( string $value, string $context, string $key = '' ) { try { - $value = \WordPressdotorg\MU_Plugins\Encryption\decrypt( $value, $additional_data, $key ); + $value = \WordPressdotorg\MU_Plugins\Encryption\decrypt( $value, $context, $key ); return new HiddenString( $value->getString(), false ); } catch ( Exception $e ) { diff --git a/mu-plugins/encryption/index.php b/mu-plugins/encryption/index.php index 5033ef731..d942ee428 100644 --- a/mu-plugins/encryption/index.php +++ b/mu-plugins/encryption/index.php @@ -34,19 +34,19 @@ /** * Encrypt a value. * - * @param string $value Value to encrypt. - * @param string $additional_data Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. Optional. - * @param string $key Key to use for encryption. Optional. + * @param string $value Value to encrypt. + * @param string $context Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. + * @param string $key Key to use for encryption. Optional. * @return string Encrypted value, exceptions thrown on error. */ -function encrypt( $value, string $additional_data, string $key = '' ) { +function encrypt( $value, string $context, string $key = '' ) { $nonce = random_bytes( NONCE_LENGTH ); if ( ! $nonce ) { throw new Exception( 'Unable to create a nonce.' ); } - if ( empty( $additional_data ) ) { - throw new Exception( '$additional_data cannot be empty.' ); + if ( empty( $context ) ) { + throw new Exception( '$context cannot be empty.' ); } if ( $value instanceOf HiddenString ) { @@ -54,7 +54,7 @@ function encrypt( $value, string $additional_data, string $key = '' ) { } $key = get_encryption_key( $key ); - $encrypted = sodium_crypto_aead_xchacha20poly1305_ietf_encrypt( $value, $additional_data, $nonce, $key->getString() ); + $encrypted = sodium_crypto_aead_xchacha20poly1305_ietf_encrypt( $value, $context, $nonce, $key->getString() ); sodium_memzero( $value ); @@ -64,12 +64,12 @@ function encrypt( $value, string $additional_data, string $key = '' ) { /** * Decrypt a value. * - * @param string $value Value to decrypt. - * @param string $additional_data Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. Optional. - * @param string $key Key to use for decryption. Optional. + * @param string $value Value to decrypt. + * @param string $context Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. + * @param string $key Key to use for decryption. Optional. * @return HiddenString Decrypted value. */ -function decrypt( $value, string $additional_data, string $key = '' ) : HiddenString { +function decrypt( $value, string $context, string $key = '' ) : HiddenString { if ( $value instanceOf HiddenString ) { $value = $value->getString(); } @@ -89,7 +89,7 @@ function decrypt( $value, string $additional_data, string $key = '' ) : HiddenSt $key = get_encryption_key( $key ); $nonce = mb_substr( $value, 0, NONCE_LENGTH, '8bit' ); $value = mb_substr( $value, NONCE_LENGTH, null, '8bit' ); - $plaintext = sodium_crypto_aead_xchacha20poly1305_ietf_decrypt( $value, $additional_data, $nonce, $key->getString() ); + $plaintext = sodium_crypto_aead_xchacha20poly1305_ietf_decrypt( $value, $context, $nonce, $key->getString() ); sodium_memzero( $nonce ); sodium_memzero( $value ); diff --git a/phpunit/test-encryption.php b/phpunit/test-encryption.php index 41d5e8691..1c9131e5f 100644 --- a/phpunit/test-encryption.php +++ b/phpunit/test-encryption.php @@ -29,15 +29,14 @@ function wporg_encryption_keys() { } public function test_encrypt_decrypt() { - $input = 'This is a plaintext string. It contains no sensitive data.'; - $additional_data = 'USER1'; - - $encrypted = encrypt( $input, $additional_data ); + $input = 'This is a plaintext string. It contains no sensitive data.'; + $context = 'USER1'; + $encrypted = encrypt( $input, $context ); $this->assertNotEquals( $input, $encrypted ); - $this->assertStringNotContainsString( $additional_data, $encrypted ); + $this->assertStringNotContainsString( $context, $encrypted ); - // Decrypt without $additional_data. + // Decrypt without $context. try { decrypt( $encrypted, '' ); } catch( Exception $e ) { @@ -47,7 +46,7 @@ public function test_encrypt_decrypt() { unset( $e ); } - // Decrypt with incorrect $additional_data. + // Decrypt with incorrect $context. try { decrypt( $encrypted, 'USER2' ); } catch( Exception $e ) { @@ -59,7 +58,7 @@ public function test_encrypt_decrypt() { // Decrypt with incorrect key specified. try { - decrypt( $encrypted, $additional_data, 'secondary' ); + decrypt( $encrypted, $context, 'secondary' ); } catch( Exception $e ) { $this->assertEquals( 'Invalid cipher text.', $e->getMessage() ); } finally { @@ -69,7 +68,7 @@ public function test_encrypt_decrypt() { // Decrypt with unknown key specified. try { - decrypt( $encrypted, $additional_data, 'unknown-key' ); + decrypt( $encrypted, $context, 'unknown-key' ); } catch( Exception $e ) { $this->assertEquals( 'Encryption key "unknown-key" not defined.', $e->getMessage() ); } finally { @@ -77,7 +76,7 @@ public function test_encrypt_decrypt() { unset( $e ); } - $decrypted = decrypt( $encrypted, $additional_data ); + $decrypted = decrypt( $encrypted, $context ); $this->assertTrue( $decrypted instanceOf HiddenString ); @@ -100,7 +99,7 @@ public function test_is_encrypted() { $this->assertTrue( is_encrypted( PREFIX . $string_nonce_length . 'TEST STRING' ) ); $test_string = 'This is a plaintext string. It contains no sensitive data.'; - $this->assertTrue( is_encrypted( encrypt( $test_string, 'additional_data' ) ) ); + $this->assertTrue( is_encrypted( encrypt( $test_string, 'context' ) ) ); } public function test_generate_key_different() { @@ -133,18 +132,21 @@ public function test_get_encryption_key() { public function test_can_encrypt_hiddenstring() { $hidden_string = new HiddenString( "TEST STRING" ); + $context = 'test-context'; - $encrypted = encrypt( $hidden_string, 'additional_data' ); + $encrypted = encrypt( $hidden_string, $context ); $this->assertTrue( is_encrypted( $encrypted ) ); - $this->assertSame( $hidden_string->getString(), decrypt( $encrypted, 'additional_data' )->getString() ); + $this->assertSame( $hidden_string->getString(), decrypt( $encrypted, $context )->getString() ); } public function test_encrypt_decrypt_invalid_inputs() { + $context = 'test-context'; + // Invalid key specified. try { - encrypt( 'TEST STRING', 'additional_data', 'unknown-key' ); + encrypt( 'TEST STRING', $context, 'unknown-key' ); } catch( Exception $e ) { $this->assertEquals( 'Encryption key "unknown-key" not defined.', $e->getMessage() ); } finally { @@ -154,7 +156,7 @@ public function test_encrypt_decrypt_invalid_inputs() { // Not-encrypted Invalid data that. try { - decrypt( PREFIX . 'TESTSTRINGTESTSTRINGTESTSTRINGTESTSTRINGTESTSTRINGTESTSTRING', 'additional_data' ); + decrypt( PREFIX . 'TESTSTRINGTESTSTRINGTESTSTRINGTESTSTRINGTESTSTRINGTESTSTRING', $context ); } catch( Exception $e ) { // This is thrown by sodium_hex2bin(). $this->assertEquals( 'invalid hex string', $e->getMessage() ); @@ -165,7 +167,7 @@ public function test_encrypt_decrypt_invalid_inputs() { // Not-encrypted Possibly-valid data. try { - decrypt( PREFIX . '012345678901234567890123456789012345678901234567890123456789', 'additional_data' ); + decrypt( PREFIX . '012345678901234567890123456789012345678901234567890123456789', $context ); } catch( Exception $e ) { $this->assertEquals( 'Invalid cipher text.', $e->getMessage() ); } finally { @@ -175,7 +177,7 @@ public function test_encrypt_decrypt_invalid_inputs() { // Invalid key specified, not-encrypted data that's not long enough. try { - decrypt( 'TEST STRING', 'additional_data', 'unknown-key' ); + decrypt( 'TEST STRING', $context, 'unknown-key' ); } catch( Exception $e ) { $this->assertEquals( 'Value is not encrypted.', $e->getMessage() ); } finally { @@ -185,7 +187,7 @@ public function test_encrypt_decrypt_invalid_inputs() { // Not-encrypted data that's not long enough. try { - decrypt( 'TEST STRING', 'additional_data' ); + decrypt( 'TEST STRING', $context ); } catch( Exception $e ) { $this->assertEquals( 'Value is not encrypted.', $e->getMessage() ); } finally { @@ -198,14 +200,14 @@ public function test_encrypt_decrypt_invalid_inputs() { public function test_exported_functions() { // This only tests the behavioural functions, not the encryption/decryption. - $input = 'This is a plaintext string. It contains no sensitive data.'; - $additional_data = 'additional_data'; + $input = 'This is a plaintext string. It contains no sensitive data.'; + $context = 'test-context'; - $encrypted = wporg_encrypt( $input, $additional_data ); + $encrypted = wporg_encrypt( $input, $context ); $this->assertNotEquals( $input, $encrypted ); - $decrypted = wporg_decrypt( $encrypted, $additional_data ); + $decrypted = wporg_decrypt( $encrypted, $context ); $this->assertTrue( $decrypted instanceOf HiddenString ); @@ -213,9 +215,9 @@ public function test_exported_functions() { $this->assertEquals( $input, $decrypted->getString() ); $this->assertEquals( $input, (string) $decrypted ); - $this->assertFalse( wporg_encrypt( '', $additional_data, 'unknown-key' ) ); - $this->assertFalse( wporg_decrypt( '', $additional_data, 'unknown-key' ) ); - $this->assertFalse( wporg_decrypt( 'TEST STRING', $additional_data ) ); + $this->assertFalse( wporg_encrypt( '', $context, 'unknown-key' ) ); + $this->assertFalse( wporg_decrypt( '', $context, 'unknown-key' ) ); + $this->assertFalse( wporg_decrypt( 'TEST STRING', $context ) ); } } From 6468c2ddf987572f2966b895add6c909fea4fada Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Tue, 18 Apr 2023 12:53:22 +1000 Subject: [PATCH 16/19] Rename the $key parameters to $key_name to better define that it's the name/slug of a key, rather than the actual key value. --- mu-plugins/encryption/exports.php | 20 +++++++++--------- mu-plugins/encryption/index.php | 34 +++++++++++++++---------------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/mu-plugins/encryption/exports.php b/mu-plugins/encryption/exports.php index b443b4058..93e77be12 100644 --- a/mu-plugins/encryption/exports.php +++ b/mu-plugins/encryption/exports.php @@ -15,14 +15,14 @@ * * Unlike the Encryption plugin, this function simply returns false for any errors. * - * @param string $value The plaintext value. - * @param string $context Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. - * @param string $key The key to use for encryption. Optional. + * @param string $value The plaintext value. + * @param string $context Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. + * @param string $key_name The name of the key to use for encryption. Optional. * @return string|false The encrypted value, or false on error. */ -function wporg_encrypt( $value, string $context, string $key = '' ) { +function wporg_encrypt( $value, string $context, string $key_name = '' ) { try { - return \WordPressdotorg\MU_Plugins\Encryption\encrypt( $value, $context, $key ); + return \WordPressdotorg\MU_Plugins\Encryption\encrypt( $value, $context, $key_name ); } catch ( Exception $e ) { return false; } @@ -34,14 +34,14 @@ function wporg_encrypt( $value, string $context, string $key = '' ) { * Unlike the Encryption plugin, this function simply returns false for any errors, and * HiddenStrings that can be cast to string as needed. * - * @param string $value The encrypted value. - * @param string $context Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. - * @param string $key The key to use for decryption. Optional. + * @param string $value The encrypted value. + * @param string $context Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. + * @param string $key_name The name of the key to use for decryption. Optional. * @return string|false The decrypted value, or false on error. */ -function wporg_decrypt( string $value, string $context, string $key = '' ) { +function wporg_decrypt( string $value, string $context, string $key_name = '' ) { try { - $value = \WordPressdotorg\MU_Plugins\Encryption\decrypt( $value, $context, $key ); + $value = \WordPressdotorg\MU_Plugins\Encryption\decrypt( $value, $context, $key_name ); return new HiddenString( $value->getString(), false ); } catch ( Exception $e ) { diff --git a/mu-plugins/encryption/index.php b/mu-plugins/encryption/index.php index d942ee428..a960551bf 100644 --- a/mu-plugins/encryption/index.php +++ b/mu-plugins/encryption/index.php @@ -34,12 +34,12 @@ /** * Encrypt a value. * - * @param string $value Value to encrypt. - * @param string $context Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. - * @param string $key Key to use for encryption. Optional. + * @param string $value Value to encrypt. + * @param string $context Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. + * @param string $key_name The name of the key to use for encryption. Optional. * @return string Encrypted value, exceptions thrown on error. */ -function encrypt( $value, string $context, string $key = '' ) { +function encrypt( $value, string $context, string $key_name = '' ) { $nonce = random_bytes( NONCE_LENGTH ); if ( ! $nonce ) { throw new Exception( 'Unable to create a nonce.' ); @@ -53,7 +53,7 @@ function encrypt( $value, string $context, string $key = '' ) { $value = $value->getString(); } - $key = get_encryption_key( $key ); + $key = get_encryption_key( $key_name ); $encrypted = sodium_crypto_aead_xchacha20poly1305_ietf_encrypt( $value, $context, $nonce, $key->getString() ); sodium_memzero( $value ); @@ -64,12 +64,12 @@ function encrypt( $value, string $context, string $key = '' ) { /** * Decrypt a value. * - * @param string $value Value to decrypt. - * @param string $context Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. - * @param string $key Key to use for decryption. Optional. + * @param string $value Value to decrypt. + * @param string $context Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. + * @param string $key_name The name of the key to use for decryption. Optional. * @return HiddenString Decrypted value. */ -function decrypt( $value, string $context, string $key = '' ) : HiddenString { +function decrypt( $value, string $context, string $key_name = '' ) : HiddenString { if ( $value instanceOf HiddenString ) { $value = $value->getString(); } @@ -86,7 +86,7 @@ function decrypt( $value, string $context, string $key = '' ) : HiddenString { throw new Exception( 'Invalid cipher text.' ); } - $key = get_encryption_key( $key ); + $key = get_encryption_key( $key_name ); $nonce = mb_substr( $value, 0, NONCE_LENGTH, '8bit' ); $value = mb_substr( $value, NONCE_LENGTH, null, '8bit' ); $plaintext = sodium_crypto_aead_xchacha20poly1305_ietf_decrypt( $value, $context, $nonce, $key->getString() ); @@ -128,25 +128,25 @@ function is_encrypted( $value ) { /** * Get the encryption key. * - * @param string $key The key to use for decryption. + * @param string $key_name The name of the key to use for decryption. * @return string The encryption key. */ -function get_encryption_key( string $key = '' ) { +function get_encryption_key( string $key_name = '' ) { $keys = []; if ( function_exists( 'wporg_encryption_keys' ) ) { $keys = wporg_encryption_keys(); } - if ( ! $key ) { - $key = 'default'; + if ( ! $key_name ) { + $key_name = 'default'; } - if ( ! isset( $keys[ $key ] ) ) { - throw new Exception( sprintf( 'Encryption key "%s" not defined.', $key ) ); + if ( ! isset( $keys[ $key_name ] ) ) { + throw new Exception( sprintf( 'Encryption key "%s" not defined.', $key_name ) ); } - return $keys[ $key ]; + return $keys[ $key_name ]; } /** From c12a1075753d5b33aefb572ae051efe430d9e98d Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Tue, 18 Apr 2023 12:54:56 +1000 Subject: [PATCH 17/19] encrypt() no longer returns HiddenString, so when decrypting we don't need to be worried about that being the input. --- mu-plugins/encryption/index.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/mu-plugins/encryption/index.php b/mu-plugins/encryption/index.php index a960551bf..ebaf4170d 100644 --- a/mu-plugins/encryption/index.php +++ b/mu-plugins/encryption/index.php @@ -69,11 +69,7 @@ function encrypt( $value, string $context, string $key_name = '' ) { * @param string $key_name The name of the key to use for decryption. Optional. * @return HiddenString Decrypted value. */ -function decrypt( $value, string $context, string $key_name = '' ) : HiddenString { - if ( $value instanceOf HiddenString ) { - $value = $value->getString(); - } - +function decrypt( string $value, string $context, string $key_name = '' ) : HiddenString { if ( ! is_encrypted( $value ) ) { throw new Exception( 'Value is not encrypted.' ); } From 1face2894ee3f453a5a375824200e455bd05849e Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Tue, 18 Apr 2023 13:57:54 +1000 Subject: [PATCH 18/19] Document the key methods as returning a HiddenString. --- mu-plugins/encryption/index.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mu-plugins/encryption/index.php b/mu-plugins/encryption/index.php index ebaf4170d..b55d97db1 100644 --- a/mu-plugins/encryption/index.php +++ b/mu-plugins/encryption/index.php @@ -125,7 +125,7 @@ function is_encrypted( $value ) { * Get the encryption key. * * @param string $key_name The name of the key to use for decryption. - * @return string The encryption key. + * @return HiddenString The encryption key. */ function get_encryption_key( string $key_name = '' ) { @@ -148,7 +148,7 @@ function get_encryption_key( string $key_name = '' ) { /** * Generate a random encryption key. * - * @return string The encryption key. + * @return HiddenString The encryption key. */ function generate_encryption_key() { return new HiddenString( random_bytes( KEY_LENGTH ) ); From c5356039175baf40bfdd7763fd70f1a25ac28ef7 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 20 Apr 2023 11:12:20 +1000 Subject: [PATCH 19/19] Correct incorrect types in docblock. --- mu-plugins/encryption/exports.php | 4 ++-- mu-plugins/encryption/index.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mu-plugins/encryption/exports.php b/mu-plugins/encryption/exports.php index 93e77be12..3ac8229de 100644 --- a/mu-plugins/encryption/exports.php +++ b/mu-plugins/encryption/exports.php @@ -37,7 +37,7 @@ function wporg_encrypt( $value, string $context, string $key_name = '' ) { * @param string $value The encrypted value. * @param string $context Additional, authenticated data. This is used in the verification of the authentication tag appended to the ciphertext, but it is not encrypted or stored in the ciphertext. * @param string $key_name The name of the key to use for decryption. Optional. - * @return string|false The decrypted value, or false on error. + * @return HiddenString|false The decrypted value stored within a HiddenString instance, or false on error. */ function wporg_decrypt( string $value, string $context, string $key_name = '' ) { try { @@ -52,7 +52,7 @@ function wporg_decrypt( string $value, string $context, string $key_name = '' ) /** * Determine if a value is encrypted. * - * @param string $value The value to check. + * @param HiddenString|string $value The value to check. * @return bool True if the value is encrypted, false otherwise. */ function wporg_is_encrypted( string $value ) : bool { diff --git a/mu-plugins/encryption/index.php b/mu-plugins/encryption/index.php index b55d97db1..d22e20bdd 100644 --- a/mu-plugins/encryption/index.php +++ b/mu-plugins/encryption/index.php @@ -100,7 +100,7 @@ function decrypt( string $value, string $context, string $key_name = '' ) : Hidd /** * Check if a value is encrypted. * - * @param string $value Value to check. + * @param HiddenString|string $value Value to check. * @return bool True if the value is encrypted, false otherwise. */ function is_encrypted( $value ) {