Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't litter /root/ with GPG-related files. #56

Merged
merged 1 commit into from
Jul 9, 2018
Merged

Don't litter /root/ with GPG-related files. #56

merged 1 commit into from
Jul 9, 2018

Conversation

djl
Copy link

@djl djl commented Jul 16, 2017

The previous command created (or modified existing) .gpg files under /root/, even on --noop runs.

The previous command created (or modified existing) .gpg files under
/root/, even on --noop runs.
@juniorsysadmin juniorsysadmin added the needs-feedback Further information is requested label Sep 29, 2017
@bastelfreak
Copy link
Member

Hi @djl, sorry for the long response time. Could you add a bit of explanation what those commits do? Can you somehow add a test for it? The change looks a bit cryptic.

@djl
Copy link
Author

djl commented Jan 27, 2018

Sure!

The previous gpg call wrote a couple of GPG-related files /root/. For example:

$ ls
$ gpg --export --armor deadbeef > RPM-GPG-KEY-TEST
$ ls
RPM-GPG-KEY-TEST
$ gpg --quiet --with-colon --homedir=$(pwd) --throw-keyids < RPM-GPG-KEY-TEST
... snip ...
$ ls
RPM-GPG-KEY-TEST  pubring.kbx  trustdb.gpg
$

And because this command is always executed, these files were created even when puppet was running with the --noop flag.

Additionally, if those files (pubring.kdx and trustdb.gpg) already exist and are not considered valid by GPG then the command, and entire Puppet run, fails:

$ echo foo > trustdb.gpg
$ gpg --quiet --with-colon --homedir=$(pwd) --throw-keyids < RPM-GPG-KEY-TEST
gpg: trustdb: read failed (n=4): Inappropriate ioctl for device
gpg: Fatal: /tmp/gpg/trustdb.gpg: invalid trustdb

The new call doesn't write to the filesystem or care about existing/invalid files:

$ gpg --quiet RPM-GPG-KEY-TEST | head -1 | cut -c12-20 | tr '[A-Z]' '[a-z]' > /dev/null
$ ls
$

As far as writing tests, I'm not sure how you could test this. Unless I'm missing something there doesn't appear to be any existing tests for this specific functionality.

(Not sure if it matters but I ran about a dozen machines with this change for ~5 months before submitting the pull request. The change was fine right up until I had to migrate those machines back to Debian about a month ago.)

@TJM
Copy link

TJM commented May 4, 2018

I could get behind this one too. This one needs pulled.

@TJM
Copy link

TJM commented May 4, 2018

For what its worth, this looks similar to the puppetlabs-puppet_agent repo...

https://github.com/puppetlabs/puppetlabs-puppet_agent/blob/27fba1b48ae8538b03b9445ca5c2aff2b59ccee3/manifests/osfamily/redhat.pp#L99-L106

  # Given the path to a key, see if it is imported, if not, import it
  exec {  "import-${keyname}":
    path      => '/bin:/usr/bin:/sbin:/usr/sbin',
    command   => "rpm --import ${gpg_path}",
    unless    => "rpm -q gpg-pubkey-`echo $(gpg --throw-keyids < ${gpg_path}) | cut --characters=11-18 | tr '[:upper:]' '[:lower:]'`",
    require   => File[$gpg_path],
    logoutput => 'on_failure',
  }

@bastelfreak bastelfreak added bug Something isn't working and removed needs-feedback Further information is requested labels Jul 9, 2018
@bastelfreak bastelfreak merged commit 43a35bb into voxpupuli:master Jul 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants