Don't use pointless AAD for WAL keys #511
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (82.14%) 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 #511 +/- ##
=====================================================
+ Coverage 82.00% 82.14% +0.13%
=====================================================
Files 24 25 +1
Lines 3162 3186 +24
Branches 514 518 +4
=====================================================
+ Hits 2593 2617 +24
Misses 460 460
Partials 109 109
🚀 New features to boost your workflow:
|
|
AAD mean "additional authenticated data" not "additional authentication data" so you should fix that in the commit message. |
|
I am not opposed to removing it but just saying it is silly is a bit unclear and you should elaborate on it. |
Using the type field as additional authenticated data when encrypting the WAL keys doesn't add any additional protection as the AAD will be the same for every valid entry in the file. It's more clear to explicitly not use any AAD so that there is no risk of confusion over this. The best option for AAD we have currently is the wal_start field, however that will require some reworking of the machinery to update the wal_start of the last key in the file.
AndersAstrand
force-pushed
the
tde/dont-use-useless-aad-for-wal-keys
branch
from
1016bd4 to
d9a6577
Compare
Yeah I think I might abandon this and to it properly instead with using the wal_start as AAD. |
Using the type field as additional authentication data when encrypting the WAL keys seems silly. It's better to be explicit about no AAD being used.
I did want to use the
wal_startas AAD, but that would have needed some rework of the mechanism to write the lsn to the last key after a written wal segment. If there is time I might try to do it before release, but this will have to do for now.