On Thu, 30 Jan 2020 at 20:36, Sehrope Sarkuni <sehr...@jackdb.com> wrote: > > On Wed, Jan 29, 2020 at 3:43 AM Masahiko Sawada > <masahiko.saw...@2ndquadrant.com> wrote: > > Thank you for the suggestion. I like your suggestion. We can do an > > integrity check of the user input wrapped key by using HMAC when > > unwrapping. Regarding the output format you meant to use aes-256 > > rather than aes-256-key-wrap? I think that DATA in the output is the > > user input key so it still must be multiples of 16-bytes if we use > > aes-256-key-wrap. > > Yes I'm suggesting not using the key wrap functions and instead using > the regular EVP_aes_256_cbc with a random IV per invocation. For > internal usage (e.g. the encrypted key) it does not need padding as we > know the input value would always be a multiple of 16-bytes.
That makes sense. > That > would allow the internal usage to have a fixed output length of > LEN(IV) + LEN(HMAC) + LEN(DATA) = 16 + 32 + 64 = 112 bytes. Probably you meant LEN(DATA) is 32? DATA will be an encryption key for AES256 (master key) internally generated. > > For the user facing piece, padding would enabled to support arbitrary > input data lengths. That would make the output length grow by up to > 16-bytes (rounding the data length up to the AES block size) plus one > more byte if a version field is added. I think the length of padding also needs to be added to the output. Anyway, in the first version the same methods of wrapping/unwrapping key are used for both internal use and user facing function. And user input key needs to be a multiple of 16 bytes value. > > > BTW regarding the implementation of cipher function using opensssl in > > the src/common I'm concerned whether we should integrate it with the > > openssl.c in pgcrypto. Since pgcrypto with openssl currently supports > > aes, des and bf etc the cipher function code in this patch also has > > similar functionality. Similarly when we introduced SCRAM we moved > > sha2 functions from pgcrypto to src/common. I thought we move all > > cipher functions in pgcrypto to src/common but it might be overkill > > because the internal KMS will use only aes with only 256 key length as > > of now. > > I'd keep the patch smaller and the functions internal to the KMS for > now. Maybe address it again after the patch is complete as it'll be > easier to see overlaps that could be combined. Agreed. BTW I think this topic is better to be discussed on a separate thread as the scope no longer includes TDE. I'll start a new thread for introducing internal KMS. Regards, -- Masahiko Sawada http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
