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

Skip only if variable is checked.

Also ensure systemd.unit contents is about okay. This disables
possibility to have something that needs to be quoted there. But as
ansible misses proper way to ensure right quotation (see man
systemd.syntax), it is better to fail such configs.

tasks/install.yml

@@ -78,7 +78,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 +107,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,64 @@
+---
+- name: Test hostkeys
+ hosts: all
+ environment:
+ TMPDIR: >-
+ /tmp/a {{ __badflag }} ' {{ __badflag }} '" {{ __badflag }} "b
+ vars:
+ __sshd_test_backup_files:
+ - /etc/ssh/sshd_config
+ - /etc/ssh/sshd_config.d/00-ansible_system_role.conf
+ __badflag: >-
+ $(touch -- "$(echo {{ '/tmp/BADFLAG' | b64encode }} | base64 -d)")
+
+ tasks:
+ - name: Ensure BADFLAG does not exist
+ ansible.builtin.file:
+ path: /tmp/BADFLAG
+ state: absent
+
+ - name: Assert TMPDIR is set
+ ansible.builtin.assert:
+ that:
+ - ansible_facts.env.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: []
+ sshd_config_file: "/tmp/sshd.d/foo.conf"
+
+ - 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: /tmp/BADFLAG
+ 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