This whole thing feels a little fragile, partially on account of attempting to keep it minimally invasive on the DB structure. In particular, it's not easy to tell that an encryption key is definitely no longer in use. This is very important to security people, especially when you are rotating a key that you think might have been compromised.

The Sentinel is a nice idea, but its current powers are limited. It just makes sure that you have the old key before starting to encrypt with a new key, and it only affects things on startup.

So, if you have multiple coderds working, and update some but not all of them with a new key, you might not immediately notice problems. The old coderds keep working, and when they encounter data they can't decrypt, they make the user re-authenticate and encrypt with the old key. The new coderds don't notice this at all --- they can decrypt data from the old key.

Then, you go and run coder dbcrypt-rotate and encrypt all the data with the new key, and then restart without the old key. In this thought experiment, you miss the same coderds for the same reason, so there are still some coderds out there happily encrypting with the old key you thought you got rid of.

Instead of a singleton sentinel, consider a table of key identities

CREATE TABLE dbcrypt_keys (
    number int NOT NULL PRIMARY KEY,
    active_key_id text,
    revoked_key_id text,
    test text
);

Active keys have the active_key_id set, and revoked keys have the revoked_key_id set (for posterity). test is like the current sentinel, it contains the string coder encrypted with the key, so you can verify that you have the matching key by trying to decrypt it.

For encrypted colums, we break them into two columns: a xxxxxxx_key_id column that has a foreign key relationship to dbcrypt_keys.active_key_id and the regular value column that contains unencrypted data (if key_id is NULL) or encrypted data. The foreign key relationship prevents revoking the key if there is still data encrypted with it, and prevents new data from being inserted once it is revoked.

To rotate a key, you do a transaction that reads the dbcrypt_keys table, verifies you have all the active keys, and then inserts a new key with the next number. The number is a primary key for the table, so multiple coderds can't insert different successor keys.

Then, you migrate all the data off they key you want to revoke, and update the row to NULL active_key_id and set revoked_key_id. Once revoked, coderds can be safely restarted without the key, as they just have to check that they have all active keys.

Instead of a singleton sentinel, consider a table of key identities

CREATE TABLE dbcrypt_keys (
    number int NOT NULL PRIMARY KEY,
    active_key_id text,
    revoked_key_id text,
    test text
);

Active keys have the active_key_id set, and revoked keys have the revoked_key_id set (for posterity). test is like the current sentinel, it contains the string coder encrypted with the key, so you can verify that you have the matching key by trying to decrypt it.

For encrypted colums, we break them into two columns: a xxxxxxx_key_id column that has a foreign key relationship to dbcrypt_keys.active_key_id and the regular value column that contains unencrypted data (if key_id is NULL) or encrypted data. The foreign key relationship prevents revoking the key if there is still data encrypted with it, and prevents new data from being inserted once it is revoked.

To rotate a key, you do a transaction that reads the dbcrypt_keys table, verifies you have all the active keys, and then inserts a new key with the next number. The number is a primary key for the table, so multiple coderds can't insert different successor keys.

Then, you migrate all the data off they key you want to revoke, and update the row to NULL active_key_id and set revoked_key_id. Once revoked, coderds can be safely restarted without the key, as they just have to check that they have all active keys.

I was initially avoiding making too many schema changes, but this has a number of nice properties:

1. We can drop the dbcrypt- magic prefix as the null-ness of xxx_key_id determines if the data is encrypted or not,
2. We can additionally stop prefixing the encrypted value with the key's checksum, as (I assume at least) we would be storing the key checksum in the xxx_key_id column.
3. We have schema-level referential integrity of encrypted data (cannot overstate how huge this is)

It also has some cons:

1. You get a proliferation of xxx_key_id columns everywhere that the the AGPL code needs to be at least cursorily aware of even though they should never be set in AGPL code. This is fine from a licensing perspective though as the structs, schema etc. are all AGPL.
2. Adding new encrypted fields becomes a database migration (although arguably this is also a plus)

Overall it definitely seems like a useful and worthwhile change 👍

Closing in favour of #9522