salt-key doesn't handle PermissionError exception when moving keys

[eliasp]

Description of Issue

When the user executing salt-key doesn't have the necessary permissions to move the key files in the PKI directory to their destination, salt-key silently ignores those errors.

It's not about how to properly set those permissions (permissive_pki_access etc), but about not letting the user know about this issue - so that's mostly about the UX here.

Setup

• 2019.2.0 Master running as non-root
• permissive_pki_access is not (yet) set to True

Steps to Reproduce Issue

Change the permissions of Minion keys in the PKI dir so the user executing salt-key is unable to move them.
Execute an operation like salt-key -A as this user.

Using pdb clearly shows the exception:

PermissionError: [Errno 13] Permission denied: '/var/lib/salt/pki/master/minions/test-minion'
> /usr/lib/python3/dist-packages/salt/key.py(726)accept()

But my quickly hacked together attempt to rectify ended up nowhere and had zero effect - maybe someone else can take it from here and fix this:

--- a/salt/key.py.orig     2019-07-29 16:41:19.288904662 +0000
+++ b/salt/key.py  2019-07-29 17:26:57.563442154 +0000
@@ -729,6 +729,9 @@
                              'id': key}
                     self.event.fire_event(eload,
                                           salt.utils.event.tagify(prefix='key'))
+                except PermissionError as pe:
+                    log.error('Unable to accept key for "%r": %r', key, pe)
+                    return self.list_keys()
                 except (IOError, OSError):
                     pass
         return (
@@ -758,6 +761,9 @@
                          'id': key}
                 self.event.fire_event(eload,
                                       salt.utils.event.tagify(prefix='key'))
+            except PermissionError as pe:
+                log.error('Unable to accept key for "%r": %r', key, pe)
+                return self.list_keys()
             except (IOError, OSError):
                 pass
         return self.list_keys()

Versions Report

Salt Version:
           Salt: 2019.2.0

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: 2.0.3
      gitpython: 2.1.8
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.6.5 (default, Apr  1 2018, 05:46:30)
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: 2.0.3
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5

System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-51-generic
         system: Linux
        version: Ubuntu 18.04 bionic

[garethgreenaway]

@eliasp Thanks for the report. From a user experience perspective this would be a good thing to have in place. It would be new functionality so marking as a new feature.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[stale[bot]]

Thank you for updating this issue. It is no longer marked as stale.