Horizon fails with FilePermissionError: Insecure permissions on key file

[Yuki Hirano]

I deployed Horizon from source using the openstack-ansible master branch. When accessing the Horizon screen, an Internal Server Error is generated.
The following error recorded in the journal log of the Horizon container:

horizon.utils.secret_key.FilePermissionError: Insecure permissions on key file /openstack/venvs/horizon-28.0.0/lib/python3.10/site-packages/openstack_dashboard/local/.secret_key_store, should be 0600.

Please see the attachment for the complete traceback.

The first deployment was successful. Applying the ansible playbook twice may reproduce this.
The mode of all non-executable regular files under the `/openstack/venvs/horizon-28.0.0/lib/python3.10/site-packages/` is incorrect.

$ sudo lxc-attach -n opstk-1_horizon_container-876f9700 -- ls -lha /openstack/venvs/horizon-28.0.0/lib/python3.10/site-packages/openstack_dashboard/local/
total 36K
drwxr-xr-x 5 horizon horizon 4.0K Feb 1 08:16 .
drwxr-xr-x 19 horizon horizon 4.0K Feb 1 08:16 ..
-rwxr-xr-x 1 horizon horizon 64 Feb 1 08:16 .secret_key_store
-rwxr-xr-x 1 horizon horizon 0 Feb 1 08:16 __init__.py
drwxr-xr-x 2 horizon horizon 4.0K Feb 1 08:16 __pycache__
-rwxr-xr-x 1 horizon horizon 0 Feb 1 08:16 _openstack_venvs_horizon-28.0.0_lib_python3.10_site-packages_openstack_dashboard_local_.secret_key_store.lock
drwxr-xr-x 3 horizon horizon 4.0K Feb 1 08:16 enabled
drwxr-xr-x 2 horizon horizon 4.0K Feb 1 08:16 local_settings.d
lrwxrwxrwx 1 horizon horizon 30 Feb 1 08:16 local_settings.py -> /etc/horizon/local_settings.py
-rwxr-xr-x 1 horizon horizon 12K Feb 1 08:16 local_settings.py.example

The location where the problem is occurring is the `Ensure horizon dirs are accessible by user` task in `/etc/ansible/roles/os_horizon/tasks/horizon_post_install.yml`. The patch below solves this problem.

diff --git a/tasks/horizon_post_install.yml b/tasks/horizon_post_install.yml
index 5739e70..6cfc2c1 100644
--- a/tasks/horizon_post_install.yml
+++ b/tasks/horizon_post_install.yml
@@ -36,7 +36,6 @@
     state: directory
     owner: "{{ horizon_system_user_name }}"
     group: "{{ horizon_system_group_name }}"
- mode: "0755"
     recurse: yes
   with_items:
     - { path: "{{ horizon_lib_dir }}", fixup: True }

The above patch gets Horizon working, but it won't fix the underlying problem.

It is inappropriate to create files written by app such as `.secret_key_store` inside the python site-packages directory. The write destination should be under /srv/, /var/lib/, /var/cache/.

----

Additional information:

openstack-ansible version: master brainch (32e503af74ba89a30a97945ca7399ed2e7fa1ca5)
Same error reproduced using openstack-ansible v2023.2.

Command to deploy the Horizon: (cd /opt/openstack-ansible/playbooks/ && openstack-ansible setup-everything.yml)
OS: Ubuntu 22.04

Related bug: https://bugs.launchpad.net/openstack-ansible/+bug/1403917

[Dmitriy Rabotyagov (noonedeadpunk)]

Hi,

I finally got some time to work on this issue.

Looking at my sandbox right now, I think there're 2 different issues right now.

First one, which is obviously wrong, is regarding wrong permissions set for `openstack_dashboard/local/` folder. And this one I will fix right away.

The second part is regarding .secret_key_store specifically. This is actually a bit different. In a default template of user_secrets.yml we do have `horizon_secret_key` variable, which is expected to be populated during initial setup:
https://opendev.org/openstack/openstack-ansible/src/branch/master/etc/openstack_deploy/user_secrets.yml#L136

This variable is then used to define the `SECRET_KEY` and ensures, that such key will be the same across all backends.

However, if the variable `horizon_secret_key` is not defined, the suggested by horizon approach will be used, which is to place .secret_key_store under LOCAL_PATH:
https://docs.openstack.org/horizon/latest/configuration/settings.html#secret-key

