Re: [Proposal] Table-level Transparent Data Encryption (TDE) and KeyManagement Service (KMS) - Mailing list pgsql-hackers

On Sat, 4 Jan 2020 at 15:11, cary huang <hcary328@gmail.com> wrote:
>
> Hello Sawada and all
>
> I would like to elaborate more on Sehrope and Sawada's discussion on passing NULL IV in "pg_cipher_encrypt/decrypt"
functionsduring kmgr_wrap_key and kmgr_unwrap_key routines in kmgr_utils.c. Openssl implements key wrap according to
RFC3394as Sawada mentioned and passing NULL will make openssl to use default IV, which equals to A6A6A6A6A6A6A6A6. I
haveconfirmed this on my side; A key wrapped with "NULL" IV can be unwrapped successfully with IV=A6A6A6A6A6A6A6A6, and
unwrapwill fail if IV is set to anything else other than NULL or A6A6A6A6A6A6A6A6. 
>

Sehrope also suggested me not to use the fixed IV in order to avoid
getting the same result from the same value. I'm researching it now.
Also, currently it's using key wrap algorithm[1] but it accepts only
multiple of 8 bytes as input. Since it's not good for some cases it's
better to use key wrap with padding algorithm[2] instead, which seems
available in OpenSSL 1.1.0 or later.

> I would like to provide some comments on the encryption and decryption routines provided by cipher_openssl.c in which
cipher.cand kmgr_utils.c are using. I see that "ossl_cipher_encrypt" calls "EVP_EncryptInit_ex" and "EVP_EncryptUpdate"
onlyto complete the encryption. Same thing applies to decryption routines. According to my past experience with openssl
andthe usages online, it is highly recommended to use "init-update-final" cycle to complete the encryption and I see
thatthe "final" part (EVP_EncryptFinal) is missing. This call will properly handle the last block of data especially
whenpadding is taken into account. The functions still works now because the input is encryption key and its size is
multipleof each cipher block and no padding is used. I think it will be safer to use the proper "init-update-final"
cyclefor encryption/decryption 

Agreed.

>
> According to openssl EVP documentation, "EVP_EncryptUpdate" can be called multiple times at different offset to the
inputdata to be encrypted. I see that "pg_cipher_encrypt" only calls "EVP_EncryptUpdate" once, which makes me assume
thatthe application should invoke "pg_cipher_encrypt" multiple times until the entire data block is encrypted? I am
askingbecause if we were to use "EVP_EncryptFinal" to complete the encryption cycle, then it is better to let
"pg_cipher_encrypt"to figure out how many times "EVP_EncryptUpdate" should be called and finalize it with
"EVP_EncryptFinal"at last block. 

IIUC EVP_EncryptUpdate can encrypt the entire data block.
EVP_EncryptFinal_ex encrypts any data that remains in a partial block.

>
> Lastly, I think we are missing a cleanup routine that calls "EVP_CIPHER_CTX_free()" to free up the EVP_CIPHER_CTX
whenencryption is done. 

Right.

While reading pgcrypto code I thought that it's better to change the
cryptographic code (cipher.c) so that pgcrypto can use them instead of
having duplicated code. I'm trying to change it so.

[1] https://tools.ietf.org/html/rfc3394
[2] https://tools.ietf.org/html/rfc5649

Regards,

--
Masahiko Sawada            http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services