Skip to content

pg_basebackup: encrypt streamed WAL with new key #537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 19, 2025

Conversation

dAdAbird
Copy link
Member

Before, pg_basebackup would encrypt streamed WAL according to the keys in pg_tde/wal_keys in the destination dir.

This commit introduces the number of changes:
pg_basebackup encrypts WAL only if the "-E --encrypt-wal" flag is provided. In such a case, it would extract the principal key, truncate pg_tde/wal_keys and encrypt WAL with a newly generated WAL key. We still expect pg_tde/wal_keys and pg_tde/1664_providers in the destination dir. In case these files are not provided, but "-E" is specified, it fails with an error.

@codecov-commenter
Copy link

codecov-commenter commented Aug 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.48%. Comparing base (6df714b) to head (78c0ecc).

❌ Your project status has failed because the head coverage (82.48%) is below the target coverage (90.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@                  Coverage Diff                  @@
##           TDE_REL_17_STABLE     #537      +/-   ##
=====================================================
+ Coverage              82.40%   82.48%   +0.07%     
=====================================================
  Files                     25       25              
  Lines                   3229     3232       +3     
  Branches                 510      510              
=====================================================
+ Hits                    2661     2666       +5     
+ Misses                   457      455       -2     
  Partials                 111      111              
Components Coverage Δ
access 84.70% <100.00%> (+0.32%) ⬆️
catalog 87.65% <ø> (ø)
common 77.77% <ø> (ø)
encryption 72.97% <ø> (ø)
keyring 73.21% <ø> (ø)
src 94.18% <ø> (ø)
smgr 96.53% <ø> (ø)
transam ∅ <ø> (∅)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dAdAbird dAdAbird force-pushed the backup_new_wal_key branch from e7f8c94 to b0830e3 Compare August 18, 2025 19:51
Copy link
Collaborator

@dutow dutow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor comment about the copyright comment, otherwise looks good

@@ -0,0 +1,49 @@

# Copyright (c) 2021-2024, PostgreSQL Global Development Group
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This copyright seems incorrect, as this mostly tests encryption flags

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

@dAdAbird dAdAbird force-pushed the backup_new_wal_key branch 2 times, most recently from fc48b60 to 3a333e6 Compare August 19, 2025 08:29
@dAdAbird dAdAbird requested a review from artemgavrilov August 19, 2025 08:51
@AndersAstrand
Copy link
Collaborator

AndersAstrand commented Aug 19, 2025

A lot of tests seems to fail with this patch. I haven't looked into the details. I'll push an updated version of #542

EDIT: SEems like the issue is with using --encrypt-wal instead of -E

@dAdAbird dAdAbird force-pushed the backup_new_wal_key branch from 3a333e6 to cc2a46f Compare August 19, 2025 13:15
Before, pg_basebackup would encrypt streamed WAL according to the keys
in pg_tde/wal_keys in the destination dir.

This commit introduces the number of changes:
pg_basebackup encrypts WAL only if the "-E --encrypt-wal" flag is
provided. In such a case, it would extract the principal key, truncate
pg_tde/wal_keys and encrypt WAL with a newly generated WAL key. We
still expect pg_tde/wal_keys and pg_tde/1664_providers in the
destination dir. In case these files are not provided, but "-E" is
specified, it fails with an error.

We also throw a warning if pg_basebackup runs w/o -E, but there is
wal_keys on the source as WAL might be compromised, and the backup
is broken

For PG-1603, PG-1857
@dAdAbird dAdAbird force-pushed the backup_new_wal_key branch from cc2a46f to 78c0ecc Compare August 19, 2025 15:02
@dAdAbird dAdAbird merged commit 481030d into percona:TDE_REL_17_STABLE Aug 19, 2025
19 checks passed
@dAdAbird dAdAbird deleted the backup_new_wal_key branch August 19, 2025 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants