security: use quote with command, shell and validate with variable

tasks/install.yml

@@ -4,6 +4,23 @@
  when:
  - not __sshd_os_supported | bool
 
+- name: Ensure variables used in templates in shell or systemd unit contexts are okay
+ ansible.builtin.assert:
+ that:
+ - (sshd_sysconfig_use_strong_rng | string) == (sshd_sysconfig_use_strong_rng | quote)
+ - sshd_binary == (sshd_binary | quote)
+ - sshd_config_file == (sshd_config_file | quote)
+ - (__sshd_runtime_directory is none) or (__sshd_runtime_directory == (__sshd_runtime_directory | quote))
+ - __sshd_runtime_directory_mode == (__sshd_runtime_directory_mode | quote)
+ msg: |
+ sshd_sysconfig_use_strong_rng: {{ sshd_sysconfig_use_strong_rng }} == {{ sshd_sysconfig_use_strong_rng | quote }}
+ sshd_binary: {{ sshd_binary }} == {{ sshd_binary | quote }}
+ sshd_config_file: {{ sshd_config_file }} == {{ sshd_config_file | quote }}
+ {%- if __sshd_runtime_directory is not none %}
+ __sshd_runtime_directory: {{ __sshd_runtime_directory }} == {{ __sshd_runtime_directory | quote }}
+ {% endif %}
+ __sshd_runtime_directory_mode: {{ __sshd_runtime_directory_mode }} == {{ __sshd_runtime_directory_mode | quote }}
+
 - name: Install ssh packages
  ansible.builtin.package:
  name: "{{ sshd_packages }}"
@@ -78,7 +95,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 }}"
@@ -107,7 +124,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
 

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

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

tasks/install_service.yml

@@ -39,7 +39,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_os_family == 'RedHat'

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 {{ item }} {{ __sshd_test_backup.path }}/$(dirname {{ item }})
+ if test -f {{ item | quote }}; then
+ mkdir -p {{ __sshd_test_backup.path | quote }}/$(dirname {{ item | quote }})
+ cp {{ item }} {{ __sshd_test_backup.path | quote }}/$(dirname {{ item | quote }})
  fi
  changed_when: false
  loop: "{{ __sshd_test_backup_files | d([]) }}"

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 {{ __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 {{ __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([]) }}"

tests/tests_hostkeys_unsafe_path.yml

@@ -0,0 +1,68 @@
+---
+- 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: []
+
+ - 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