Skip to content

Commit

Permalink
(MODULES-10996) Improve GPG key import check
Browse files Browse the repository at this point in the history
The puppet GPG signing key with ID 4528b6cd9e61ef26 had a subkey in it
until February 2021. This caused GPG checks on systems with RPM versions
that do not support subkeys[1] (SLES 11 and EL 5) to fail.

We added this GPG key in the puppet_agent module in January, and
included it in the 4.4.0 release of the module.

We discovered the subkey issue in February and promptly removed the
subkey from the existing key. The new key is available since version
4.5.0 of the puppet_agent module.

This module imports GPG keys based on their ID. Since in our case both
the good key and the bad key have the same ID, the module will not
import the correct key if the bad one is already installed (or any other
key with the same ID for that matter).

To circumvent this, we now specifically compare the contents of the GPG
key from the RPM database with the contents of the GPG key laid by
Puppet in `/etc/pki/rpm-gpg`. If any differences are found, the imported
key is purged and reimported, which should ensure that the key shipped
in the module is identical to the from the RPM database.

[1] https://technosorcery.net/blog/2010/10/pitfalls-with-rpm-and-gpg/
  • Loading branch information
GabrielNagy committed Apr 20, 2021
1 parent a2a9f11 commit df35318
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 27 deletions.
19 changes: 19 additions & 0 deletions files/rpm_gpg_import_check.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#!/bin/bash
# shellcheck disable=SC2086

ACTION=$1
GPG_HOMEDIR=$2
GPG_KEY_PATH=$3

GPG_ARGS="--homedir $GPG_HOMEDIR --with-colons"
GPG_PUBKEY=gpg-pubkey-$(gpg ${GPG_ARGS} "${GPG_KEY_PATH}" 2>&1 | grep ^pub | cut -d':' -f5 | cut --characters=9-16 | tr '[:upper:]' '[:lower:]')

if [ "${ACTION}" = "check" ]; then
# This will return 1 if there are differences between the key imported in the
# RPM database and the local keyfile. This means we need to purge the key and
# reimport it.
diff <(rpm -qi "${GPG_PUBKEY}" | gpg ${GPG_ARGS}) <(gpg ${GPG_ARGS} "${GPG_KEY_PATH}")
elif [ "${ACTION}" == "import" ]; then
(rpm -q "${GPG_PUBKEY}" && rpm -e --allmatches "${GPG_PUBKEY}") || true
rpm --import "${GPG_KEY_PATH}"
fi
32 changes: 16 additions & 16 deletions manifests/osfamily/redhat.pp
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
$legacy_gpg_path = "/etc/pki/rpm-gpg/RPM-${legacy_keyname}"
$keyname = 'GPG-KEY-puppet-20250406'
$gpg_path = "/etc/pki/rpm-gpg/RPM-${keyname}"
$gpg_homedir = '/root/.gnupg'
$gpg_keys = "file://${legacy_gpg_path}
file://${gpg_path}"

Expand All @@ -103,17 +104,6 @@
source => "puppet:///modules/puppet_agent/${legacy_keyname}",
}

# Given the path to a key, see if it is imported, if not, import it
$legacy_gpg_pubkey = "gpg-pubkey-$(echo $(${gpg_cmd} --with-colons ${legacy_gpg_path} 2>&1 | grep ^pub | awk -F ':' '{print \$5}' | cut --characters=9-16 | tr '[:upper:]' '[:lower:]'))"

exec { "import-${legacy_keyname}":
path => '/bin:/usr/bin:/sbin:/usr/sbin',
command => "rpm --import ${legacy_gpg_path}",
unless => "rpm -q ${legacy_gpg_pubkey}",
require => File[$legacy_gpg_path],
logoutput => 'on_failure',
}

file { $gpg_path:
ensure => present,
owner => 0,
Expand All @@ -122,12 +112,22 @@
source => "puppet:///modules/puppet_agent/${keyname}",
}

