Skip to content

Some comments on documentation #436

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

Draft
wants to merge 1 commit into
base: TDE_REL_17_STABLE
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions contrib/pg_tde/documentation/docs/architecture/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

Let's break down what it means.

AÅ: We need to make it clear that this file are the ambitions for pg_tde, because a lot of this is not yet in there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So, I wanted to mention that architecture is similar to design here, so I thought why not combine these two. Added the following:

This topic outlines the long-term architectural goals and intended design for pg_tde. While many of the described components are implemented, however some features are still under active development and may not yet be available in the current release.

Let's break down the key building blocks of this design.

**Customizable** means that `pg_tde` aims to support many different use cases:

* Encrypting either every table in every database or only some tables in some databases
Expand Down Expand Up @@ -349,8 +351,10 @@ In this case existing references to global providers, or the global default prin
## Typical setup scenarios

### Simple "one principal key" encryption
AÅ: I'm no linguist, but since these are instructions for someone to follow the progressive tense doesn't seem right to me. Even weirder that step 5 and 6 does not use the progressive tense but the rest does.

1. Passing the option from the postgres config file the extension: `shared_preload_libraries=‘pg_tde’`
AÅ: The above sentence makes no sense to me
2. `CREATE EXTENSION pg_tde;` in `template1`
3. Adding a global key provider
4. Adding a default principal key using the same global provider
Expand All @@ -371,6 +375,7 @@ encryption is managed by the admins, normal users only have to create tables wit
specific databases HAVE to use the global key provider

Note: setting the `default_table_access_method` to `tde_heap` is possible, but instead of `ALTER SYSTEM` only per database using `ALTER DATABASE`, after a principal key is configured for that specific database.
AÅ: Above language seems a bit awkward here to me

Alternatively `ALTER SYSTEM` is possible, but table creation in the database will fail if there's no principal key for the database, that has to be created first.

Expand All @@ -382,5 +387,6 @@ Alternatively `ALTER SYSTEM` is possible, but table creation in the database wil
4. Changing the WAL encryption to use the proper global key provider

No default configuration: key providers / principal keys are configured as a per database level, permissions are managed per database
AÅ: what does "configured as a per database level" mean?

Same note about `default_table_access_method` as above - but in a multi tenant setup, `ALTER SYSTEM` doesn't make much sense.
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# pg_waldump

AÅ: We should probably include a notice about WAL encrypting being BETA and this information is subject to change

