Bug 33934 - 'No encryption_key in koha-conf.xml' needs more detail
Summary: 'No encryption_key in koha-conf.xml' needs more detail
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major
Assignee: Jonathan Druart
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 28998
Blocks:
  Show dependency treegraph
 
Reported: 2023-06-07 07:27 UTC by Jonathan Druart
Modified: 2024-07-04 20:40 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes an issue that can cause upgrades to Koha 23.05 to fail with an error message that includes 'No encryption_key in koha-conf.xml'. It also requires the configuration entry in the instance koha-conf.xml to be something other than __ENCRYPTION_KEY__. It is recommended that the key is generated using pwgen 32
Version(s) released in:
23.11.00,23.05.01,22.11.07
Circulation function:


Attachments
Bug 33934: Add more detail to 'No encryption_key in koha-conf.xml' (2.49 KB, patch)
2023-06-07 07:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33934: Add more detail to 'No encryption_key in koha-conf.xml' (2.53 KB, patch)
2023-06-07 08:00 UTC, David Nind
Details | Diff | Splinter Review
Bug 33934: Add more detail to 'No encryption_key in koha-conf.xml' (2.60 KB, patch)
2023-06-07 08:28 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2023-06-07 07:27:56 UTC
If encryption_key is not set in $KOHA_CONF we are raising an exception.

This key was only needed for a couple of feature, but now we are using Koha::Encryption from the update DB process, and so the upgrade fails with no more info than 'No encryption_key in koha-conf.xml'.

We need to provide more detail in this error.
Comment 1 Jonathan Druart 2023-06-07 07:30:10 UTC
https://lists.katipo.co.nz/pipermail/koha/2023-June/059566.html
https://lists.katipo.co.nz/pipermail/koha/2023-June/059581.html

Additionally the following answer can be very confusing:

"""
I added the lines:
 <!-- Encryption key for crypted password or sensitive data -->
 <encryption_key>__ENCRYPTION_KEY__</encryption_key>

The update went through without a problem.
"""

https://lists.katipo.co.nz/pipermail/koha/2023-June/059582.html

We must reject "__ENCRYPTION_KEY__", just in case...
Comment 2 Jonathan Druart 2023-06-07 07:33:41 UTC
Created attachment 152074 [details] [review]
Bug 33934: Add more detail to 'No encryption_key in koha-conf.xml'

If encryption_key is not set in $KOHA_CONF we are raising an exception.

This key was only needed for a couple of feature, but now we are using Koha::Encryption from the update DB process, and so the upgrade fails with no more info than 'No encryption_key in koha-conf.xml'.

We need to provide more detail in this error.

Additionally we reject "__ENCRYPTION_KEY__", in case people will simple
copy/paste that

Test plan:
Apply the patch
Edit $KOHA_CONf, remove the encryption_key entry (or blank)
restart_all
Go to the about page and enable 2FA
=> warning on the about page, and 500 server-side are displaying more
info about how to generate the missing entry
Edit $KOHA_CONF and set the value to __ENCRYPTION_KEY__
restart_all
=> Same messages
Edit $KOHA_CONF and set a correct value
restart_all
=> No error, everything is working correctly
Comment 3 David Nind 2023-06-07 08:00:53 UTC
Created attachment 152075 [details] [review]
Bug 33934: Add more detail to 'No encryption_key in koha-conf.xml'

If encryption_key is not set in $KOHA_CONF we are raising an exception.

This key was only needed for a couple of feature, but now we are using Koha::Encryption from the update DB process, and so the upgrade fails with no more info than 'No encryption_key in koha-conf.xml'.

We need to provide more detail in this error.

Additionally we reject "__ENCRYPTION_KEY__", in case people will simple
copy/paste that

Test plan:
Apply the patch
Edit $KOHA_CONf, remove the encryption_key entry (or blank)
restart_all
Go to the about page and enable 2FA
=> warning on the about page, and 500 server-side are displaying more
info about how to generate the missing entry
Edit $KOHA_CONF and set the value to __ENCRYPTION_KEY__
restart_all
=> Same messages
Edit $KOHA_CONF and set a correct value
restart_all
=> No error, everything is working correctly

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2023-06-07 08:17:18 UTC
Testing notes (using KTD):

