| From: | Henson Choi <assam258(at)gmail(dot)com> |
|---|---|
| To: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
| Cc: | Konstantin Knizhnik <knizhnik(at)garret(dot)ru>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Tomas Vondra <tomas(at)vondra(dot)me> |
| Subject: | Re: RFC: PostgreSQL Storage I/O Transformation Hooks |
| Date: | 2025-12-30 00:39:17 |
| Message-ID: | CAAAe_zBAY67JRREy3qV_c7QiYgeEzkszTpXpc2X-+6UHWxDbbQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Zsolt,
Thank you for the detailed feedback.
## SMGR Patch
You're right - I shouldn't argue about SMGR without actually reviewing
the patch. Let me step back from the SMGR discussion for now and focus
this proposal on WAL hooks only.
Could you point me to where I can access the SMGR extensibility patch?
I'd like to review it properly before any further discussion.
For SMGR, I'm also thinking about a different approach that could cover
Bootstrap and Frontend processes as well - but that's a separate
conversation after I understand the current SMGR proposal better.
## Reference Implementation Scope
As mentioned in my earlier messages, the reference implementation
(test_tde) intentionally doesn't cover key rotation and other
production concerns. Its purpose is to demonstrate the
hook API, not to be a production-ready TDE solution.
## Encryption Library Abstraction
I agree in principle that an abstraction layer would be ideal.
Personally, I prefer developing with OpenSSL and getting an OpenSSL
Provider certified at the company level.
However, our CTO (who comes from a cryptography company background)
insists on using their long-maintained proprietary encryption library.
It's a complex C++ implementation that cannot follow critical section
constraints at all. :)
## 2-Level Key Architecture
Our production implementation also uses a 2-level key architecture
(master key → separate smgr/wal keys). The reference implementation
uses a simplified single-key approach just for demonstration purposes.
I've been considering further key granularity (e.g., per-tablespace
or per-database keys), but there are unresolved challenges:
- Key distribution to replicas
- Some DDL operations that complete by simple file copying
Until these are solved, we're keeping the smgr key at a coarser
granularity.
I'm also exploring TPM integration for auto-login master key
protection. How does pg_tde handle master key storage and auto-login
scenarios?
## Timeline ID in IV
Good catch - I hadn't considered that. Including timeline ID would
make the IV more robust. Thank you for sharing this insight.
Best regards,
Henson
2025년 12월 29일 (월) PM 4:37, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>님이 작성:
> > The main difference is timing and current availability:
> >
> > - The hook approach is working today and can be used immediately
> . - Your SMGR extensibility work provides a more comprehensive
> long-term solution
>
> I disagree with this. The SMGR patch is available since 2023/PG16 as a
> patch, and it is already used by at least 3 companies I know of (Neon,
> Nile, Percona), and probably also by others I don't know of. It is
> available immediately.
>
> Compared to that this proposal is something new, and more limited.
>
> The actual advantage of this proposal is that it includes WAL, but I
> still think the two should be separate discussions.
>
> > Regarding what to protect (WAL vs heap vs both), there's flexibility
> depending on the organization and jurisdiction. The hook approach allows
> extensions to choose - you can implement only the buffer hooks if that
> satisfies your requirements, or add WAL hooks if needed.
>
> My concern is that these two separate discussion about 2 extensibility
> points, with different concerns by different people. One part
> shouldn't stall the other, as for some, even getting half of it into
> the core for PG19 would be useful.
>
> > You're absolutely right that extension developers need to understand
> multiprocess architecture, memory management, critical sections, and so on.
>
> > This is precisely why test_tde exists as a reference implementation.
>
> The reference implementation ignores the tricky steps, like key
> rotation, caching, configuration, providing a user interface, etc,
> which all require knowledge of postgres internals.
>
> > ARIA and SEED are already implemented in OpenSSL. However, Korean law
> requires certified implementations. Specifically, companies must use
> nationally-certified builds and provide the hash codes of those specific
> library binaries to regulators. You cannot simply use the OpenSSL version,
> even if the algorithm is identical.
>
> That could be still solved by introducing an abstraction layer in the
> encryption code of a TDE extension :) Encryption is only a small part
> of an extension, the other parts (user interface, rotation, key
> storage integrations, etc) are a much bigger part. It is still
> questionable to reimplement everything because of an encryption
> library difference. But I see your point, that is a bit more
> difficult.
>
> > That's a reasonable approach for SMGR-based solutions where you control
> the storage layer. However, with the hook approach, we don't have the
> ability to inject custom WAL records for encryption events.
>
> > Currently, in a replication environment, the reference implementation
> requires the same key to be configured in the settings on both primary and
> replicas (shared key model). For future KMS integration, I'm considering
> mechanisms to propagate keys to replicas through external channels rather
> than WAL.
>
> I originally wrote a long answer about how I don't think this is
> related to where the hooks are, and then I realized that the problem
> is probably completely different - and this also shows why adding a
> few bits to the pages is not a good generic solution for all
> extensions.
>
> Our extension uses a 2 level key architecture, as used by most
> database servers (there's a master key, and it encodes separate
> internal keys, one for each database file). The proposed sample code
> in your patch uses a single key, with the IV encoding the database
> file. That means you want to encode which key is used for each page
> instead of for each file.
>
> So we approach how we map data/pages to keys completely differently.
> But I don't think the page header addition is a good solution, because
> it is specific to your implementation, not for encryption solutions in
> general.
>
> (Also, I just noticed that you forgot about timelineid in derive_iv,
> you probably want to include that somehow)
>
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Bryan Green | 2025-12-30 01:05:53 | Re: [PATCH] Allow complex data for GUC extra. |
| Previous Message | Masahiko Sawada | 2025-12-30 00:01:18 | Re: Newly created replication slot may be invalidated by checkpoint |