[`pg_waldump` :octicons-link-external-16:](https://www.postgresql.org/docs/current/pgwaldump.html) is a tool to display a human-readable rendering of the Write-Ahead Log (WAL) of a PostgreSQL database cluster.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added this note:

The WAL encryption feature is currently in beta and is not effective unless explicitly enabled. It is not yet production ready. Do not enable this feature in production environments.

To read encrypted WAL records, `pg_waldump` supports the following additional arguments:
Expand Down
4 changes: 4 additions & 0 deletions contrib/pg_tde/documentation/docs/faq.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ The initial decision on what file to encrypt is based on the table access method
The principal key is used to encrypt the internal keys. The principal key is stored in the key management store. When you query the table, the principal key is retrieved from the key store to decrypt the table. Then the internal key for that table is used to decrypt the data.

### WAL encryption
AÅ: We should mention this is a BETA feature and subject to change.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added a note:

WAL encryption is currently in beta and is not effective unless explicitly enabled. It is not yet production ready. Do not enable this feature in production environments.

WAL encryption is done globally for the entire database cluster. All modifications to any database within a PostgreSQL cluster are written to the same WAL to maintain data consistency and integrity and ensure that PostgreSQL cluster can be restored to a consistent state. Therefore, WAL is encrypted globally.

Expand Down Expand Up @@ -117,6 +118,7 @@ The support of other encryption mechanisms such as AES256 is planned for future
## Is post-quantum encryption supported?

No, it's not yet supported. In our implementation we reply on OpenSSL libraries that don't yet support post-quantum encryption.
AÅ: Afaik newer versions of openssl support post-quantum algorithms. Maybe we should use some better excuse or mention what version we use?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Updated the paragraph to:

No, pg_tde does not support post-quantum encryption.

## Can I encrypt an existing table?

Expand All @@ -134,6 +136,7 @@ You must restart the database in the following cases to apply the changes:

* after you enabled the `pg_tde` extension
* to turn on / off the WAL encryption
AÅ: WAL encrytion is not yet production ready and should probably not be casually mentioned as something you can "turn on".

Copy link
Collaborator

@Andriciuc Andriciuc Jun 17, 2025

Choose a reason for hiding this comment

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

updated this to turn on/off line to:

  • when enabling WAL encryption, which is currently in beta. Do not enable this feature in production environments.

After that, no database restart is required. When you create or alter the table using the `tde_heap` access method, the files are marked as those that require encryption. The encryption happens at the storage manager level, before a transaction is written to disk. Read more about [how tde_heap works](index/table-access-method.md#how-tde_heap-works).

Expand All @@ -150,6 +153,7 @@ In `pg_tde`, multi-tenancy is supported via a separate principal key per databas
To control user access to the databases, you can use role-based access control (RBAC).

WAL files are encrypted globally across the entire PostgreSQL cluster using the same encryption keys. Users don't interact with WAL files as these are used by the database management system to ensure data integrity and durability.
AÅ: Again, WAL encryption hould probably not be mentioned as if it was a feature already.

Copy link
Collaborator

@Andriciuc Andriciuc Jun 17, 2025

Choose a reason for hiding this comment

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

commented the above paragraph out for now regarding WAL

## Are my backups safe? Can I restore from them?

Expand Down
3 changes: 3 additions & 0 deletions contrib/pg_tde/documentation/docs/features.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,18 @@ The following features are available for the extension:
* Index data for encrypted tables
* TOAST tables
* Temporary tables created during database operations
AÅ: Created how?

!!! note
Metadata of those tables is not encrypted.

* Global Write-Ahead Log (WAL) encryption for data in both encrypted and non-encrypted tables
AÅ: WAL encryption should not be mentioned as a feature since we don't yet have it in more than a beta preview.
* Single-tenancy support via a global keyring provider
* Multi-tenancy support
* Table-level granularity for encryption and access control
* Multiple Key management options
* Logical replication of encrypted tables
AÅ: We do not do anything for logical replication, do we?

Copy link
Collaborator

Choose a reason for hiding this comment

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

  • Removed temporary tables feature to clear confusion, removed logical replication mention, removed WAL encryption as a feature.

[Overview](index/index.md){.md-button} [Get Started](install.md){.md-button}
5 changes: 5 additions & 0 deletions contrib/pg_tde/documentation/docs/functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ However, database owners can run the “view keys” and “set principal key”

* `GRANT EXECUTE`
* `REVOKE EXECUTE`
AÅ: This should probably say `GRANT EXECUTE ON FUNCTION` and similar for revoke

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fixed:

  • GRANT EXECUTE ON FUNCTION
  • REVOKE EXECUTE ON FUNCTION

## Key provider management

Expand Down Expand Up @@ -52,6 +53,8 @@ The `change` functions require the same parameters as the `add` functions. They

Provider specific parameters differ for each implementation. Refer to the respective subsection for details.

AÅ: We should probably have a notice here about the modified provider settings will need to be able to provide the exact same principal keys as the original settings.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added the following note for this:

!!! note
The updated provider must be able to retrieve the same principal keys as the original configuration.
If the new configuration cannot access existing keys, encrypted data and backups will become unreadable.

**Some provider specific parameters contain sensitive information, such as passwords. Never specify these directly, use the remote configuration option instead.**

#### Adding or modifying Vault providers
Expand Down Expand Up @@ -231,6 +234,7 @@ These functions list the details of all key providers for the current database o
## Principal key management

Use these functions to create a new principal key at a given keyprover, and to use those keys for a specific scope such as a current database, a global or default scope. You can also use them to start using a different existing key for a specific scope.
AÅ: "keyprover"?

Copy link
Collaborator

Choose a reason for hiding this comment

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

fixed, key provider.

Princial keys are stored on key providers by the name specified in this function - for example, when using the Vault provider, after creating a key named "foo", a key named "foo" will be visible on the Vault server at the specified mount point.

Expand Down Expand Up @@ -281,6 +285,7 @@ SELECT pg_tde_set_key_using_global_key_provider(
### pg_tde_set_server_key_using_global_key_provider

Sets or rotates the server principal key using the specified global key provider. Use this function to set a principal key for WAL encryption.
AÅ: Note that WAL encryption is not yet production ready

Copy link
Collaborator

@Andriciuc Andriciuc Jun 18, 2025

Choose a reason for hiding this comment

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

added the standard WAL warning here

```sql
SELECT pg_tde_set_server_key_using_global_key_provider(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ The key material (actual cryptographic key) is auto-generated by `pg_tde` and st
!!! note
This process sets the **default principal key** for the server. Any database without its own key configuration will use this key.

AÅ: This is confusing to me, but maybe makes sense in the rendered version. The Example below does _not_ set a default key, does it?

## Example

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have reworded the note and the below example to properly describe the correct param:

  • updated with correct code example using set_server_key_using_global parameter
  • updated note to reflect correct config

This example is for testing purposes only. Replace the key name and provider name with your values:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ You must do these steps for every database where you have created the extension.
=== "With HashiCorp Vault"

The Vault server setup is out of scope of this document.
AÅ: Why doesn't KMIP have a similar comment? Is KMIP more in scope than vault? Why isn't OpenBao mentioned here?

Copy link
Collaborator

@Andriciuc Andriciuc Jun 18, 2025

Choose a reason for hiding this comment

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

Good point! Added the same paragraph for KMIP

```sql
SELECT pg_tde_add_database_key_provider_vault_v2('provider-name', 'url', 'mount', 'secret_token_path', 'ca_path');
Expand Down Expand Up @@ -144,6 +145,7 @@ You must do these steps for every database where you have created the extension.

* `name-of-the-key` is the name of the principal key. You will use this name to identify the key.
* `provider-name` is the name of the key provider you added before. The principal key will be associated with this provider.
AÅ: It will not only be "associated" with it, it is where the key will be stored and fetched from.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Updated:

  • provider-name is the name of the key provider you added before. The principal key is associated with this provider and it is the location where it is stored and fetched from.

<i warning>:material-information: Warning:</i> This example is for testing purposes only:

Expand Down
2 changes: 2 additions & 0 deletions contrib/pg_tde/documentation/docs/how-to/uninstall.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ If you no longer wish to use TDE in your deployment, you can remove the `pg_tde`

Here's how to do it:

AÅ: We don't think that anyone would like to drop the extension, but keep their data? Ie. should this document mention decryption of tables before dropping rather than only how to drop the tables?

Copy link
Collaborator

@Andriciuc Andriciuc Jun 19, 2025

Choose a reason for hiding this comment

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

This is great feedback! To ensure we covered all possible scenarios I added a new step 1:

  1. Decrypt or drop encrypted tables:

Before removing the extension, you must either decrypt or drop all encrypted tables. pg_tde does not support decrypting tables in-place yet. Therefore:

  • To preserve the data: manually copy it into unencrypted tables
  • To discard data: drop the encrypted tables

Also, added a note of warning in the intro and reworded that a bit too:

!!! warning
This process removes the extension, but does not decrypt data automatically. Only uninstall the extension after all encrypted data has been removed or decrypted.

Step 2 is also updated:

  1. Drop the extension using the DROP EXTENSION command:

    DROP EXTENSION pg_tde;

    Alternatively, to remove everything at once:

    DROP EXTENSION pg_tde CASCADE;

    !!! note
    The DROP EXTENSION command does not delete the underlying pg_tde-specific data files from disk.

1. Drop the extension using the `DROP EXTENSION` command:

```sql
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ The extension is tightly integrated with Percona Server for PostgreSQL to delive
By using our PostgreSQL distribution, you get:

- **Full encryption support** through the `tde_heap` access method, including tables, indexes, WAL data, and logical replication.
AÅ: Logical replication?
Copy link
Collaborator

Choose a reason for hiding this comment

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

removed

- **Enhanced performance and enterprise-ready features** not available in community builds.
- **Regular updates and security patches** backed by Percona’s expert support team.
- **Professional support** and guidance for secure PostgreSQL deployments.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ Here is how you can set the new default table access method:
1. Add the access method to the `default_table_access_method` parameter:

=== "via the SQL statement"
AÅ: I think "the" shouldn't be here, should it? It's also weird that it's called a "statement" here but "command" below. Maybe "via ALTER SYSTEM" is a better heading?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Updated as such:

  1. Add the access method to the default_table_access_method parameter:

    === "via the ALTER SYSTEM command"

     Use `ALTER SYSTEM`, it requires superuser or `ALTER SYSTEM` privileges.
     
     ```sql
     ALTER SYSTEM SET default_table_access_method = tde_heap;
     ```
    

Use the `ALTER SYSTEM` command. This requires superuser or ALTER SYSTEM privileges.

Expand Down
2 changes: 2 additions & 0 deletions contrib/pg_tde/documentation/docs/index/tde-limitations.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Limitations of pg_tde

AÅ: We should mention that WAL encryption isn't yet ready for use rather than mentioning pg_rewind specifically.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Reorganized limitations, removed rewind mention and added WAL note as text, made small changes to RC version. Added note for KMS, improved system tables text.

* Keys in the local keyfile are stored unencrypted. For better security we recommend using the Key management storage.
* System tables are currently not encrypted. This means that statistics data and database metadata are currently not encrypted.

Expand Down
1 change: 1 addition & 0 deletions contrib/pg_tde/documentation/docs/setup.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ The `pg_tde` extension requires additional shared memory. You need to configure
You can configure the `shared_preload_libraries` parameter in two ways:

* Add the following line to the `shared_preload_libraries` file:
AÅ: Which file?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fixed this: * Add the following line to the postgresql.conf file: ...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also updated the last note:

!!! note
It’s recommended to use an external key provider (KMS) to manage encryption keys. For configuration instructions, see Next steps.

```bash
shared_preload_libraries = 'pg_tde'
Expand Down
1 change: 1 addition & 0 deletions contrib/pg_tde/documentation/docs/test.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ After enabling the `pg_tde` extension for a database, you can begin encrypting d
```

The function returns `t` if the table is encrypted and `f` - if not.
AÅ: The function returns `true` or `false`. the psql client specifically renders them as `t` or `f` however.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Updated with:

The function returns true or false. The psql client specifically renders them as t or f respectively.

3. (Optional) Rotate the principal key.

Expand Down
1 change: 1 addition & 0 deletions contrib/pg_tde/documentation/docs/variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
The `pg_tde` extension provides GUC variables to configure the behaviour of the extension:

## pg_tde.wal_encrypt
AÅ: Note that this shouldn't be enabled for production systems

Copy link
Collaborator

Choose a reason for hiding this comment

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

added warning note

**Type** - boolean <br>
**Default** - off
Expand Down
1 change: 1 addition & 0 deletions contrib/pg_tde/documentation/docs/yum.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Make sure you check the [list of supported platforms](install.md#__tabbed_1_1) b
The `pg_tde` uses memory locks (mlocks) to keep internal encryption keys in RAM, both for WAL and for user data.

A memory lock (`mlock`) is a system call to lock a specified memory range in RAM for a process. The maximum amount of memory that can be locked differs between systems. You can check the current setting with this command:
AÅ: This isn't necessarily true. Maybe OpenSSL does this under the hood? We don't do it in our code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, we have two options here, just mention that you can check the system limit using the command if user uses mlock, or this:

A memory lock (mlock) is a system call that prevents specified memory ranges from being swapped to disk. Some libraries, such as OpenSSL, may use mlock internally to protect sensitive data like encryption keys. However, pg_tde does not directly invoke mlock in its own code.

You can check the current system limits for locked memory using:

ulimit -l

Thoughts?

```bash
ulimit -a
Expand Down