Key management with tests

On Thu, Dec 31, 2020 at 11:50:47PM -0500, Bruce Momjian wrote:

I have completed the key management patch with tests created by Stephen
Frost. Original patch by Masahiko Sawada. It requires the hex
reorganization patch first. The key patch is now 2.1MB because of the
tests, so attaching it here seems unwise:

https://github.com/postgres/postgres/compare/master...bmomjian:hex.diff
https://github.com/postgres/postgres/compare/master...bmomjian:key.diff

I will add it to the commitfest. I think we need to figure out how much
of the tests we want to add.

I am getting regression test errors using OpenSSL 1.1.1d 10 Sep 2019
with zero-length input data (no -p), while Stephen is able for those
tests to pass. This needs more research, plus I think higher-level
tests.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

On Fri, Jan 1, 2021 at 01:07:50AM -0500, Bruce Momjian wrote:

On Thu, Dec 31, 2020 at 11:50:47PM -0500, Bruce Momjian wrote:

I have completed the key management patch with tests created by Stephen
Frost. Original patch by Masahiko Sawada. It requires the hex
reorganization patch first. The key patch is now 2.1MB because of the
tests, so attaching it here seems unwise:

https://github.com/postgres/postgres/compare/master...bmomjian:hex.diff
https://github.com/postgres/postgres/compare/master...bmomjian:key.diff

I will add it to the commitfest. I think we need to figure out how much
of the tests we want to add.

I am getting regression test errors using OpenSSL 1.1.1d 10 Sep 2019
with zero-length input data (no -p), while Stephen is able for those
tests to pass. This needs more research, plus I think higher-level
tests.

I have found the cause of the failure, which I added as a C comment:

/*
* OpenSSL 1.1.1d and earlier crashes on some zero-length plaintext
* and ciphertext strings. It crashes on an encryption call to
* EVP_EncryptFinal_ex(() in GCM mode of zero-length strings if
* plaintext is NULL, even though plaintext_len is zero. Setting
* plaintext to non-NULL allows it to work. In KW/KWP mode,
* zero-length strings fail if plaintext_len = 0 and plaintext is
* non-NULL (the opposite). OpenSSL 1.1.1e+ is fine with all options.
*/
else if (cipher == PG_CIPHER_AES_GCM)
{
plaintext_len = 0;
plaintext = pg_malloc0(1);
}

All the tests pass now. The current src/test directory is 19MB, and
adding these tests takes it to 23MB, or a 20% increase. That seems like
a lot. It is testing 128-bit and 256-bit keys --- should we do fewer
tests, or just test 256, or use gzip to compress the tests by 50%?
(Does every platform have gzip?)

My next step is to add the high-level tests.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

On 2021-Jan-07, Bruce Momjian wrote:

All the tests pass now. The current src/test directory is 19MB, and
adding these tests takes it to 23MB, or a 20% increase. That seems like
a lot. It is testing 128-bit and 256-bit keys --- should we do fewer
tests, or just test 256, or use gzip to compress the tests by 50%?
(Does every platform have gzip?)

So the tests are about 95% of the patch ... do we really need that many
tests?

--
�lvaro Herrera

On Thu, Jan 7, 2021 at 04:08:49PM -0300, �lvaro Herrera wrote:

On 2021-Jan-07, Bruce Momjian wrote:

All the tests pass now. The current src/test directory is 19MB, and
adding these tests takes it to 23MB, or a 20% increase. That seems like
a lot. It is testing 128-bit and 256-bit keys --- should we do fewer
tests, or just test 256, or use gzip to compress the tests by 50%?
(Does every platform have gzip?)

So the tests are about 95% of the patch ... do we really need that many
tests?

No, I don't think so. Stephen imported the entire NIST test suite. It
was so comperhensive, it detected several OpenSSL bugs for zero-length
strings, which I already reported, but we would never be encrypting
zero-length strings, so there wasn't a lot of value to it.

Anyway, I think we need to figure out how to trim. The first part would
be to figure out whether we need 128 _and_ 256-bit tests, and then see
what items are really useful. Stephen, do you have any ideas on that?
We currently have 10296 tests, and I think we could get away with 100.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

On Thu, Jan 7, 2021 at 10:02:14AM -0500, Bruce Momjian wrote:

