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

[BUG] gpg.absent does not honor user/gnupghome, gpg.delete_key does not honor gnupghome #63159

Closed
lkubb opened this issue Nov 30, 2022 · 0 comments · Fixed by #63162
Closed
Labels
Bug broken, incorrect, or confusing behavior needs-triage State-Module

Comments

@lkubb
Copy link
Contributor

lkubb commented Nov 30, 2022

Description
gpg.absent cannot delete keys for any user (apart from root and salt) or a different GNUPGHOME. It checks for the presence of keys in the keychain without passing the necessary params:

_current_keys = __salt__["gpg.list_keys"]()

Setup
irrelevant

Steps to Reproduce the behavior

/tmp/gnupghome:
  file.directory

Make sure key is present:
  gpg.present:
    - name: D75899B9A724937A
    - keyserver: keyserver.ubuntu.com
    - gnupghome: /tmp/gnupghome
    - require:
      - file: /tmp/gnupghome

Ensure the key is actually present:
  module.run:
    - gpg.get_key:
      - keyid: D75899B9A724937A
      - gnupghome: /tmp/gnupghome
    - require:
      - Make sure key is present

This will report the key as absent:
  gpg.absent:
    - name: D75899B9A724937A
    - gnupghome: /tmp/gnupghome
    - require:
      - Ensure the key is actually present

And it is still here:
  module.run:
    - gpg.get_key:
      - keyid: D75899B9A724937A
      - gnupghome: /tmp/gnupghome
    - require:
      - This will report the key as absent

Expected behavior
The key being deleted

Screenshots

local:
----------
          ID: /tmp/gnupghome
    Function: file.directory
      Result: True
     Comment:
     Started: 21:11:23.073900
    Duration: 4.668 ms
     Changes:
              ----------
              /tmp/gnupghome:
                  ----------
                  directory:
                      new
----------
          ID: Make sure key is present
    Function: gpg.present
        Name: D75899B9A724937A
      Result: True
     Comment: Adding D75899B9A724937A to GPG keychain
     Started: 21:11:23.079134
    Duration: 151.256 ms
     Changes:
----------
          ID: Ensure the key is actually present
    Function: module.run
      Result: True
     Comment: gpg.get_key: Success
     Started: 21:11:23.231724
    Duration: 8.732 ms
     Changes:
              ----------
              gpg.get_key:
                  ----------
                  created:
                      2016-06-14
                  fingerprint:
                      28806A878AE423A28372792ED75899B9A724937A
                  keyLength:
                      4096
                  keyid:
                      D75899B9A724937A
                  ownerTrust:
                      Unknown
                  trust:
                      Unknown
                  uids:
                      - Nextcloud Security <[email protected]>
----------
          ID: This will report the key as absent
    Function: gpg.absent
        Name: D75899B9A724937A
      Result: True
     Comment: D75899B9A724937A not found in GPG keychain
     Started: 21:11:23.240977
    Duration: 7.787 ms
     Changes:
----------
          ID: And it is still here
    Function: module.run
      Result: True
     Comment: gpg.get_key: Success
     Started: 21:11:23.249287
    Duration: 8.722 ms
     Changes:
              ----------
              gpg.get_key:
                  ----------
                  created:
                      2016-06-14
                  fingerprint:
                      28806A878AE423A28372792ED75899B9A724937A
                  keyLength:
                      4096
                  keyid:
                      D75899B9A724937A
                  ownerTrust:
                      Unknown
                  trust:
                      Unknown
                  uids:
                      - Nextcloud Security <[email protected]>

Summary for local
------------
Succeeded: 5 (changed=3)
Failed:    0
------------
Total states run:     5
Total run time: 181.165 ms

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
Salt Version:
          Salt: 3005.1

Dependency Versions:
          cffi: 1.14.6
      cherrypy: unknown
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: 4.0.9
     gitpython: 3.1.29
        Jinja2: 3.1.0
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.9.8
        pygit2: Not Installed
        Python: 3.9.14 (main, Sep 27 2022, 00:00:00)
  python-gnupg: 0.4.8
        PyYAML: 5.4.1
         PyZMQ: 23.2.0
         smmap: 5.0.0
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4

System Versions:
          dist: rocky 9.0 Blue Onyx
        locale: utf-8
       machine: x86_64
       release: 5.14.0-70.26.1.el9_0.x86_64
        system: Linux
       version: Rocky Linux 9.0 Blue Onyx

Additional context
Another issue: gpg.absent state confuses gpg.delete_key kwargs by using positional args and passes user as fingerprint and gnupghome as delete_secret as seen here:

salt/salt/states/gpg.py

Lines 175 to 179 in 474ea03

result = __salt__["gpg.delete_key"](
key,
user,
gnupghome,
)

salt/salt/modules/gpg.py

Lines 495 to 502 in 474ea03

def delete_key(
keyid=None,
fingerprint=None,
delete_secret=False,
user=None,
gnupghome=None,
use_passphrase=True,
):

This is masked by the first issue, which causes it to almost never reach that line if any of the mentioned params are set.

Another issue: gpg.delete_key tries to look up the key from the keychain to determine if it can be deleted, again forgetting the gnupghome parameter.

key = get_key(keyid, fingerprint, user)

But I really want to stop flooding the issue tracker with the gpg modules (this being the 6th). The described issues seemed like a sensible fit, so I hope it's fine to mash them together like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior needs-triage State-Module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants