Destramic Posted August 18, 2016 Share Posted August 18, 2016 hey guys i have some help from jacques a while back regarding aes encryption in php, which works great!...i made the encryption/decryptions compatable with nodejs, but i'd like to know how i can make it compatable with mysql also. here is what i use to encrypt $data = 'hello'; $encryption_algorithm = 'AES-128-CBC'; $master_key = 'fba05a681b7606c57d6218d4cca387f5cd4f8e0ae098cb4d9b7e'; $init_vector = openssl_random_pseudo_bytes(openssl_cipher_iv_length($encryption_algorithm)); $ciphertext = openssl_encrypt(serialize($data), $encryption_algorithm, $master_key, false, $init_vector); result: e4a844130207ad0364c442387d5b9b8e:4d43ca26887385ee94eb27010b8d48fa i was in work thinking of my project and i plan on encypting everything except for primary key. ie: users ----------------------- user_id username (encrypted) password (encrypted) email_address (encrypted) telephone_number (encrypted) ----------------------- now if i decided to select user where username = 'JohnDoe' i'd been doomed as the username is encypted. i know mysql offers the AES_DECRYPT and AES_ENCRYPT function but i've read its not very safe? is it possible to find an enrypted row by column value which is compatable to my php script? any advise/links would be appreciated thank you Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted August 18, 2016 Share Posted August 18, 2016 (edited) Encrypt specific sensitive data, not everything. Then you don't have this problem. No, you cannot make the MySQL cryptographic functions compatible with your current script, because MySQL only supports the insecure ECB mode. Another major problem is that you would have to transfer the plaintext key to the database system over and over again, which massively increases the risk of exposing it. For example, if query logging is enabled, the key will appear in the log. So do the cryptography in the application. There are also problems with the implementation: You should avoid deserializing data at all cost, because this can be used for attacks like object injection. Use a standard data format like JSON, YAML or XML. AES-CBC only provides confidentiality, it does not protect the integrity of your data. This is particulary bad when data manipulation can have serious consequences (like in the case of deserialization). Unless you need compatibility with other parts of the application, you should consider switching to a high-level library like libsodium and an algorithm which provides authenticated encryption like ChaCha20-Poly1305. Edited August 18, 2016 by Jacques1 1 Quote Link to comment Share on other sites More sharing options...
Destramic Posted August 29, 2016 Author Share Posted August 29, 2016 firstly i apologies for such a late reply on the topic as i know guys spend a lot of your time helping out us members. well i didn't realize i was open to such big attacks when using unserialize() and deserialize() thanks for the advise. regarding encyrption at some point i need to encrypt sessions, cookies, passwords and any other sessitive data added to the database, i downloaded libsodium now i'm trying to understand how it is best to use library. i have found an example just of plain text encrypting and decrypting....would this be a good example? <?php // This requires the libsodium PECL extension /** * Encrypt a message * * @param string $message - message to encrypt * @param string $key - encryption key * @return string */ function safeEncrypt($message, $key) { $nonce = \Sodium\randombytes_buf( \Sodium\CRYPTO_SECRETBOX_NONCEBYTES ); $cipher = base64_encode( $nonce. \Sodium\crypto_secretbox( $message, $nonce, $key ) ); \Sodium\memzero($message); \Sodium\memzero($key); return $cipher; } /** * Decrypt a message * * @param string $encrypted - message encrypted with safeEncrypt() * @param string $key - encryption key * @return string */ function safeDecrypt($encrypted, $key) { $decoded = base64_decode($encrypted); $nonce = mb_substr($decoded, 0, \Sodium\CRYPTO_SECRETBOX_NONCEBYTES, '8bit'); $ciphertext = mb_substr($decoded, \Sodium\CRYPTO_SECRETBOX_NONCEBYTES, null, '8bit'); $plain = \Sodium\crypto_secretbox_open( $ciphertext, $nonce, $key ); \Sodium\memzero($ciphertext); \Sodium\memzero($key); return $plain; } thank you Quote Link to comment Share on other sites More sharing options...
Solution Jacques1 Posted August 29, 2016 Solution Share Posted August 29, 2016 The code is OK, but it relies on implementation details and may break when libsodium changes its default algorithms. Since the encryption function simply concatenates the nonce and the ciphertext to produce the output, the only way to tell those substrings apart is to rely on a specific length of the nonce. But this length may change at any time. When it does, your application won't be able to decrypt anything until you've figured out which libsodium version you need for your legacy ciphertexts. A more robust and future-proof solution would be to store the nonce and the ciphertext in separate fields and add an extra column for some kind of algorithm identifier (or the libsodium version): \Sodium\version_string(); Alternatively, use a specific algorithm so that you're not dependent on any defaults. If you need a single string, choose an unambiguous format. For example: <version identifier>:<Base64-encoded nonce>:<Base64-encoded ciphertext> 1 Quote Link to comment Share on other sites More sharing options...
Destramic Posted August 30, 2016 Author Share Posted August 30, 2016 (edited) thank you for the good information...i took what you said onboard and here is what i made class Text extends Encryption_Abstract { public function encrypt($message) { if (is_array($message)) { $message = json_encode($message); } $nonce = \Sodium\randombytes_buf(\Sodium\CRYPTO_AEAD_CHACHA20POLY1305_NPUBBYTES); $ciphertext = \Sodium\crypto_aead_chacha20poly1305_encrypt( $message, $this->additional_data(), $nonce, $this->secret_key() ); return '<' . $this->version() . '>:<' . base64_encode($nonce) . '>:<' . base64_encode($ciphertext) . '>'; } public function decrypt($encryption) { $encryption = $this->get_encryption($encryption); $decryption = \Sodium\crypto_aead_chacha20poly1305_decrypt( $encryption['ciphertext'], $this->additional_data(), $encryption['nonce'], $this->secret_key() ); return $this->get_decryption($decryption); } } abstract - so it can be used for gloabls(session, cookie) and passwords abstract class Encryption_Abstract { private $_keys_directory; private $_secret_key; private $_public_key; public function __construct($keys_directory) { $this->_keys_directory = $keys_directory; if (!extension_loaded('libsodium')) { throw new Exception('Encryption: PHP libsodium extension not loaded.'); } } protected function secret_key() { if (!is_null($this->_secret_key)) { return $this->_secret_key; } $this->_secret_key = $this->get_key('secret.key'); return $this->_secret_key; } protected function public_key() { if (!is_null($this->_public_key)) { return $this->_public_key; } $this->_public_key = $this->get_key('public.key'); return $this->_public_key; } protected function additional_data() { return 'fba05a681b7606c57d6218d4cca387f5cd4f8e0ae098cb4d9b7e'; } protected function version() { $libsodium = new ReflectionExtension('libsodium'); return $libsodium->getVersion(); } private function get_key($filename) { try { $filename = $this->_keys_directory . '/' . $filename; if (!is_readable($filename)) { throw new Exception('Encryption: Unable to get key.'); } return base64_decode(file_get_contents($filename)); } catch (Exception $exception) { echo $exception->getMessage(); } } protected function get_decryption($string) { $decryption = json_decode($string); if (!is_null($decryption)) { return $decryption; } return $string; } protected function get_encryption($encryption) { $encryption = substr($encryption, 1, -1); $encryption = explode('>:<', $encryption); try { if (count($encryption) !== 3) { throw new Exception('Encryption: Unknown encryption.'); } else if ($encryption[0] < $this->version()) { throw new Exception('Encryption: Encrypted data out of date.'); } if ($encryption[0] > $this->version()) { throw new Exception('Encryption: PHP libsodium extension out od date.'); } return array( 'nonce' => base64_decode($encryption[1]), 'ciphertext' => base64_decode($encryption[2]) ); } catch (Exception $exception) { echo $exception->getMessage(); } } } usage $text_encryption = new Text_Encryption(PRIVATE_DIRECTORY . 'config/keys'); $ciphertext = $text_encryption->encrypt(array(test')); print_r($text_encryption->decrypt($ciphertext)); $ciphertext2 = $text_encryption->encrypt('test'); echo $text_encryption->decrypt($ciphertext2); thanks again Edited August 30, 2016 by Destramic Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted August 30, 2016 Share Posted August 30, 2016 The class architecture doesn't really make sense. There should be one class which takes care of all the low-level cryptography. If you call the libsodium functions in the different subclasses for encrypting sessions, cookies etc., you'll quickly end up with duplicate code. From a cryptography standpoint, there's no difference between encrypting a session and encrypting a cookie. It's just different data. Get rid of the try statements. Not only are they useless and make you lose important information like the error location and the stack trace. They defeat the whole purpose of exceptions, because now you're back having to check return values. It's particularly strange when you throw an exception and then immediately catch it. A low-level cryptography class should not do any JSON-encoding. In fact, you're making the data ambiguous, because it's no longer clear whether it's actually a JSON document or just happens to look like the JSON format. Who says that [42] is a JSON array? Maybe it's supposed to be a literal pair of brackets with a number. Do all the encoding and decoding outside of the class. The angle brackets <> in my format were meant for you, the reader. The actual format is just a colon-separated list of parameters. Since you now use a fixed algorithm, there's no need for a version. If you still want to use versions, don't compare them with < or >, because those operators don't understand the semantics of version numbers. For example, "2.0.0" is considered greater than "10.0.0". It also doesn't really make sense to reject a ciphertext just because there was an unrelated change somewhere in the libsodium library. Your “additional data” parameter for the AEAD algorithm is currently a long random string. This makes no sense. The purpose of the parameter is that you can split the input into two parts: a secret part which should be encrypted and protected from manipulation, and a non-secret part which should only be protected from manipulation. If you don't need this (which is most of the time), simply use an empty string as the additional data. The naming isn't ideal. “get_encryption” and “get_decryption” don't make much sense. When you remove the JSON stuff, you can probably get rid of get_decryption() altogether. And get_encryption() should be renamed to something like parse_ciphertext(). Quote Link to comment Share on other sites More sharing options...
Destramic Posted August 30, 2016 Author Share Posted August 30, 2016 The class architecture doesn't really make sense. There should be one class which takes care of all the low-level cryptography. If you call the libsodium functions in the different subclasses for encrypting sessions, cookies etc., you'll quickly end up with duplicate code. From a cryptography standpoint, there's no difference between encrypting a session and encrypting a cookie. It's just different data ummm when looking at https://paragonie.com/book/pecl-libsodium/read/09-recipes.md i see they encrypt and decrypt thier cookies different to how i've done my text class...also i see versions of encrypting passwords and verifying so i assumed i'd no longer need password_verfiy() and password_hash() functons...my plan was to make a session_cookie class which extends encryption_abstract same with password...so this one class is all i need for all encryptions? Get rid of the try statements. Not only are they useless and make you lose important information like the error location and the stack trace. They defeat the whole purpose of exceptions, because now you're back having to check return values. It's particularly strange when you throw an exception and then immediately catch it. A low-level cryptography class should not do any JSON-encoding. In fact, you're making the data ambiguous, because it's no longer clear whether it's actually a JSON document or just happens to look like the JSON format. Who says that [42] is a JSON array? Maybe it's supposed to be a literal pair of brackets with a number. Do all the encoding and decoding outside of the class. The angle brackets <> in my format were meant for you, the reader. The actual format is just a colon-separated list of parameters. Since you now use a fixed algorithm, there's no need for a version. If you still want to use versions, don't compare them with < or >, because those operators don't understand the semantics of version numbers. For example, "2.0.0" is considered greater than "10.0.0". It also doesn't really make sense to reject a ciphertext just because there was an unrelated change somewhere in the libsodium library. Your “additional data” parameter for the AEAD algorithm is currently a long random string. This makes no sense. The purpose of the parameter is that you can split the input into two parts: a secret part which should be encrypted and protected from manipulation, and a non-secret part which should only be protected from manipulation. If you don't need this (which is most of the time), simply use an empty string as the additional data. The naming isn't ideal. “get_encryption” and “get_decryption” don't make much sense. When you remove the JSON stuff, you can probably get rid of get_decryption() altogether. And get_encryption() should be renamed to something like parse_ciphertext(). haha last night when looking at: <version identifier>:<Base64-encoded nonce>:<Base64-encoded ciphertext> i made it so the encryption returned as version:nonce:ciphertext, but when i looked at it again today i thought he might mean with <>....but all that been said i'll change my class with all your possitive comments in mind. thank you much appreciated Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted August 30, 2016 Share Posted August 30, 2016 ummm when looking at https://paragonie.com/book/pecl-libsodium/read/09-recipes.md i see they encrypt and decrypt thier cookies different to how i've done my text class...also i see versions of encrypting passwords and verifying so i assumed i'd no longer need password_verfiy() and password_hash() functons...my plan was to make a session_cookie class which extends encryption_abstract same with password...so this one class is all i need for all encryptions? My point is that you should have a single class which takes care of the basic crypto infrastructure (key management, algorithm selection, calling libsodium methods, parsing etc.) and provides a simple encrypt($plaintext) and a decrypt($ciphertext) method. On top of that class, you can then build specific security features like encrypted sessions. Right now, all the low-level cryptography happens in the individual feature classes. This isn't sensible, because the basic steps are always the same: Generate a nonce, call the ChaCha20 method etc. So instead of repeating them for every feature, put them into a common class. You must still hash your passwords. Encryption alone is dangerous, because if an attacker manages to obtain the key, they suddenly have an entire database of plaintext passwords. What you can of course do is hash the password and additionally encrypt the hash. This is valid and can provide more security than hashing alone. Encrypting cookies only makes sense if they contain sensitive data – which they really shouldn't. If you just want to prevent users from changing the data, an HMAC is more appropriate. This is essentially a “signature”. In short: confidentiality without integrity/authenticity (not recommended) ⇒ classical encryption, e. g. AES-CBC confidentiality with integrity/authenticity ⇒ authenticated encryption, e. g. ChaCha20 + Poly1305 integrity/authenticity without confidentiality ⇒ HMAC passwords ⇒ password hash algorithm, e. g. bcrypt; optionally encryption In any case, don't overestimate the security of your features, especially since your resources and your experience are limited. There can always be fatal flaws, so expect the worst and write your application as if there were no cryptographic features. Quote Link to comment Share on other sites More sharing options...
Destramic Posted September 2, 2016 Author Share Posted September 2, 2016 ok so my class here is good to encrypt everything really...when looking at https://paragonie.com/book/pecl-libsodium/read/09-recipes.md it makes you think you need to make a encryption and decryption class especially for cookies and passwords thank you for all the great information i'll definitly be keeping this in mind confidentiality without integrity/authenticity (not recommended) ⇒ classical encryption, e. g. AES-CBC confidentiality with integrity/authenticity ⇒ authenticated encryption, e. g. ChaCha20 + Poly1305 integrity/authenticity without confidentiality ⇒ HMAC passwords ⇒ password hash algorithm, e. g. bcrypt; optionally encryption also here my class updated from your suggestions <?php namespace Encryption; use Exception; class Encryption { private $_keys_directory; private $_secret_key; private $_public_key; public function __construct($keys_directory) { $this->_keys_directory = $keys_directory; if (!extension_loaded('libsodium')) { throw new Exception('Encryption: PHP libsodium extension not loaded.'); } } public function encrypt($data) { $nonce = \Sodium\randombytes_buf(\Sodium\CRYPTO_AEAD_CHACHA20POLY1305_NPUBBYTES); $secret_key = $this->secret_key(); if (!$secret_key) { return false; } $ciphertext = \Sodium\crypto_aead_chacha20poly1305_encrypt( $data, $this->additional_data(), $nonce, $this->secret_key() ); return base64_encode($nonce) . ':' . base64_encode($ciphertext); } public function decrypt($ciphertext) { $ciphertext = $this->parse_ciphertext($ciphertext); $secret_key = $this->secret_key(); if (!$ciphertext || !$secret_key) { return false; } list($nonce, $ciphertext) = $ciphertext; $decryption = \Sodium\crypto_aead_chacha20poly1305_decrypt( $ciphertext, $this->additional_data(), $nonce, $this->secret_key() ); return $decryption; } private function secret_key() { if (!is_null($this->_secret_key)) { return $this->_secret_key; } $this->_secret_key = $this->get_key('secret.key'); return $this->_secret_key; } private function public_key() { if (!is_null($this->_public_key)) { return $this->_public_key; } $this->_public_key = $this->get_key('public.key'); return $this->_public_key; } private function get_key($filename) { $filename = $this->_keys_directory . '/' . $filename; if (!is_readable($filename)) { return false; } return base64_decode(file_get_contents($filename)); } private function parse_ciphertext($ciphertext) { $ciphertext = explode(':', $ciphertext); if (count($ciphertext) !== 2) { return false; } return array( base64_decode($ciphertext[0]), base64_decode($ciphertext[1]) ); } private function additional_data() { return 'fba05a681b7606c57d6218d4cca387f5cd4f8e0ae098cb4d9b7e'; } } thank you Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted September 2, 2016 Share Posted September 2, 2016 You still have the nonsensical “additional data”. You're using magical return values (false) to indicate errors. This is bad, because it forces the caller to manually check for those return values and increases the risk of missing the error altogether. PHP code in general and critical code in particular should throw exceptions. The class should generally perform a lot more validation. For example, the $filename of the key is currently an arbitrary string which is just appended to the directory. So if the caller asks for /etc/passwd, that's what they get – which is clearly not acceptable. Come up with a sensible pattern for the key identifier (e.g. [a-zA-Z][a-zA-Z0-9]*) and reject anything else. If you're using PHP 7, consider adding type annotations to the method signatures for automatic type validation. Also consider switching to the PSR coding standards. For example, underscore prefixes for protected/private methods are long obsolete, because PHP now has explicit keywords for that. In the long run, the file operations should be in a seperate class so that the crypto class isn't bound to one particular way of storing the keys. Quote Link to comment Share on other sites More sharing options...
Destramic Posted September 10, 2016 Author Share Posted September 10, 2016 ok i see now so the “additional data" is used to reinforce the encryption?...should i be change the string depeding on what im encrypting. ie. if i'm encrypting users information then the user would have a private key and that would be used for my “additional data" as i put it. understood regarding throwing exception...due to the nature of the script it should just return fatal error. i've also been looking into symfony's annoations so yeah i think i'm going to writes something to validate type hinting but thank you for the brilliant feedback...much appreciated Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted September 10, 2016 Share Posted September 10, 2016 ok i see now so the “additional data" is used to reinforce the encryption?...should i be change the string depeding on what im encrypting. ie. if i'm encrypting users information then the user would have a private key and that would be used for my “additional data" as i put it. No. The parameter for additional data is used when you have non-secret input together with secret input. Both have to be covered by the authentication tag, but only the secret data needs to be encrypted. That's why the algorithm has two separate parameters. If you only have secret data, you don't use the parameter at all (simply supply an empty string). If you do have non-secret data, you can pass it to the second parameter to exclude it from the encryption step. Or you simply encrypt the entire input and waste a few CPU cycles. As an example: When you encrypt a message, it can make sense to store the character encoding as meta data. The encoding information should not be manipulated, but it's perfectly fine for anybody to know it. So you would pass it as additional data rather than encrypt it together with the message. 1 Quote Link to comment Share on other sites More sharing options...
Destramic Posted September 11, 2016 Author Share Posted September 11, 2016 ah ok understood...well all the information you've supplied is awesome...i appreciate your time and efforts thank you jacques Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.