My next step is to add the high-level tests.

Here is the high-level script, and the log output. I used the
pg_upgrade test.sh as a model.

It uses "CFE DEBUG" lines that are already in the code to compare the
initdb encryption with the other initdb decryption and pg_ctl
decryption. It was easier than I thought.

What it does not do is to test the file descriptor passing from
/dev/tty, or the sample scripts. This seems acceptable to me since I
test them and they rarely change.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

Greetings,

* Bruce Momjian (bruce@momjian.us) wrote:

On Thu, Jan 7, 2021 at 04:08:49PM -0300, Álvaro Herrera wrote:

On 2021-Jan-07, Bruce Momjian wrote:

All the tests pass now. The current src/test directory is 19MB, and
adding these tests takes it to 23MB, or a 20% increase. That seems like
a lot. It is testing 128-bit and 256-bit keys --- should we do fewer
tests, or just test 256, or use gzip to compress the tests by 50%?
(Does every platform have gzip?)

So the tests are about 95% of the patch ... do we really need that many
tests?

No, I don't think so. Stephen imported the entire NIST test suite. It
was so comperhensive, it detected several OpenSSL bugs for zero-length
strings, which I already reported, but we would never be encrypting
zero-length strings, so there wasn't a lot of value to it.

I ran the entire test suite locally to ensure everything worked, but I
didn't actually include all of it in the PR which you merged- I had
already reduced it quite a bit by removing all 'additional
authenticated data' test cases (which the tests will automatically skip
and which we haven't implemented support for in the common library
wrappers) and by removing the 192-bit cases. This reduced the overall
test set by about 2/3rd's or so, as I recall.

Anyway, I think we need to figure out how to trim. The first part would
be to figure out whether we need 128 _and_ 256-bit tests, and then see
what items are really useful. Stephen, do you have any ideas on that?
We currently have 10296 tests, and I think we could get away with 100.

Yeah, it's probably still too much, but I don't have any particularly
justifiable suggestions as to exactly what we should remove or what we
should keep.

Perhaps it'd make sense to try and cover the cases that are more likely
to be issues between our wrapper functions and OpenSSL, and not stress
too much about constantly testing cases that should really be up to
OpenSSL. As such, I'd propose:

- Add back in some 192-bit tests, so we cover all three bit lengths.
- Add back in some additional authenticated test cases, just to make
sure that, until/unless we implement support, the test code properly
skips over those.
- Keep tests for various length plaintext/ciphertext (including 0-byte
cases, so we make sure those work, since they really should).
- Keep at least one test for each length of tag that's included in the
test suite.

I'm not sure how many tests we'd end up with from that, but my swag /
gut feeling is that it'd probably be on the order of 100ish and a small
enough set that it won't dwarf the rest of the patch.

Would be nice if we had a way for some buildfarm animal or something to
pull in the entire suite and test it, imv.. If anyone wants to
volunteer, I'd be happy to explain how to make that happen (it's not
hard though- download/unzip the files, drop them in the directory,
update the test script to add all the files into the array).

Thanks,

Stephen

Greetings Bruce,

* Bruce Momjian (bruce@momjian.us) wrote:

On Fri, Jan 1, 2021 at 01:07:50AM -0500, Bruce Momjian wrote:

On Thu, Dec 31, 2020 at 11:50:47PM -0500, Bruce Momjian wrote:

I have completed the key management patch with tests created by Stephen
Frost. Original patch by Masahiko Sawada. It requires the hex
reorganization patch first. The key patch is now 2.1MB because of the
tests, so attaching it here seems unwise:

https://github.com/postgres/postgres/compare/master...bmomjian:hex.diff
https://github.com/postgres/postgres/compare/master...bmomjian:key.diff

I will add it to the commitfest. I think we need to figure out how much
of the tests we want to add.

I am getting regression test errors using OpenSSL 1.1.1d 10 Sep 2019
with zero-length input data (no -p), while Stephen is able for those
tests to pass. This needs more research, plus I think higher-level
tests.

I have found the cause of the failure, which I added as a C comment:

