From dc97027453a3bc27963da726b09e3fb23307779b Mon Sep 17 00:00:00 2001 From: Rick Elrod Date: Mon, 31 Aug 2020 10:05:30 -0500 Subject: [PATCH] [dnf] ensure packages are gpg-verified (#71539) Change: - By default the dnf API does not gpg-verify packages. This is a feature that is executed in its CLI code. It never made it into Ansible's usage of the API, so packages were previously not verified. - This fixes CVE-2020-14365. Test Plan: - New integration tests Signed-off-by: Rick Elrod --- changelogs/fragments/dnf_gpg.yml | 2 + lib/ansible/modules/dnf.py | 22 +++++++ test/integration/targets/dnf/tasks/dnf.yml | 2 + test/integration/targets/dnf/tasks/gpg.yml | 72 +++++++++++++++++++++ test/integration/targets/dnf/tasks/main.yml | 4 ++ test/integration/targets/dnf/tasks/repo.yml | 5 ++ 6 files changed, 107 insertions(+) create mode 100644 changelogs/fragments/dnf_gpg.yml create mode 100644 test/integration/targets/dnf/tasks/gpg.yml diff --git a/changelogs/fragments/dnf_gpg.yml b/changelogs/fragments/dnf_gpg.yml new file mode 100644 index 00000000000..2e156c509b0 --- /dev/null +++ b/changelogs/fragments/dnf_gpg.yml @@ -0,0 +1,2 @@ +security_fixes: + - dnf - Previously, regardless of the ``disable_gpg_check`` option, packages were not GPG validated. They are now. (CVE-2020-14365) diff --git a/lib/ansible/modules/dnf.py b/lib/ansible/modules/dnf.py index ab49528766e..70448d17534 100644 --- a/lib/ansible/modules/dnf.py +++ b/lib/ansible/modules/dnf.py @@ -62,6 +62,8 @@ options: description: - Whether to disable the GPG checking of signatures of packages being installed. Has an effect only if state is I(present) or I(latest). + - This setting affects packages installed from a repository as well as + "local" packages installed from the filesystem or a URL. type: bool default: 'no' @@ -1165,6 +1167,26 @@ class DnfModule(YumDnf): results=[], ) + # Validate GPG. This is NOT done in dnf.Base (it's done in the + # upstream CLI subclass of dnf.Base) + if not self.disable_gpg_check: + for package in self.base.transaction.install_set: + fail = False + gpgres, gpgerr = self.base._sig_check_pkg(package) + if gpgres == 0: # validated successfully + continue + elif gpgres == 1: # validation failed, install cert? + try: + self.base._get_key_for_package(package) + except dnf.exceptions.Error as e: + fail = True + else: # fatal error + fail = True + + if fail: + msg = 'Failed to validate GPG signature for {0}'.format(package) + self.module.fail_json(msg) + if self.download_only: for package in self.base.transaction.install_set: response['results'].append("Downloaded: {0}".format(package)) diff --git a/test/integration/targets/dnf/tasks/dnf.yml b/test/integration/targets/dnf/tasks/dnf.yml index 3688e7d085a..04d90a538a9 100644 --- a/test/integration/targets/dnf/tasks/dnf.yml +++ b/test/integration/targets/dnf/tasks/dnf.yml @@ -557,6 +557,7 @@ dnf: name: "/tmp/{{ pkg_name }}.rpm" state: present + disable_gpg_check: true register: dnf_result - name: verify installation @@ -586,6 +587,7 @@ dnf: name: "{{ pkg_url }}" state: present + disable_gpg_check: true register: dnf_result - name: verify installation diff --git a/test/integration/targets/dnf/tasks/gpg.yml b/test/integration/targets/dnf/tasks/gpg.yml new file mode 100644 index 00000000000..2b6f4079a9e --- /dev/null +++ b/test/integration/targets/dnf/tasks/gpg.yml @@ -0,0 +1,72 @@ +# Set up a repo of unsigned rpms +- block: + - name: Ensure our test package isn't already installed + dnf: + name: + - fpaste + state: absent + + - name: Install rpm-sign + dnf: + name: + - rpm-sign + state: present + + - name: Create directory to use as local repo + file: + path: "{{ remote_tmp_dir }}/unsigned" + state: directory + + - name: Download an RPM + get_url: + url: https://s3.amazonaws.com/ansible-ci-files/test/integration/targets/dnf/fpaste-0.3.9.1-1.fc27.noarch.rpm + dest: "{{ remote_tmp_dir }}/unsigned/fpaste-0.3.9.1-1.fc27.noarch.rpm" + mode: 0644 + + - name: Unsign the RPM + command: rpmsign --delsign "{{ remote_tmp_dir }}/unsigned/fpaste-0.3.9.1-1.fc27.noarch.rpm" + + - name: createrepo + command: createrepo . + args: + chdir: "{{ remote_tmp_dir }}/unsigned" + + - name: Add the repo + yum_repository: + name: unsigned + description: unsigned rpms + baseurl: "file://{{ remote_tmp_dir }}/unsigned/" + # we want to ensure that signing is verified + gpgcheck: true + + - name: Install fpaste from above + dnf: + name: + - fpaste + disablerepo: '*' + enablerepo: unsigned + register: res + ignore_errors: yes + + - assert: + that: + - res is failed + - "'Failed to validate GPG signature' in res.msg" + + always: + - name: Remove rpm-sign (and fpaste if it got installed) + dnf: + name: + - rpm-sign + - fpaste + state: absent + + - name: Remove test repo + yum_repository: + name: unsigned + state: absent + + - name: Remove repo dir + file: + path: "{{ remote_tmp_dir }}/unsigned" + state: absent diff --git a/test/integration/targets/dnf/tasks/main.yml b/test/integration/targets/dnf/tasks/main.yml index 05881184039..fdaf227a983 100644 --- a/test/integration/targets/dnf/tasks/main.yml +++ b/test/integration/targets/dnf/tasks/main.yml @@ -23,6 +23,10 @@ when: (ansible_distribution == 'Fedora' and ansible_distribution_major_version is version('23', '>=')) or (ansible_distribution in ['RedHat', 'CentOS'] and ansible_distribution_major_version is version('8', '>=')) +- include_tasks: gpg.yml + when: (ansible_distribution == 'Fedora' and ansible_distribution_major_version is version('23', '>=')) or + (ansible_distribution in ['RedHat', 'CentOS'] and ansible_distribution_major_version is version('8', '>=')) + - include_tasks: repo.yml when: (ansible_distribution == 'Fedora' and ansible_distribution_major_version is version('23', '>=')) or (ansible_distribution in ['RedHat', 'CentOS'] and ansible_distribution_major_version is version('8', '>=')) diff --git a/test/integration/targets/dnf/tasks/repo.yml b/test/integration/targets/dnf/tasks/repo.yml index 19c5747172d..4f82899cd37 100644 --- a/test/integration/targets/dnf/tasks/repo.yml +++ b/test/integration/targets/dnf/tasks/repo.yml @@ -106,6 +106,7 @@ name: "{{ repodir }}/dinginessentail-1.0-1.{{ ansible_architecture }}.rpm" state: present allow_downgrade: True + disable_gpg_check: True register: dnf_result - name: Check dinginessentail with rpm @@ -132,6 +133,7 @@ dnf: name: "{{ repodir }}/dinginessentail-1.0-1.{{ ansible_architecture }}.rpm" state: present + disable_gpg_check: True register: dnf_result - name: Check dinginessentail with rpm @@ -153,6 +155,7 @@ dnf: name: "{{ repodir }}/dinginessentail-1.0-1.{{ ansible_architecture }}.rpm" state: present + disable_gpg_check: True register: dnf_result - name: Check dinginessentail with rpm @@ -169,6 +172,7 @@ dnf: name: "{{ repodir }}/dinginessentail-1.0-2.{{ ansible_architecture }}.rpm" state: present + disable_gpg_check: True register: dnf_result - name: Check dinginessentail with rpm @@ -190,6 +194,7 @@ dnf: name: "{{ repodir }}/dinginessentail-1.0-2.{{ ansible_architecture }}.rpm" state: present + disable_gpg_check: True register: dnf_result - name: Check dinginessentail with rpm