Which is exactly what we are doing:
https://opendev.org/openstack/openstack-ansible-os_horizon/src/commit/d4ef66fc028477709d6e4bd36a2bb39c957c3eae/templates/horizon_local_settings.py.j2#L140-L154

You should be also able to define any arbitrary path for the secret key, by leveraging `horizon_config_overrides` variable, for example:

horizon_config_overrides:
  SECRET_KEY: secret_key.generate_or_read_from_file('/var/cache/.secret_key_store')

But for multi-backend setup I'd suggest to use `horizon_secret_key` as secret key should be the same across all workers.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/openstack-ansible-os_horizon/+/911420

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/openstack-ansible-os_horizon/+/911420
Committed: https://opendev.org/openstack/openstack-ansible-os_horizon/commit/47364bcadd017372dba88b78d8a89d263637afa0
Submitter: "Zuul (22348)"
Branch: master

commit 47364bcadd017372dba88b78d8a89d263637afa0
Author: Dmitriy Rabotyagov <email address hidden>
Date: Wed Mar 6 11:24:40 2024 +0100

    Do not change mode of files recursively

    Current behavior leads to all files having executable bit which is not
    anticipated or required behaviour.

    Thus, we should avoid defining mode recursively to the directory

    Closes-Bug: #2052011
    Change-Id: I30b9b6a70d2cabfb1f1f434cd883ea2503d867bc

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/2023.2
Review: https://review.opendev.org/c/openstack/openstack-ansible-os_horizon/+/914015

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/2023.1
Review: https://review.opendev.org/c/openstack/openstack-ansible-os_horizon/+/914016

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/zed
Review: https://review.opendev.org/c/openstack/openstack-ansible-os_horizon/+/914017

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/openstack-ansible-os_horizon/+/914015
Committed: https://opendev.org/openstack/openstack-ansible-os_horizon/commit/b737e476958d904852cfbebbc2e346c677b0e89b
Submitter: "Zuul (22348)"
Branch: stable/2023.2

commit b737e476958d904852cfbebbc2e346c677b0e89b
Author: Dmitriy Rabotyagov <email address hidden>
Date: Wed Mar 6 11:24:40 2024 +0100

    Do not change mode of files recursively

    Current behavior leads to all files having executable bit which is not
    anticipated or required behaviour.

    Thus, we should avoid defining mode recursively to the directory

    Closes-Bug: #2052011
    Change-Id: I30b9b6a70d2cabfb1f1f434cd883ea2503d867bc
    (cherry picked from commit 47364bcadd017372dba88b78d8a89d263637afa0)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/openstack-ansible-os_horizon/+/914016
Committed: https://opendev.org/openstack/openstack-ansible-os_horizon/commit/a30d5219525081382377e165587e5796d4fdb245
Submitter: "Zuul (22348)"
Branch: stable/2023.1

commit a30d5219525081382377e165587e5796d4fdb245
Author: Dmitriy Rabotyagov <email address hidden>
Date: Wed Mar 6 11:24:40 2024 +0100

    Do not change mode of files recursively

    Current behavior leads to all files having executable bit which is not
    anticipated or required behaviour.

    Thus, we should avoid defining mode recursively to the directory

    Closes-Bug: #2052011
    Change-Id: I30b9b6a70d2cabfb1f1f434cd883ea2503d867bc
    (cherry picked from commit 47364bcadd017372dba88b78d8a89d263637afa0)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/openstack-ansible-os_horizon/+/914017
Committed: https://opendev.org/openstack/openstack-ansible-os_horizon/commit/caa87160de22bb4d72cccd4aa78aa571c00fa56f
Submitter: "Zuul (22348)"
Branch: stable/zed

commit caa87160de22bb4d72cccd4aa78aa571c00fa56f
Author: Dmitriy Rabotyagov <email address hidden>
Date: Wed Mar 6 11:24:40 2024 +0100

    Do not change mode of files recursively

    Current behavior leads to all files having executable bit which is not
    anticipated or required behaviour.

    Thus, we should avoid defining mode recursively to the directory

    Closes-Bug: #2052011
    Change-Id: I30b9b6a70d2cabfb1f1f434cd883ea2503d867bc
    (cherry picked from commit 47364bcadd017372dba88b78d8a89d263637afa0)