/*
* OpenSSL 1.1.1d and earlier crashes on some zero-length plaintext
* and ciphertext strings. It crashes on an encryption call to
* EVP_EncryptFinal_ex(() in GCM mode of zero-length strings if
* plaintext is NULL, even though plaintext_len is zero. Setting
* plaintext to non-NULL allows it to work. In KW/KWP mode,
* zero-length strings fail if plaintext_len = 0 and plaintext is
* non-NULL (the opposite). OpenSSL 1.1.1e+ is fine with all options.
*/
else if (cipher == PG_CIPHER_AES_GCM)
{
plaintext_len = 0;
plaintext = pg_malloc0(1);
}

All the tests pass now. The current src/test directory is 19MB, and
adding these tests takes it to 23MB, or a 20% increase. That seems like
a lot. It is testing 128-bit and 256-bit keys --- should we do fewer
tests, or just test 256, or use gzip to compress the tests by 50%?
(Does every platform have gzip?)

Thanks a lot for working on this and figuring out what the issue was and
fixing it! That's great that we got all those cases passing for you
too.

Thanks again,

Stephen

On Fri, Jan 8, 2021 at 03:33:44PM -0500, Stephen Frost wrote:

No, I don't think so. Stephen imported the entire NIST test suite. It
was so comperhensive, it detected several OpenSSL bugs for zero-length
strings, which I already reported, but we would never be encrypting
zero-length strings, so there wasn't a lot of value to it.

I ran the entire test suite locally to ensure everything worked, but I
didn't actually include all of it in the PR which you merged- I had
already reduced it quite a bit by removing all 'additional
authenticated data' test cases (which the tests will automatically skip
and which we haven't implemented support for in the common library
wrappers) and by removing the 192-bit cases. This reduced the overall
test set by about 2/3rd's or so, as I recall.

Wow, so that was reduced!

Anyway, I think we need to figure out how to trim. The first part would
be to figure out whether we need 128 _and_ 256-bit tests, and then see
what items are really useful. Stephen, do you have any ideas on that?
We currently have 10296 tests, and I think we could get away with 100.

Yeah, it's probably still too much, but I don't have any particularly
justifiable suggestions as to exactly what we should remove or what we
should keep.

Perhaps it'd make sense to try and cover the cases that are more likely
to be issues between our wrapper functions and OpenSSL, and not stress
too much about constantly testing cases that should really be up to
OpenSSL. As such, I'd propose:

- Add back in some 192-bit tests, so we cover all three bit lengths.
- Add back in some additional authenticated test cases, just to make
sure that, until/unless we implement support, the test code properly
skips over those.
- Keep tests for various length plaintext/ciphertext (including 0-byte
cases, so we make sure those work, since they really should).
- Keep at least one test for each length of tag that's included in the
test suite.

Makes sense. I did a simplistic trim-down to 90 tests but it still was
40% of the patch; attached. The hex strings are very long.

I'm not sure how many tests we'd end up with from that, but my swag /
gut feeling is that it'd probably be on the order of 100ish and a small
enough set that it won't dwarf the rest of the patch.

Would be nice if we had a way for some buildfarm animal or something to
pull in the entire suite and test it, imv.. If anyone wants to
volunteer, I'd be happy to explain how to make that happen (it's not
hard though- download/unzip the files, drop them in the directory,
update the test script to add all the files into the array).

Yes, do we have a place to store more comprehensive tests outside of our
git tree? Has this been done before?

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

On Fri, Jan 8, 2021 at 03:34:23PM -0500, Stephen Frost wrote:

All the tests pass now. The current src/test directory is 19MB, and
adding these tests takes it to 23MB, or a 20% increase. That seems like
a lot. It is testing 128-bit and 256-bit keys --- should we do fewer
tests, or just test 256, or use gzip to compress the tests by 50%?
(Does every platform have gzip?)

Thanks a lot for working on this and figuring out what the issue was and
fixing it! That's great that we got all those cases passing for you
too.

Yes, I was relieved. The pattern of when zero-length strings fail in
which modes is still very odd, but at least it reports an error, so it
isn't returning incorrect data.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

Greetings,

* Bruce Momjian (bruce@momjian.us) wrote:

On Fri, Jan 8, 2021 at 03:33:44PM -0500, Stephen Frost wrote:

Anyway, I think we need to figure out how to trim. The first part would
be to figure out whether we need 128 _and_ 256-bit tests, and then see
what items are really useful. Stephen, do you have any ideas on that?
We currently have 10296 tests, and I think we could get away with 100.

