pgsql: Add key management system
Add key management system
This adds a key management system that stores (currently) two data
encryption keys of length 128, 192, or 256 bits. The data keys are
AES256 encrypted using a key encryption key, and validated via GCM
cipher mode. A command to obtain the key encryption key must be
specified at initdb time, and will be run at every database server
start. New parameters allow a file descriptor open to the terminal to
be passed. pg_upgrade support has also been added.
Discussion: /messages/by-id/CA+fd4k7q5o6Nc_AaX6BcYM9yqTbC6_pnH-6nSD=54Zp6NBQTCQ@mail.gmail.com
Discussion: /messages/by-id/20201202213814.GG20285@momjian.us
Author: Masahiko Sawada, me, Stephen Frost
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/978f869b992f9fca343e99d6fdb71073c76e869a
Modified Files
--------------
doc/src/sgml/config.sgml | 62 ++++
doc/src/sgml/database-encryption.sgml | 97 +++++
doc/src/sgml/filelist.sgml | 1 +
doc/src/sgml/installation.sgml | 5 +-
doc/src/sgml/postgres.sgml | 1 +
doc/src/sgml/ref/initdb.sgml | 46 +++
doc/src/sgml/ref/pg_ctl-ref.sgml | 13 +
doc/src/sgml/ref/pgupgrade.sgml | 18 +-
doc/src/sgml/ref/postgres-ref.sgml | 13 +
doc/src/sgml/storage.sgml | 5 +
src/backend/Makefile | 2 +-
src/backend/access/transam/xlog.c | 21 ++
src/backend/bootstrap/bootstrap.c | 21 +-
src/backend/crypto/Makefile | 18 +
src/backend/crypto/kmgr.c | 372 +++++++++++++++++++
src/backend/main/main.c | 3 +
src/backend/postmaster/pgstat.c | 9 +
src/backend/postmaster/postmaster.c | 13 +-
src/backend/replication/basebackup.c | 5 +
src/backend/storage/ipc/ipci.c | 3 +
src/backend/storage/lmgr/lwlocknames.txt | 1 +
src/backend/tcop/postgres.c | 25 +-
src/backend/utils/misc/guc.c | 24 ++
src/backend/utils/misc/pg_controldata.c | 11 +-
src/backend/utils/misc/postgresql.conf.sample | 5 +
src/bin/initdb/initdb.c | 116 +++++-
src/bin/pg_controldata/pg_controldata.c | 3 +
src/bin/pg_ctl/pg_ctl.c | 59 ++-
src/bin/pg_resetwal/pg_resetwal.c | 2 +
src/bin/pg_rewind/filemap.c | 8 +
src/bin/pg_upgrade/check.c | 34 ++
src/bin/pg_upgrade/controldata.c | 42 ++-
src/bin/pg_upgrade/file.c | 2 +
src/bin/pg_upgrade/option.c | 7 +-
src/bin/pg_upgrade/pg_upgrade.h | 3 +
src/bin/pg_upgrade/server.c | 5 +-
src/common/Makefile | 3 +
src/common/cipher.c | 67 ++++
src/common/cipher_openssl.c | 268 ++++++++++++++
src/common/kmgr_utils.c | 507 ++++++++++++++++++++++++++
src/include/catalog/pg_control.h | 5 +-
src/include/common/cipher.h | 62 ++++
src/include/common/kmgr_utils.h | 98 +++++
src/include/crypto/kmgr.h | 29 ++
src/include/pgstat.h | 3 +
src/include/postmaster/postmaster.h | 2 +
src/include/utils/guc_tables.h | 1 +
src/test/Makefile | 2 +-
src/tools/msvc/Mkvcbuild.pm | 4 +-
49 files changed, 2091 insertions(+), 35 deletions(-)
On 2020-12-25 16:19, Bruce Momjian wrote:
Add key management system
doc/src/sgml/database-encryption.sgml | 97 +++++
Attached are a few typos.
I also noticed that this option does not occur in the initdb --help:
-u --copy-encryption-keys
Was that deliberate?
Thanks,
Erik Rijkers
Attachments:
database-encryption.sgml.20201225.difftext/x-diff; name=database-encryption.sgml.20201225.diffDownload
--- ./doc/src/sgml/database-encryption.sgml.orig 2020-12-25 19:11:55.809303009 +0100
+++ ./doc/src/sgml/database-encryption.sgml 2020-12-25 19:22:22.558936395 +0100
@@ -13,7 +13,7 @@
log from being able to access the data stored in those files.
For example, when using cluster file encryption, users who have read
access to the cluster directories for backup purposes will not be able
- to decrypt the data stored in the these files.
+ to decrypt the data stored in these files.
</para>
<para>
@@ -24,7 +24,7 @@
Key one is used to encrypt write-ahead log (WAL) files. Two different
keys are used so that primary and standby servers can use different zero
(heap/index/temp) keys, but the same one (WAL) key, so that these keys
- can eventually be rotated by switching the primary to the standby as
+ can eventually be rotated by switching the primary to the standby
and then changing the WAL key.
</para>
@@ -68,7 +68,7 @@
During the <command>initdb</command> process, if
<option>--cluster-key-command</option> is specified, two data-level
encryption keys are created. These two keys are then encrypted with
- the key enryption key (KEK) supplied by the cluster key command before
+ the key encryption key (KEK) supplied by the cluster key command before
being stored in the database directory. The key or passphrase that
derives the key must be supplied from the terminal or stored in a
trusted key store, such as key vault software, hardware security module.
@@ -87,7 +87,7 @@
</para>
<para>
- The data encryption keys are randomly generated and are of 128, 192,
+ The data encryption keys are randomly generated and are 128, 192,
or 256-bits in length. They are encrypted by the key encryption key
(KEK) using Advanced Encryption Standard (<acronym>AES256</acronym>)
encryption in Galois/Counter Mode (<acronym>GCM</acronym>), which also
On Fri, Dec 25, 2020 at 07:30:01PM +0100, Erik Rijkers wrote:
On 2020-12-25 16:19, Bruce Momjian wrote:
Add key management system
doc/src/sgml/database-encryption.sgml | 97 +++++Attached are a few typos.
I also noticed that this option does not occur in the initdb --help:
-u --copy-encryption-keys
Was that deliberate?
No. :-( Attached patch applied. Thanks.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
Attachments:
master.difftext/x-diff; charset=us-asciiDownload
diff --git a/doc/src/sgml/database-encryption.sgml b/doc/src/sgml/database-encryption.sgml
index f938c9f574..82bc137a61 100644
--- a/doc/src/sgml/database-encryption.sgml
+++ b/doc/src/sgml/database-encryption.sgml
@@ -13,7 +13,7 @@
log from being able to access the data stored in those files.
For example, when using cluster file encryption, users who have read
access to the cluster directories for backup purposes will not be able
- to decrypt the data stored in the these files.
+ to decrypt the data stored in these files.
</para>
<para>
@@ -24,7 +24,7 @@
Key one is used to encrypt write-ahead log (WAL) files. Two different
keys are used so that primary and standby servers can use different zero
(heap/index/temp) keys, but the same one (WAL) key, so that these keys
- can eventually be rotated by switching the primary to the standby as
+ can eventually be rotated by switching the primary to the standby
and then changing the WAL key.
</para>
@@ -68,7 +68,7 @@ initdb -D dbname --cluster-key-command='ckey_passphrase.sh'
During the <command>initdb</command> process, if
<option>--cluster-key-command</option> is specified, two data-level
encryption keys are created. These two keys are then encrypted with
- the key enryption key (KEK) supplied by the cluster key command before
+ the key encryption key (KEK) supplied by the cluster key command before
being stored in the database directory. The key or passphrase that
derives the key must be supplied from the terminal or stored in a
trusted key store, such as key vault software, hardware security module.
@@ -87,7 +87,7 @@ initdb -D dbname --cluster-key-command='ckey_passphrase.sh'
</para>
<para>
- The data encryption keys are randomly generated and are of 128, 192,
+ The data encryption keys are randomly generated and are 128, 192,
or 256-bits in length. They are encrypted by the key encryption key
(KEK) using Advanced Encryption Standard (<acronym>AES256</acronym>)
encryption in Galois/Counter Mode (<acronym>GCM</acronym>), which also
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 92594772f6..4d07ce6e3f 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -2326,6 +2326,8 @@ usage(const char *progname)
printf(_(" -R, --authprompt prompt for a passphrase or PIN\n"));
printf(_(" -s, --show show internal settings\n"));
printf(_(" -S, --sync-only only sync data directory\n"));
+ printf(_(" -u, --copy-encryption-keys=DATADIR\n"
+ " copy the file encryption key from another cluster\n"));
printf(_("\nOther options:\n"));
printf(_(" -V, --version output version information, then exit\n"));
printf(_(" -?, --help show this help, then exit\n"));
I think cipher_failure() should be marked pg_attribute_noreturn().
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2 -I./src/include -D_GNU_SOURCE -I/usr/include/libxml2 -c -o src/common/cipher.o src/common/cipher.c
src/common/cipher.c: In function ‘pg_cipher_ctx_create’:
src/common/cipher.c:28:1: warning: control reaches end of non-void function [-Wreturn-type]
28 | }
--
Justin
Justin Pryzby <pryzby@telsasoft.com> writes:
I think cipher_failure() should be marked pg_attribute_noreturn().
Perhaps more to the point, it still doesn't build at all without
--with-openssl.
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sifaka&dt=2020-12-25%2019%3A13%3A19
regards, tom lane
I wrote:
Justin Pryzby <pryzby@telsasoft.com> writes:
I think cipher_failure() should be marked pg_attribute_noreturn().
Perhaps more to the point, it still doesn't build at all without
--with-openssl.
[ looks closer ... ] Oh, we're on about the same thing -- the difference
is that sifaka is using -Werror.
pg_attribute_noreturn() seems like a good idea, but we're also going to
need dummy return statements in the callers, to satisfy compilers that
don't understand that.
regards, tom lane
On Fri, Dec 25, 2020 at 02:37:06PM -0500, Tom Lane wrote:
I wrote:
Justin Pryzby <pryzby@telsasoft.com> writes:
I think cipher_failure() should be marked pg_attribute_noreturn().
Perhaps more to the point, it still doesn't build at all without
--with-openssl.[ looks closer ... ] Oh, we're on about the same thing -- the difference
is that sifaka is using -Werror.pg_attribute_noreturn() seems like a good idea, but we're also going to
need dummy return statements in the callers, to satisfy compilers that
don't understand that.
Yes, done. I tested it with a non-OpenSSL configure run now and it
worked. Thanks for the report.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
Bruce Momjian <bruce@momjian.us> writes:
On Fri, Dec 25, 2020 at 02:37:06PM -0500, Tom Lane wrote:
pg_attribute_noreturn() seems like a good idea, but we're also going to
need dummy return statements in the callers, to satisfy compilers that
don't understand that.
Yes, done. I tested it with a non-OpenSSL configure run now and it
worked. Thanks for the report.
Now that it compiles cleanly, what about test cases? It looks
to me like you broke src/test/Makefile:
@@ -30,7 +30,7 @@ endif
endif
ifeq ($(with_openssl),yes)
ifneq (,$(filter ssl,$(PG_TEST_EXTRA)))
-SUBDIRS += ssl
+SUBDIRS += ssl crypto
endif
endif
because there is no such subdirectory. Assuming that that's a
case of forgetting to "git add" the whole subdirectory, I still
don't much care for this implementation: the user should be able
to decide which subdirectories get run. Maybe more like
ifeq ($(with_openssl),yes)
ifneq (,$(filter ssl,$(PG_TEST_EXTRA)))
SUBDIRS += ssl
endif
+ifneq (,$(filter crypto,$(PG_TEST_EXTRA)))
+SUBDIRS += crypto
+endif
endifregards, tom lane
On Fri, Dec 25, 2020 at 02:53:10PM -0500, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
On Fri, Dec 25, 2020 at 02:37:06PM -0500, Tom Lane wrote:
pg_attribute_noreturn() seems like a good idea, but we're also going to
need dummy return statements in the callers, to satisfy compilers that
don't understand that.Yes, done. I tested it with a non-OpenSSL configure run now and it
worked. Thanks for the report.Now that it compiles cleanly, what about test cases? It looks
to me like you broke src/test/Makefile:@@ -30,7 +30,7 @@ endif
endif
ifeq ($(with_openssl),yes)
ifneq (,$(filter ssl,$(PG_TEST_EXTRA)))
-SUBDIRS += ssl
+SUBDIRS += ssl crypto
endif
endif
Odd I am running the regression tests.
because there is no such subdirectory. Assuming that that's a
case of forgetting to "git add" the whole subdirectory, I still
don't much care for this implementation: the user should be able
to decide which subdirectories get run. Maybe more likeifeq ($(with_openssl),yes) ifneq (,$(filter ssl,$(PG_TEST_EXTRA))) SUBDIRS += ssl endif +ifneq (,$(filter crypto,$(PG_TEST_EXTRA))) +SUBDIRS += crypto +endif endif
There are no tests yet. I need to write those in TAP, and I am going to
wait until I have something more substantial to test. I do have a test
framework here I am using.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
Hi,
On 2020-12-25 15:12:44 -0500, Bruce Momjian wrote:
There are no tests yet. I need to write those in TAP, and I am going to
wait until I have something more substantial to test. I do have a test
framework here I am using.
That's not great. It's not a tiny amount of code, and not having
anything to exercise it until some unclear future time means that other
people will have a hard time working anywhere in the vicinity. And that
might quite plausibly stay that way for v14?
Greetings,
Andres Freund
On Fri, Dec 25, 2020 at 03:12:08PM -0800, Andres Freund wrote:
Hi,
On 2020-12-25 15:12:44 -0500, Bruce Momjian wrote:
There are no tests yet. I need to write those in TAP, and I am going to
wait until I have something more substantial to test. I do have a test
framework here I am using.That's not great. It's not a tiny amount of code, and not having
anything to exercise it until some unclear future time means that other
people will have a hard time working anywhere in the vicinity. And that
might quite plausibly stay that way for v14?
It might. It depends on who else wants to work on it, and who wants to
write tests. I did test Stephen's GCM changes.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
Hi,
Thank you for developing a great feature. I tested it immediately.
The attached patch changes the value of the --file-encryption-keylen option of the initdb command to mandatory. No value is currently required.
I also changed the help message and the manual.
Regards,
Noriyoshi Shinoda
-----Original Message-----
From: Bruce Momjian [mailto:bruce@momjian.us]
Sent: Saturday, December 26, 2020 4:01 AM
To: Erik Rijkers <er@xs4all.nl>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: pgsql: Add key management system
On Fri, Dec 25, 2020 at 07:30:01PM +0100, Erik Rijkers wrote:
On 2020-12-25 16:19, Bruce Momjian wrote:
Add key management system
doc/src/sgml/database-encryption.sgml | 97 +++++Attached are a few typos.
I also noticed that this option does not occur in the initdb --help:
-u --copy-encryption-keys
Was that deliberate?
No. :-( Attached patch applied. Thanks.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
Attachments:
keylength.diffapplication/octet-stream; name=keylength.diffDownload
diff --git a/doc/src/sgml/ref/initdb.sgml b/doc/src/sgml/ref/initdb.sgml
index 44a2d69..d994f40 100644
--- a/doc/src/sgml/ref/initdb.sgml
+++ b/doc/src/sgml/ref/initdb.sgml
@@ -236,8 +236,8 @@ PostgreSQL documentation
<varlistentry id="app-initdb-file-encryption-keylen"
xreflabel="file encryption">
- <term><option>-K</option></term>
- <term><option>--file-encryption-keylen</option></term>
+ <term><option>-K <replaceable class="parameter">length</replaceable></option></term>
+ <term><option>--file-encryption-keylen=<replaceable class="parameter">length</replaceable></option></term>
<listitem>
<para>
Specifies the number of bits for the file encryption keys. The
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 4d07ce6..33a11e0 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -2318,7 +2318,7 @@ usage(const char *progname)
" to obtain the cluster key\n"));
printf(_(" -d, --debug generate lots of debugging output\n"));
printf(_(" -k, --data-checksums use data page checksums\n"));
- printf(_(" -K, --file-encryption-keylen\n"
+ printf(_(" -K, --file-encryption-keylen=LENGTH\n"
" bit length of the file encryption key\n"));
printf(_(" -L DIRECTORY where to find the input files\n"));
printf(_(" -n, --no-clean do not clean up after errors\n"));
@@ -3008,7 +3008,7 @@ main(int argc, char *argv[])
{"wal-segsize", required_argument, NULL, 12},
{"data-checksums", no_argument, NULL, 'k'},
{"authprompt", no_argument, NULL, 'R'},
- {"file-encryption-keylen", no_argument, NULL, 'K'},
+ {"file-encryption-keylen", required_argument, NULL, 'K'},
{"allow-group-access", no_argument, NULL, 'g'},
{"cluster-key-command", required_argument, NULL, 'c'},
{"copy-encryption-keys", required_argument, NULL, 'u'},
On Sat, Dec 26, 2020 at 02:38:40PM +0000, Shinoda, Noriyoshi (PN Japan FSIP) wrote:
Hi,
Thank you for developing a great feature. I tested it immediately.
The attached patch changes the value of the --file-encryption-keylen option of the initdb command to mandatory. No value is currently required.
I also changed the help message and the manual.
Thank you, applied.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee