Before this commit, we Xlogged the binary result of the _map file
content during key rotation. This led to issues:
1. Replicas would rewrite their own WAL keys with the primary's ones.
And WAL keys are different on replicas. The same would have happened
with SMGR keys since we're also planning to have them different across
replicas.
2. The crash recovery would rewrite the latest WAL key as it's being
created before redo.
This commit switches to rather Xlogging the event of rotation (to which
key should rotate) and lets redo/replicas perform the actual rotation.
Fixes PG-1468, PG-1541
There is a restart function so there is not and need to call first stop
and then start. And since by default a start, stop or restart call does
not return on error it is totally pointless to assert anything about the
return value. And since PostgreSQL's own tests also are fine with just
bailing out on error we do the same.
While at it we also always call these three functions without
parentheses to be consistent.
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 to validate that our c files conform to postgres
coding standards, we also run pgperltidy to do the same for perl files.
We only run it on our own code in contrib/pg_tde/
This doesn't actually run pgperltidy as we need to inject some options
in a way that didn't seem possible in that script. Instead it does the
same thing with some slight changes.
We also bump the ubuntu version for this google actions job to the
newest LTS as the older ubuntu version seems to have a version of
perltidy that doesn't support the options used by pgperltidy.
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.
The reason to do this is that the old approach created an unnecessary
diff against upstream where they had forgot SinglePartitionSpec in
typedefs.list.
Additionally add two new structs from our SMGR patch to the list.
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
e0978a8be6 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.
- Added code coverage to link repo to codecov.io for coverage stats on
PR and merge.
- Added coverage badge on the landing page (readme) of the repo.
- Updated GH action to run on PUSH/MERGE, as this is required for code
coverage.
- Updated bash files in ci_scripts folder to accommodate tde
installcheck only.
- Added percona server version scheme verification TAP test case.
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
Anders Åstrand