Yeah, it's probably still too much, but I don't have any particularly
justifiable suggestions as to exactly what we should remove or what we
should keep.

Perhaps it'd make sense to try and cover the cases that are more likely
to be issues between our wrapper functions and OpenSSL, and not stress
too much about constantly testing cases that should really be up to
OpenSSL. As such, I'd propose:

- Add back in some 192-bit tests, so we cover all three bit lengths.
- Add back in some additional authenticated test cases, just to make
sure that, until/unless we implement support, the test code properly
skips over those.
- Keep tests for various length plaintext/ciphertext (including 0-byte
cases, so we make sure those work, since they really should).
- Keep at least one test for each length of tag that's included in the
test suite.

Makes sense. I did a simplistic trim-down to 90 tests but it still was
40% of the patch; attached. The hex strings are very long.

I don't think we actually need to stress over the size of the test data
relative to the size of the patch- it's not like it's all that much perl
code. I can appreciate that we don't want to add megabytes worth of
test data to the git repo though.

I'm not sure how many tests we'd end up with from that, but my swag /
gut feeling is that it'd probably be on the order of 100ish and a small
enough set that it won't dwarf the rest of the patch.

Would be nice if we had a way for some buildfarm animal or something to
pull in the entire suite and test it, imv.. If anyone wants to
volunteer, I'd be happy to explain how to make that happen (it's not
hard though- download/unzip the files, drop them in the directory,
update the test script to add all the files into the array).

Yes, do we have a place to store more comprehensive tests outside of our
git tree? Has this been done before?

Not that I'm aware of.

Thanks,

Stephen

On Fri, Jan 1, 2021 at 01:07:50AM -0500, Bruce Momjian wrote:

On Thu, Dec 31, 2020 at 11:50:47PM -0500, Bruce Momjian wrote:

I have completed the key management patch with tests created by Stephen
Frost. Original patch by Masahiko Sawada. It requires the hex
reorganization patch first. The key patch is now 2.1MB because of the
tests, so attaching it here seems unwise:

https://github.com/postgres/postgres/compare/master...bmomjian:hex.diff
https://github.com/postgres/postgres/compare/master...bmomjian:key.diff

I will add it to the commitfest. I think we need to figure out how much
of the tests we want to add.

I am getting regression test errors using OpenSSL 1.1.1d 10 Sep 2019
with zero-length input data (no -p), while Stephen is able for those
tests to pass. This needs more research, plus I think higher-level
tests.

I know we are still working on the hex patch (dest-len) and the crypto
tests, but I wanted to post this so people can see where we are, and we
can get some current cfbot testing.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

On Sat, Jan 9, 2021 at 01:17:36PM -0500, Bruce Momjian wrote:

On Fri, Jan 1, 2021 at 01:07:50AM -0500, Bruce Momjian wrote:

On Thu, Dec 31, 2020 at 11:50:47PM -0500, Bruce Momjian wrote:

I have completed the key management patch with tests created by Stephen
Frost. Original patch by Masahiko Sawada. It requires the hex
reorganization patch first. The key patch is now 2.1MB because of the
tests, so attaching it here seems unwise:

https://github.com/postgres/postgres/compare/master...bmomjian:hex.diff
https://github.com/postgres/postgres/compare/master...bmomjian:key.diff

I will add it to the commitfest. I think we need to figure out how much
of the tests we want to add.

I am getting regression test errors using OpenSSL 1.1.1d 10 Sep 2019
with zero-length input data (no -p), while Stephen is able for those
tests to pass. This needs more research, plus I think higher-level
tests.

I know we are still working on the hex patch (dest-len) and the crypto
tests, but I wanted to post this so people can see where we are, and we
can get some current cfbot testing.

Here is an updated version that covers all the possible
testing/configuration options.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

On Sat, Jan 9, 2021 at 08:08:16PM -0500, Bruce Momjian wrote:

On Sat, Jan 9, 2021 at 01:17:36PM -0500, Bruce Momjian wrote:

I know we are still working on the hex patch (dest-len) and the crypto
tests, but I wanted to post this so people can see where we are, and we
can get some current cfbot testing.

Here is an updated version that covers all the possible
testing/configuration options.

