Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Task extracting console encryption key from configuration confused by Lua comment #122

Open
joel-ling opened this issue Mar 11, 2024 · 1 comment · May be fixed by #123
Open

Task extracting console encryption key from configuration confused by Lua comment #122

joel-ling opened this issue Mar 11, 2024 · 1 comment · May be fixed by #123

Comments

@joel-ling
Copy link
Member

joel-ling commented Mar 11, 2024 •
edited
Loading

This issue pertains to the following task:

dnsdist-ansible/tasks/main.yml

Lines 20 to 32 in ca381db

- name: Create encryption key
when: dnsdist_generatekey
tags:
- install
- configure
- molecule-idempotence-notest
block:
- name: Check if the key is already present in the DNSdist configuration file
ansible.builtin.shell: |
set -o pipefail && fgrep setKey "{{ default_dnsdist_config_location }}" | sed 's/setKey("\(.*\)")/\1/'
register: dnsdist_grepkey_cmd
changed_when: false
failed_when: false

Using fgrep and sed, it aims to extract and register the argument of setKey() as dnsdist_grepkey_cmd.stdout.

Problem

Since the playbook installs DNSdist in the preceding task, the default config file distributed with DNSdist (link) is read as input, resulting in the following phenomenon.

fgrep setKey in Line 29 matches the following comment in Line 15 of the default config file:

-- setKey("please generate a fresh private key with makeKey()")

Running the command in Line 29 against the default config file gives the following output:

$ set -o pipefail && fgrep setKey dnsdistconf.lua | sed 's/setKey("\(.*\)")/\1/'
-- please generate a fresh private key with makeKey()

(The leading double hyphen -- happens to be preserved because it is not captured by the sed expression.)

The above output ends up as the value of fact dnsdist_setkey:

dnsdist-ansible/tasks/main.yml

Lines 47 to 50 in ca381db

- name: Set old encryption key
ansible.builtin.set_fact:
dnsdist_setkey: "{{ dnsdist_grepkey_cmd.stdout }}"
when: dnsdist_grepkey_cmd.rc == 0 and dnsdist_grepkey_cmd.stdout | length > 0

This becomes the encryption key when dnsdist.conf.j2 is expanded by another task to generate the new config file:

dnsdist-ansible/templates/dnsdist.conf.j2

Lines 21 to 26 in ca381db

{% if dnsdist_controlsocket != "" %}
controlSocket("{{ dnsdist_controlsocket }}")
{% endif %}
{% if dnsdist_setkey is defined and dnsdist_setkey != "" %}
setKey("{{ dnsdist_setkey }}")
{% endif %}

Workaround

Truncate the automatically-generated DNSdist config file and re-run the playbook. This causes the command in Line 29 to fail with exit code 1, hence triggering the subsequent task that generates an encryption key from scratch:

dnsdist-ansible/tasks/main.yml

Lines 34 to 41 in ca381db

- name: Set up new encryption key
# if the config file does not contains a key already
when: dnsdist_grepkey_cmd.rc > 0 or dnsdist_grepkey_cmd.stdout | length == 0
block:
- name: Generate encryption key # noqa no-changed-when
ansible.builtin.shell: head -c 32 /dev/urandom | base64
register: dnsdist_setkey_cmd
changed_when: true

Possible Solution

Consider replacing fgrep setKey with grep ^setKey (or possibly grep '^\s*setKey' if indentation is to be expected), to avoid matching lines where the setKey() invocation is prefixed.

The regular expression in the sed script should also be prefixed and suffixed with .* to remove leading and trailing characters around the function:

s/.*setKey("\(.*\)").*/\1/
@joel-ling
Copy link
Member Author

Tangentially related to #119 as it pertains to the same line in the playbook, despite no overlap in cause nor effect.

joel-ling added a commit to joel-ling/dnsdist-ansible that referenced this issue Mar 11, 2024
@joel-ling
implement solution described in PowerDNS#122
ca54c9f
@joel-ling joel-ling linked a pull request Mar 11, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

implement solution described in PowerDNS#122 joel-ling/dnsdist-ansible
1 participant
@joel-ling