diff --git a/tasks/check_vars.yml b/tasks/check_vars.yml new file mode 100644 index 0000000..0fe9a3e --- /dev/null +++ b/tasks/check_vars.yml @@ -0,0 +1,24 @@ +--- +- name: Ensure sshd_sysconfig_use_strong_rng is safe to use in shell/command + ansible.builtin.assert: + that: + - (sshd_sysconfig_use_strong_rng | string) == (sshd_sysconfig_use_strong_rng | quote) + msg: | + The variable `sshd_sysconfig_use_strong_rng` is not safe for shell/command/template expansions: + sshd_sysconfig_use_strong_rng: {{ sshd_sysconfig_use_strong_rng }} == {{ sshd_sysconfig_use_strong_rng | quote }} + +- name: Ensure sshd_binary is safe to use in shell/command + ansible.builtin.assert: + that: + - sshd_binary == (sshd_binary | quote) + msg: | + The variable `sshd_binary` is not safe for shell/command/template expansions: + sshd_binary: {{ sshd_binary }} == {{ sshd_binary | quote }} + +- name: Ensure sshd_config_file is safe to use in shell/command + ansible.builtin.assert: + that: + - sshd_config_file == (sshd_config_file | quote) + msg: | + The variable `sshd_sysconfig_use_strong_rng` is not safe for shell/command/template expansions: + sshd_config_file: {{ sshd_config_file }} == {{ sshd_config_file | quote }} diff --git a/tasks/install.yml b/tasks/install.yml index 9e6c409..cbc4b7d 100644 --- a/tasks/install.yml +++ b/tasks/install.yml @@ -4,6 +4,9 @@ when: - not __sshd_os_supported | bool +- name: Check variables are safe for use for shell expansions and word splitting + ansible.builtin.include_tasks: check_vars.yml + - name: Install ssh packages ansible.builtin.package: name: "{{ sshd_packages }}" @@ -80,7 +83,7 @@ {% if sshd_sysconfig %} source /etc/sysconfig/sshd {% endif %} - ssh-keygen -q -t {{ item | regex_search('(rsa|dsa|ecdsa|ed25519)') }} -f {{ item }} -C '' -N '' + ssh-keygen -q -t {{ item | regex_search('(rsa|dsa|ecdsa|ed25519)') }} -f {{ item | quote }} -C '' -N '' args: creates: "{{ item }}" loop: "{{ __sshd_verify_hostkeys | from_json | list }}" @@ -109,7 +112,7 @@ - name: Generate temporary hostkey ansible.builtin.command: > - ssh-keygen -q -t rsa -f '{{ sshd_test_hostkey.path }}/rsa_key' -C '' -N '' + ssh-keygen -q -t rsa -f {{ sshd_test_hostkey.path | quote }}/rsa_key -C '' -N '' changed_when: false when: sshd_test_hostkey.path is defined diff --git a/tasks/install_config.yml b/tasks/install_config.yml index 1c4efb1..4aa17a2 100644 --- a/tasks/install_config.yml +++ b/tasks/install_config.yml @@ -19,9 +19,9 @@ {% if not __sshd_supports_validate %} true %s {% elif sshd_test_hostkey is defined and sshd_test_hostkey.path is defined %} - {{ sshd_binary }} -t -f %s -h {{ sshd_test_hostkey.path }}/rsa_key + {{ sshd_binary | quote }} -t -f %s -h {{ sshd_test_hostkey.path | quote }}/rsa_key {% else %} - {{ sshd_binary }} -t -f %s + {{ sshd_binary | quote }} -t -f %s {% endif %} backup: "{{ sshd_backup }}" notify: Reload_sshd @@ -38,9 +38,9 @@ {% if not __sshd_supports_validate %} true %s {% elif sshd_test_hostkey is defined and sshd_test_hostkey.path is defined %} - {{ sshd_binary }} -t -f %s -h {{ sshd_test_hostkey.path }}/rsa_key + {{ sshd_binary | quote }} -t -f %s -h {{ sshd_test_hostkey.path | quote }}/rsa_key {% else %} - {{ sshd_binary }} -t -f %s + {{ sshd_binary | quote }} -t -f %s {% endif %} backup: "{{ sshd_backup }}" notify: Reload_sshd diff --git a/tasks/install_namespace.yml b/tasks/install_namespace.yml index 1e7e1b6..3a879e0 100644 --- a/tasks/install_namespace.yml +++ b/tasks/install_namespace.yml @@ -16,9 +16,9 @@ {% if not __sshd_supports_validate %} true %s {% elif sshd_test_hostkey is defined and sshd_test_hostkey.path is defined %} - {{ sshd_binary }} -t -f %s -h {{ sshd_test_hostkey.path }}/rsa_key + {{ sshd_binary | quote }} -t -f %s -h {{ sshd_test_hostkey.path | quote }}/rsa_key {% else %} - {{ sshd_binary }} -t -f %s + {{ sshd_binary | quote }} -t -f %s {% endif %} backup: "{{ sshd_backup }}" notify: Reload_sshd diff --git a/tasks/install_service.yml b/tasks/install_service.yml index b843449..41e164f 100644 --- a/tasks/install_service.yml +++ b/tasks/install_service.yml @@ -46,7 +46,7 @@ # Due to ansible bug 21026, cannot use service module on RHEL 7 - name: Enable service in chroot - ansible.builtin.command: systemctl enable {{ sshd_service }} # noqa command-instead-of-module + ansible.builtin.command: systemctl enable {{ sshd_service | quote }} # noqa command-instead-of-module when: - ansible_connection == 'chroot' - ansible_facts['os_family'] == 'RedHat' diff --git a/tests/tasks/backup.yml b/tests/tasks/backup.yml index fe64624..f30ab4c 100644 --- a/tests/tasks/backup.yml +++ b/tests/tasks/backup.yml @@ -16,9 +16,9 @@ set -o pipefail fi set -eu - if test -f {{ item }}; then - mkdir -p {{ __sshd_test_backup.path }}/$(dirname {{ item }}) - cp -a {{ item }} {{ __sshd_test_backup.path }}/$(dirname {{ item }}) + if test -f {{ item | quote }}; then + mkdir -p {{ __sshd_test_backup.path | quote }}/$(dirname {{ item | quote }}) + cp -a {{ item | quote }} {{ __sshd_test_backup.path | quote }}/$(dirname {{ item | quote }}) fi changed_when: false loop: "{{ __sshd_test_backup_files | d([]) }}" diff --git a/tests/tasks/restore.yml b/tests/tasks/restore.yml index 2a1959a..9999d85 100644 --- a/tests/tasks/restore.yml +++ b/tests/tasks/restore.yml @@ -5,10 +5,10 @@ if set -o | grep pipefail 2>&1 /dev/null ; then set -o pipefail fi - if test -f {{ __sshd_test_backup.path }}/{{ item }}; then - cp -a {{ __sshd_test_backup.path }}/{{ item }} $(dirname {{ item }}) - elif test -f {{ item }}; then - rm {{ item }} + if test -f {{ __sshd_test_backup.path | quote }}/{{ item | quote }}; then + cp -a {{ __sshd_test_backup.path | quote }}/{{ item | quote }} $(dirname {{ item | quote }}) + elif test -f {{ item | quote }}; then + rm {{ item | quote }} fi changed_when: false loop: "{{ __sshd_test_backup_files | d([]) }}" diff --git a/tests/tests_hostkeys_unsafe_path.yml b/tests/tests_hostkeys_unsafe_path.yml new file mode 100644 index 0000000..560894a --- /dev/null +++ b/tests/tests_hostkeys_unsafe_path.yml @@ -0,0 +1,70 @@ +--- +- name: Test quote with unsafe input + hosts: all + environment: + TMPDIR: "{{ __tmpdir }}" + vars: + __sshd_test_backup_files: + - /etc/ssh/sshd_config + - /etc/ssh/sshd_config.d/00-ansible_system_role.conf + __badflag_file: /tmp/BADFLAG + # Avoid / in TMPDIR file name + __badflag: >- + $(touch -- "$(echo {{ __badflag_file | b64encode }} | base64 -d)") + # Iterate w/o quote, w/ ' and w/ " + __tmpdir: >- + /tmp/a {{ __badflag }} ' {{ __badflag }} '" {{ __badflag }} "b + + tasks: + - name: Ensure BADFLAG does not exist + ansible.builtin.file: + path: /tmp/BADFLAG + state: absent + + - name: Assert TMPDIR is correctly set + ansible.builtin.assert: + that: + - __tmpdir != '' + - ansible_facts.env.TMPDIR == __tmpdir + + - name: "Backup configuration files" + ansible.builtin.include_tasks: tasks/backup.yml + + - name: Create BAD TMPDIR + ansible.builtin.file: + state: directory + path: "{{ ansible_facts.env.TMPDIR }}" + mode: '0755' + + - name: Configure sshd with BAD config + ansible.builtin.include_role: + name: ansible-sshd + vars: + sshd_skip_defaults: true + sshd_verify_hostkeys: [] + when: + - ansible_facts['os_family'] != 'RedHat' or ansible_facts['distribution_major_version'] | int != 8 + + - name: Verify the options are correctly set + tags: tests::verify + block: + - name: Flush handlers + ansible.builtin.meta: flush_handlers + + - name: Get status BADFLAG + ansible.builtin.stat: + path: "{{ __badflag_file }}" + register: badflag + + - name: Ensure BADFLAG does not exist + ansible.builtin.assert: + that: + - not badflag.stat.exists + + - name: Remove BAD TMPDIR + ansible.builtin.file: + state: absent + path: "{{ ansible_facts.env.TMPDIR }}" + + - name: "Restore configuration files" + ansible.builtin.include_tasks: tasks/restore.yml diff --git a/tests/tests_unsafe_options.yml b/tests/tests_unsafe_options.yml new file mode 100644 index 0000000..a36f799 --- /dev/null +++ b/tests/tests_unsafe_options.yml @@ -0,0 +1,93 @@ +--- +- name: Test quote with unsafe input + hosts: all + vars: + __sshd_test_backup_files: + - /etc/ssh/sshd_config + - /etc/ssh/sshd_config.d/00-ansible_system_role.conf + + tasks: + - name: "Backup configuration files" + ansible.builtin.include_tasks: tasks/backup.yml + + - name: Configure sshd with bad sysconfig configuration + block: + - name: Include the role + ansible.builtin.include_role: + name: ansible-sshd + vars: + sshd_skip_defaults: true + sshd_verify_hostkeys: [] + sshd_sysconfig_use_strong_rng: "maybe yes" + register: role_result + + - name: Unreachable task -- the role should have failed! + ansible.builtin.fail: + msg: UNREACH + + rescue: + - name: Check that we failed in the role + ansible.builtin.assert: + that: + - ansible_failed_result.msg != 'UNREACH' + - not role_result.changed + msg: "Role has not failed when it should have with invalid inputs" + + - name: Configure sshd with bad path to sshd binary + block: + - name: Include the role + ansible.builtin.include_role: + name: ansible-sshd + vars: + sshd_skip_defaults: true + sshd_verify_hostkeys: [] + sshd_binary: "/usr/sbin/sshd binary" + register: role_result + + - name: Unreachable task -- the role should have failed! + ansible.builtin.fail: + msg: UNREACH + + rescue: + - name: Check that we failed in the role + ansible.builtin.assert: + that: + - ansible_failed_result.msg != 'UNREACH' + - not role_result.changed + msg: "Role has not failed when it should have with invalid inputs" + + - name: Configure sshd with bad path sshd config + block: + - name: Include the role + ansible.builtin.include_role: + name: ansible-sshd + vars: + sshd_skip_defaults: true + sshd_verify_hostkeys: [] + sshd_config_file: /etc/ssh/sshd.config.d/my fancy config + register: role_result + + - name: Unreachable task -- the role should have failed! + ansible.builtin.fail: + msg: UNREACH + + rescue: + - name: Check that we failed in the role + ansible.builtin.assert: + that: + - ansible_failed_result.msg != 'UNREACH' + - not role_result.changed + msg: "Role has not failed when it should have with invalid inputs" + + - name: Make sure service is still running + ansible.builtin.service: + name: sshd + state: started + register: result + failed_when: result.changed + tags: tests::verify + when: + - not (ansible_facts['os_family'] == 'RedHat' and ansible_facts['distribution_major_version'] == '6') + + - name: "Restore configuration files" + ansible.builtin.include_tasks: tasks/restore.yml