# Given the path to a key, see if it is imported, if not, import it
$gpg_pubkey = "gpg-pubkey-$(echo $(${gpg_cmd} --with-colons ${gpg_path} 2>&1 | grep ^pub | awk -F ':' '{print \$5}' | cut --characters=9-16 | tr '[:upper:]' '[:lower:]'))"
exec { "import-${keyname}":
file { "${::env_temp_variable}/rpm_gpg_import_check.sh":
ensure => file,
source => 'puppet:///modules/puppet_agent/rpm_gpg_import_check.sh',
mode => '0755',
}
-> exec { "import-${legacy_keyname}":
path => '/bin:/usr/bin:/sbin:/usr/sbin',
command => "${::env_temp_variable}/rpm_gpg_import_check.sh import ${gpg_homedir} ${legacy_gpg_path}",
unless => "${::env_temp_variable}/rpm_gpg_import_check.sh check ${gpg_homedir} ${legacy_gpg_path}",
require => File[$legacy_gpg_path],
logoutput => 'on_failure',
}
-> exec { "import-${keyname}":
path => '/bin:/usr/bin:/sbin:/usr/sbin',
command => "rpm --import ${gpg_path}",
unless => "rpm -q ${gpg_pubkey}",
command => "${::env_temp_variable}/rpm_gpg_import_check.sh import ${gpg_homedir} ${gpg_path}",
unless => "${::env_temp_variable}/rpm_gpg_import_check.sh check ${gpg_homedir} ${gpg_path}",
require => File[$gpg_path],
logoutput => 'on_failure',
}
Expand Down
22 changes: 11 additions & 11 deletions manifests/osfamily/suse.pp
Original file line number Diff line number Diff line change
Expand Up @@ -85,22 +85,22 @@
source => "puppet:///modules/puppet_agent/${legacy_keyname}",
}

# Given the path to a key, see if it is imported, if not, import it
$legacy_gpg_pubkey = "gpg-pubkey-$(echo $(gpg --homedir ${gpg_homedir} --with-colons ${legacy_gpg_path} 2>&1 | grep ^pub | awk -F ':' '{print \$5}' | cut --characters=9-16 | tr '[:upper:]' '[:lower:]'))"
$gpg_pubkey = "gpg-pubkey-$(echo $(gpg --homedir ${gpg_homedir} --with-colons ${gpg_path} 2>&1 | grep ^pub | awk -F ':' '{print \$5}' | cut --characters=9-16 | tr '[:upper:]' '[:lower:]'))"

exec { "import-${legacy_keyname}":
file { "${::env_temp_variable}/rpm_gpg_import_check.sh":
ensure => file,
source => 'puppet:///modules/puppet_agent/rpm_gpg_import_check.sh',
mode => '0755',
}
-> exec { "import-${legacy_keyname}":
path => '/bin:/usr/bin:/sbin:/usr/sbin',
command => "rpm --import ${legacy_gpg_path}",
unless => "rpm -q ${legacy_gpg_pubkey}",
command => "${::env_temp_variable}/rpm_gpg_import_check.sh import ${gpg_homedir} ${legacy_gpg_path}",
unless => "${::env_temp_variable}/rpm_gpg_import_check.sh check ${gpg_homedir} ${legacy_gpg_path}",
require => File[$legacy_gpg_path],
logoutput => 'on_failure',
}

exec { "import-${keyname}":
-> exec { "import-${keyname}":
path => '/bin:/usr/bin:/sbin:/usr/sbin',
command => "rpm --import ${gpg_path}",
unless => "rpm -q ${gpg_pubkey}",
command => "${::env_temp_variable}/rpm_gpg_import_check.sh import ${gpg_homedir} ${gpg_path}",
unless => "${::env_temp_variable}/rpm_gpg_import_check.sh check ${gpg_homedir} ${gpg_path}",
require => File[$gpg_path],
logoutput => 'on_failure',
}
Expand Down
1 change: 1 addition & 0 deletions spec/classes/puppet_agent_osfamily_redhat_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
:architecture => 'x64',
:puppet_master_server => 'master.example.vm',
:clientcert => 'foo.example.vm',
:env_temp_variable => '/tmp',
}
end

Expand Down
1 change: 1 addition & 0 deletions spec/classes/puppet_agent_osfamily_suse_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
:operatingsystemmajrelease => '12',
:architecture => 'x86_64',
:puppet_master_server => 'master.example.vm',
:env_temp_variable => '/tmp',
:clientcert => 'foo.example.vm',
}

Expand Down

0 comments on commit df35318

Please sign in to comment.