Does anyone know why the cfbot applied the patch listed second first
here?

http://cfbot.cputube.org/patch_31_2925.log

Specifically, it applied hex..key.diff.gz before hex.diff.gz. I assumed
it would apply attachments in the order they appear in the email.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

On Sun, Jan 10, 2021 at 3:45 PM Bruce Momjian <bruce@momjian.us> wrote:

Does anyone know why the cfbot applied the patch listed second first
here?

http://cfbot.cputube.org/patch_31_2925.log

Specifically, it applied hex..key.diff.gz before hex.diff.gz. I assumed
it would apply attachments in the order they appear in the email.

It sorts the filenames (in this case after decompressing step removes
the .gz endings). That works pretty well for the patches that "git
format-patch" spits out, but it's a bit hit and miss with cases like
yours.

On Sun, Jan 10, 2021 at 06:04:12PM +1300, Thomas Munro wrote:

On Sun, Jan 10, 2021 at 3:45 PM Bruce Momjian <bruce@momjian.us> wrote:

Does anyone know why the cfbot applied the patch listed second first
here?

http://cfbot.cputube.org/patch_31_2925.log

Specifically, it applied hex..key.diff.gz before hex.diff.gz. I assumed
it would apply attachments in the order they appear in the email.

It sorts the filenames (in this case after decompressing step removes
the .gz endings). That works pretty well for the patches that "git
format-patch" spits out, but it's a bit hit and miss with cases like
yours.

OK, here they are with numeric prefixes. It was actually tricky to
figure out how to create a squashed format-patch based on another branch.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

On Sun, Jan 10, 2021 at 11:51 PM Bruce Momjian <bruce@momjian.us> wrote:

On Sun, Jan 10, 2021 at 06:04:12PM +1300, Thomas Munro wrote:

On Sun, Jan 10, 2021 at 3:45 PM Bruce Momjian <bruce@momjian.us> wrote:

Does anyone know why the cfbot applied the patch listed second first
here?

http://cfbot.cputube.org/patch_31_2925.log

Specifically, it applied hex..key.diff.gz before hex.diff.gz. I assumed
it would apply attachments in the order they appear in the email.

It sorts the filenames (in this case after decompressing step removes
the .gz endings). That works pretty well for the patches that "git
format-patch" spits out, but it's a bit hit and miss with cases like
yours.

OK, here they are with numeric prefixes. It was actually tricky to
figure out how to create a squashed format-patch based on another branch.

Thank you for attaching the patches. It passes all cfbot tests, great.

Looking at the patch, it supports three algorithms but only
PG_CIPHER_AES_KWP is used in the core for now:

+/*
+ * Supported symmetric encryption algorithm. These identifiers are passed
+ * to pg_cipher_ctx_create() function, and then actual encryption
+ * implementations need to initialize their context of the given encryption
+ * algorithm.
+ */
+#define PG_CIPHER_AES_GCM          0
+#define PG_CIPHER_AES_KW           1
+#define PG_CIPHER_AES_KWP          2
+#define PG_MAX_CIPHER_ID           3

Are we in the process of experimenting which algorithms are better? If
we support one algorithm that is actually used in the core, we would
reduce the tests as well.

FWIW, I've written a PoC patch for buffer encryption to make sure the
kms patch would be workable with other components using the encryption
key managed by kmgr.

Overall it’s good. While the buffer encryption patch is still PoC
quality and there are some problems regarding nonce generation we need
to deal with, it easily can use the relation key managed by the kmgr
to encrypt/decrypt buffers.

Regards,

--
Masahiko Sawada
EnterpriseDB: https://www.enterprisedb.com/

On Mon, Jan 11, 2021 at 08:12:00PM +0900, Masahiko Sawada wrote:

On Sun, Jan 10, 2021 at 11:51 PM Bruce Momjian <bruce@momjian.us> wrote:

OK, here they are with numeric prefixes. It was actually tricky to
figure out how to create a squashed format-patch based on another branch.

Thank you for attaching the patches. It passes all cfbot tests, great.

Yeah, I saw that. :-) I head to learn a lot about how to create
squashed format-patches on non-master branches. I have now automated it
so it will be easy going forward.

