[BUG] Syndic key permissions overwritten

[tacerus]

Your setup

Formula commit hash / release tag

2a7a9de

Versions reports (master & minion)

Versions output

$ salt --versions
Salt Version:
          Salt: 3004
 
Dependency Versions:
          cffi: 1.13.2
      cherrypy: unknown
      dateutil: Not Installed
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 2.10.1
       libgit2: 1.3.0
      M2Crypto: 0.38.0
          Mako: Not Installed
       msgpack: 0.5.6
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: 2.17
      pycrypto: Not Installed
  pycryptodome: Not Installed
        pygit2: 1.7.0
        Python: 3.6.15 (default, Sep 23 2021, 15:41:43) [GCC]
  python-gnupg: Not Installed
        PyYAML: 5.4.1
         PyZMQ: 17.1.2
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.2.3
 
System Versions:
          dist: opensuse-leap 15.4 
        locale: UTF-8
       machine: x86_64
       release: 5.14.21-150400.24.38-default
        system: Linux
       version: openSUSE Leap 15.4

Pillar / config used

salt:master:
https://git.com.de/LibertaCasa/salt/src/branch/production/pillar/role/salt/master.sls

salt:minion:
https://git.com.de/LibertaCasa/salt/src/branch/production/pillar/role/salt/minion.sls

salt:syndic:
https://git.com.de/LibertaCasa/salt/src/branch/production/pillar/role/salt/syndic.sls

---

Bug details

Describe the bug

On SUSE based distributions, the minion runs as root by default, but the Syndic is run under the separate salt user.
The Syndic needs key ownership in /etc/salt/pki/ to be different from the key ownership of the Minion if both are run on the same machine.
Currently the formula overwrites root:salt or salt:salt key ownerships:

----------                                                                                                                                                                                     
          ID: salt-minion-pki-dir                                                                                                                                                              
    Function: file.directory                                                                                                                                                                   
        Name: /etc/salt/pki/minion                                                                                                                                                             
      Result: None                                                                                                                                                                             
     Comment: The following files will be changed:                                                                                                                                             
              /etc/salt/pki/minion: user - root                                                                                                                                                
              /etc/salt/pki/minion: group - root                                                                                                                                               
     Started: 01:42:35.464669                                                                                                                                                                  
    Duration: 2.747 ms                                                                                                                                                                         
     Changes:                                                                                                                                                                                  
              ----------                                                                                                                                                                       
              /etc/salt/pki/minion:                                                                                                                                                            
                  ----------                                                                                                                                                                   
                  group:                                                                                                                                                                       
                      root                                                                                                                                                                     
                  user:                                                                                                                                                                        
                      root                                                                     
----------                                                                                     
          ID: permissions-minion.pem                                                                                                                                                           
    Function: file.managed                                                                     
        Name: /etc/salt/pki/minion/minion.pem                                                  
      Result: None                                                                             
     Comment: File /etc/salt/pki/minion/minion.pem not updated                                 
     Started: 01:42:35.468222                                                                  
    Duration: 2.631 ms                                                                         
     Changes:                                                                                  
              ----------                   
              group:                                                                           
                  root                                                                         
              user:                                                                            
                  root      

Which causes the Syndic to fail - at first with not being able to authenticate to the master, followed by a traceback about the permission error.

Traceback

Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]: Traceback (most recent call last):
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib64/python3.6/site-packages/M2Crypto/BIO.py", line 279, in openfile
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     f = open(filename, mode)
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]: PermissionError: [Errno 13] Permission denied: '/etc/salt/pki/minion/minion.pem'
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]: During handling of the above exception, another exception occurred:
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]: Traceback (most recent call last):
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/ext/tornado/ioloop.py", line 606, in _run_callback
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     ret = callback()
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/ext/tornado/stack_context.py", line 278, in null_wrapper
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     return fn(*args, **kwargs)
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/ext/tornado/ioloop.py", line 628, in _discard_future_result
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     future.result()
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/ext/tornado/concurrent.py", line 249, in result
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     raise_exc_info(self._exc_info)
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "<string>", line 4, in raise_exc_info
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/ext/tornado/gen.py", line 1064, in run
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     yielded = self.gen.throw(*exc_info)
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/crypt.py", line 654, in _authenticate
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     creds = yield self.sign_in(channel=channel)
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/ext/tornado/gen.py", line 1056, in run
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     value = future.result()
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/ext/tornado/concurrent.py", line 249, in result
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     raise_exc_info(self._exc_info)
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "<string>", line 4, in raise_exc_info
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/ext/tornado/gen.py", line 1070, in run
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     yielded = self.gen.send(value)
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/crypt.py", line 780, in sign_in
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     ret = self.handle_signin_response(sign_in_payload, payload)
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/crypt.py", line 797, in handle_signin_response
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     payload, master_pub="token" in sign_in_payload
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/crypt.py", line 1229, in verify_master
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     return self.extract_aes(payload, master_pub=False)
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/crypt.py", line 1149, in extract_aes
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     aes, token = self.decrypt_aes(payload, master_pub)
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/crypt.py", line 979, in decrypt_aes
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     key = self.get_keys()
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/crypt.py", line 903, in get_keys
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     key = get_rsa_key(self.rsa_path, None)
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/crypt.py", line 213, in get_rsa_key
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     return _get_key_with_evict(path, str(os.path.getmtime(path)), passphrase)
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/utils/decorators/__init__.py", line 298, in _memoize
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     cache[args_] = func(*args, **kwargs)
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib/python3.6/site-packages/salt/crypt.py", line 194, in _get_key_with_evict
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     key = RSA.load_key(path, lambda x: bytes(passphrase))
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib64/python3.6/site-packages/M2Crypto/RSA.py", line 379, in load_key
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     with BIO.openfile(file) as bio:
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:   File "/usr/lib64/python3.6/site-packages/M2Crypto/BIO.py", line 281, in openfile
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]:     raise BIOError(ex.args)
Jan 28 01:34:38 derutil01.rigel.lysergic.dev salt-syndic[1391]: M2Crypto.BIO.BIOError: (13, 'Permission denied')

One needs to manually correct the ownership of the directory.

systemctl stop salt-syndic
chown salt:salt /etc/salt/pki/minion
chown salt:salt /etc/salt/pki/minion/*.pem
rm /etc/salt/pki/minion/syndic_master.pub
systemctl start salt-syndic

Steps to reproduce the bug

Configure both a Syndic and a Minion on the same machine using the formula.

Expected behaviour

The formula should differentiate between a Minion-only-minion and a Syndic-minion, and set file permissions accordingly.

Attempts to fix the bug

Upon reading the code, I found a salt:rootgroup setting - defining it to salt helped with the directory permissions. The minion.pem key file however is generated with 400 permissions, requiring the owner of it to be salt - I could not find a way to change this without changing the whole minion user.

Additionally the rootgroup setting changes some other files, like minion.pub, which re-creations by the Salt minion would in return change back to root:root, possibly causing a back-and-forth.

Additional context