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 encryption support for hookshot #2979
draft encryption support for hookshot #2979
Conversation
cmd: | | ||
{{ devture_systemd_docker_base_host_command_docker }} run | ||
--rm | ||
--name={{ matrix_hookshot_container_url }}-reset-crypto |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's quite unfortunate that the container-name (and what we call _ident
in our roles) is called matrix_hookshot_container_url
here, making you think it's http://something-something
-- a value unsuitable for a container name.
In actuality, matrix_hookshot_container_url
is just matrix-hookshot
- a container name / identifier.
port: 6379 | ||
host: matrix-redis |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These assumptions should not be hardcoded and should use variables (matrix_hookshot_queue_host
and matrix_hookshot_queue_port
).
The port could default to 6379
, since that's quite standard for Redis.
The host should default to empty. Perhaps you can only enable the queue (if
statement) if the host is non-empty. Example: {% if matrix_hookshot_queue_host %}
The matrix_hookshot_queue_host
and matrix_hookshot_queue_port
variables can be set to the correct values (matrix-redis
, ..) via group_vars/matrix_servers
- the file we use for wiring the different roles together. It's better to explicitly wire things via this central file, instead of having roles randomly assume things about other roles or each into each other's variables.
matrix_hookshot_queue_host
would be defined as such in group_vars/matrix_servers
:
matrix_hookshot_queue_host: "{{ redis_identifier if matrix_hookshot_experimental_encryption_enabled and redis_enabled else '' }}"
This: provides sane defaults for these variables, potentially overrides them via the group vars file.. and still allows people to use external Redis instances, if they want to.
experimentalEncryption: | ||
storagePath: /data/encryption |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be out of the if
that deals with the queue
stuff, since one may wish to enable the queue without enabling experimental encryption.
Experimental encryption does probably require the queue
configuration, but I suppose Hookshot checks for this by itself. You may add a check in validate_config.yml
if you're feeling like it though.
experimentalEncryption
settings should be dependant on {% if matrix_hookshot_experimental_encryption_enabled %}
.
{% if matrix_hookshot_experimental_encryption_enabled %} | ||
ExecStartPre={{ devture_systemd_docker_base_host_command_docker }} network connect matrix-redis {{ matrix_hookshot_container_url }} | ||
{% endif %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed before, it's not great for roles to make assumptions about other roles.
The assumption that this role makes there is that when encryption is enabled, Redis will also be enabled via nothing else but matrix-redis
.
It's better to redo this by:
- adding a new
matrix_hookshot_container_additional_networks
variable, which can connect the Hookshot container to any arbitrary additional network. In some roles, we even split this into multiple lists of networks (multiple variables) indefaults/main.yml
:matrix_hookshot_container_additional_networks: "{{ matrix_hookshot_container_additional_networks_auto + matrix_hookshot_container_additional_networks_custom }}"
matrix_hookshot_container_additional_networks_auto: []
matrix_hookshot_container_additional_networks_custom: []
- With the above,
matrix_hookshot_container_additional_networks_auto
is supposed to be managed by the playbook, whilematrix_hookshot_container_additional_networks_custom
is supposed to be managed by the user
matrix_hookshot_container_additional_networks_auto
may be redefined ingroup_vars/matrix_servers
to injectredis_container_network
(equal tomatrix-redis
) into the list of networks under certain conditions (ifmatrix_hookshot_queue_host == redis_identifier and redis_enabled
). You can find various examples of_additional_networks
redefinitions ingroup_vars/matrix_servers
- redoing this systemd
.service
file, so that it loops over the list of networks (matrix_hookshot_container_additional_networks
) and connects to them all
With this, the role is configurable and does not hardcode anything about matrix-redis
. All wiring happens in group_vars/matrix_servers
and people can do their own network wiring or use external Redis if they need to.
@@ -23,6 +23,11 @@ Other configuration options are available via the `matrix_hookshot_configuration | |||
|
|||
Finally, run the playbook (see [installing](installing.md)). | |||
|
|||
### End-to-bridge endcryption |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: endcryption -> encryption
Squashed based on the work done in #3042 commit 49932b8 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:21:31 2023 +0200 Fix syntax in matrix-bridge-hookshot/tasks/reset_encryption.yml Also, this task always does work and side-effects, so it should always report changes (`changed_when: true`). commit 6bdf7a9 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:12:41 2023 +0200 Add Hookshot validation task to ensure queue settings are set when encryption is enabled commit 8c531b7 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:10:17 2023 +0200 Add missing variables rewiring in group_vars/matrix_servers for Hookshot commit 7d26dab Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:08:19 2023 +0200 Add defaults for matrix_hookshot_queue_host and matrix_hookshot_queue_port commit 74f9113 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:06:17 2023 +0200 Fix syntax for connecting to additional networks for Hookshot commit ca7b41f Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:05:28 2023 +0200 Fix indentation and remove unnecessary if-statements commit ac4a918 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:04:44 2023 +0200 Add missing --network for Hookshot This seems to have been removed by accident. commit 6a81fa2 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:02:47 2023 +0200 Make automatic Redis enabling safer, when Hookshot encryption enabled If we ever default encryption to enabled for Hookshot, we only wish to force-enable Redis if Hookshot is actually enabled. commit 75a8e0f Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:01:10 2023 +0200 Fix typo commit 98ad182 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:37:40 2023 +0100 Add defaults for Hookshot's encryption commit 29fa9fa Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:35:11 2023 +0100 Improve wording of Hookshot's encryption section commit 4f835e0 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:28:52 2023 +0100 use safer mount options for the container's files commit 8c93327 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:26:01 2023 +0100 fix filename commit 03a7bb6 Merge: e55d769 0604776 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:23:44 2023 +0100 Merge branch 'HarHarLinks/hookshot-encryption' of https://github.com/real-joshua/matrix-docker-ansible-deploy into HarHarLinks/hookshot-encryption commit 0604776 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:15:54 2023 +0100 Update roles/custom/matrix-bridge-hookshot/templates/config.yml.j2 change the if statement to not require a variable with a length > 0 and add a filter to json for the redis host Co-authored-by: Slavi Pantaleev <slavi@devture.com> commit e55d769 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:13:50 2023 +0100 clarify that Redis is required, standardadise on Hookshot with an upper-case first letter for consistency commit 66706e4 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:08:20 2023 +0100 Update roles/custom/matrix-bridge-hookshot/templates/config.yml.j2 fix for a typo Co-authored-by: Slavi Pantaleev <slavi@devture.com> commit f6aaeb9 Merge: e5d3400 869dd33 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 00:22:34 2023 +0100 Merge branch 'master' into HarHarLinks/hookshot-encryption commit e5d3400 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 00:09:27 2023 +0100 Add Jinja loop to allow adding multiple networks commit 69f9477 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Thu Dec 14 23:52:41 2023 +0100 split if statements for the message queue and experimental encryption support into seperate statements commit 4c13be1 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Thu Dec 14 23:31:19 2023 +0100 change variable name per spantaleev's suggestion (#2979 (comment)) commit 9905309 Author: HarHarLinks <kim.brose@rwth-aachen.de> Date: Wed Nov 1 16:14:04 2023 +0100 amend docs commit 94abf2d Author: HarHarLinks <kim.brose@rwth-aachen.de> Date: Wed Nov 1 16:05:22 2023 +0100 draft encryption support for hookshot
Squashed based on the work done in spantaleev#3042 commit 49932b8 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:21:31 2023 +0200 Fix syntax in matrix-bridge-hookshot/tasks/reset_encryption.yml Also, this task always does work and side-effects, so it should always report changes (`changed_when: true`). commit 6bdf7a9 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:12:41 2023 +0200 Add Hookshot validation task to ensure queue settings are set when encryption is enabled commit 8c531b7 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:10:17 2023 +0200 Add missing variables rewiring in group_vars/matrix_servers for Hookshot commit 7d26dab Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:08:19 2023 +0200 Add defaults for matrix_hookshot_queue_host and matrix_hookshot_queue_port commit 74f9113 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:06:17 2023 +0200 Fix syntax for connecting to additional networks for Hookshot commit ca7b41f Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:05:28 2023 +0200 Fix indentation and remove unnecessary if-statements commit ac4a918 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:04:44 2023 +0200 Add missing --network for Hookshot This seems to have been removed by accident. commit 6a81fa2 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:02:47 2023 +0200 Make automatic Redis enabling safer, when Hookshot encryption enabled If we ever default encryption to enabled for Hookshot, we only wish to force-enable Redis if Hookshot is actually enabled. commit 75a8e0f Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:01:10 2023 +0200 Fix typo commit 98ad182 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:37:40 2023 +0100 Add defaults for Hookshot's encryption commit 29fa9fa Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:35:11 2023 +0100 Improve wording of Hookshot's encryption section commit 4f835e0 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:28:52 2023 +0100 use safer mount options for the container's files commit 8c93327 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:26:01 2023 +0100 fix filename commit 03a7bb6 Merge: e55d769 0604776 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:23:44 2023 +0100 Merge branch 'HarHarLinks/hookshot-encryption' of https://github.com/real-joshua/matrix-docker-ansible-deploy into HarHarLinks/hookshot-encryption commit 0604776 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:15:54 2023 +0100 Update roles/custom/matrix-bridge-hookshot/templates/config.yml.j2 change the if statement to not require a variable with a length > 0 and add a filter to json for the redis host Co-authored-by: Slavi Pantaleev <slavi@devture.com> commit e55d769 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:13:50 2023 +0100 clarify that Redis is required, standardadise on Hookshot with an upper-case first letter for consistency commit 66706e4 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:08:20 2023 +0100 Update roles/custom/matrix-bridge-hookshot/templates/config.yml.j2 fix for a typo Co-authored-by: Slavi Pantaleev <slavi@devture.com> commit f6aaeb9 Merge: e5d3400 869dd33 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 00:22:34 2023 +0100 Merge branch 'master' into HarHarLinks/hookshot-encryption commit e5d3400 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 00:09:27 2023 +0100 Add Jinja loop to allow adding multiple networks commit 69f9477 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Thu Dec 14 23:52:41 2023 +0100 split if statements for the message queue and experimental encryption support into seperate statements commit 4c13be1 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Thu Dec 14 23:31:19 2023 +0100 change variable name per spantaleev's suggestion (spantaleev#2979 (comment)) commit 9905309 Author: HarHarLinks <kim.brose@rwth-aachen.de> Date: Wed Nov 1 16:14:04 2023 +0100 amend docs commit 94abf2d Author: HarHarLinks <kim.brose@rwth-aachen.de> Date: Wed Nov 1 16:05:22 2023 +0100 draft encryption support for hookshot
Squashed based on the work done in spantaleev#3042 commit 49932b8 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:21:31 2023 +0200 Fix syntax in matrix-bridge-hookshot/tasks/reset_encryption.yml Also, this task always does work and side-effects, so it should always report changes (`changed_when: true`). commit 6bdf7a9 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:12:41 2023 +0200 Add Hookshot validation task to ensure queue settings are set when encryption is enabled commit 8c531b7 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:10:17 2023 +0200 Add missing variables rewiring in group_vars/matrix_servers for Hookshot commit 7d26dab Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:08:19 2023 +0200 Add defaults for matrix_hookshot_queue_host and matrix_hookshot_queue_port commit 74f9113 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:06:17 2023 +0200 Fix syntax for connecting to additional networks for Hookshot commit ca7b41f Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:05:28 2023 +0200 Fix indentation and remove unnecessary if-statements commit ac4a918 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:04:44 2023 +0200 Add missing --network for Hookshot This seems to have been removed by accident. commit 6a81fa2 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:02:47 2023 +0200 Make automatic Redis enabling safer, when Hookshot encryption enabled If we ever default encryption to enabled for Hookshot, we only wish to force-enable Redis if Hookshot is actually enabled. commit 75a8e0f Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:01:10 2023 +0200 Fix typo commit 98ad182 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:37:40 2023 +0100 Add defaults for Hookshot's encryption commit 29fa9fa Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:35:11 2023 +0100 Improve wording of Hookshot's encryption section commit 4f835e0 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:28:52 2023 +0100 use safer mount options for the container's files commit 8c93327 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:26:01 2023 +0100 fix filename commit 03a7bb6 Merge: e55d769 0604776 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:23:44 2023 +0100 Merge branch 'HarHarLinks/hookshot-encryption' of https://github.com/real-joshua/matrix-docker-ansible-deploy into HarHarLinks/hookshot-encryption commit 0604776 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:15:54 2023 +0100 Update roles/custom/matrix-bridge-hookshot/templates/config.yml.j2 change the if statement to not require a variable with a length > 0 and add a filter to json for the redis host Co-authored-by: Slavi Pantaleev <slavi@devture.com> commit e55d769 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:13:50 2023 +0100 clarify that Redis is required, standardadise on Hookshot with an upper-case first letter for consistency commit 66706e4 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:08:20 2023 +0100 Update roles/custom/matrix-bridge-hookshot/templates/config.yml.j2 fix for a typo Co-authored-by: Slavi Pantaleev <slavi@devture.com> commit f6aaeb9 Merge: e5d3400 869dd33 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 00:22:34 2023 +0100 Merge branch 'master' into HarHarLinks/hookshot-encryption commit e5d3400 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 00:09:27 2023 +0100 Add Jinja loop to allow adding multiple networks commit 69f9477 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Thu Dec 14 23:52:41 2023 +0100 split if statements for the message queue and experimental encryption support into seperate statements commit 4c13be1 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Thu Dec 14 23:31:19 2023 +0100 change variable name per spantaleev's suggestion (spantaleev#2979 (comment)) commit 9905309 Author: HarHarLinks <kim.brose@rwth-aachen.de> Date: Wed Nov 1 16:14:04 2023 +0100 amend docs commit 94abf2d Author: HarHarLinks <kim.brose@rwth-aachen.de> Date: Wed Nov 1 16:05:22 2023 +0100 draft encryption support for hookshot
per the comment in main.yml, this requires some experimental synapse features to also be enabled, which the role does not do yet at the time of writing (94abf2d).
i manage that by setting (iirc) in my host vars:
other than that, i tried this out at some point which worked for a while but eventually broke in a way that hookshot's messages were not decryptable anymore. i couldn't figure out what broke it and had no time to try it again, so i never contributed this patch. i think the feature is not quite stable yet, unless i made a mistake somewhere, which is for you to review. i am sharing it now out of interest from members in the hookshot room.
this PR is a draft for the above 2 reasons. feel free to take it as the base for something production ready.