Skip to content

Commit

Permalink
[dnf] ensure packages are gpg-verified (#71537)
Browse files Browse the repository at this point in the history
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 <rick@elrod.me>
  • Loading branch information
relrod committed Aug 31, 2020
1 parent d083307 commit 9bea33f
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 0 deletions.
2 changes: 2 additions & 0 deletions 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)
22 changes: 22 additions & 0 deletions lib/ansible/modules/dnf.py
Expand Up @@ -62,6 +62,8 @@
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'
Expand Down Expand Up @@ -1189,6 +1191,26 @@ def ensure(self):
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:
# No further work left to do, and the results were already updated above.
# Just return them.
Expand Down
2 changes: 2 additions & 0 deletions test/integration/targets/dnf/tasks/dnf.yml
Expand Up @@ -617,6 +617,7 @@
dnf:
name: "/tmp/{{ pkg_name }}.rpm"
state: present
disable_gpg_check: true
register: dnf_result

- name: verify installation
Expand Down Expand Up @@ -646,6 +647,7 @@
dnf:
name: "{{ pkg_url }}"
state: present
disable_gpg_check: true
register: dnf_result

- name: verify installation
Expand Down
72 changes: 72 additions & 0 deletions 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
4 changes: 4 additions & 0 deletions test/integration/targets/dnf/tasks/main.yml
Expand Up @@ -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', '>='))
Expand Down
5 changes: 5 additions & 0 deletions test/integration/targets/dnf/tasks/repo.yml
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 9bea33f

Please sign in to comment.