1. After removing the <encryption_key> entry in /etc/koha/sites/kohadev/koha-conf.xml and restarting, there is:
  - a warning in About Koha > System information about the encryption key missing
  - after enabling the TwoFactorAuthentication system preference, got to 'Manage two-factor authentication' for the koha user - you should get an error message pop up when trying to set up 2FA

2. If you try to add <encryption_key>__ENCRYPTION_KEY__</encryption_key> you will also get the warning and error message.

3. Changing __ENCRYPTION_KEY__ to something else will remove the warning and let you set up 2FA.
Comment 5 Martin Renvoize (ashimema) 2023-06-07 08:28:17 UTC
Created attachment 152076 [details] [review]
Bug 33934: Add more detail to 'No encryption_key in koha-conf.xml'

If encryption_key is not set in $KOHA_CONF we are raising an exception.

This key was only needed for a couple of feature, but now we are using Koha::Encryption from the update DB process, and so the upgrade fails with no more info than 'No encryption_key in koha-conf.xml'.

We need to provide more detail in this error.

Additionally we reject "__ENCRYPTION_KEY__", in case people will simple
copy/paste that

Test plan:
Apply the patch
Edit $KOHA_CONf, remove the encryption_key entry (or blank)
restart_all
Go to the about page and enable 2FA
=> warning on the about page, and 500 server-side are displaying more
info about how to generate the missing entry
Edit $KOHA_CONF and set the value to __ENCRYPTION_KEY__
restart_all
=> Same messages
Edit $KOHA_CONF and set a correct value
restart_all
=> No error, everything is working correctly

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize (ashimema) 2023-06-07 08:29:20 UTC
All working as described.  Did we work out a better way to accomplish the DB update that was causing issues?

Anyway, this can go PQA as is.. scripts happy, works clearly.

Passing
Comment 7 Jonathan Druart 2023-06-07 09:21:45 UTC
Removing the example from the release notes.
Comment 8 Jonathan Druart 2023-06-07 09:22:31 UTC
(In reply to Martin Renvoize from comment #6)
> All working as described.  Did we work out a better way to accomplish the DB
> update that was causing issues?

See bug 30649 comment 35.
Comment 9 Tomás Cohen Arazi (tcohen) 2023-06-07 18:34:13 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 10 David Cook 2023-06-08 01:21:18 UTC
Very useful change ﷐[U+1F44D]﷑
Comment 11 Matt Blenkinsop 2023-06-12 11:56:30 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x
Comment 14 Jonathan Druart 2023-07-13 06:56:58 UTC
Please keep in mind that this key shall not be shared!
Comment 15 Marcel de Rooy 2023-10-03 14:34:23 UTC
For purists, taken from Crypt::CBC

-pass,-key      The encryption/decryption passphrase. These arguments
                   are interchangeable, but -pass is preferred
                   ("key" is a misnomer, as it is not the literal 
                   encryption key).

So our "encryption key" is a misnnomer too :)
It is a passphrase used to generate the real encryption key.

We say now: We recommend one of at least 32 bytes.
It should be formally at least 16 bytes (AES blocksize) in order to be safe. So 32 is fine. Shouid we enforce a minimum length in Koha::Encryption?
Comment 16 David Cook 2023-10-03 23:23:08 UTC
(In reply to Marcel de Rooy from comment #15)
> For purists, taken from Crypt::CBC
> 
> -pass,-key      The encryption/decryption passphrase. These arguments
>                    are interchangeable, but -pass is preferred
>                    ("key" is a misnomer, as it is not the literal 
>                    encryption key).
> 
> So our "encryption key" is a misnnomer too :)
> It is a passphrase used to generate the real encryption key.

That's true although I think colloquially it's all right to call it the encryption key. 

> We say now: We recommend one of at least 32 bytes.
> It should be formally at least 16 bytes (AES blocksize) in order to be safe.
> So 32 is fine. Shouid we enforce a minimum length in Koha::Encryption?

Probably a good idea.