salt-key can't delete a single key if name is duplicated

[boltronics]

Description of Issue/Question

Sometimes when replacing instances outside of salt-cloud, I find myself in a situation where old minion keys are left over on the master and need to be cleaned up. Since a new key was created for the replacement host with the same name, salt-key is unable to remove just the old host key. The only known work-around is to have salt-key delete both keys, and then restart the minion on the new host to have it send through a new key, which then also has to be manually accepted.

Setup

Launch a host using EC2 or whatever using salt-cloud. Nuke it from outside salt-cloud. Launch a new host using salt-cloud. You will end up with multiple keys with different fingerprints but the same name.

$ sudo salt-key -L
Accepted Keys:
salt-master0.internal
vpn0.internal
Denied Keys:
salt-master0.internal
Unaccepted Keys:
Rejected Keys:
$ sudo salt-key -d salt-master0.internal
The following keys are going to be deleted:
Accepted Keys:
salt-master0.internal
Denied Keys:
salt-master0.internal
Proceed? [N/y] y
Key for minion salt-master0.internal deleted.
Key for minion salt-master0.internal deleted.
$ 

If I try to delete by fingerprint:

$ sudo salt-key -d df:18:84:7b:e3:2b:fa:e8:cc:af:c8:80:92:78:79:51:38:54:d2:0f:cd:31:55:2d:f7:00:6f:23:35:50:fd:00
The key glob 'df:18:84:7b:e3:2b:fa:e8:cc:af:c8:80:92:78:79:51:38:54:d2:0f:cd:31:55:2d:f7:00:6f:23:35:50:fd:00' does not match any unaccepted keys.

AFAIK there's no way to just delete the old key when such naming conflicts exist using salt-key, but salt-cloud will happily allow such conflicts to be created.

Steps to Reproduce Issue

Nothing beyond the steps in the Setup section.

Versions Report

salt --versions-report
Salt Version:
           Salt: 2017.7.2

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.5.3
      docker-py: Not Installed
          gitdb: 2.0.0
      gitpython: 2.1.1
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.13 (default, Nov 24 2017, 17:33:09)
   python-gnupg: 0.3.9
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: 2.0.1
        timelib: Not Installed
        Tornado: 4.4.3
            ZMQ: 4.2.1

System Versions:
           dist: debian 9.3 
         locale: UTF-8
        machine: x86_64
        release: 4.9.0-5-amd64
         system: Linux
        version: debian 9.3 

The issue has been around for a long time. I thought I would have already had a bug report open about this, but either I forgot to or can't find it.

[boltronics]

Looks like the bulk of the logic to handle this exists in https://github.com/saltstack/salt/blob/develop/salt/key.py and https://github.com/saltstack/salt/blob/develop/salt/utils/parsers.py#L2249. Probably not too hard to fix.

One way to address this would be to add another flag with action=store_true to the optparser, and have the existing glob matching functionality of -d replaced with fingerprint matching. Alternatively we could just add a --delete-fingerprint option, which is probably the nicer way to go since there's not too many other options that could make use of switching away from glob matching.

Either way, I guess the option would probably be implemented without a short flag option since -d, -D, -f and -F are all already taken. Thoughts?

[gtmanfred]

You can specify the type of minions that you want with -l or --list

So salt-key -l denied -d salt-master0.internal would only delete the denied keys with that name.

Does this work for you?

Thanks,
Daniel

[boltronics]

Thanks - good to know. It would work for the above example, but unless I'm mistaken, I'm reasonably sure I have also encountered situations where there are duplicate key names that are both in the accepted section.

[boltronics]

Another option would be to change the behaviour of apt-key to prompt on each key when deleting by default, showing the fingerprint each time. In theory that shouldn't cause too much trouble for people since there is a --yes argument that could be used to skip all the prompts for use cases where many keys are to be deleted at once, and previewing what will be deleted can be done via salt-key -f <some glob> (although the --help documentation output should probably be updated to indicate -f accepts globs).

[gtmanfred]

Yeah, that is really reasonable.

I am marking this as a feature request.

Thanks!
Daniel

[mbunkus]

So salt-key -l denied -d salt-master0.internal would only delete the denied keys with that name.

For the record, this doesn't work. It only lists all denied keys but doesn't offer to (or actually does) delete the key.

[daveneeley]

Other CLI options described in #28154

[lordcirth]

This feature request would also solve this request, by allowing salt-key -l denied -d salt-master0.internal to work: https://github.com/saltstack/salt/issues/48951

[gtmanfred]

ping @saltstack/team-triage

[poige]

boltronics opened this issue on Feb 15, 2018 · 9 comments

So this is 2 years (soon) lasting bug report. Quite disappointing to be honest.

[waynew]

@poige since this seems to affect you, would you be interested in working on adding this functionality? If not, that's fine, but it's typically the fastest way to get some functionality implemented.

[grinapo]

@poige since this seems to affect you,

Silent masses speaking here. No, it's not. Like I periodically have the problem that:

    The function "state.apply" is running as PID 459723 and was started at 2020, Feb 06 09:53:47.903569 with jid 20200206095347903569

Which is caused by a key called twice (or existing two active, alowed keys with the same name).

# /usr/bin/time salt 'spamd2*' test.ping                                                                                         
spamd2.x:
    True
spamd2.x:
    True

I cannot even list them (I guess they are the same keys coming from different hosts?), and the only way to detect them is to have calls fail, then delete all the keys (same name, I can't do it otherwise anyway) and re-add them.

I can't quite see where master stores this, as /var/cache/salt/master/minions/spamd2.x/ is a directory, so it can't be the same name twice....

[_UPDATE_]: I believe the reason is that there are _two_ minions running on the same host (which is a different bug). This is, then, not really related to the original problem but I'd leave it here for the search.

[isaachui]

This doesn't really address the cli for salt-key not being able to delete only the denied key, but its a workaround.

If you're able to access the salt-master, which I assume you are, then the denied keys are located here:
/etc/salt/pki/master/minions_denied

I just deleted the one I wanted to delete.

[rjc]

Fingerprint-only delete option doesn't solve the issue either, however.

Let's say you clone a machine and reuse the key but change the minion_id and then accept both keys - two keys with the same fingerprint will work just fine, no errors or warnings are triggered. Then, one of these machine fails so you set up a new machine with the same minion_id (from scratch this time) and you'd like to clean-up old key afterwards - you'd remove two keys - the key which is still being used in the first machine and the one which has failed.

If anything, both the minion_id and the fingerprint should be used, or a different approach taken altogether.