use the openssl EVP API for SHA256 and AES256 instead of the deprecated lo… #4840
Conversation
|
/runtests |
There was a problem hiding this comment.
Coding style check: Success ✓.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: 3/588 tests failed ⚠.
The first 10 failing tests are:
sc_inserts_logicalsc_generated [setup failure]
snap_ha_retry_newsi_generated
auth
| @@ -144,7 +159,16 @@ __aes_encrypt(dbenv, aes_data, iv, data, data_len) | |||
| if ((ret = __db_generate_iv(dbenv, (uint32_t*)orig)) != 0) | |||
| return (ret); | |||
| memcpy(copy, orig, DB_IV_BYTES); | |||
| AES_cbc_encrypt(data, data, data_len, &aes->encrypt_key, copy, AES_ENCRYPT); | |||
|
|
|||
| if (!EVP_EncryptInit_ex(aes->encrypt_ctx, NULL, NULL, NULL, copy)) | |||
There was a problem hiding this comment.
Would you not need a key here?
There was a problem hiding this comment.
Sorry for taking so long to go back and look at this @riverszhang89 - the key is set into the EVP_CIPHER_CTX in the __aes_derivekeys method called during the init. So passing NULL along with the aes->encrypt_ctx in the __aes_encrypt is telling EVP_EncryptInit to use the already defined key in the context. This looks correct. I hope to do better testing of this once I setup another comdb2 test environment.
There was a problem hiding this comment.
I've now tested the code in creating an encrypted DB and it works. I ran it through valgrind as well to ensure no (new) memory leaks. @riverszhang89
frumioj
changed the title
|
Noting that using SHA256 and AES256 will produce larger encrypted files than SHA1 + AES128, but if you wish, it is easy to switch back to AES128. |
To help us review your pull request, please consider providing an overview of the following:
This is sort of a bug-fix, but also improves security
Current behaviour is that you use a
#pragmato remove warnings about using deprecated openssl features. This fix implements non-deprecated usage of the EVP APIs of openssl.Try building and testing with the new code. Behaviour should be unchanged (still using AES and SHA-1 for encryption and key stretching) but the build should work correctly while not using the
#pragma.This new behaviour is better as it is more future-proof use of openssl, and is a more secure way to use the openssl APIs.