Bug 32078 - We should have an easy way for an administrator to update the encryption keys
Summary: We should have an easy way for an administrator to update the encryption keys
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-11-02 16:00 UTC by Martin Renvoize
Modified: 2023-10-03 14:52 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2022-11-02 16:00:44 UTC
We now use encryption on a number of Koha database fields and utilise a key sorted in koha-conf.xml..

However, we have no easy way to change key should that key be leaked or found to be to simple to crack.  We should add a script to allow updating of our encrpyted values from one key to the next... (or alternatively, perhaps we should allow for an array of keys in our config and update the encryption on access whenever we find an prior key is in use?.. I believe this is what we did when we upgraded from SHA to BCrypt for user passwords).
Comment 1 David Cook 2022-11-02 22:53:24 UTC
Good call on adding functionality for key rotation!

The tricky thing is that we don't currently have a way of noting which key was used to encrypt which field.

In master, I'm only seeing Koha::Encryption used in Koha::Patron, and within Koha::Patron it's used in the function "encode_secret".

So we could put a "key_id" or "encrypt_key_id" field in the patron table row, and then look up the key that way.

Then the re-encryption tool could take a source ID and a destination ID for doing the re-encryption. 

If we wanted to automate it more, we could add additional metadata to the keys to indicate something like "revoked", "preferred", or whatever. But I think recording the key ID would be the first step. (You can see this all over AWS with KMS IDs.)
Comment 2 Victor Grousset/tuxayo 2022-11-27 04:09:43 UTC
> The tricky thing is that we don't currently have a way of noting which key was used to encrypt which field.

As long as there is one key at the time, it's not needed. The update process should be one transaction to guaranty that though.


> However, we have no easy way to change key should that key be leaked or found to be to simple to crack


It's generated so cracking shouldn't be an issue. As for a leak, yes a webserver misconfiguration or a vulnerability in Koha or another app on the same server could expose the config file while still having the DB unleaked.
Comment 3 David Cook 2022-11-27 23:57:25 UTC
(In reply to Victor Grousset/tuxayo from comment #2)
> > The tricky thing is that we don't currently have a way of noting which key was used to encrypt which field.
> 
> As long as there is one key at the time, it's not needed. The update process
> should be one transaction to guaranty that though.

That's not going to scale. If you have a large number of amount of data, that's going to be slow, require downtime, and potentially put unnecessary load on the database server. Large transactions are rarely a programmer's friend. 

That said, it would be easier in the short-term and for small databases. So certainly better than nothing.

> > However, we have no easy way to change key should that key be leaked or found to be to simple to crack
> 
> 
> It's generated so cracking shouldn't be an issue. As for a leak, yes a
> webserver misconfiguration or a vulnerability in Koha or another app on the
> same server could expose the config file while still having the DB unleaked.

Theoretically we might learn of new computing methods that mean the generated key is too weak, and we need to employ a different algorithm. What Martin is saying is that there's no way to currently re-encrypt using a stronger/unknown key.
Comment 4 Marcel de Rooy 2023-10-03 14:51:52 UTC
Bug 34976 - Encryption keys should not be shared between modules
Comment 5 Marcel de Rooy 2023-10-03 14:52:40 UTC
(In reply to David Cook from comment #1)
> In master, I'm only seeing Koha::Encryption used in Koha::Patron, and within
> Koha::Patron it's used in the function "encode_secret".

Unfortunately this is no longer true. EDI and Acquisition started using it too. See the other report.