On Thu, 30 Jan 2020 at 20:36, Sehrope Sarkuni <sehrope@jackdb.com> wrote:
>
> On Wed, Jan 29, 2020 at 3:43 AM Masahiko Sawada
> <masahiko.sawada@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