Looking at the patch, it supports three algorithms but only
PG_CIPHER_AES_KWP is used in the core for now:

+/*
+ * Supported symmetric encryption algorithm. These identifiers are passed
+ * to pg_cipher_ctx_create() function, and then actual encryption
+ * implementations need to initialize their context of the given encryption
+ * algorithm.
+ */
+#define PG_CIPHER_AES_GCM          0
+#define PG_CIPHER_AES_KW           1
+#define PG_CIPHER_AES_KWP          2
+#define PG_MAX_CIPHER_ID           3

Are we in the process of experimenting which algorithms are better? If
we support one algorithm that is actually used in the core, we would
reduce the tests as well.

I think we are only using KWP (Key Wrap with Padding) because that is
for wrapping keys:

https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Algorithm-Validation-Program/documents/mac/KWVS.pdf

I am not sure about KW. I think we are using GCM for the WAP/heap/index
pages. Stephen would know more.

FWIW, I've written a PoC patch for buffer encryption to make sure the
kms patch would be workable with other components using the encryption
key managed by kmgr.

Wow, it is a small patch --- nice.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

Greetings,

* Bruce Momjian (bruce@momjian.us) wrote:

On Mon, Jan 11, 2021 at 08:12:00PM +0900, Masahiko Sawada wrote:

Looking at the patch, it supports three algorithms but only
PG_CIPHER_AES_KWP is used in the core for now:

+/*
+ * Supported symmetric encryption algorithm. These identifiers are passed
+ * to pg_cipher_ctx_create() function, and then actual encryption
+ * implementations need to initialize their context of the given encryption
+ * algorithm.
+ */
+#define PG_CIPHER_AES_GCM          0
+#define PG_CIPHER_AES_KW           1
+#define PG_CIPHER_AES_KWP          2
+#define PG_MAX_CIPHER_ID           3

Are we in the process of experimenting which algorithms are better? If
we support one algorithm that is actually used in the core, we would
reduce the tests as well.

I think we are only using KWP (Key Wrap with Padding) because that is
for wrapping keys:

https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Algorithm-Validation-Program/documents/mac/KWVS.pdf

Yes.

I am not sure about KW. I think we are using GCM for the WAP/heap/index
pages. Stephen would know more.

KW was more-or-less 'for free' and there were tests for it, which is why
it was included. Yes, GCM would be for WAL/heap/index pages, it
wouldn't be appropriate to use KW or KWP for that. Using KW/KWP for the
key wrapping also makes the API simpler- and therefore easier for other
implementations to be written which provide the same API.

FWIW, I've written a PoC patch for buffer encryption to make sure the
kms patch would be workable with other components using the encryption
key managed by kmgr.

Wow, it is a small patch --- nice.

I agree that the actual encryption patch, for just the main heap/index,
won't be too bad. The larger part will be dealing with all of the
temporary files we create that have user data in them... I've been
contemplating a way to try and make that part of the patch smaller
though and hopefully that will bear fruit and we can avoid having to
change a lof of, eg, reorderbuffer.c and pgstat.c.

There's a few places where we need to be sure to be updating the LSN for
both logged and unlogged relations properly, including dealing with
things like the magic GIST "GistBuildLSN" fake-LSN too, and we will
absolutely need to have a bit used in the IV to distinguish if it's a
real LSN or an unlogged LSN.

Although, another approach and one that I've discussed a bit with Bruce,
is to have more keys- such as a key for temporary files, and perhaps
even a key for logged relations and a different for unlogged.. Or
perhaps sets of keys for each which automatically are rotating every X
number of GB based on the LSN... Which is a big part of why key
management is such an important part of this effort.

Thanks,

Stephen

On Mon, Jan 11, 2021 at 12:54:49PM -0500, Stephen Frost wrote:

Although, another approach and one that I've discussed a bit with Bruce,
is to have more keys- such as a key for temporary files, and perhaps
even a key for logged relations and a different for unlogged.. Or

Yes, we have to make sure the nonce (computed as LSN/pageno) is never
reused, so if we have several LSN usage "spaces", they need different
data keys.

perhaps sets of keys for each which automatically are rotating every X
number of GB based on the LSN... Which is a big part of why key
management is such an important part of this effort.

Yes, this would avoid the need to failover to a standby for data key
rotation.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee