[security] master_sign.pem never used

jerem991 commented 4 years ago

Description of Issue

While testing the verify master public key feature, we were able to replace the master_sign.pem on the master with a wrong private key and successfully initiate a new minion connection on a master. This is a security issue as if an attacker is able to retrieve the master_sign public key, he would be able to create a malicious master and potentially grab grains from minions that can contain sensitive information.

Setup

Minion configuration

verify_master_pubkey_sign: True
id: minionid
master:
  - salt-master-1-IP
  - salt-master-2-IP
master_type: failover
master_alive_interval: 10
random_master: True
retry_dns: 0
autosign_grains:
  - test-uuid
grains:
  roles:
    - super-role
  test-uuid: randomuuiiiiiiiiiiid

Master 1 configuration

user: salt
id: salt-master-1
master_sign_pubkey: True
autosign_grains_dir: /etc/salt/autosign_grains

Steps to Reproduce Issue

Install salt-master using the booostrap on the master1 with the configuration - master_sign_pubkey: True (and with the autosign_grains folder and the test-uuid file with the randomuuiiiiiiiiiiid)
Install salt-minion with the configuration file (see above)
Observe successful connection
Stop minion and master
Remove with salt-key the minion on the master
Copy master.pem -> mastersign.pem (should break the configuration)_
Run minion and master
Observe successful connection (should not happen)

Versions Report

Minion & Master version : 2019.2.2 (Fluorine)


           Salt: 2019.2.2

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: 0.26.0
        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: 0.26.2
         Python: 3.6.9 (default, Nov  7 2019, 10:44:02)
   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-1054-aws
         system: Linux
        version: Ubuntu 18.04 bionic```

frogunder commented 4 years ago

@jerem991 Thank you for reporting this issue.

I have a few questions to better understand the issue you are seeing.

When you use verify_master_pubkey_sign: True on the master you generate a new key pair on the master, did you copy the master_sign.pub to the minion key directory?

Did you check the minion log upon connection to see that it indeed used the master_sign.pub key? You should see this

[DEBUG   ] salt.crypt.get_rsa_pub_key: Loading public key
[DEBUG   ] salt.crypt.verify_signature: Loading public key
[DEBUG   ] salt.crypt.get_rsa_pub_key: Loading public key
[DEBUG   ] salt.crypt.verify_signature: Verifying signature
[DEBUG   ] Successfully verified signature of master public key with verification public key master_sign.pub

Also after you removed the minion key on the master and copied master.pem -> master_sign.pem and restarted, did you see this in the minion log?

[DEBUG   ] salt.crypt.get_rsa_pub_key: Loading public key
[DEBUG   ] Decrypting the current master AES key
[DEBUG   ] salt.crypt.get_rsa_key: Loading private key
[DEBUG   ] salt.crypt._get_key_with_evict: Loading private key
[DEBUG   ] Loaded minion key: /etc/salt/pki/minion/minion.pem

Seems like for me we restart it is not using the master_sign.pub key.

Wonder if you are seeing the same things in the logs?

I will also check with the core team if they have any thoughts in the issue. @saltstack/team-core

Thanks.

jerem991 commented 4 years ago

Hi @frogunder,

When you use verify_master_pubkey_sign: True on the master you generate a new key pair on the master, did you copy the master_sign.pub to the minion key directory?

Yes i did it.

Did you check the minion log upon connection to see that it indeed used the master_sign.pub key?

Yes there was a successful connection.

Also after you removed the minion key on the master and copied master.pem -> master_sign.pem and restarted, did you see this in the minion log?

I have the exact same behaviour.

Wonder if you are seeing the same things in the logs?

Same things ! 👍

Thanks !

frogunder commented 4 years ago

@saltstack/team-core Can one of you take a look at this issue? Thanks

stale[bot] commented 4 years ago

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] commented 4 years ago

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

sagetherage commented 4 years ago

I will get someone assigned from the core team.

stale[bot] commented 4 years ago

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.

sagetherage commented 4 years ago

@dwoz

stale[bot] commented 4 years ago

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

saltstack / salt