Since we tried to check if flags & MAP_ENTRY_EMPTY was true when
searching for empty entries the code was broken since x & 0 always is
false. We fix this by refactoring pg_tde_read_one_map_entry() so the
filtering of the entries is done outside the function. This make
implementing search for empty entries much easier.
Replaces the old way we deleted keys which was built for tde_heap_basic
with deleting the the relation key when smgr_unlink() is called on the
main fork. This function is always called after commit/abort when a
relation deletion has been registered, even if no main fork would exist.
This approach means we do not need to WAL log any event for deleting
relation keys, the normal SMGR unlink also handles that which fits well
into the current approach of doing most of the encryption at the SMGR
layer.
We also remove the subtransaction test which is no longer useful since
it tested things very specific to the old key deleteion.
Just as we use pgindent we should probably use pgperltidy. This is an
initial run of it using the following command:
src/tools/pgindent/pgperltidy contrib/pg_tde
This helper mostly added confusion by making it seem like it did more
work than is actually did. And especially since we will want to call
init in the future with different parameters for some tests or
initialize from a backup.
Since until we actually have CI for older versions the code is likely
broken anyway we might as well not try to support versions we do not
actually support. It is easy to re-add this once we want to add support
for PostgreSQL 14.
The code wrongly assumed that the databaseId set in the keyInfo returned
from GetPrincipalKeyNoDefault() would be the Oid of the key provider
owner, while in reality it is the Oid of the database using it as a
principal key.
While there is a quite big variation already among PostgreSQL's own
record types and decriptions at least try not to invent something
totally different.
This simplifies working with tests a lot since now we will also always
get the errors from failed queries directly in the test output instead
of it being truncated when the TAP tests aborts due to the query
failing.
There is still a good case for why we should instead write idiomatic TAP
tests but this at least does a lot to improve the expereince of people
who have to work with these tests without changing the way the tests
work. Plus that the code is cleaner now so it should be easier to move
away from this way of testing in the future.
We have not been using autotools since commit
e0978a8be6c70b2fccc86ca1cb8fc5499dd83a88 so stop pretending that we do
related to config.h and instead directly have the necessary defines with
the right names in pg_tde.h.
Since as soon as we have installed pg_tde the database owner can call
any function created by the extension so any database owner can meddle
with any global key provider. The only way to prevent the database owner
to do whatever they want add permissions checks to the C code and here
we keep that check simple by limiting modifying the global key provider
to only the super user.
Additionally we also protect the function for settting the WAL key, for
setting the default key and to be paranoid also the function for using a
global key provider to set the database key. The third is not obvious if
it is necessary or not but I chose to be paranoid and relax that
restirction later once we have demed it to be secure.
Before this commit, we XLogged an unsigned PrincipalKey info when
creating the key. Which leads to:
1. In case of crash recovery, the redo would rewrite a map_ file with
an empty sign info. And the server would later fail to start with
"Failed to verify principal key header..."
2. Replicas would create a _map file with an empty sign info. Which in
turn leads to a fail on restart.
For PG-1539
Since you can take a copy of a PostgreSQL data directory and start both
the old and the new version you could get two versions where the same
encrypted counter is used for CTR which would mean we could comapre
them and potentially decrypt the data. For this reason we need to
generate a new WAL key every time we start the server.
According to to the manual we should return a null pointer, but it is
contradicted by an example on the same page but let's follow what
plpgsql does.
An event trigger function must return a NULL pointer
(not an SQL null value, that is, do not
set isNull true).
The security of the encryption is reduced if we reuse the same
initiation vector more than necessary so we make sure to use a unique IV
per relation fork, with the exception of the initialization fork which
is used by unlogged indexes when restarting the server after a crash. It
is copied with low-level file system functions to the main fork on crash
recovery so it needs to use the same IV as the main fork.
The init fork issue is in no way more a security issue than to the
extent that ideally we should pick a new IV when truncating unlogged
tables on crash recovery but to fix this we would need to change the
SMGR API and moving the copying of the intialization fork into that. And
in the long term this might be what we want to do.
