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.

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

fix: use quote with command, shell and validate with variable #298

Merged
merged 7 commits into from
Sep 12, 2024
Merged
24 changes: 24 additions & 0 deletions tasks/check_vars.yml
Original file line number Diff line number Diff line change
@@ -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 }}
7 changes: 5 additions & 2 deletions tasks/install.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}"
Expand Down Expand Up @@ -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 }}"
Expand Down Expand Up @@ -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

Expand Down
8 changes: 4 additions & 4 deletions tasks/install_config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tasks/install_namespace.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion tasks/install_service.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
6 changes: 3 additions & 3 deletions tests/tasks/backup.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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([]) }}"
Expand Down
8 changes: 4 additions & 4 deletions tests/tasks/restore.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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([]) }}"
Expand Down
70 changes: 70 additions & 0 deletions tests/tests_hostkeys_unsafe_path.yml
Original file line number Diff line number Diff line change
@@ -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: []
Jakuje marked this conversation as resolved.
Show resolved Hide resolved
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
93 changes: 93 additions & 0 deletions tests/tests_unsafe_options.yml
Original file line number Diff line number Diff line change
@@ -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